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 <[email protected]>
---
.../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
+ *************************************************************/
+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,
--
2.40.1
On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> 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 <[email protected]>
> ---
> .../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 <[email protected]>
--
With best wishes
Dmitry
On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> 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 <[email protected]>
> ---
> .../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
>
--
With best wishes
Dmitry
On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote:
> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> 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 <[email protected]>
>> ---
>> .../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 <[email protected]> wrote:
>
>
>
> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote:
> > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> 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 <[email protected]>
> >> ---
> >> .../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
> >>
> >
> >
--
With best wishes
Dmitry
On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote:
> On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <[email protected]> wrote:
>>
>>
>>
>> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote:
>>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> 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 <[email protected]>
>>>> ---
>>>> .../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 <[email protected]> wrote:
>
>
>
> On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote:
> > On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <[email protected]> wrote:
> >>
> >>
> >>
> >> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote:
> >>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> 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 <[email protected]>
> >>>> ---
> >>>> .../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
> >>>>
> >>>
> >>>
> >
> >
> >
--
With best wishes
Dmitry
On 12/11/2023 1:42 PM, Dmitry Baryshkov wrote:
> On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar <[email protected]> wrote:
>>
>>
>>
>> On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote:
>>> On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote:
>>>>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> 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 <[email protected]>
>>>>>> ---
>>>>>> .../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 <[email protected]> wrote:
>
>
>
> On 12/11/2023 1:42 PM, Dmitry Baryshkov wrote:
> > On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar <[email protected]> wrote:
> >>
> >>
> >>
> >> On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote:
> >>> On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <[email protected]> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote:
> >>>>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> 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 <[email protected]>
> >>>>>> ---
> >>>>>> .../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
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>>
> >
> >
> >
--
With best wishes
Dmitry
Hi Abhinav,
On Tue, 12 Dec 2023 at 08:49, Dmitry Baryshkov
<[email protected]> wrote:
>
> On Mon, 11 Dec 2023 at 23:48, Abhinav Kumar <[email protected]> wrote:
> >
> >
> >
> > On 12/11/2023 1:42 PM, Dmitry Baryshkov wrote:
> > > On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar <[email protected]> wrote:
> > >>
> > >>
> > >>
> > >> On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote:
> > >>> On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar <[email protected]> wrote:
> > >>>>
> > >>>>
> > >>>>
> > >>>> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote:
> > >>>>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> 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 <[email protected]>
> > >>>>>> ---
> > >>>>>> .../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
--
With best wishes
Dmitry