| Message ID | 20240627205328.2912859-1-quic_abhinavk@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] drm/msm/dpu: check ubwc support before adding compressed formats | expand |
On Thu, Jun 27, 2024 at 1:53 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > On QCM2290 chipset DPU does not support UBWC. > > Add a dpu cap to indicate this and do not expose compressed formats > in this case. > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 ++++- > 3 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h > index 3cbb2fe8aba2..6671f798bacc 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h > @@ -12,6 +12,7 @@ static const struct dpu_caps qcm2290_dpu_caps = { > .max_mixer_blendstages = 0x4, > .has_dim_layer = true, > .has_idle_pc = true, > + .has_no_ubwc = true, > .max_linewidth = 2160, > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > }; > 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 af2ead1c4886..676d0a283922 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > @@ -342,6 +342,7 @@ struct dpu_rotation_cfg { > * @has_dim_layer dim layer feature status > * @has_idle_pc indicate if idle power collapse feature is supported > * @has_3d_merge indicate if 3D merge is supported > + * @has_no_ubwc indicate if UBWC is supported > * @max_linewidth max linewidth for sspp > * @pixel_ram_size size of latency hiding and de-tiling buffer in bytes > * @max_hdeci_exp max horizontal decimation supported (max is 2^value) > @@ -354,6 +355,7 @@ struct dpu_caps { > bool has_dim_layer; > bool has_idle_pc; > bool has_3d_merge; > + bool has_no_ubwc; has_no_ubwc sounds kinda awkward compared to has_ubwc. But I guess you wanted to avoid all that churn.. How about instead, if msm_mdss_data::ubwc_{enc,dec}_version are zero, then we know there is no ubwc support in the display. BR, -R > /* SSPP limits */ > u32 max_linewidth; > u32 pixel_ram_size; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > index 6000e84598c2..31fe0fc4c02e 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > @@ -1341,10 +1341,13 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable) > static bool dpu_plane_format_mod_supported(struct drm_plane *plane, > uint32_t format, uint64_t modifier) > { > + struct dpu_plane *pdpu = to_dpu_plane(plane); > + const struct dpu_caps *caps = pdpu->catalog->caps; > + > if (modifier == DRM_FORMAT_MOD_LINEAR) > return true; > > - if (modifier == DRM_FORMAT_MOD_QCOM_COMPRESSED) > + if (modifier == DRM_FORMAT_MOD_QCOM_COMPRESSED && !caps->has_no_ubwc) > return dpu_find_format(format, qcom_compressed_supported_formats, > ARRAY_SIZE(qcom_compressed_supported_formats)); > > -- > 2.44.0 >
On 6/27/2024 2:13 PM, Rob Clark wrote: > On Thu, Jun 27, 2024 at 1:53 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> On QCM2290 chipset DPU does not support UBWC. >> >> Add a dpu cap to indicate this and do not expose compressed formats >> in this case. >> >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> --- >> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 1 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 ++++- >> 3 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h >> index 3cbb2fe8aba2..6671f798bacc 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h >> @@ -12,6 +12,7 @@ static const struct dpu_caps qcm2290_dpu_caps = { >> .max_mixer_blendstages = 0x4, >> .has_dim_layer = true, >> .has_idle_pc = true, >> + .has_no_ubwc = true, >> .max_linewidth = 2160, >> .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, >> }; >> 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 af2ead1c4886..676d0a283922 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >> @@ -342,6 +342,7 @@ struct dpu_rotation_cfg { >> * @has_dim_layer dim layer feature status >> * @has_idle_pc indicate if idle power collapse feature is supported >> * @has_3d_merge indicate if 3D merge is supported >> + * @has_no_ubwc indicate if UBWC is supported >> * @max_linewidth max linewidth for sspp >> * @pixel_ram_size size of latency hiding and de-tiling buffer in bytes >> * @max_hdeci_exp max horizontal decimation supported (max is 2^value) >> @@ -354,6 +355,7 @@ struct dpu_caps { >> bool has_dim_layer; >> bool has_idle_pc; >> bool has_3d_merge; >> + bool has_no_ubwc; > > has_no_ubwc sounds kinda awkward compared to has_ubwc. But I guess > you wanted to avoid all that churn.. > Yes I wanted to avoid modifying all the catalogs. > How about instead, if msm_mdss_data::ubwc_{enc,dec}_version are zero, > then we know there is no ubwc support in the display. > hmm ... should work .... I can post a v2 with this and avoid touching the catalog altogether. > BR, > -R > > >> /* SSPP limits */ >> u32 max_linewidth; >> u32 pixel_ram_size; >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> index 6000e84598c2..31fe0fc4c02e 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> @@ -1341,10 +1341,13 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable) >> static bool dpu_plane_format_mod_supported(struct drm_plane *plane, >> uint32_t format, uint64_t modifier) >> { >> + struct dpu_plane *pdpu = to_dpu_plane(plane); >> + const struct dpu_caps *caps = pdpu->catalog->caps; >> + >> if (modifier == DRM_FORMAT_MOD_LINEAR) >> return true; >> >> - if (modifier == DRM_FORMAT_MOD_QCOM_COMPRESSED) >> + if (modifier == DRM_FORMAT_MOD_QCOM_COMPRESSED && !caps->has_no_ubwc) >> return dpu_find_format(format, qcom_compressed_supported_formats, >> ARRAY_SIZE(qcom_compressed_supported_formats)); >> >> -- >> 2.44.0 >>
On Fri, 28 Jun 2024 at 00:21, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 6/27/2024 2:13 PM, Rob Clark wrote: > > On Thu, Jun 27, 2024 at 1:53 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >> > >> On QCM2290 chipset DPU does not support UBWC. > >> > >> Add a dpu cap to indicate this and do not expose compressed formats > >> in this case. > >> > >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 1 + > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 ++++- > >> 3 files changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h > >> index 3cbb2fe8aba2..6671f798bacc 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h > >> @@ -12,6 +12,7 @@ static const struct dpu_caps qcm2290_dpu_caps = { > >> .max_mixer_blendstages = 0x4, > >> .has_dim_layer = true, > >> .has_idle_pc = true, > >> + .has_no_ubwc = true, > >> .max_linewidth = 2160, > >> .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > >> }; > >> 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 af2ead1c4886..676d0a283922 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >> @@ -342,6 +342,7 @@ struct dpu_rotation_cfg { > >> * @has_dim_layer dim layer feature status > >> * @has_idle_pc indicate if idle power collapse feature is supported > >> * @has_3d_merge indicate if 3D merge is supported > >> + * @has_no_ubwc indicate if UBWC is supported > >> * @max_linewidth max linewidth for sspp > >> * @pixel_ram_size size of latency hiding and de-tiling buffer in bytes > >> * @max_hdeci_exp max horizontal decimation supported (max is 2^value) > >> @@ -354,6 +355,7 @@ struct dpu_caps { > >> bool has_dim_layer; > >> bool has_idle_pc; > >> bool has_3d_merge; > >> + bool has_no_ubwc; > > > > has_no_ubwc sounds kinda awkward compared to has_ubwc. But I guess > > you wanted to avoid all that churn.. > > > > Yes I wanted to avoid modifying all the catalogs. > > > How about instead, if msm_mdss_data::ubwc_{enc,dec}_version are zero, > > then we know there is no ubwc support in the display. > > > > hmm ... should work .... I can post a v2 with this and avoid touching > the catalog altogether. Yes, this sounds much better.
On 6/27/2024 4:22 PM, Dmitry Baryshkov wrote: > On Fri, 28 Jun 2024 at 00:21, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 6/27/2024 2:13 PM, Rob Clark wrote: >>> On Thu, Jun 27, 2024 at 1:53 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >>>> >>>> On QCM2290 chipset DPU does not support UBWC. >>>> >>>> Add a dpu cap to indicate this and do not expose compressed formats >>>> in this case. >>>> >>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>>> --- >>>> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 1 + >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 ++++- >>>> 3 files changed, 7 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h >>>> index 3cbb2fe8aba2..6671f798bacc 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h >>>> @@ -12,6 +12,7 @@ static const struct dpu_caps qcm2290_dpu_caps = { >>>> .max_mixer_blendstages = 0x4, >>>> .has_dim_layer = true, >>>> .has_idle_pc = true, >>>> + .has_no_ubwc = true, >>>> .max_linewidth = 2160, >>>> .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, >>>> }; >>>> 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 af2ead1c4886..676d0a283922 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >>>> @@ -342,6 +342,7 @@ struct dpu_rotation_cfg { >>>> * @has_dim_layer dim layer feature status >>>> * @has_idle_pc indicate if idle power collapse feature is supported >>>> * @has_3d_merge indicate if 3D merge is supported >>>> + * @has_no_ubwc indicate if UBWC is supported >>>> * @max_linewidth max linewidth for sspp >>>> * @pixel_ram_size size of latency hiding and de-tiling buffer in bytes >>>> * @max_hdeci_exp max horizontal decimation supported (max is 2^value) >>>> @@ -354,6 +355,7 @@ struct dpu_caps { >>>> bool has_dim_layer; >>>> bool has_idle_pc; >>>> bool has_3d_merge; >>>> + bool has_no_ubwc; >>> >>> has_no_ubwc sounds kinda awkward compared to has_ubwc. But I guess >>> you wanted to avoid all that churn.. >>> >> >> Yes I wanted to avoid modifying all the catalogs. >> >>> How about instead, if msm_mdss_data::ubwc_{enc,dec}_version are zero, >>> then we know there is no ubwc support in the display. >>> >> >> hmm ... should work .... I can post a v2 with this and avoid touching >> the catalog altogether. > > Yes, this sounds much better. > Ok, does this qualify for a Fixes tag too? Because exposing ubwc formats on non-ubwc supported chipsets seems like a bug.
On Fri, 28 Jun 2024 at 08:44, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 6/27/2024 4:22 PM, Dmitry Baryshkov wrote: > > On Fri, 28 Jun 2024 at 00:21, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >> > >> > >> > >> On 6/27/2024 2:13 PM, Rob Clark wrote: > >>> On Thu, Jun 27, 2024 at 1:53 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >>>> > >>>> On QCM2290 chipset DPU does not support UBWC. > >>>> > >>>> Add a dpu cap to indicate this and do not expose compressed formats > >>>> in this case. > >>>> > >>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >>>> --- > >>>> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 1 + > >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ > >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 ++++- > >>>> 3 files changed, 7 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h > >>>> index 3cbb2fe8aba2..6671f798bacc 100644 > >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h > >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h > >>>> @@ -12,6 +12,7 @@ static const struct dpu_caps qcm2290_dpu_caps = { > >>>> .max_mixer_blendstages = 0x4, > >>>> .has_dim_layer = true, > >>>> .has_idle_pc = true, > >>>> + .has_no_ubwc = true, > >>>> .max_linewidth = 2160, > >>>> .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > >>>> }; > >>>> 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 af2ead1c4886..676d0a283922 100644 > >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >>>> @@ -342,6 +342,7 @@ struct dpu_rotation_cfg { > >>>> * @has_dim_layer dim layer feature status > >>>> * @has_idle_pc indicate if idle power collapse feature is supported > >>>> * @has_3d_merge indicate if 3D merge is supported > >>>> + * @has_no_ubwc indicate if UBWC is supported > >>>> * @max_linewidth max linewidth for sspp > >>>> * @pixel_ram_size size of latency hiding and de-tiling buffer in bytes > >>>> * @max_hdeci_exp max horizontal decimation supported (max is 2^value) > >>>> @@ -354,6 +355,7 @@ struct dpu_caps { > >>>> bool has_dim_layer; > >>>> bool has_idle_pc; > >>>> bool has_3d_merge; > >>>> + bool has_no_ubwc; > >>> > >>> has_no_ubwc sounds kinda awkward compared to has_ubwc. But I guess > >>> you wanted to avoid all that churn.. > >>> > >> > >> Yes I wanted to avoid modifying all the catalogs. > >> > >>> How about instead, if msm_mdss_data::ubwc_{enc,dec}_version are zero, > >>> then we know there is no ubwc support in the display. > >>> > >> > >> hmm ... should work .... I can post a v2 with this and avoid touching > >> the catalog altogether. > > > > Yes, this sounds much better. > > > > Ok, does this qualify for a Fixes tag too? Because exposing ubwc formats > on non-ubwc supported chipsets seems like a bug. I think it does. For example, I've also added Fixes: to the patch disabling YUV on qcm2290.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h index 3cbb2fe8aba2..6671f798bacc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h @@ -12,6 +12,7 @@ static const struct dpu_caps qcm2290_dpu_caps = { .max_mixer_blendstages = 0x4, .has_dim_layer = true, .has_idle_pc = true, + .has_no_ubwc = true, .max_linewidth = 2160, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, }; 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 af2ead1c4886..676d0a283922 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -342,6 +342,7 @@ struct dpu_rotation_cfg { * @has_dim_layer dim layer feature status * @has_idle_pc indicate if idle power collapse feature is supported * @has_3d_merge indicate if 3D merge is supported + * @has_no_ubwc indicate if UBWC is supported * @max_linewidth max linewidth for sspp * @pixel_ram_size size of latency hiding and de-tiling buffer in bytes * @max_hdeci_exp max horizontal decimation supported (max is 2^value) @@ -354,6 +355,7 @@ struct dpu_caps { bool has_dim_layer; bool has_idle_pc; bool has_3d_merge; + bool has_no_ubwc; /* SSPP limits */ u32 max_linewidth; u32 pixel_ram_size; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 6000e84598c2..31fe0fc4c02e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1341,10 +1341,13 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable) static bool dpu_plane_format_mod_supported(struct drm_plane *plane, uint32_t format, uint64_t modifier) { + struct dpu_plane *pdpu = to_dpu_plane(plane); + const struct dpu_caps *caps = pdpu->catalog->caps; + if (modifier == DRM_FORMAT_MOD_LINEAR) return true; - if (modifier == DRM_FORMAT_MOD_QCOM_COMPRESSED) + if (modifier == DRM_FORMAT_MOD_QCOM_COMPRESSED && !caps->has_no_ubwc) return dpu_find_format(format, qcom_compressed_supported_formats, ARRAY_SIZE(qcom_compressed_supported_formats));
On QCM2290 chipset DPU does not support UBWC. Add a dpu cap to indicate this and do not expose compressed formats in this case. Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 ++++- 3 files changed, 7 insertions(+), 1 deletion(-)