All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
@ 2023-06-12 18:25 ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-06-12 18:25 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel,
	Stephen Boyd, Yongqin Liu

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(-)

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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
@ 2023-06-12 18:25 ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-06-12 18:25 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno, Yongqin Liu

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(-)

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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
  2023-06-12 18:25 ` Dmitry Baryshkov
@ 2023-06-12 18:25   ` Dmitry Baryshkov
  -1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-06-12 18:25 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Add definitions of DSPP blocks present on the sdm845 platform. This
should enable color-management on sdm845-bassed devices.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
index 36ea1af10894..b6098141bb9b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
@@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
 
 static const struct dpu_lm_cfg sdm845_lm[] = {
 	LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
-		&sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
+		&sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
 	LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
-		&sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
+		&sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
 	LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
-		&sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
+		&sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
 	LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
-		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
+		&sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
 	LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
 		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
 	LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
 		&sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
 };
 
+static const struct dpu_dspp_cfg sdm845_dspp[] = {
+	DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+		 &sm8150_dspp_sblk),
+	DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
+		 &sm8150_dspp_sblk),
+	DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
+		 &sm8150_dspp_sblk),
+	DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
+		 &sm8150_dspp_sblk),
+};
+
 static const struct dpu_pingpong_cfg sdm845_pp[] = {
 	PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
 			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
@@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
 	.sspp = sdm845_sspp,
 	.mixer_count = ARRAY_SIZE(sdm845_lm),
 	.mixer = sdm845_lm,
+	.dspp_count = ARRAY_SIZE(sdm845_dspp),
+	.dspp = sdm845_dspp,
 	.pingpong_count = ARRAY_SIZE(sdm845_pp),
 	.pingpong = sdm845_pp,
 	.dsc_count = ARRAY_SIZE(sdm845_dsc),
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
@ 2023-06-12 18:25   ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-06-12 18:25 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno

Add definitions of DSPP blocks present on the sdm845 platform. This
should enable color-management on sdm845-bassed devices.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
index 36ea1af10894..b6098141bb9b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
@@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
 
 static const struct dpu_lm_cfg sdm845_lm[] = {
 	LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
-		&sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
+		&sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
 	LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
-		&sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
+		&sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
 	LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
-		&sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
+		&sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
 	LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
-		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
+		&sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
 	LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
 		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
 	LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
 		&sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
 };
 
+static const struct dpu_dspp_cfg sdm845_dspp[] = {
+	DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+		 &sm8150_dspp_sblk),
+	DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
+		 &sm8150_dspp_sblk),
+	DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
+		 &sm8150_dspp_sblk),
+	DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
+		 &sm8150_dspp_sblk),
+};
+
 static const struct dpu_pingpong_cfg sdm845_pp[] = {
 	PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
 			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
@@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
 	.sspp = sdm845_sspp,
 	.mixer_count = ARRAY_SIZE(sdm845_lm),
 	.mixer = sdm845_lm,
+	.dspp_count = ARRAY_SIZE(sdm845_dspp),
+	.dspp = sdm845_dspp,
 	.pingpong_count = ARRAY_SIZE(sdm845_pp),
 	.pingpong = sdm845_pp,
 	.dsc_count = ARRAY_SIZE(sdm845_dsc),
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
  2023-06-12 18:25 ` Dmitry Baryshkov
@ 2023-06-12 18:42   ` Abhinav Kumar
  -1 siblings, 0 replies; 22+ messages in thread
From: Abhinav Kumar @ 2023-06-12 18:42 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno, Yongqin Liu



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>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
@ 2023-06-12 18:42   ` Abhinav Kumar
  0 siblings, 0 replies; 22+ messages in thread
From: Abhinav Kumar @ 2023-06-12 18:42 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel,
	Stephen Boyd, Yongqin Liu



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>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
  2023-06-12 18:25   ` Dmitry Baryshkov
@ 2023-06-12 19:35     ` Abhinav Kumar
  -1 siblings, 0 replies; 22+ messages in thread
From: Abhinav Kumar @ 2023-06-12 19:35 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno



On 6/12/2023 11:25 AM, Dmitry Baryshkov wrote:
> Add definitions of DSPP blocks present on the sdm845 platform. This
> should enable color-management on sdm845-bassed devices.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)
> 

This change itself is fine, hence

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

one note below for a future cleanup:

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index 36ea1af10894..b6098141bb9b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>   
>   static const struct dpu_lm_cfg sdm845_lm[] = {
>   	LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
> +		&sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
>   	LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
> +		&sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
>   	LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
> +		&sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
>   	LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
> +		&sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
>   	LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
>   		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>   	LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
>   		&sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
>   };
>   
> +static const struct dpu_dspp_cfg sdm845_dspp[] = {
> +	DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +};
> +

I see the len of pcc blocks should be 0x88 not 0x90 as sm8150_dspp_sblk 
explains.

Also, I need to do some digging on the PCC version here. Can you pls 
provide me the link to downstream source which mentions PCC version is 
4.0 for sdm845?

>   static const struct dpu_pingpong_cfg sdm845_pp[] = {
>   	PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
>   			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> @@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
>   	.sspp = sdm845_sspp,
>   	.mixer_count = ARRAY_SIZE(sdm845_lm),
>   	.mixer = sdm845_lm,
> +	.dspp_count = ARRAY_SIZE(sdm845_dspp),
> +	.dspp = sdm845_dspp,
>   	.pingpong_count = ARRAY_SIZE(sdm845_pp),
>   	.pingpong = sdm845_pp,
>   	.dsc_count = ARRAY_SIZE(sdm845_dsc),

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
@ 2023-06-12 19:35     ` Abhinav Kumar
  0 siblings, 0 replies; 22+ messages in thread
From: Abhinav Kumar @ 2023-06-12 19:35 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd



On 6/12/2023 11:25 AM, Dmitry Baryshkov wrote:
> Add definitions of DSPP blocks present on the sdm845 platform. This
> should enable color-management on sdm845-bassed devices.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)
> 

This change itself is fine, hence

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

one note below for a future cleanup:

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index 36ea1af10894..b6098141bb9b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>   
>   static const struct dpu_lm_cfg sdm845_lm[] = {
>   	LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
> +		&sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
>   	LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
> +		&sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
>   	LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
> +		&sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
>   	LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
> +		&sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
>   	LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
>   		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>   	LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
>   		&sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
>   };
>   
> +static const struct dpu_dspp_cfg sdm845_dspp[] = {
> +	DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +};
> +

I see the len of pcc blocks should be 0x88 not 0x90 as sm8150_dspp_sblk 
explains.

Also, I need to do some digging on the PCC version here. Can you pls 
provide me the link to downstream source which mentions PCC version is 
4.0 for sdm845?

>   static const struct dpu_pingpong_cfg sdm845_pp[] = {
>   	PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
>   			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> @@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
>   	.sspp = sdm845_sspp,
>   	.mixer_count = ARRAY_SIZE(sdm845_lm),
>   	.mixer = sdm845_lm,
> +	.dspp_count = ARRAY_SIZE(sdm845_dspp),
> +	.dspp = sdm845_dspp,
>   	.pingpong_count = ARRAY_SIZE(sdm845_pp),
>   	.pingpong = sdm845_pp,
>   	.dsc_count = ARRAY_SIZE(sdm845_dsc),

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
  2023-06-12 19:35     ` Abhinav Kumar
@ 2023-06-12 19:39       ` Dmitry Baryshkov
  -1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-06-12 19:39 UTC (permalink / raw)
  To: Abhinav Kumar, Rob Clark, Sean Paul, Marijn Suijten
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno

On 12/06/2023 22:35, Abhinav Kumar wrote:
> 
> 
> On 6/12/2023 11:25 AM, Dmitry Baryshkov wrote:
>> Add definitions of DSPP blocks present on the sdm845 platform. This
>> should enable color-management on sdm845-bassed devices.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
>>   1 file changed, 17 insertions(+), 4 deletions(-)
>>
> 
> This change itself is fine, hence
> 
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
> one note below for a future cleanup:
> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
>> index 36ea1af10894..b6098141bb9b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
>> @@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>>   static const struct dpu_lm_cfg sdm845_lm[] = {
>>       LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
>> -        &sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
>> +        &sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
>>       LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
>> -        &sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
>> +        &sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
>>       LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
>> -        &sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
>> +        &sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
>>       LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
>> -        &sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>> +        &sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
>>       LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
>>           &sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>>       LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
>>           &sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
>>   };
>> +static const struct dpu_dspp_cfg sdm845_dspp[] = {
>> +    DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
>> +         &sm8150_dspp_sblk),
>> +    DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
>> +         &sm8150_dspp_sblk),
>> +    DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
>> +         &sm8150_dspp_sblk),
>> +    DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
>> +         &sm8150_dspp_sblk),
>> +};
>> +
> 
> I see the len of pcc blocks should be 0x88 not 0x90 as sm8150_dspp_sblk 
> explains.
> 
> Also, I need to do some digging on the PCC version here. Can you pls 
> provide me the link to downstream source which mentions PCC version is 
> 4.0 for sdm845?

Sure, 
https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.8.3.c25-08000-sdm845.0/arch/arm64/boot/dts/qcom/sdm845-sde.dtsi#L233

> 
>>   static const struct dpu_pingpong_cfg sdm845_pp[] = {
>>       PP_BLK("pingpong_0", PINGPONG_0, 0x70000, 
>> PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
>>               DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
>> @@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
>>       .sspp = sdm845_sspp,
>>       .mixer_count = ARRAY_SIZE(sdm845_lm),
>>       .mixer = sdm845_lm,
>> +    .dspp_count = ARRAY_SIZE(sdm845_dspp),
>> +    .dspp = sdm845_dspp,
>>       .pingpong_count = ARRAY_SIZE(sdm845_pp),
>>       .pingpong = sdm845_pp,
>>       .dsc_count = ARRAY_SIZE(sdm845_dsc),

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
@ 2023-06-12 19:39       ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-06-12 19:39 UTC (permalink / raw)
  To: Abhinav Kumar, Rob Clark, Sean Paul, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

On 12/06/2023 22:35, Abhinav Kumar wrote:
> 
> 
> On 6/12/2023 11:25 AM, Dmitry Baryshkov wrote:
>> Add definitions of DSPP blocks present on the sdm845 platform. This
>> should enable color-management on sdm845-bassed devices.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
>>   1 file changed, 17 insertions(+), 4 deletions(-)
>>
> 
> This change itself is fine, hence
> 
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
> one note below for a future cleanup:
> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
>> index 36ea1af10894..b6098141bb9b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
>> @@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>>   static const struct dpu_lm_cfg sdm845_lm[] = {
>>       LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
>> -        &sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
>> +        &sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
>>       LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
>> -        &sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
>> +        &sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
>>       LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
>> -        &sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
>> +        &sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
>>       LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
>> -        &sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>> +        &sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
>>       LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
>>           &sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>>       LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
>>           &sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
>>   };
>> +static const struct dpu_dspp_cfg sdm845_dspp[] = {
>> +    DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
>> +         &sm8150_dspp_sblk),
>> +    DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
>> +         &sm8150_dspp_sblk),
>> +    DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
>> +         &sm8150_dspp_sblk),
>> +    DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
>> +         &sm8150_dspp_sblk),
>> +};
>> +
> 
> I see the len of pcc blocks should be 0x88 not 0x90 as sm8150_dspp_sblk 
> explains.
> 
> Also, I need to do some digging on the PCC version here. Can you pls 
> provide me the link to downstream source which mentions PCC version is 
> 4.0 for sdm845?

Sure, 
https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.8.3.c25-08000-sdm845.0/arch/arm64/boot/dts/qcom/sdm845-sde.dtsi#L233

> 
>>   static const struct dpu_pingpong_cfg sdm845_pp[] = {
>>       PP_BLK("pingpong_0", PINGPONG_0, 0x70000, 
>> PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
>>               DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
>> @@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
>>       .sspp = sdm845_sspp,
>>       .mixer_count = ARRAY_SIZE(sdm845_lm),
>>       .mixer = sdm845_lm,
>> +    .dspp_count = ARRAY_SIZE(sdm845_dspp),
>> +    .dspp = sdm845_dspp,
>>       .pingpong_count = ARRAY_SIZE(sdm845_pp),
>>       .pingpong = sdm845_pp,
>>       .dsc_count = ARRAY_SIZE(sdm845_dsc),

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
  2023-06-12 18:25 ` Dmitry Baryshkov
@ 2023-06-12 23:09   ` Marijn Suijten
  -1 siblings, 0 replies; 22+ messages in thread
From: Marijn Suijten @ 2023-06-12 23:09 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Stephen Boyd, David Airlie,
	Daniel Vetter, Bjorn Andersson, linux-arm-msm, dri-devel,
	freedreno, Yongqin Liu

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
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
@ 2023-06-12 23:09   ` Marijn Suijten
  0 siblings, 0 replies; 22+ messages in thread
From: Marijn Suijten @ 2023-06-12 23:09 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: freedreno, Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
	Stephen Boyd, Yongqin Liu, linux-arm-msm

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
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
  2023-06-12 18:25   ` Dmitry Baryshkov
@ 2023-06-12 23:11     ` Marijn Suijten
  -1 siblings, 0 replies; 22+ messages in thread
From: Marijn Suijten @ 2023-06-12 23:11 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Stephen Boyd, David Airlie,
	Daniel Vetter, Bjorn Andersson, linux-arm-msm, dri-devel,
	freedreno

On 2023-06-12 21:25:34, Dmitry Baryshkov wrote:
> Add definitions of DSPP blocks present on the sdm845 platform. This
> should enable color-management on sdm845-bassed devices.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

And with this, the CTM property is back on SDM845 and functions as
intended: I am able to set a color transformation of choice with a
quickly-pieced-together DRM utility:

    https://github.com/MarijnS95/drm-tools

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

> ---
>  .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index 36ea1af10894..b6098141bb9b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>  
>  static const struct dpu_lm_cfg sdm845_lm[] = {
>  	LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
> +		&sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
>  	LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
> +		&sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
>  	LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
> +		&sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
>  	LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
> +		&sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
>  	LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
>  		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>  	LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
>  		&sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
>  };
>  
> +static const struct dpu_dspp_cfg sdm845_dspp[] = {
> +	DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +};
> +
>  static const struct dpu_pingpong_cfg sdm845_pp[] = {
>  	PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
>  			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> @@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
>  	.sspp = sdm845_sspp,
>  	.mixer_count = ARRAY_SIZE(sdm845_lm),
>  	.mixer = sdm845_lm,
> +	.dspp_count = ARRAY_SIZE(sdm845_dspp),
> +	.dspp = sdm845_dspp,
>  	.pingpong_count = ARRAY_SIZE(sdm845_pp),
>  	.pingpong = sdm845_pp,
>  	.dsc_count = ARRAY_SIZE(sdm845_dsc),
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
@ 2023-06-12 23:11     ` Marijn Suijten
  0 siblings, 0 replies; 22+ messages in thread
From: Marijn Suijten @ 2023-06-12 23:11 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: freedreno, Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
	Stephen Boyd, linux-arm-msm

On 2023-06-12 21:25:34, Dmitry Baryshkov wrote:
> Add definitions of DSPP blocks present on the sdm845 platform. This
> should enable color-management on sdm845-bassed devices.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

And with this, the CTM property is back on SDM845 and functions as
intended: I am able to set a color transformation of choice with a
quickly-pieced-together DRM utility:

    https://github.com/MarijnS95/drm-tools

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

> ---
>  .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index 36ea1af10894..b6098141bb9b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>  
>  static const struct dpu_lm_cfg sdm845_lm[] = {
>  	LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
> +		&sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
>  	LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
> +		&sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
>  	LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
> +		&sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
>  	LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
> -		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
> +		&sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
>  	LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
>  		&sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>  	LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
>  		&sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
>  };
>  
> +static const struct dpu_dspp_cfg sdm845_dspp[] = {
> +	DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +	DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
> +		 &sm8150_dspp_sblk),
> +};
> +
>  static const struct dpu_pingpong_cfg sdm845_pp[] = {
>  	PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
>  			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> @@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
>  	.sspp = sdm845_sspp,
>  	.mixer_count = ARRAY_SIZE(sdm845_lm),
>  	.mixer = sdm845_lm,
> +	.dspp_count = ARRAY_SIZE(sdm845_dspp),
> +	.dspp = sdm845_dspp,
>  	.pingpong_count = ARRAY_SIZE(sdm845_pp),
>  	.pingpong = sdm845_pp,
>  	.dsc_count = ARRAY_SIZE(sdm845_dsc),
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
  2023-06-12 18:25 ` Dmitry Baryshkov
@ 2023-06-14  7:49   ` Sumit Semwal
  -1 siblings, 0 replies; 22+ messages in thread
From: Sumit Semwal @ 2023-06-14  7:49 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
	Stephen Boyd, Yongqin Liu, linux-arm-msm, Marijn Suijten,
	freedreno

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
>


-- 
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
@ 2023-06-14  7:49   ` Sumit Semwal
  0 siblings, 0 replies; 22+ messages in thread
From: Sumit Semwal @ 2023-06-14  7:49 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, freedreno,
	linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
	Yongqin Liu, Daniel Vetter, David Airlie

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
>


-- 
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Freedreno] [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
  2023-06-12 18:25   ` Dmitry Baryshkov
@ 2023-06-14  7:51     ` Sumit Semwal
  -1 siblings, 0 replies; 22+ messages in thread
From: Sumit Semwal @ 2023-06-14  7:51 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
	Stephen Boyd, linux-arm-msm, Marijn Suijten, freedreno

On Mon, 12 Jun 2023 at 23:55, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Add definitions of DSPP blocks present on the sdm845 platform. This
> should enable color-management on sdm845-bassed devices.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)

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/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index 36ea1af10894..b6098141bb9b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>
>  static const struct dpu_lm_cfg sdm845_lm[] = {
>         LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
> -               &sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
> +               &sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
>         LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
> -               &sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
> +               &sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
>         LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
> -               &sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
> +               &sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
>         LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
> -               &sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
> +               &sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
>         LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
>                 &sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>         LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
>                 &sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
>  };
>
> +static const struct dpu_dspp_cfg sdm845_dspp[] = {
> +       DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
> +                &sm8150_dspp_sblk),
> +       DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
> +                &sm8150_dspp_sblk),
> +       DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
> +                &sm8150_dspp_sblk),
> +       DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
> +                &sm8150_dspp_sblk),
> +};
> +
>  static const struct dpu_pingpong_cfg sdm845_pp[] = {
>         PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
>                         DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> @@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
>         .sspp = sdm845_sspp,
>         .mixer_count = ARRAY_SIZE(sdm845_lm),
>         .mixer = sdm845_lm,
> +       .dspp_count = ARRAY_SIZE(sdm845_dspp),
> +       .dspp = sdm845_dspp,
>         .pingpong_count = ARRAY_SIZE(sdm845_pp),
>         .pingpong = sdm845_pp,
>         .dsc_count = ARRAY_SIZE(sdm845_dsc),
> --
> 2.39.2
>


-- 
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Freedreno] [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
@ 2023-06-14  7:51     ` Sumit Semwal
  0 siblings, 0 replies; 22+ messages in thread
From: Sumit Semwal @ 2023-06-14  7:51 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, freedreno,
	linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
	Daniel Vetter, David Airlie

On Mon, 12 Jun 2023 at 23:55, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Add definitions of DSPP blocks present on the sdm845 platform. This
> should enable color-management on sdm845-bassed devices.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h    | 21 +++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)

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/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index 36ea1af10894..b6098141bb9b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>
>  static const struct dpu_lm_cfg sdm845_lm[] = {
>         LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
> -               &sdm845_lm_sblk, PINGPONG_0, LM_1, 0),
> +               &sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
>         LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
> -               &sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
> +               &sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
>         LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
> -               &sdm845_lm_sblk, PINGPONG_2, LM_5, 0),
> +               &sdm845_lm_sblk, PINGPONG_2, LM_5, DSPP_2),
>         LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
> -               &sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
> +               &sdm845_lm_sblk, PINGPONG_NONE, 0, DSPP_3),
>         LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
>                 &sdm845_lm_sblk, PINGPONG_NONE, 0, 0),
>         LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
>                 &sdm845_lm_sblk, PINGPONG_3, LM_2, 0),
>  };
>
> +static const struct dpu_dspp_cfg sdm845_dspp[] = {
> +       DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
> +                &sm8150_dspp_sblk),
> +       DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
> +                &sm8150_dspp_sblk),
> +       DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
> +                &sm8150_dspp_sblk),
> +       DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
> +                &sm8150_dspp_sblk),
> +};
> +
>  static const struct dpu_pingpong_cfg sdm845_pp[] = {
>         PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
>                         DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> @@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
>         .sspp = sdm845_sspp,
>         .mixer_count = ARRAY_SIZE(sdm845_lm),
>         .mixer = sdm845_lm,
> +       .dspp_count = ARRAY_SIZE(sdm845_dspp),
> +       .dspp = sdm845_dspp,
>         .pingpong_count = ARRAY_SIZE(sdm845_pp),
>         .pingpong = sdm845_pp,
>         .dsc_count = ARRAY_SIZE(sdm845_dsc),
> --
> 2.39.2
>


-- 
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
  2023-06-12 18:25 ` Dmitry Baryshkov
@ 2023-06-15  6:28   ` Yongqin Liu
  -1 siblings, 0 replies; 22+ messages in thread
From: Yongqin Liu @ 2023-06-15  6:28 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten,
	Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno

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
>

-- 
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
@ 2023-06-15  6:28   ` Yongqin Liu
  0 siblings, 0 replies; 22+ messages in thread
From: Yongqin Liu @ 2023-06-15  6:28 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: freedreno, Bjorn Andersson, Abhinav Kumar, dri-devel,
	Stephen Boyd, linux-arm-msm, Marijn Suijten, Sean Paul

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
>

-- 
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
  2023-06-12 18:25 ` Dmitry Baryshkov
@ 2023-06-15 11:31   ` Dmitry Baryshkov
  -1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-06-15 11:31 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, Dmitry Baryshkov
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno, Yongqin Liu


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,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
@ 2023-06-15 11:31   ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-06-15 11:31 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, Dmitry Baryshkov
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel,
	Stephen Boyd, Yongqin Liu


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,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-06-15 11:31 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-12 18:25 [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available Dmitry Baryshkov
2023-06-12 18:25 ` Dmitry Baryshkov
2023-06-12 18:25 ` [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845 Dmitry Baryshkov
2023-06-12 18:25   ` Dmitry Baryshkov
2023-06-12 19:35   ` Abhinav Kumar
2023-06-12 19:35     ` Abhinav Kumar
2023-06-12 19:39     ` Dmitry Baryshkov
2023-06-12 19:39       ` Dmitry Baryshkov
2023-06-12 23:11   ` Marijn Suijten
2023-06-12 23:11     ` Marijn Suijten
2023-06-14  7:51   ` [Freedreno] " Sumit Semwal
2023-06-14  7:51     ` Sumit Semwal
2023-06-12 18:42 ` [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available Abhinav Kumar
2023-06-12 18:42   ` Abhinav Kumar
2023-06-12 23:09 ` Marijn Suijten
2023-06-12 23:09   ` Marijn Suijten
2023-06-14  7:49 ` [Freedreno] " Sumit Semwal
2023-06-14  7:49   ` Sumit Semwal
2023-06-15  6:28 ` Yongqin Liu
2023-06-15  6:28   ` Yongqin Liu
2023-06-15 11:31 ` Dmitry Baryshkov
2023-06-15 11:31   ` Dmitry Baryshkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.