Message ID | 20231208050641.32582-6-quic_abhinavk@quicinc.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | [v2,01/16] drm/msm/dpu: add formats check for writeback encoder | expand |
On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > Add CDM blocks to the sc7280 dpu_hw_catalog to support > YUV format output from writeback block. > > changes in v2: > - remove explicit zero assignment for features > - move sc7280_cdm to dpu_hw_catalog from the sc7280 > catalog file as its definition can be re-used > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > --- > .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ > 4 files changed, 29 insertions(+) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > Add CDM blocks to the sc7280 dpu_hw_catalog to support > YUV format output from writeback block. > > changes in v2: > - remove explicit zero assignment for features > - move sc7280_cdm to dpu_hw_catalog from the sc7280 > catalog file as its definition can be re-used > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > --- > .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ > 4 files changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > index 209675de6742..19c2b7454796 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { > .mdss_ver = &sc7280_mdss_ver, > .caps = &sc7280_dpu_caps, > .mdp = &sc7280_mdp, > + .cdm = &sc7280_cdm, > .ctl_count = ARRAY_SIZE(sc7280_ctl), > .ctl = sc7280_ctl, > .sspp_count = ARRAY_SIZE(sc7280_sspp), > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > index d52aae54bbd5..1be3156cde05 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { > .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, > }; > > +/************************************************************* > + * CDM sub block config Nit: it is not a subblock config. > + *************************************************************/ > +static const struct dpu_cdm_cfg sc7280_cdm = { I know that I have r-b'ed this patch. But then one thing occurred to me. If this definition is common to all (or almost all) platforms, can we just call it dpu_cdm or dpu_common_cdm? > + .name = "cdm_0", > + .id = CDM_0, > + .len = 0x228, > + .base = 0x79200, > +}; > + > /************************************************************* > * VBIF sub blocks config > *************************************************************/ > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > index e3c0d007481b..ba82ef4560a6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { > u32 memtype[MAX_XIN_COUNT]; > }; > > +/** > + * struct dpu_cdm_cfg - information of chroma down blocks > + * @name string name for debug purposes > + * @id enum identifying this block > + * @base register offset of this block > + * @features bit mask identifying sub-blocks/features > + */ > +struct dpu_cdm_cfg { > + DPU_HW_BLK_INFO; > +}; > + > /** > * Define CDP use cases > * @DPU_PERF_CDP_UDAGE_RT: real-time use cases > @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { > u32 wb_count; > const struct dpu_wb_cfg *wb; > > + const struct dpu_cdm_cfg *cdm; > + > u32 ad_count; > > u32 dspp_count; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > index a6702b2bfc68..f319c8232ea5 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > @@ -185,6 +185,11 @@ enum dpu_dsc { > DSC_MAX > }; > > +enum dpu_cdm { > + CDM_0 = 1, > + CDM_MAX > +}; > + > enum dpu_pingpong { > PINGPONG_NONE, > PINGPONG_0, > -- > 2.40.1 >
On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> Add CDM blocks to the sc7280 dpu_hw_catalog to support >> YUV format output from writeback block. >> >> changes in v2: >> - remove explicit zero assignment for features >> - move sc7280_cdm to dpu_hw_catalog from the sc7280 >> catalog file as its definition can be re-used >> >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> --- >> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ >> 4 files changed, 29 insertions(+) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h >> index 209675de6742..19c2b7454796 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h >> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { >> .mdss_ver = &sc7280_mdss_ver, >> .caps = &sc7280_dpu_caps, >> .mdp = &sc7280_mdp, >> + .cdm = &sc7280_cdm, >> .ctl_count = ARRAY_SIZE(sc7280_ctl), >> .ctl = sc7280_ctl, >> .sspp_count = ARRAY_SIZE(sc7280_sspp), >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >> index d52aae54bbd5..1be3156cde05 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { >> .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, >> }; >> >> +/************************************************************* >> + * CDM sub block config > > Nit: it is not a subblock config. > Ack. >> + *************************************************************/ >> +static const struct dpu_cdm_cfg sc7280_cdm = { > > I know that I have r-b'ed this patch. But then one thing occurred to > me. If this definition is common to all (or almost all) platforms, can > we just call it dpu_cdm or dpu_common_cdm? > >> + .name = "cdm_0", >> + .id = CDM_0, >> + .len = 0x228, >> + .base = 0x79200, >> +}; hmmm .... almost common but not entirely ... msm8998's CDM has a shorter len of 0x224 :( >> + >> /************************************************************* >> * VBIF sub blocks config >> *************************************************************/ >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >> index e3c0d007481b..ba82ef4560a6 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >> @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { >> u32 memtype[MAX_XIN_COUNT]; >> }; >> >> +/** >> + * struct dpu_cdm_cfg - information of chroma down blocks >> + * @name string name for debug purposes >> + * @id enum identifying this block >> + * @base register offset of this block >> + * @features bit mask identifying sub-blocks/features >> + */ >> +struct dpu_cdm_cfg { >> + DPU_HW_BLK_INFO; >> +}; >> + >> /** >> * Define CDP use cases >> * @DPU_PERF_CDP_UDAGE_RT: real-time use cases >> @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { >> u32 wb_count; >> const struct dpu_wb_cfg *wb; >> >> + const struct dpu_cdm_cfg *cdm; >> + >> u32 ad_count; >> >> u32 dspp_count; >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h >> index a6702b2bfc68..f319c8232ea5 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h >> @@ -185,6 +185,11 @@ enum dpu_dsc { >> DSC_MAX >> }; >> >> +enum dpu_cdm { >> + CDM_0 = 1, >> + CDM_MAX >> +}; >> + >> enum dpu_pingpong { >> PINGPONG_NONE, >> PINGPONG_0, >> -- >> 2.40.1 >> > >
On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: > > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >> > >> Add CDM blocks to the sc7280 dpu_hw_catalog to support > >> YUV format output from writeback block. > >> > >> changes in v2: > >> - remove explicit zero assignment for features > >> - move sc7280_cdm to dpu_hw_catalog from the sc7280 > >> catalog file as its definition can be re-used > >> > >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >> --- > >> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ > >> 4 files changed, 29 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> index 209675de6742..19c2b7454796 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { > >> .mdss_ver = &sc7280_mdss_ver, > >> .caps = &sc7280_dpu_caps, > >> .mdp = &sc7280_mdp, > >> + .cdm = &sc7280_cdm, > >> .ctl_count = ARRAY_SIZE(sc7280_ctl), > >> .ctl = sc7280_ctl, > >> .sspp_count = ARRAY_SIZE(sc7280_sspp), > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> index d52aae54bbd5..1be3156cde05 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { > >> .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, > >> }; > >> > >> +/************************************************************* > >> + * CDM sub block config > > > > Nit: it is not a subblock config. > > > > Ack. > > >> + *************************************************************/ > >> +static const struct dpu_cdm_cfg sc7280_cdm = { > > > > I know that I have r-b'ed this patch. But then one thing occurred to > > me. If this definition is common to all (or almost all) platforms, can > > we just call it dpu_cdm or dpu_common_cdm? > > > >> + .name = "cdm_0", > >> + .id = CDM_0, > >> + .len = 0x228, > >> + .base = 0x79200, > >> +}; > > hmmm .... almost common but not entirely ... msm8998's CDM has a shorter > len of 0x224 :( Then sdm845_cdm? > > >> + > >> /************************************************************* > >> * VBIF sub blocks config > >> *************************************************************/ > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >> index e3c0d007481b..ba82ef4560a6 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >> @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { > >> u32 memtype[MAX_XIN_COUNT]; > >> }; > >> > >> +/** > >> + * struct dpu_cdm_cfg - information of chroma down blocks > >> + * @name string name for debug purposes > >> + * @id enum identifying this block > >> + * @base register offset of this block > >> + * @features bit mask identifying sub-blocks/features > >> + */ > >> +struct dpu_cdm_cfg { > >> + DPU_HW_BLK_INFO; > >> +}; > >> + > >> /** > >> * Define CDP use cases > >> * @DPU_PERF_CDP_UDAGE_RT: real-time use cases > >> @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { > >> u32 wb_count; > >> const struct dpu_wb_cfg *wb; > >> > >> + const struct dpu_cdm_cfg *cdm; > >> + > >> u32 ad_count; > >> > >> u32 dspp_count; > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >> index a6702b2bfc68..f319c8232ea5 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >> @@ -185,6 +185,11 @@ enum dpu_dsc { > >> DSC_MAX > >> }; > >> > >> +enum dpu_cdm { > >> + CDM_0 = 1, > >> + CDM_MAX > >> +}; > >> + > >> enum dpu_pingpong { > >> PINGPONG_NONE, > >> PINGPONG_0, > >> -- > >> 2.40.1 > >> > > > >
On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote: > On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: >>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >>>> >>>> Add CDM blocks to the sc7280 dpu_hw_catalog to support >>>> YUV format output from writeback block. >>>> >>>> changes in v2: >>>> - remove explicit zero assignment for features >>>> - move sc7280_cdm to dpu_hw_catalog from the sc7280 >>>> catalog file as its definition can be re-used >>>> >>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>>> --- >>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ >>>> 4 files changed, 29 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h >>>> index 209675de6742..19c2b7454796 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h >>>> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { >>>> .mdss_ver = &sc7280_mdss_ver, >>>> .caps = &sc7280_dpu_caps, >>>> .mdp = &sc7280_mdp, >>>> + .cdm = &sc7280_cdm, >>>> .ctl_count = ARRAY_SIZE(sc7280_ctl), >>>> .ctl = sc7280_ctl, >>>> .sspp_count = ARRAY_SIZE(sc7280_sspp), >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>> index d52aae54bbd5..1be3156cde05 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { >>>> .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, >>>> }; >>>> >>>> +/************************************************************* >>>> + * CDM sub block config >>> >>> Nit: it is not a subblock config. >>> >> >> Ack. >> >>>> + *************************************************************/ >>>> +static const struct dpu_cdm_cfg sc7280_cdm = { >>> >>> I know that I have r-b'ed this patch. But then one thing occurred to >>> me. If this definition is common to all (or almost all) platforms, can >>> we just call it dpu_cdm or dpu_common_cdm? >>> >>>> + .name = "cdm_0", >>>> + .id = CDM_0, >>>> + .len = 0x228, >>>> + .base = 0x79200, >>>> +}; >> >> hmmm .... almost common but not entirely ... msm8998's CDM has a shorter >> len of 0x224 :( > > Then sdm845_cdm? > That also has a shorter cdm length. BTW, sdm845 is not in this series. It will be part of RFT as we discussed. >> >>>> + >>>> /************************************************************* >>>> * VBIF sub blocks config >>>> *************************************************************/ >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >>>> index e3c0d007481b..ba82ef4560a6 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >>>> @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { >>>> u32 memtype[MAX_XIN_COUNT]; >>>> }; >>>> >>>> +/** >>>> + * struct dpu_cdm_cfg - information of chroma down blocks >>>> + * @name string name for debug purposes >>>> + * @id enum identifying this block >>>> + * @base register offset of this block >>>> + * @features bit mask identifying sub-blocks/features >>>> + */ >>>> +struct dpu_cdm_cfg { >>>> + DPU_HW_BLK_INFO; >>>> +}; >>>> + >>>> /** >>>> * Define CDP use cases >>>> * @DPU_PERF_CDP_UDAGE_RT: real-time use cases >>>> @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { >>>> u32 wb_count; >>>> const struct dpu_wb_cfg *wb; >>>> >>>> + const struct dpu_cdm_cfg *cdm; >>>> + >>>> u32 ad_count; >>>> >>>> u32 dspp_count; >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h >>>> index a6702b2bfc68..f319c8232ea5 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h >>>> @@ -185,6 +185,11 @@ enum dpu_dsc { >>>> DSC_MAX >>>> }; >>>> >>>> +enum dpu_cdm { >>>> + CDM_0 = 1, >>>> + CDM_MAX >>>> +}; >>>> + >>>> enum dpu_pingpong { >>>> PINGPONG_NONE, >>>> PINGPONG_0, >>>> -- >>>> 2.40.1 >>>> >>> >>> > > >
On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote: > > On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >> > >> > >> > >> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: > >>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >>>> > >>>> Add CDM blocks to the sc7280 dpu_hw_catalog to support > >>>> YUV format output from writeback block. > >>>> > >>>> changes in v2: > >>>> - remove explicit zero assignment for features > >>>> - move sc7280_cdm to dpu_hw_catalog from the sc7280 > >>>> catalog file as its definition can be re-used > >>>> > >>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >>>> --- > >>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + > >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ > >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ > >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ > >>>> 4 files changed, 29 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >>>> index 209675de6742..19c2b7454796 100644 > >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >>>> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { > >>>> .mdss_ver = &sc7280_mdss_ver, > >>>> .caps = &sc7280_dpu_caps, > >>>> .mdp = &sc7280_mdp, > >>>> + .cdm = &sc7280_cdm, > >>>> .ctl_count = ARRAY_SIZE(sc7280_ctl), > >>>> .ctl = sc7280_ctl, > >>>> .sspp_count = ARRAY_SIZE(sc7280_sspp), > >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>>> index d52aae54bbd5..1be3156cde05 100644 > >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>>> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { > >>>> .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, > >>>> }; > >>>> > >>>> +/************************************************************* > >>>> + * CDM sub block config > >>> > >>> Nit: it is not a subblock config. > >>> > >> > >> Ack. > >> > >>>> + *************************************************************/ > >>>> +static const struct dpu_cdm_cfg sc7280_cdm = { > >>> > >>> I know that I have r-b'ed this patch. But then one thing occurred to > >>> me. If this definition is common to all (or almost all) platforms, can > >>> we just call it dpu_cdm or dpu_common_cdm? > >>> > >>>> + .name = "cdm_0", > >>>> + .id = CDM_0, > >>>> + .len = 0x228, > >>>> + .base = 0x79200, > >>>> +}; > >> > >> hmmm .... almost common but not entirely ... msm8998's CDM has a shorter > >> len of 0x224 :( > > > > Then sdm845_cdm? > > > > That also has a shorter cdm length. Could you please clarify. According to the downstream DT files all CDM blocks up to (but not including) sm8550 had length 0x224. SM8550 and SM8650 got qcom,sde-cdm-size = <0x220>. But I don't see any registers after 0x204. > > BTW, sdm845 is not in this series. It will be part of RFT as we discussed. > > >> > >>>> + > >>>> /************************************************************* > >>>> * VBIF sub blocks config > >>>> *************************************************************/ > >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >>>> index e3c0d007481b..ba82ef4560a6 100644 > >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >>>> @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { > >>>> u32 memtype[MAX_XIN_COUNT]; > >>>> }; > >>>> > >>>> +/** > >>>> + * struct dpu_cdm_cfg - information of chroma down blocks > >>>> + * @name string name for debug purposes > >>>> + * @id enum identifying this block > >>>> + * @base register offset of this block > >>>> + * @features bit mask identifying sub-blocks/features > >>>> + */ > >>>> +struct dpu_cdm_cfg { > >>>> + DPU_HW_BLK_INFO; > >>>> +}; > >>>> + > >>>> /** > >>>> * Define CDP use cases > >>>> * @DPU_PERF_CDP_UDAGE_RT: real-time use cases > >>>> @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { > >>>> u32 wb_count; > >>>> const struct dpu_wb_cfg *wb; > >>>> > >>>> + const struct dpu_cdm_cfg *cdm; > >>>> + > >>>> u32 ad_count; > >>>> > >>>> u32 dspp_count; > >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >>>> index a6702b2bfc68..f319c8232ea5 100644 > >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >>>> @@ -185,6 +185,11 @@ enum dpu_dsc { > >>>> DSC_MAX > >>>> }; > >>>> > >>>> +enum dpu_cdm { > >>>> + CDM_0 = 1, > >>>> + CDM_MAX > >>>> +}; > >>>> + > >>>> enum dpu_pingpong { > >>>> PINGPONG_NONE, > >>>> PINGPONG_0, > >>>> -- > >>>> 2.40.1 > >>>> > >>> > >>> > > > > > >
On 12/11/2023 1:42 PM, Dmitry Baryshkov wrote: > On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote: >>> On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >>>> >>>> >>>> >>>> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: >>>>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >>>>>> >>>>>> Add CDM blocks to the sc7280 dpu_hw_catalog to support >>>>>> YUV format output from writeback block. >>>>>> >>>>>> changes in v2: >>>>>> - remove explicit zero assignment for features >>>>>> - move sc7280_cdm to dpu_hw_catalog from the sc7280 >>>>>> catalog file as its definition can be re-used >>>>>> >>>>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>>>>> --- >>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ >>>>>> 4 files changed, 29 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h >>>>>> index 209675de6742..19c2b7454796 100644 >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h >>>>>> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { >>>>>> .mdss_ver = &sc7280_mdss_ver, >>>>>> .caps = &sc7280_dpu_caps, >>>>>> .mdp = &sc7280_mdp, >>>>>> + .cdm = &sc7280_cdm, >>>>>> .ctl_count = ARRAY_SIZE(sc7280_ctl), >>>>>> .ctl = sc7280_ctl, >>>>>> .sspp_count = ARRAY_SIZE(sc7280_sspp), >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>>>> index d52aae54bbd5..1be3156cde05 100644 >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>>>> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { >>>>>> .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, >>>>>> }; >>>>>> >>>>>> +/************************************************************* >>>>>> + * CDM sub block config >>>>> >>>>> Nit: it is not a subblock config. >>>>> >>>> >>>> Ack. >>>> >>>>>> + *************************************************************/ >>>>>> +static const struct dpu_cdm_cfg sc7280_cdm = { >>>>> >>>>> I know that I have r-b'ed this patch. But then one thing occurred to >>>>> me. If this definition is common to all (or almost all) platforms, can >>>>> we just call it dpu_cdm or dpu_common_cdm? >>>>> >>>>>> + .name = "cdm_0", >>>>>> + .id = CDM_0, >>>>>> + .len = 0x228, >>>>>> + .base = 0x79200, >>>>>> +}; >>>> >>>> hmmm .... almost common but not entirely ... msm8998's CDM has a shorter >>>> len of 0x224 :( >>> >>> Then sdm845_cdm? >>> >> >> That also has a shorter cdm length. > > Could you please clarify. According to the downstream DT files all CDM > blocks up to (but not including) sm8550 had length 0x224. SM8550 and > SM8650 got qcom,sde-cdm-size = <0x220>. But I don't see any registers > after 0x204. >> We always list 0x4 more than the last offset. In chipsets sdm845 and msm8998, I only see the last offset of CDM as 0x220 but in sm8250 and sc7280, the last offset is 0x224. Hence the total length is more in sc7280/sm8250 as compared to sdm845/msm8998. I didnt follow that you do not see any registers after 0x204. The CDM_MUX is the last offset which has an offset 0x224 from the base address. So thats the last offset. The newer chipsets have CDM_MUX and the older ones did not. Hence the difference in length. >> BTW, sdm845 is not in this series. It will be part of RFT as we discussed. >> >>>> >>>>>> + >>>>>> /************************************************************* >>>>>> * VBIF sub blocks config >>>>>> *************************************************************/ >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >>>>>> index e3c0d007481b..ba82ef4560a6 100644 >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >>>>>> @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { >>>>>> u32 memtype[MAX_XIN_COUNT]; >>>>>> }; >>>>>> >>>>>> +/** >>>>>> + * struct dpu_cdm_cfg - information of chroma down blocks >>>>>> + * @name string name for debug purposes >>>>>> + * @id enum identifying this block >>>>>> + * @base register offset of this block >>>>>> + * @features bit mask identifying sub-blocks/features >>>>>> + */ >>>>>> +struct dpu_cdm_cfg { >>>>>> + DPU_HW_BLK_INFO; >>>>>> +}; >>>>>> + >>>>>> /** >>>>>> * Define CDP use cases >>>>>> * @DPU_PERF_CDP_UDAGE_RT: real-time use cases >>>>>> @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { >>>>>> u32 wb_count; >>>>>> const struct dpu_wb_cfg *wb; >>>>>> >>>>>> + const struct dpu_cdm_cfg *cdm; >>>>>> + >>>>>> u32 ad_count; >>>>>> >>>>>> u32 dspp_count; >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h >>>>>> index a6702b2bfc68..f319c8232ea5 100644 >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h >>>>>> @@ -185,6 +185,11 @@ enum dpu_dsc { >>>>>> DSC_MAX >>>>>> }; >>>>>> >>>>>> +enum dpu_cdm { >>>>>> + CDM_0 = 1, >>>>>> + CDM_MAX >>>>>> +}; >>>>>> + >>>>>> enum dpu_pingpong { >>>>>> PINGPONG_NONE, >>>>>> PINGPONG_0, >>>>>> -- >>>>>> 2.40.1 >>>>>> >>>>> >>>>> >>> >>> >>> > > >
On Mon, 11 Dec 2023 at 23:48, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 12/11/2023 1:42 PM, Dmitry Baryshkov wrote: > > On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >> > >> > >> > >> On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote: > >>> On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >>>> > >>>> > >>>> > >>>> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: > >>>>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >>>>>> > >>>>>> Add CDM blocks to the sc7280 dpu_hw_catalog to support > >>>>>> YUV format output from writeback block. > >>>>>> > >>>>>> changes in v2: > >>>>>> - remove explicit zero assignment for features > >>>>>> - move sc7280_cdm to dpu_hw_catalog from the sc7280 > >>>>>> catalog file as its definition can be re-used > >>>>>> > >>>>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >>>>>> --- > >>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + > >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ > >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ > >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ > >>>>>> 4 files changed, 29 insertions(+) > >>>>>> > >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >>>>>> index 209675de6742..19c2b7454796 100644 > >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >>>>>> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { > >>>>>> .mdss_ver = &sc7280_mdss_ver, > >>>>>> .caps = &sc7280_dpu_caps, > >>>>>> .mdp = &sc7280_mdp, > >>>>>> + .cdm = &sc7280_cdm, > >>>>>> .ctl_count = ARRAY_SIZE(sc7280_ctl), > >>>>>> .ctl = sc7280_ctl, > >>>>>> .sspp_count = ARRAY_SIZE(sc7280_sspp), > >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>>>>> index d52aae54bbd5..1be3156cde05 100644 > >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>>>>> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { > >>>>>> .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, > >>>>>> }; > >>>>>> > >>>>>> +/************************************************************* > >>>>>> + * CDM sub block config > >>>>> > >>>>> Nit: it is not a subblock config. > >>>>> > >>>> > >>>> Ack. > >>>> > >>>>>> + *************************************************************/ > >>>>>> +static const struct dpu_cdm_cfg sc7280_cdm = { > >>>>> > >>>>> I know that I have r-b'ed this patch. But then one thing occurred to > >>>>> me. If this definition is common to all (or almost all) platforms, can > >>>>> we just call it dpu_cdm or dpu_common_cdm? > >>>>> > >>>>>> + .name = "cdm_0", > >>>>>> + .id = CDM_0, > >>>>>> + .len = 0x228, > >>>>>> + .base = 0x79200, > >>>>>> +}; > >>>> > >>>> hmmm .... almost common but not entirely ... msm8998's CDM has a shorter > >>>> len of 0x224 :( > >>> > >>> Then sdm845_cdm? > >>> > >> > >> That also has a shorter cdm length. > > > > Could you please clarify. According to the downstream DT files all CDM > > blocks up to (but not including) sm8550 had length 0x224. SM8550 and > > SM8650 got qcom,sde-cdm-size = <0x220>. But I don't see any registers > > after 0x204. > >> > > We always list 0x4 more than the last offset. Yes, so this makes it correct for several latest DT files, which have qcom,sde-cdm-size = <0x220>. However all the previous DT files (from msm8998 to sm8450) had qcom,sde-cdm-size = <0x224>; > > In chipsets sdm845 and msm8998, I only see the last offset of CDM as > 0x220 but in sm8250 and sc7280, the last offset is 0x224. Hence the > total length is more in sc7280/sm8250 as compared to sdm845/msm8998. > > I didnt follow that you do not see any registers after 0x204. > > The CDM_MUX is the last offset which has an offset 0x224 from the base > address. So thats the last offset. Ack > > The newer chipsets have CDM_MUX and the older ones did not. Hence the > difference in length. > > >> BTW, sdm845 is not in this series. It will be part of RFT as we discussed. > >> > >>>> > >>>>>> + > >>>>>> /************************************************************* > >>>>>> * VBIF sub blocks config > >>>>>> *************************************************************/ > >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >>>>>> index e3c0d007481b..ba82ef4560a6 100644 > >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >>>>>> @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { > >>>>>> u32 memtype[MAX_XIN_COUNT]; > >>>>>> }; > >>>>>> > >>>>>> +/** > >>>>>> + * struct dpu_cdm_cfg - information of chroma down blocks > >>>>>> + * @name string name for debug purposes > >>>>>> + * @id enum identifying this block > >>>>>> + * @base register offset of this block > >>>>>> + * @features bit mask identifying sub-blocks/features > >>>>>> + */ > >>>>>> +struct dpu_cdm_cfg { > >>>>>> + DPU_HW_BLK_INFO; > >>>>>> +}; > >>>>>> + > >>>>>> /** > >>>>>> * Define CDP use cases > >>>>>> * @DPU_PERF_CDP_UDAGE_RT: real-time use cases > >>>>>> @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { > >>>>>> u32 wb_count; > >>>>>> const struct dpu_wb_cfg *wb; > >>>>>> > >>>>>> + const struct dpu_cdm_cfg *cdm; > >>>>>> + > >>>>>> u32 ad_count; > >>>>>> > >>>>>> u32 dspp_count; > >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >>>>>> index a6702b2bfc68..f319c8232ea5 100644 > >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >>>>>> @@ -185,6 +185,11 @@ enum dpu_dsc { > >>>>>> DSC_MAX > >>>>>> }; > >>>>>> > >>>>>> +enum dpu_cdm { > >>>>>> + CDM_0 = 1, > >>>>>> + CDM_MAX > >>>>>> +}; > >>>>>> + > >>>>>> enum dpu_pingpong { > >>>>>> PINGPONG_NONE, > >>>>>> PINGPONG_0, > >>>>>> -- > >>>>>> 2.40.1 > >>>>>> > >>>>> > >>>>> > >>> > >>> > >>> > > > > > >
Hi Abhinav, On Tue, 12 Dec 2023 at 08:49, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Mon, 11 Dec 2023 at 23:48, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > > > > > On 12/11/2023 1:42 PM, Dmitry Baryshkov wrote: > > > On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > >> > > >> > > >> > > >> On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote: > > >>> On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > >>>> > > >>>> > > >>>> > > >>>> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: > > >>>>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > >>>>>> > > >>>>>> Add CDM blocks to the sc7280 dpu_hw_catalog to support > > >>>>>> YUV format output from writeback block. > > >>>>>> > > >>>>>> changes in v2: > > >>>>>> - remove explicit zero assignment for features > > >>>>>> - move sc7280_cdm to dpu_hw_catalog from the sc7280 > > >>>>>> catalog file as its definition can be re-used > > >>>>>> > > >>>>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > > >>>>>> --- > > >>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + > > >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ > > >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ > > >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ > > >>>>>> 4 files changed, 29 insertions(+) > > >>>>>> > > >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > > >>>>>> index 209675de6742..19c2b7454796 100644 > > >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > > >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > > >>>>>> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { > > >>>>>> .mdss_ver = &sc7280_mdss_ver, > > >>>>>> .caps = &sc7280_dpu_caps, > > >>>>>> .mdp = &sc7280_mdp, > > >>>>>> + .cdm = &sc7280_cdm, > > >>>>>> .ctl_count = ARRAY_SIZE(sc7280_ctl), > > >>>>>> .ctl = sc7280_ctl, > > >>>>>> .sspp_count = ARRAY_SIZE(sc7280_sspp), > > >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > >>>>>> index d52aae54bbd5..1be3156cde05 100644 > > >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > >>>>>> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { > > >>>>>> .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, > > >>>>>> }; > > >>>>>> > > >>>>>> +/************************************************************* > > >>>>>> + * CDM sub block config > > >>>>> > > >>>>> Nit: it is not a subblock config. > > >>>>> > > >>>> > > >>>> Ack. > > >>>> > > >>>>>> + *************************************************************/ > > >>>>>> +static const struct dpu_cdm_cfg sc7280_cdm = { > > >>>>> > > >>>>> I know that I have r-b'ed this patch. But then one thing occurred to > > >>>>> me. If this definition is common to all (or almost all) platforms, can > > >>>>> we just call it dpu_cdm or dpu_common_cdm? > > >>>>> > > >>>>>> + .name = "cdm_0", > > >>>>>> + .id = CDM_0, > > >>>>>> + .len = 0x228, > > >>>>>> + .base = 0x79200, > > >>>>>> +}; > > >>>> > > >>>> hmmm .... almost common but not entirely ... msm8998's CDM has a shorter > > >>>> len of 0x224 :( > > >>> > > >>> Then sdm845_cdm? > > >>> > > >> > > >> That also has a shorter cdm length. > > > > > > Could you please clarify. According to the downstream DT files all CDM > > > blocks up to (but not including) sm8550 had length 0x224. SM8550 and > > > SM8650 got qcom,sde-cdm-size = <0x220>. But I don't see any registers > > > after 0x204. > > >> > > > > We always list 0x4 more than the last offset. > > Yes, so this makes it correct for several latest DT files, which have > qcom,sde-cdm-size = <0x220>. > However all the previous DT files (from msm8998 to sm8450) had > qcom,sde-cdm-size = <0x224>; Ok, I think I got it, what you were writing about. And we can ignore the sde-cdm-size from the DT files. > > > > > In chipsets sdm845 and msm8998, I only see the last offset of CDM as > > 0x220 but in sm8250 and sc7280, the last offset is 0x224. Hence the > > total length is more in sc7280/sm8250 as compared to sdm845/msm8998. > > > > I didnt follow that you do not see any registers after 0x204. > > > > The CDM_MUX is the last offset which has an offset 0x224 from the base > > address. So thats the last offset. > > Ack > > > > > The newer chipsets have CDM_MUX and the older ones did not. Hence the > > difference in length. > > > > >> BTW, sdm845 is not in this series. It will be part of RFT as we discussed. > > >> > > >>>> > > >>>>>> + > > >>>>>> /************************************************************* > > >>>>>> * VBIF sub blocks config > > >>>>>> *************************************************************/ > > >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > > >>>>>> index e3c0d007481b..ba82ef4560a6 100644 > > >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > > >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > > >>>>>> @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { > > >>>>>> u32 memtype[MAX_XIN_COUNT]; > > >>>>>> }; > > >>>>>> > > >>>>>> +/** > > >>>>>> + * struct dpu_cdm_cfg - information of chroma down blocks > > >>>>>> + * @name string name for debug purposes > > >>>>>> + * @id enum identifying this block > > >>>>>> + * @base register offset of this block > > >>>>>> + * @features bit mask identifying sub-blocks/features > > >>>>>> + */ > > >>>>>> +struct dpu_cdm_cfg { > > >>>>>> + DPU_HW_BLK_INFO; > > >>>>>> +}; > > >>>>>> + > > >>>>>> /** > > >>>>>> * Define CDP use cases > > >>>>>> * @DPU_PERF_CDP_UDAGE_RT: real-time use cases > > >>>>>> @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { > > >>>>>> u32 wb_count; > > >>>>>> const struct dpu_wb_cfg *wb; > > >>>>>> > > >>>>>> + const struct dpu_cdm_cfg *cdm; > > >>>>>> + > > >>>>>> u32 ad_count; > > >>>>>> > > >>>>>> u32 dspp_count; > > >>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > > >>>>>> index a6702b2bfc68..f319c8232ea5 100644 > > >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > > >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > > >>>>>> @@ -185,6 +185,11 @@ enum dpu_dsc { > > >>>>>> DSC_MAX > > >>>>>> }; > > >>>>>> > > >>>>>> +enum dpu_cdm { > > >>>>>> + CDM_0 = 1, > > >>>>>> + CDM_MAX > > >>>>>> +}; > > >>>>>> + > > >>>>>> enum dpu_pingpong { > > >>>>>> PINGPONG_NONE, > > >>>>>> PINGPONG_0, > > >>>>>> -- > > >>>>>> 2.40.1 > > >>>>>> > > >>>>> > > >>>>> > > >>> > > >>> > > >>> > > > > > > > > > > > > > -- > With best wishes > Dmitry
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h index 209675de6742..19c2b7454796 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { .mdss_ver = &sc7280_mdss_ver, .caps = &sc7280_dpu_caps, .mdp = &sc7280_mdp, + .cdm = &sc7280_cdm, .ctl_count = ARRAY_SIZE(sc7280_ctl), .ctl = sc7280_ctl, .sspp_count = ARRAY_SIZE(sc7280_sspp), diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index d52aae54bbd5..1be3156cde05 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, }; +/************************************************************* + * CDM sub block config + *************************************************************/ +static const struct dpu_cdm_cfg sc7280_cdm = { + .name = "cdm_0", + .id = CDM_0, + .len = 0x228, + .base = 0x79200, +}; + /************************************************************* * VBIF sub blocks config *************************************************************/ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index e3c0d007481b..ba82ef4560a6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { u32 memtype[MAX_XIN_COUNT]; }; +/** + * struct dpu_cdm_cfg - information of chroma down blocks + * @name string name for debug purposes + * @id enum identifying this block + * @base register offset of this block + * @features bit mask identifying sub-blocks/features + */ +struct dpu_cdm_cfg { + DPU_HW_BLK_INFO; +}; + /** * Define CDP use cases * @DPU_PERF_CDP_UDAGE_RT: real-time use cases @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { u32 wb_count; const struct dpu_wb_cfg *wb; + const struct dpu_cdm_cfg *cdm; + u32 ad_count; u32 dspp_count; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h index a6702b2bfc68..f319c8232ea5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h @@ -185,6 +185,11 @@ enum dpu_dsc { DSC_MAX }; +enum dpu_cdm { + CDM_0 = 1, + CDM_MAX +}; + enum dpu_pingpong { PINGPONG_NONE, PINGPONG_0,
Add CDM blocks to the sc7280 dpu_hw_catalog to support YUV format output from writeback block. changes in v2: - remove explicit zero assignment for features - move sc7280_cdm to dpu_hw_catalog from the sc7280 catalog file as its definition can be re-used Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 +++++ 4 files changed, 29 insertions(+)