Re: [PATCH v5 12/14] media: iris: Add platform data for glymur

[Dmitry Baryshkov]

On Wed, May 13, 2026 at 08:00:39PM +0530, Vikash Garodia wrote:
> 
> 
> On 5/13/2026 7:47 PM, Dmitry Baryshkov wrote:
> > On Mon, May 11, 2026 at 09:45:01PM +0530, Vishnu Reddy wrote:
> > > 
> > > On 5/9/2026 2:35 AM, Dmitry Baryshkov wrote:
> > > > On Sat, May 09, 2026 at 12:30:01AM +0530, Vishnu Reddy wrote:
> > > > > On glymur platform, the iris core shares most properties with the
> > > > > iris core on the SM8550 platform. The major difference is that glymur
> > > > > integrates two codec cores (vcodec0 and vcodec1), while SM8550 has only
> > > > > one. Add glymur specific platform data, reusing SM8550 definitions
> > > > > wherever applicable.
> > > > This leave me in confusion. Having two cores, each with its own set of
> > > > clocks and pm domains, I'd have expected that each core scales
> > > > independently. I.e. if the load is pushed to the core0, it requires
> > > > core0 clocks to go higher (while core1 clocks can stay at the low freq).
> > > > Or, at least, the clocks would be set to the frequency corresponding to
> > > > the max of the workloads (if for some reason the cores should stay in
> > > > sync).
> > > > 
> > > > However, I don't see it in the code. All clocks and all power domains
> > > > seem do be scaled using the common workload. If my assumptions were not
> > > > correct, please explain it in the commit message.
> > > 
> > > The OPP core logic sets the rpmhpd level and clock rate based on the OPP table
> > > defined in the DT node, where the clock frequency and power rail level are
> > > tightly coupled together. Since vcodec0 and vcodec1 share the same power rails,
> > > independently scaling one clock high while keeping the other low is not
> > > straightforward within this OPP framework.
> > > 
> > > Do you have any suggestion on how best to handle per core independent clock
> > > scaling within these constraints?
> > 
> > This would require more plumbing and driver changes, but:
> > 
> > 	iris: video-codec@foo {
> > 		compatible = "qcom,glymur-iris",
> > 		clocks = <only-core-clocks>;
> > 		resets = <only-core-resets>;
> > 
> > 		/* or core@0 */
> > 		codec@0 {
> > 			clocks, resets, power-domains;
> > 			operating-points-v2 = <&iris_opp_table>
> > 		};
> > 
> > 		/* or core@1 */
> > 		codec@1 {
> > 			clocks, resets, power-domains;
> > 			operating-points-v2 = <&iris_opp_table>
> > 		};
> > 
> > 		iris_opp_table: opp-table {
> > 			compatible = "operating-points-v2"
> > 		};
> > 	};
> > 
> 
> clock source "video_cc_mvs0_clk_src" is common for both the cores. It would
> not matter if core0 is scaled for a specific workload and core1 is scaled
> for different (let say lower), as the common PLL would always generate the
> higher of them.
> 
> Infact, going with the approach of exclusive scaling would be an issue here.
> The later core scaling command would bring down/up the corner for other
> core, and could lead to under/over-voting.

Are the dividers between mvs0_clk_src and the branch clocks really R/O
in the hardware? Can they be scaled to account for the different
workloads? The commit message should capture the details of the
interaction between cores:
- Can either of them be turned off, while retaining the other one
  running?
- Can either of them run at a different frequency than the other one?
- etc.

> > > 
> > > > > Reviewed-by: Vikash Garodia <vikash.garodia@xxxxxxxxxxxxxxxx>
> > > > > Signed-off-by: Vishnu Reddy <busanna.reddy@xxxxxxxxxxxxxxxx>
> > > > > ---
> > > > >   drivers/media/platform/qcom/iris/Makefile          |  1 +
> > > > >   .../platform/qcom/iris/iris_platform_common.h      |  5 ++
> > > > >   .../media/platform/qcom/iris/iris_platform_gen2.c  | 99 ++++++++++++++++++++++
> > > > >   .../platform/qcom/iris/iris_platform_glymur.c      | 97 +++++++++++++++++++++
> > > > >   .../platform/qcom/iris/iris_platform_glymur.h      | 17 ++++
> > > > >   drivers/media/platform/qcom/iris/iris_probe.c      |  4 +
> > > > >   6 files changed, 223 insertions(+)
> > > > > 
> > 
> 

-- 
With best wishes
Dmitry