From: Vikash Garodia <quic_vgarodia@quicinc.com>
To: Konrad Dybcio <konrad.dybcio@linaro.org>,
Dikshita Agarwal <quic_dikshita@quicinc.com>,
Stanimir Varbanov <stanimir.k.varbanov@gmail.com>,
Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
"Philipp Zabel" <p.zabel@pengutronix.de>
Cc: Marijn Suijten <marijn.suijten@somainline.org>,
Stanimir Varbanov <stanimir.varbanov@linaro.org>,
Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
<linux-media@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 03/19] media: venus: pm_helpers: Add kerneldoc to venus_clks_get()
Date: Wed, 10 Apr 2024 17:33:39 +0530 [thread overview]
Message-ID: <335d1eee-a6f1-6502-7f27-c7362a53b4ba@quicinc.com> (raw)
In-Reply-To: <b4c56cad-0a3c-4b74-b9fa-0931204d1514@linaro.org>
On 4/9/2024 11:52 PM, Konrad Dybcio wrote:
>
>
> On 4/5/24 14:44, Vikash Garodia wrote:
>> Hi Konrad,
>>
>> On 4/5/2024 1:56 PM, Dikshita Agarwal wrote:
>>>
>>>
>>> On 3/27/2024 11:38 PM, Konrad Dybcio wrote:
>>>> To make it easier to understand the various clock requirements within
>>>> this driver, add kerneldoc to venus_clk_get() explaining the fluff.
>>>>
>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>> ---
>>>> drivers/media/platform/qcom/venus/pm_helpers.c | 28
>>>> ++++++++++++++++++++++++++
>>>> 1 file changed, 28 insertions(+)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c
>>>> b/drivers/media/platform/qcom/venus/pm_helpers.c
>>>> index ac7c83404c6e..cf91f50a33aa 100644
>>>> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
>>>> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
>>>> @@ -23,6 +23,34 @@
>>>> static bool legacy_binding;
>>>> +/**
>>>> + * venus_clks_get() - Get Venus clocks that are not bound to a vcodec
>>>> + * @core: A pointer to the venus core resource
>>>> + *
>>>> + * The Venus block (depending on the generation) can be split into a couple
>>>> + * of clock domains: one for main logic and one for each video core (0-2
>>>> instances).
>> s/main logic/controller. Applies to below places as well.
>
> Ok - so "controller" is the cortex-m3 (m5?) that power-sequences
> the DSP etc.?
Thats correct. The firmware runs on the controller and takes care of many
aspects of hardware (dsp or core) programming.
>>
>>>> + *
>>>> + * MSM8916 (and possibly other HFIv1 users) only feature the "main logic"
>>>> + * domain, so this function is the only kind if clk_get necessary there.
>> To be checked, unable to get the clock document to see why only core clocks
>> (VENUS0_VCODEC0_CLK). Will update.
>
> FWIW 8916 only has GCC_VENUS0_VCODEC0_CLK (and _SRC) and AHB/AXI/TBU clocks,
> no (currently registered) clock for the entire block.
>
>>
>>>> + *
>>>> + * MSM8996 (and other HFIv3 users) feature two video cores, with core0 being
>>>> + * statically defined a decoder and core1 an encoder, with both having
>>>> + * their own clock domains.
>>>> + *
>>>> + * SDM845 features two video cores, each one of which may or may not be
>> s/two video cores/two identical video cores
>>>> + * subdivided into two encoder/decoder threads.
>> decoder cannot be split into core threads. you can keep it like "each of which
>> is capable to do any encode or decode"
>
> So it's not about any splitting, but rather the ability to do both encode
> and decode, sort of like how the displayport controller can nowadays do both
> eDP and DP, depending on what init data you send to it?
It is precisely that way, just that there are cases of cores with dedicated
codec support, hence identical implies that each of them can do same processing.
>>
>>>> + *
>>>> + * Other SoCs either feature a single video core (with its own clock domain)
>>>> + * or one video core and one CVP (Computer Vision Processor) core. In both
>>>> cases
>>>> + * we treat it the same way (CVP only happens to live near-by Venus on the
>>>> SoC).
>>>> + *
>>>> + * Due to unfortunate developments in the past, we need to support legacy
>>> why unfortunate? please re-phrase this.
>>>> + * bindings (MSM8996, SDM660, SDM845) that require specifying the clocks and
>>>> + * power-domains associated with a video core domain in a bogus sub-node,
>>>> + * which means that additional fluff is necessary.
>> Some background:
>> It was done that way to support decoder core with specific clocks and similarly
>> for encoder. Earlier architectures use to have different clock source for these
>> specific decoder/encoder core clocks, now there is a common clock source for
>> both the cores. Hence if any one is enabled, others gets enabled as it is
>> derived from same source.
>> So if we see the later bindings, the clocks were moved out of sub node to main
>> venus node.
>
> Yeah and I don't really see the reason why the binding needed to be changed
> for this, you can simply get the clocks by name and poke at the specific clk*
> (or an array of them), no matter where they were _get()-ed from.
I think the reason for not doing a name based clock as it might be possible that
the clock is not available or applicable to subsequent SOC.
Regards,
Vikash
next prev parent reply other threads:[~2024-04-10 12:04 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-27 18:08 [PATCH v3 00/19] Venus cleanups Konrad Dybcio
2024-03-27 18:08 ` [PATCH v3 01/19] media: venus: pm_helpers: Only set rate of the core clock in core_clks_enable Konrad Dybcio
2024-04-05 7:31 ` Dikshita Agarwal
2024-04-05 8:49 ` Vikash Garodia
2024-03-27 18:08 ` [PATCH v3 02/19] media: venus: pm_helpers: Rename core_clks_get to venus_clks_get Konrad Dybcio
2024-04-05 7:32 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 03/19] media: venus: pm_helpers: Add kerneldoc to venus_clks_get() Konrad Dybcio
2024-04-05 8:26 ` Dikshita Agarwal
2024-04-05 12:44 ` Vikash Garodia
2024-04-09 18:22 ` Konrad Dybcio
2024-04-10 12:03 ` Vikash Garodia [this message]
2024-04-09 18:16 ` Konrad Dybcio
2024-03-27 18:08 ` [PATCH v3 04/19] media: venus: core: Set OPP clkname in a common code path Konrad Dybcio
2024-04-05 7:39 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 05/19] media: venus: pm_helpers: Kill dead code Konrad Dybcio
2024-04-05 7:49 ` Dikshita Agarwal
2024-04-09 18:24 ` Konrad Dybcio
2024-04-23 7:59 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 06/19] media: venus: pm_helpers: Move reset acquisition to common code Konrad Dybcio
2024-04-05 7:51 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 07/19] media: venus: core: Deduplicate OPP genpd names Konrad Dybcio
2024-04-05 7:52 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 08/19] media: venus: core: Get rid of vcodec_num Konrad Dybcio
2024-04-05 9:18 ` Dikshita Agarwal
2024-04-05 12:30 ` Vikash Garodia
2024-03-27 18:08 ` [PATCH v3 09/19] media: venus: core: Drop cache properties in resource struct Konrad Dybcio
2024-04-05 7:57 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 10/19] media: venus: core: Use GENMASK for dma_mask Konrad Dybcio
2024-04-05 7:59 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 11/19] media: venus: core: Remove cp_start Konrad Dybcio
2024-04-05 8:09 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 12/19] media: venus: pm_helpers: Commonize core_power Konrad Dybcio
2024-04-05 8:12 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 13/19] media: venus: pm_helpers: Remove pm_ops->core_put Konrad Dybcio
2024-04-05 9:01 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 14/19] media: venus: core: Define a pointer to core->res Konrad Dybcio
2024-04-24 0:33 ` Bryan O'Donoghue
2024-04-25 12:38 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 15/19] media: venus: pm_helpers: Simplify vcodec clock handling Konrad Dybcio
2024-04-25 12:44 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 16/19] media: venus: pm_helpers: Commonize getting clocks and GenPDs Konrad Dybcio
2024-04-25 12:46 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 17/19] media: venus: pm_helpers: Commonize vdec_get() Konrad Dybcio
2024-04-25 12:47 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 18/19] media: venus: pm_helpers: Commonize venc_get() Konrad Dybcio
2024-04-25 12:47 ` Dikshita Agarwal
2024-03-27 18:08 ` [PATCH v3 19/19] media: venus: pm_helpers: Use reset_bulk API Konrad Dybcio
2024-04-05 8:30 ` Dikshita Agarwal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=335d1eee-a6f1-6502-7f27-c7362a53b4ba@quicinc.com \
--to=quic_vgarodia@quicinc.com \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=bryan.odonoghue@linaro.org \
--cc=konrad.dybcio@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=marijn.suijten@somainline.org \
--cc=mchehab+huawei@kernel.org \
--cc=mchehab@kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=quic_dikshita@quicinc.com \
--cc=stanimir.k.varbanov@gmail.com \
--cc=stanimir.varbanov@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).