Re: [PATCH v4 3/9] media: qcom: venus: Add msm8939 resource struct
Bryan O'Donoghue <[email protected]>
| Newsgroups | org.kernel.vger.phone-devel,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-clk,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media |
|---|---|
| Message-ID | <[email protected]> |
On 12/05/2026 08:05, Erikas Bitovtas wrote:
>
>
> On 5/6/26 2:33 AM, Bryan O'Donoghue wrote:
>> On 05/05/2026 22:44, Erikas Bitovtas wrote:
>>> + .vcodec_clks = { "core0", "core1" },
>>> + .vcodec_clks_num = 2,
>>
>> This smells a bit dodgy to me.
>>
>> You already have
>>
>> struct clk *vcodec0_clks[VIDC_VCODEC_CLKS_NUM_MAX];
>> struct clk *vcodec1_clks[VIDC_VCODEC_CLKS_NUM_MAX];
>>
>> so why add vcodec_clks and put core0 and core1 into a new array.
>> vcodec0_clks and vcodec1_clks seem like a very natural place for core0
>> and core1 clocks to live ?
>>
> Sashiko found some power management issues in my code, so I'm coming
> back to this.
> The idea is to call vcodec_clks_get only once for inline cores, thus
> avoiding duplicate clock assignment. Attaching to vcodec{0,1}_clks
> instead would mean we're adding the same clocks twice. I dropped it in
> later revisions, but now I want to re-introduce this, then add them in
> core_get_v1 instead of {vdec,venc}_get_v1.> ---
>> bod
>
That's fine just please give as much detail as possible in the commit
log to justify.
---
bod