| Message ID | 20230612182534.3345805-1-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available | expand |
On 6/12/2023 11:25 AM, Dmitry Baryshkov wrote: > We can not support color management without DSPP blocks being provided > in the HW catalog. Do not enable color management for CRTCs if num_dspps > is 0. > > Fixes: 4259ff7ae509 ("drm/msm/dpu: add support for pcc color block in dpu driver") > Reported-by: Yongqin Liu <yongqin.liu@linaro.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > LGTM, Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
On 2023-06-12 21:25:33, Dmitry Baryshkov wrote: > We can not support color management without DSPP blocks being provided > in the HW catalog. Do not enable color management for CRTCs if num_dspps > is 0. > > Fixes: 4259ff7ae509 ("drm/msm/dpu: add support for pcc color block in dpu driver") > Reported-by: Yongqin Liu <yongqin.liu@linaro.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Yep, this indeed makes the CTM blob property disappear from the CRTC resource: Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 6e684a7b49a1..1edf2b6b0a26 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -1463,6 +1463,8 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = { > struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, > struct drm_plane *cursor) > { > + struct msm_drm_private *priv = dev->dev_private; > + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > struct drm_crtc *crtc = NULL; > struct dpu_crtc *dpu_crtc = NULL; > int i, ret; > @@ -1494,7 +1496,8 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, > > drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs); > > - drm_crtc_enable_color_mgmt(crtc, 0, true, 0); > + if (dpu_kms->catalog->dspp_count) > + drm_crtc_enable_color_mgmt(crtc, 0, true, 0); > > /* save user friendly CRTC name for later */ > snprintf(dpu_crtc->name, DPU_CRTC_NAME_SIZE, "crtc%u", crtc->base.id); > -- > 2.39.2 >
On Mon, 12 Jun 2023 at 23:55, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > We can not support color management without DSPP blocks being provided > in the HW catalog. Do not enable color management for CRTCs if num_dspps > is 0. > > Fixes: 4259ff7ae509 ("drm/msm/dpu: add support for pcc color block in dpu driver") > Reported-by: Yongqin Liu <yongqin.liu@linaro.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) Thanks for your patch, Dmitry. LGTM. Please feel free to add: Reviewed-by: Sumit Semwal <sumit.semwal@linaro.org> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 6e684a7b49a1..1edf2b6b0a26 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -1463,6 +1463,8 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = { > struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, > struct drm_plane *cursor) > { > + struct msm_drm_private *priv = dev->dev_private; > + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > struct drm_crtc *crtc = NULL; > struct dpu_crtc *dpu_crtc = NULL; > int i, ret; > @@ -1494,7 +1496,8 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, > > drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs); > > - drm_crtc_enable_color_mgmt(crtc, 0, true, 0); > + if (dpu_kms->catalog->dspp_count) > + drm_crtc_enable_color_mgmt(crtc, 0, true, 0); > > /* save user friendly CRTC name for later */ > snprintf(dpu_crtc->name, DPU_CRTC_NAME_SIZE, "crtc%u", crtc->base.id); > -- > 2.39.2 >
On Tue, 13 Jun 2023 at 02:25, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > We can not support color management without DSPP blocks being provided > in the HW catalog. Do not enable color management for CRTCs if num_dspps > is 0. > > Fixes: 4259ff7ae509 ("drm/msm/dpu: add support for pcc color block in dpu driver") > Reported-by: Yongqin Liu <yongqin.liu@linaro.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) Thanks for this fix! With it applied to the ACK android-mainline branch, the "dpu error" problem reported here: https://lore.kernel.org/lkml/CAMSo37VmhB1-PUp1qu8gaxOXtu98eEYmWd71FOai+cwLb-JvSg@mail.gmail.com/ is not reproduced. Tested-by: Yongqin Liu <yongqin.liu@linaro.org> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 6e684a7b49a1..1edf2b6b0a26 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -1463,6 +1463,8 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = { > struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, > struct drm_plane *cursor) > { > + struct msm_drm_private *priv = dev->dev_private; > + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > struct drm_crtc *crtc = NULL; > struct dpu_crtc *dpu_crtc = NULL; > int i, ret; > @@ -1494,7 +1496,8 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, > > drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs); > > - drm_crtc_enable_color_mgmt(crtc, 0, true, 0); > + if (dpu_kms->catalog->dspp_count) > + drm_crtc_enable_color_mgmt(crtc, 0, true, 0); > > /* save user friendly CRTC name for later */ > snprintf(dpu_crtc->name, DPU_CRTC_NAME_SIZE, "crtc%u", crtc->base.id); > -- > 2.39.2 >
On Mon, 12 Jun 2023 21:25:33 +0300, Dmitry Baryshkov wrote: > We can not support color management without DSPP blocks being provided > in the HW catalog. Do not enable color management for CRTCs if num_dspps > is 0. > > Applied, thanks! [1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available https://gitlab.freedesktop.org/lumag/msm/-/commit/3bcfc7b90465 [2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845 https://gitlab.freedesktop.org/lumag/msm/-/commit/c72375172194 Best regards,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 6e684a7b49a1..1edf2b6b0a26 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1463,6 +1463,8 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = { struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, struct drm_plane *cursor) { + struct msm_drm_private *priv = dev->dev_private; + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_crtc *crtc = NULL; struct dpu_crtc *dpu_crtc = NULL; int i, ret; @@ -1494,7 +1496,8 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane, drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs); - drm_crtc_enable_color_mgmt(crtc, 0, true, 0); + if (dpu_kms->catalog->dspp_count) + drm_crtc_enable_color_mgmt(crtc, 0, true, 0); /* save user friendly CRTC name for later */ snprintf(dpu_crtc->name, DPU_CRTC_NAME_SIZE, "crtc%u", crtc->base.id);
We can not support color management without DSPP blocks being provided in the HW catalog. Do not enable color management for CRTCs if num_dspps is 0. Fixes: 4259ff7ae509 ("drm/msm/dpu: add support for pcc color block in dpu driver") Reported-by: Yongqin Liu <yongqin.liu@linaro.org> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)