On Tue, 4 Jul 2023 at 19:10, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > > > On 7/4/2023 4:52 AM, Dmitry Baryshkov wrote: > > On Tue, 4 Jul 2023 at 13:06, Dmitry Baryshkov > > <dmitry.baryshkov@xxxxxxxxxx> wrote: > >> > >> On Tue, 4 Jul 2023 at 07:04, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > >>> > >>> > >>> > >>> On 7/3/2023 7:20 PM, Dmitry Baryshkov wrote: > >>>> On 03/07/2023 05:01, Abhinav Kumar wrote: > >>>>> > >>>>> > >>>>> On 6/19/2023 2:25 PM, Dmitry Baryshkov wrote: > >>>>>> There is always a single MDP TOP block. Drop the mdp_count field and > >>>>>> stop declaring dpu_mdp_cfg instances as arrays. > >>>>>> > >>>>>> Tested-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> > >>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > >>>>>> --- > >>>>> > >>>>> The change drops mdp_count and stops using the array which is fine and > >>>>> I will support that. > >>>>> > >>>>> But looking at the pattern I saw while using core_revision, both > >>>>> DPU_MDP_VSYNC_SEL and DPU_MDP_AUDIO_SELECT can also be dropped from > >>>>> the catalog in favor of using core_revision. > >>>>> > >>>>> Hence for that, I request you not to stop passing dpu_mdss_cfg to > >>>>> dpu_hw_mdptop_init as that has the necessary information of > >>>>> core_revision. > >>>> > >>>> Sure, I'll restore it. Please note, however, that it might be better to > >>>> pass struct dpu_caps instead of the full struct dpu_mdss_cfg. > >>>> > >>> > >>> Thanks for restoring. > >>> > >>> Can you pls explain this better? dpu_core_rev is part of dpu_mdss_cfg, > >>> so dpu_caps wont be enough for this one. > >> > >> Oh, true. For some reason I thought that version is a part of dpu_caps. > > > > And after additional thought. Maybe it would be better to add a > > separate struct dpu_mdss_version and pass it to the hw block init > > functions? > > > > I would like to see this evolve. Today, we are assuming that only the hw > block init functions are the places we would use those. > > From what I recall, the DSC over DP series needed the core_revision in > the timing gen code somewhere. I hope you are talking about the DPU driver here, not about the DP driver. For the DP driver please use struct msm_dp_desc. > > If we see that pattern is possible once that lands, why not. > > Right now, I would leave it at dpu_mdss_cfg. > > >> > >>> > >>>>> > >>>>>> .../msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_6_4_sm6350.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_6_9_sm6375.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 7 +--- > >>>>>> .../msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 7 +--- > >>>>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 - > >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 38 +++---------------- > >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 8 ++-- > >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 +- > >>>>>> 19 files changed, 41 insertions(+), 115 deletions(-) > >>>> > >> > >> > >> > >> -- > >> With best wishes > >> Dmitry > > > > > > -- With best wishes Dmitry