Re: [PATCH v5 08/14] media: iris: Rename clock and power domain macros to use vcodec prefix

[Dmitry Baryshkov]

On Wed, May 13, 2026 at 07:40:23PM +0530, Vikash Garodia wrote:
> 
> On 5/13/2026 7:17 PM, Dmitry Baryshkov wrote:
> > On Sat, May 09, 2026 at 10:37:20PM +0530, Vishnu Reddy wrote:
> > > 
> > > On 5/9/2026 1:52 AM, Dmitry Baryshkov wrote:
> > > > On Sat, May 09, 2026 at 12:29:57AM +0530, Vishnu Reddy wrote:
> > > > > The current clock and power domain enum names are too generic. Rename
> > > > > them with a vcodec prefix to make the names more meaningful and to easily
> > > > > accommodate vcodec1 enums for the secondary core for glymur platform.
> > > > > 
> > > > > No functional changes intended.
> > > > > 
> > > > > Reviewed-by: Vikash Garodia <vikash.garodia@xxxxxxxxxxxxxxxx>
> > > > > Signed-off-by: Vishnu Reddy <busanna.reddy@xxxxxxxxxxxxxxxx>
> > > > > ---
> > > > >   .../platform/qcom/iris/iris_platform_common.h      | 12 ++++----
> > > > >   .../media/platform/qcom/iris/iris_platform_gen1.c  |  6 ++--
> > > > >   .../media/platform/qcom/iris/iris_platform_gen2.c  |  6 ++--
> > > > >   .../platform/qcom/iris/iris_platform_sc7280.h      | 10 +++----
> > > > >   .../platform/qcom/iris/iris_platform_sm8750.h      | 12 ++++----
> > > > >   drivers/media/platform/qcom/iris/iris_vpu3x.c      | 21 +++++++-------
> > > > >   drivers/media/platform/qcom/iris/iris_vpu4x.c      | 30 +++++++++++---------
> > > > >   drivers/media/platform/qcom/iris/iris_vpu_common.c | 33 +++++++++++-----------
> > > > >   8 files changed, 67 insertions(+), 63 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > index 55ff6137d9a9..30e9d4d288c6 100644
> > > > > --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > @@ -49,14 +49,14 @@ extern const struct iris_platform_data sm8650_data;
> > > > >   extern const struct iris_platform_data sm8750_data;
> > > > >   enum platform_clk_type {
> > > > > -	IRIS_AXI_CLK, /* AXI0 in case of platforms with multiple AXI clocks */
> > > > > +	IRIS_AXI_VCODEC_CLK,
> > > > >   	IRIS_CTRL_CLK,
> > > > >   	IRIS_AHB_CLK,
> > > > > -	IRIS_HW_CLK,
> > > > > -	IRIS_HW_AHB_CLK,
> > > > > -	IRIS_AXI1_CLK,
> > > > > +	IRIS_VCODEC_CLK,
> > > > > +	IRIS_VCODEC_AHB_CLK,
> > > > > +	IRIS_AXI_CTRL_CLK,
> > > > Why AXI_CTRL is better than AXI1?
> > > 
> > > Glymur has 3 AXI clocks: axi_vcodec0, axi_ctrl, and axi_vcodec1. Using
> > > positional names like AXI0/AXI1/AXI2 doesn't convey their purpose, whereas
> > > naming them as AXI_VCODEC_CLK, AXI_CTRL_CLK makes the intent clearer,
> > > especially when platforms differ in the number of AXI clocks they expose.
> > 
> > => commit message
> > 
> > > 
> > > > 
> > > > >   	IRIS_CTRL_FREERUN_CLK,
> > > > > -	IRIS_HW_FREERUN_CLK,
> > > > > +	IRIS_VCODEC_FREERUN_CLK,
> > > > Can we at least group them too?
> > > > 
> > > > >   	IRIS_BSE_HW_CLK,
> > > > Is BSE a core or a codec clock?
> 
> IRIS_VCODEC_BSE_CLK would be more appropriate here.
> 
> Core and vcodec (video codec), it refers to the main processing block and
> are referred interchangeably. BSE and VPP are the sub blocks inside the
> core/vcodec block.

Thanks!

> > 
> > Unanswered
> > 
> > > > 
> > > > >   	IRIS_VPP0_HW_CLK,
> > > > >   	IRIS_VPP1_HW_CLK,
> > > > > @@ -206,7 +206,7 @@ struct icc_vote_data {
> > > > >   enum platform_pm_domain_type {
> > > > >   	IRIS_CTRL_POWER_DOMAIN,
> > > > > -	IRIS_HW_POWER_DOMAIN,
> > > > > +	IRIS_VCODEC_POWER_DOMAIN,
> > > > >   	IRIS_VPP0_HW_POWER_DOMAIN,
> > > > >   	IRIS_VPP1_HW_POWER_DOMAIN,
> > > > >   	IRIS_APV_HW_POWER_DOMAIN,
> > 
> 

-- 
With best wishes
Dmitry