[PATCH 00/11] drm/msm/dpu: tweaks for better hardware resources allocation

[PATCH 00/11] drm/msm/dpu: tweaks for better hardware resources allocation

[Arnaud Vrac]

This series include misc fixes related to hardware resource allocations
in the msm dpu driver, some specifically for msm8998 (including hw
catalog fixes and cursor sspp support for cursor planes, instead of
using Smart DMA pipes).

This series has been tested on msm8998 with additional patches to enable
hdmi support.

The following modetest example command works now; 8 planes can be
displayed simultaneously on msm8998 in 1080p, including a cursor plane,
using a single LM:

modetest -Mmsm -a \
    -s 32:1920x1080-60 \
    -P 33@87:1920x1080+0+0@XR24 \
    -P 39@87:200x200+100+600@AR24 \
    -P 45@87:200x200+200+500@AR24 \
    -P 51@87:200x200+300+400@AR24 \
    -P 57@87:200x200+400+300@AR24 \
    -P 63@87:200x200+500+200@AR24 \
    -P 69@87:200x200+600+100@AR24 \
    -P 81@87:200x200+700+000@AR24

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
Arnaud Vrac (11):
      drm/msm/dpu: tweak msm8998 hw catalog values
      drm/msm/dpu: use the actual lm maximum width instead of a hardcoded value
      drm/msm/dpu: use hsync/vsync polarity set by the encoder
      drm/msm/dpu: allow using lm mixer base stage
      drm/msm/dpu: allow using all lm mixer stages
      drm/msm/dpu: support cursor sspp hw blocks
      drm/msm/dpu: add sspp cursor blocks to msm8998 hw catalog
      drm/msm/dpu: fix cursor block register bit offset in msm8998 hw catalog
      drm/msm/dpu: set max cursor width to 512x512
      drm/msm/dpu: tweak lm pairings in msm8998 hw catalog
      drm/msm/dpu: do not use mixer that supports dspp when not required

 .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h    | 28 +++++++------
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 10 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     | 49 +++++++++++++++++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c        | 16 ++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            | 22 +++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          | 32 +++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             |  2 +-
 7 files changed, 107 insertions(+), 52 deletions(-)
---
base-commit: e3342532ecd39bbd9c2ab5b9001cec1589bc37e9
change-id: 20230419-dpu-tweaks-5475305621d9

Best regards,
-- 
Arnaud Vrac <avrac@freebox.fr>

[PATCH 01/11] drm/msm/dpu: tweak msm8998 hw catalog values

[Arnaud Vrac]

Match the values found in the downstream msm-4.4 kernel sde driver.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h |  8 ++++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c          | 15 +++++----------
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
index 2b3ae84057dfe..b07e8a9941f79 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
@@ -134,10 +134,10 @@ static const struct dpu_dspp_cfg msm8998_dspp[] = {
 };
 
 static const struct dpu_intf_cfg msm8998_intf[] = {
-	INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
-	INTF_BLK("intf_1", INTF_1, 0x6a800, 0x280, INTF_DSI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
-	INTF_BLK("intf_2", INTF_2, 0x6b000, 0x280, INTF_DSI, 1, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29),
-	INTF_BLK("intf_3", INTF_3, 0x6b800, 0x280, INTF_HDMI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31),
+	INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 0, 21, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
+	INTF_BLK("intf_1", INTF_1, 0x6a800, 0x280, INTF_DSI, 0, 21, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
+	INTF_BLK("intf_2", INTF_2, 0x6b000, 0x280, INTF_DSI, 1, 21, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29),
+	INTF_BLK("intf_3", INTF_3, 0x6b800, 0x280, INTF_HDMI, 0, 21, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31),
 };
 
 static const struct dpu_perf_cfg msm8998_perf_data = {
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 03f162af1a50b..8d5d782a43398 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -587,12 +587,12 @@ static const u32 sdm845_nrt_pri_lvl[] = {3, 3, 3, 3, 3, 3, 3, 3};
 
 static const struct dpu_vbif_dynamic_ot_cfg msm8998_ot_rdwr_cfg[] = {
 	{
-		.pps = 1088 * 1920 * 30,
+		.pps = 1920 * 1080 * 30,
 		.ot_limit = 2,
 	},
 	{
-		.pps = 1088 * 1920 * 60,
-		.ot_limit = 6,
+		.pps = 1920 * 1080 * 60,
+		.ot_limit = 4,
 	},
 	{
 		.pps = 3840 * 2160 * 30,
@@ -705,10 +705,7 @@ static const struct dpu_qos_lut_entry msm8998_qos_linear[] = {
 	{.fl = 10, .lut = 0x1555b},
 	{.fl = 11, .lut = 0x5555b},
 	{.fl = 12, .lut = 0x15555b},
-	{.fl = 13, .lut = 0x55555b},
-	{.fl = 14, .lut = 0},
-	{.fl = 1,  .lut = 0x1b},
-	{.fl = 0,  .lut = 0}
+	{.fl = 0,  .lut = 0x55555b}
 };
 
 static const struct dpu_qos_lut_entry sdm845_qos_linear[] = {
@@ -730,9 +727,7 @@ static const struct dpu_qos_lut_entry msm8998_qos_macrotile[] = {
 	{.fl = 10, .lut = 0x1aaff},
 	{.fl = 11, .lut = 0x5aaff},
 	{.fl = 12, .lut = 0x15aaff},
-	{.fl = 13, .lut = 0x55aaff},
-	{.fl = 1,  .lut = 0x1aaff},
-	{.fl = 0,  .lut = 0},
+	{.fl = 0,  .lut = 0x55aaff},
 };
 
 static const struct dpu_qos_lut_entry sc7180_qos_linear[] = {

-- 
2.40.0

Re: [PATCH 01/11] drm/msm/dpu: tweak msm8998 hw catalog values

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> Match the values found in the downstream msm-4.4 kernel sde driver.
> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>

Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h |  8 ++++----
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c          | 15 +++++----------
>   2 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> index 2b3ae84057dfe..b07e8a9941f79 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> @@ -134,10 +134,10 @@ static const struct dpu_dspp_cfg msm8998_dspp[] = {
>   };
>   
>   static const struct dpu_intf_cfg msm8998_intf[] = {
> -	INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
> -	INTF_BLK("intf_1", INTF_1, 0x6a800, 0x280, INTF_DSI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
> -	INTF_BLK("intf_2", INTF_2, 0x6b000, 0x280, INTF_DSI, 1, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29),
> -	INTF_BLK("intf_3", INTF_3, 0x6b800, 0x280, INTF_HDMI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31),
> +	INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 0, 21, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
> +	INTF_BLK("intf_1", INTF_1, 0x6a800, 0x280, INTF_DSI, 0, 21, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
> +	INTF_BLK("intf_2", INTF_2, 0x6b000, 0x280, INTF_DSI, 1, 21, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29),
> +	INTF_BLK("intf_3", INTF_3, 0x6b800, 0x280, INTF_HDMI, 0, 21, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31),
>   };
>   
>   static const struct dpu_perf_cfg msm8998_perf_data = {
> 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 03f162af1a50b..8d5d782a43398 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -587,12 +587,12 @@ static const u32 sdm845_nrt_pri_lvl[] = {3, 3, 3, 3, 3, 3, 3, 3};
>   
>   static const struct dpu_vbif_dynamic_ot_cfg msm8998_ot_rdwr_cfg[] = {
>   	{
> -		.pps = 1088 * 1920 * 30,
> +		.pps = 1920 * 1080 * 30,
>   		.ot_limit = 2,
>   	},
>   	{
> -		.pps = 1088 * 1920 * 60,
> -		.ot_limit = 6,
> +		.pps = 1920 * 1080 * 60,
> +		.ot_limit = 4,
>   	},
>   	{
>   		.pps = 3840 * 2160 * 30,
> @@ -705,10 +705,7 @@ static const struct dpu_qos_lut_entry msm8998_qos_linear[] = {
>   	{.fl = 10, .lut = 0x1555b},
>   	{.fl = 11, .lut = 0x5555b},
>   	{.fl = 12, .lut = 0x15555b},
> -	{.fl = 13, .lut = 0x55555b},
> -	{.fl = 14, .lut = 0},
> -	{.fl = 1,  .lut = 0x1b},
> -	{.fl = 0,  .lut = 0}
> +	{.fl = 0,  .lut = 0x55555b}
>   };
>   
>   static const struct dpu_qos_lut_entry sdm845_qos_linear[] = {
> @@ -730,9 +727,7 @@ static const struct dpu_qos_lut_entry msm8998_qos_macrotile[] = {
>   	{.fl = 10, .lut = 0x1aaff},
>   	{.fl = 11, .lut = 0x5aaff},
>   	{.fl = 12, .lut = 0x15aaff},
> -	{.fl = 13, .lut = 0x55aaff},
> -	{.fl = 1,  .lut = 0x1aaff},
> -	{.fl = 0,  .lut = 0},
> +	{.fl = 0,  .lut = 0x55aaff},
>   };
>   
>   static const struct dpu_qos_lut_entry sc7180_qos_linear[] = {
> 

-- 
With best wishes
Dmitry

Re: [PATCH 01/11] drm/msm/dpu: tweak msm8998 hw catalog values

[Abhinav Kumar]

On 4/19/2023 7:41 AM, Arnaud Vrac wrote:
> Match the values found in the downstream msm-4.4 kernel sde driver.
> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---

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

[PATCH 02/11] drm/msm/dpu: use the actual lm maximum width instead of a hardcoded value

[Arnaud Vrac]

This avoids using two LMs instead of one when the display width is lower
than the maximum supported value. For example on MSM8996/MSM8998, the
actual maxwidth is 2560, so we would use two LMs for 1280x720 or
1920x1080 resolutions, while one is enough.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1dc5dbe585723..dd2914726c4f6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -53,8 +53,6 @@
 
 #define IDLE_SHORT_TIMEOUT	1
 
-#define MAX_HDISPLAY_SPLIT 1080
-
 /* timeout in frames waiting for frame done */
 #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5
 
@@ -568,10 +566,12 @@ static struct msm_display_topology dpu_encoder_get_topology(
 	 */
 	if (intf_count == 2)
 		topology.num_lm = 2;
-	else if (!dpu_kms->catalog->caps->has_3d_merge)
-		topology.num_lm = 1;
+	else if (dpu_kms->catalog->caps->has_3d_merge &&
+		 dpu_kms->catalog->mixer_count > 0 &&
+		 mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth)
+		topology.num_lm = 2;
 	else
-		topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
+		topology.num_lm = 1;
 
 	if (crtc_state->ctm)
 		topology.num_dspp = topology.num_lm;

-- 
2.40.0

Re: [PATCH 02/11] drm/msm/dpu: use the actual lm maximum width instead of a hardcoded value

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> This avoids using two LMs instead of one when the display width is lower
> than the maximum supported value. For example on MSM8996/MSM8998, the
> actual maxwidth is 2560, so we would use two LMs for 1280x720 or
> 1920x1080 resolutions, while one is enough.
> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>

While this looks correct (and following what we have in 4.4), later 
vendor kernels specify the topology explicitly. Probably we should check 
this with the hw guys, because it might be the following case: even 
though a single LM can supply the mode, it will spend more power 
compared to two LMs.


> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 1dc5dbe585723..dd2914726c4f6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -53,8 +53,6 @@
>   
>   #define IDLE_SHORT_TIMEOUT	1
>   
> -#define MAX_HDISPLAY_SPLIT 1080
> -
>   /* timeout in frames waiting for frame done */
>   #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5
>   
> @@ -568,10 +566,12 @@ static struct msm_display_topology dpu_encoder_get_topology(
>   	 */
>   	if (intf_count == 2)
>   		topology.num_lm = 2;
> -	else if (!dpu_kms->catalog->caps->has_3d_merge)
> -		topology.num_lm = 1;
> +	else if (dpu_kms->catalog->caps->has_3d_merge &&
> +		 dpu_kms->catalog->mixer_count > 0 &&
> +		 mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth)
> +		topology.num_lm = 2;
>   	else
> -		topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
> +		topology.num_lm = 1;
>   
>   	if (crtc_state->ctm)
>   		topology.num_dspp = topology.num_lm;
> 

-- 
With best wishes
Dmitry

Re: [Freedreno] [PATCH 02/11] drm/msm/dpu: use the actual lm maximum width instead of a hardcoded value

[Jeykumar Sankaran]

On 4/19/2023 3:23 PM, Dmitry Baryshkov wrote:
> On 19/04/2023 17:41, Arnaud Vrac wrote:
>> This avoids using two LMs instead of one when the display width is lower
>> than the maximum supported value. For example on MSM8996/MSM8998, the
>> actual maxwidth is 2560, so we would use two LMs for 1280x720 or
>> 1920x1080 resolutions, while one is enough.
>>
>> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> 
> While this looks correct (and following what we have in 4.4), later 
> vendor kernels specify the topology explicitly. Probably we should check 
> this with the hw guys, because it might be the following case: even 
> though a single LM can supply the mode, it will spend more power 
> compared to two LMs.
> 
> 
Yes. 2 LM split will allow the HW to run in lower mdp core clock. Can 
you maintain the split_threshold in the hw catalog until per mode 
topology is available?

Jeykumar S
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 1dc5dbe585723..dd2914726c4f6 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -53,8 +53,6 @@
>>   #define IDLE_SHORT_TIMEOUT    1
>> -#define MAX_HDISPLAY_SPLIT 1080
>> -
>>   /* timeout in frames waiting for frame done */
>>   #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5
>> @@ -568,10 +566,12 @@ static struct msm_display_topology 
>> dpu_encoder_get_topology(
>>        */
>>       if (intf_count == 2)
>>           topology.num_lm = 2;
>> -    else if (!dpu_kms->catalog->caps->has_3d_merge)
>> -        topology.num_lm = 1;
>> +    else if (dpu_kms->catalog->caps->has_3d_merge &&
>> +         dpu_kms->catalog->mixer_count > 0 &&
>> +         mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth)
>> +        topology.num_lm = 2;
>>       else
>> -        topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
>> +        topology.num_lm = 1;
>>       if (crtc_state->ctm)
>>           topology.num_dspp = topology.num_lm;
>>
> 

Re: [Freedreno] [PATCH 02/11] drm/msm/dpu: use the actual lm maximum width instead of a hardcoded value

[Dmitry Baryshkov]

On 20/04/2023 20:47, Jeykumar Sankaran wrote:
> 
> 
> On 4/19/2023 3:23 PM, Dmitry Baryshkov wrote:
>> On 19/04/2023 17:41, Arnaud Vrac wrote:
>>> This avoids using two LMs instead of one when the display width is lower
>>> than the maximum supported value. For example on MSM8996/MSM8998, the
>>> actual maxwidth is 2560, so we would use two LMs for 1280x720 or
>>> 1920x1080 resolutions, while one is enough.
>>>
>>> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
>>
>> While this looks correct (and following what we have in 4.4), later 
>> vendor kernels specify the topology explicitly. Probably we should 
>> check this with the hw guys, because it might be the following case: 
>> even though a single LM can supply the mode, it will spend more power 
>> compared to two LMs.
>>
>>
> Yes. 2 LM split will allow the HW to run in lower mdp core clock. Can 
> you maintain the split_threshold in the hw catalog until per mode 
> topology is available?

I don't think it warrants the trouble, unless we have a real usecase 
when the device is short of LMs.

Arnaud, I'll mark this patch as Rejected for now, unless it fixes an LM 
shortage for your platform.

> 
> Jeykumar S
>>> ---
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++-----
>>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> index 1dc5dbe585723..dd2914726c4f6 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> @@ -53,8 +53,6 @@
>>>   #define IDLE_SHORT_TIMEOUT    1
>>> -#define MAX_HDISPLAY_SPLIT 1080
>>> -
>>>   /* timeout in frames waiting for frame done */
>>>   #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5
>>> @@ -568,10 +566,12 @@ static struct msm_display_topology 
>>> dpu_encoder_get_topology(
>>>        */
>>>       if (intf_count == 2)
>>>           topology.num_lm = 2;
>>> -    else if (!dpu_kms->catalog->caps->has_3d_merge)
>>> -        topology.num_lm = 1;
>>> +    else if (dpu_kms->catalog->caps->has_3d_merge &&
>>> +         dpu_kms->catalog->mixer_count > 0 &&
>>> +         mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth)
>>> +        topology.num_lm = 2;
>>>       else
>>> -        topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 
>>> : 1;
>>> +        topology.num_lm = 1;
>>>       if (crtc_state->ctm)
>>>           topology.num_dspp = topology.num_lm;
>>>
>>

-- 
With best wishes
Dmitry

Re: [Freedreno] [PATCH 02/11] drm/msm/dpu: use the actual lm maximum width instead of a hardcoded value

[Arnaud Vrac]

Le sam. 20 mai 2023 à 22:49, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> a écrit :
>
> On 20/04/2023 20:47, Jeykumar Sankaran wrote:
> >
> >
> > On 4/19/2023 3:23 PM, Dmitry Baryshkov wrote:
> >> On 19/04/2023 17:41, Arnaud Vrac wrote:
> >>> This avoids using two LMs instead of one when the display width is lower
> >>> than the maximum supported value. For example on MSM8996/MSM8998, the
> >>> actual maxwidth is 2560, so we would use two LMs for 1280x720 or
> >>> 1920x1080 resolutions, while one is enough.
> >>>
> >>> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> >>
> >> While this looks correct (and following what we have in 4.4), later
> >> vendor kernels specify the topology explicitly. Probably we should
> >> check this with the hw guys, because it might be the following case:
> >> even though a single LM can supply the mode, it will spend more power
> >> compared to two LMs.
> >>
> >>
> > Yes. 2 LM split will allow the HW to run in lower mdp core clock. Can
> > you maintain the split_threshold in the hw catalog until per mode
> > topology is available?
>
> I don't think it warrants the trouble, unless we have a real usecase
> when the device is short of LMs.
>
> Arnaud, I'll mark this patch as Rejected for now, unless it fixes an LM
> shortage for your platform.

It's fine, if I remember correctly I wrote this patch because display
wouldn't work before I fixed the LM pairings on msm8998, but now it's
not a requirement anymore.

>
> >
> > Jeykumar S
> >>> ---
> >>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++-----
> >>>   1 file changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >>> index 1dc5dbe585723..dd2914726c4f6 100644
> >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >>> @@ -53,8 +53,6 @@
> >>>   #define IDLE_SHORT_TIMEOUT    1
> >>> -#define MAX_HDISPLAY_SPLIT 1080
> >>> -
> >>>   /* timeout in frames waiting for frame done */
> >>>   #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5
> >>> @@ -568,10 +566,12 @@ static struct msm_display_topology
> >>> dpu_encoder_get_topology(
> >>>        */
> >>>       if (intf_count == 2)
> >>>           topology.num_lm = 2;
> >>> -    else if (!dpu_kms->catalog->caps->has_3d_merge)
> >>> -        topology.num_lm = 1;
> >>> +    else if (dpu_kms->catalog->caps->has_3d_merge &&
> >>> +         dpu_kms->catalog->mixer_count > 0 &&
> >>> +         mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth)
> >>> +        topology.num_lm = 2;
> >>>       else
> >>> -        topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2
> >>> : 1;
> >>> +        topology.num_lm = 1;
> >>>       if (crtc_state->ctm)
> >>>           topology.num_dspp = topology.num_lm;
> >>>
> >>
>
> --
> With best wishes
> Dmitry
>

[PATCH 03/11] drm/msm/dpu: use hsync/vsync polarity set by the encoder

[Arnaud Vrac]

Do not override the hsync/vsync polarity passed by the encoder when
setting up intf timings. The same logic was used in both the encoder and
intf code to set the DP and DSI polarities, so those interfaces are not
impacted. However for HDMI, the polarities were overriden to static
values based on the vertical resolution, instead of using the actual
mode polarities.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 84ee2efa9c664..9f05417eb1213 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -104,7 +104,7 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
 	u32 active_h_start, active_h_end;
 	u32 active_v_start, active_v_end;
 	u32 active_hctl, display_hctl, hsync_ctl;
-	u32 polarity_ctl, den_polarity, hsync_polarity, vsync_polarity;
+	u32 polarity_ctl, den_polarity;
 	u32 panel_format;
 	u32 intf_cfg, intf_cfg2 = 0;
 	u32 display_data_hctl = 0, active_data_hctl = 0;
@@ -191,19 +191,9 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
 	}
 
 	den_polarity = 0;
-	if (ctx->cap->type == INTF_HDMI) {
-		hsync_polarity = p->yres >= 720 ? 0 : 1;
-		vsync_polarity = p->yres >= 720 ? 0 : 1;
-	} else if (ctx->cap->type == INTF_DP) {
-		hsync_polarity = p->hsync_polarity;
-		vsync_polarity = p->vsync_polarity;
-	} else {
-		hsync_polarity = 0;
-		vsync_polarity = 0;
-	}
 	polarity_ctl = (den_polarity << 2) | /*  DEN Polarity  */
-		(vsync_polarity << 1) | /* VSYNC Polarity */
-		(hsync_polarity << 0);  /* HSYNC Polarity */
+		(p->vsync_polarity << 1) | /* VSYNC Polarity */
+		(p->hsync_polarity << 0);  /* HSYNC Polarity */
 
 	if (!DPU_FORMAT_IS_YUV(fmt))
 		panel_format = (fmt->bits[C0_G_Y] |

-- 
2.40.0

Re: [PATCH 03/11] drm/msm/dpu: use hsync/vsync polarity set by the encoder

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> Do not override the hsync/vsync polarity passed by the encoder when
> setting up intf timings. The same logic was used in both the encoder and
> intf code to set the DP and DSI polarities, so those interfaces are not
> impacted. However for HDMI, the polarities were overriden to static
> values based on the vertical resolution, instead of using the actual
> mode polarities.
> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 16 +++-------------
>   1 file changed, 3 insertions(+), 13 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

As a side note: I think at some point we should get rid of the override 
in dpu_encoder too and move it to dsi bridge code.

-- 
With best wishes
Dmitry

Re: [Freedreno] [PATCH 03/11] drm/msm/dpu: use hsync/vsync polarity set by the encoder

[Jeykumar Sankaran]

On 4/19/2023 7:41 AM, Arnaud Vrac wrote:
> Do not override the hsync/vsync polarity passed by the encoder when
> setting up intf timings. The same logic was used in both the encoder and
> intf code to set the DP and DSI polarities, so those interfaces are not
> impacted. However for HDMI, the polarities were overriden to static
> values based on the vertical resolution, instead of using the actual
> mode polarities.
> 
Any idea why vres based static polarity override was in place? Hope you 
had tested HDMI resolutions with yres > and < than 720.

Jeykumar S.

> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 16 +++-------------
>   1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 84ee2efa9c664..9f05417eb1213 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -104,7 +104,7 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
>   	u32 active_h_start, active_h_end;
>   	u32 active_v_start, active_v_end;
>   	u32 active_hctl, display_hctl, hsync_ctl;
> -	u32 polarity_ctl, den_polarity, hsync_polarity, vsync_polarity;
> +	u32 polarity_ctl, den_polarity;
>   	u32 panel_format;
>   	u32 intf_cfg, intf_cfg2 = 0;
>   	u32 display_data_hctl = 0, active_data_hctl = 0;
> @@ -191,19 +191,9 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
>   	}
>   
>   	den_polarity = 0;
> -	if (ctx->cap->type == INTF_HDMI) {
> -		hsync_polarity = p->yres >= 720 ? 0 : 1;
> -		vsync_polarity = p->yres >= 720 ? 0 : 1;
> -	} else if (ctx->cap->type == INTF_DP) {
> -		hsync_polarity = p->hsync_polarity;
> -		vsync_polarity = p->vsync_polarity;
> -	} else {
> -		hsync_polarity = 0;
> -		vsync_polarity = 0;
> -	}
>   	polarity_ctl = (den_polarity << 2) | /*  DEN Polarity  */
> -		(vsync_polarity << 1) | /* VSYNC Polarity */
> -		(hsync_polarity << 0);  /* HSYNC Polarity */
> +		(p->vsync_polarity << 1) | /* VSYNC Polarity */
> +		(p->hsync_polarity << 0);  /* HSYNC Polarity */
>   
>   	if (!DPU_FORMAT_IS_YUV(fmt))
>   		panel_format = (fmt->bits[C0_G_Y] |
> 

Re: [Freedreno] [PATCH 03/11] drm/msm/dpu: use hsync/vsync polarity set by the encoder

[Dmitry Baryshkov]

On Thu, 20 Apr 2023 at 21:01, Jeykumar Sankaran
<quic_jeykumar@quicinc.com> wrote:
>
>
>
> On 4/19/2023 7:41 AM, Arnaud Vrac wrote:
> > Do not override the hsync/vsync polarity passed by the encoder when
> > setting up intf timings. The same logic was used in both the encoder and
> > intf code to set the DP and DSI polarities, so those interfaces are not
> > impacted. However for HDMI, the polarities were overriden to static
> > values based on the vertical resolution, instead of using the actual
> > mode polarities.
> >
> Any idea why vres based static polarity override was in place? Hope you
> had tested HDMI resolutions with yres > and < than 720.

I think it migrated from the old fbdev, see [1].
Tthis easily causes issues with the DVI monitors, which have different
rules for hsync/vsync. I have observed this on other platforms.
In practice it was observed that EDID provides correct sync polarity,
so it is better to follow the EDID data. If for some reason EDID is
incorrect, DRM provides a way to override it.

[1] https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/commit/29c3d616bf3f89f13bbab46c58151ae7a3a79b98

>
> Jeykumar S.
>
> > Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 16 +++-------------
> >   1 file changed, 3 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > index 84ee2efa9c664..9f05417eb1213 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > @@ -104,7 +104,7 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
> >       u32 active_h_start, active_h_end;
> >       u32 active_v_start, active_v_end;
> >       u32 active_hctl, display_hctl, hsync_ctl;
> > -     u32 polarity_ctl, den_polarity, hsync_polarity, vsync_polarity;
> > +     u32 polarity_ctl, den_polarity;
> >       u32 panel_format;
> >       u32 intf_cfg, intf_cfg2 = 0;
> >       u32 display_data_hctl = 0, active_data_hctl = 0;
> > @@ -191,19 +191,9 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
> >       }
> >
> >       den_polarity = 0;
> > -     if (ctx->cap->type == INTF_HDMI) {
> > -             hsync_polarity = p->yres >= 720 ? 0 : 1;
> > -             vsync_polarity = p->yres >= 720 ? 0 : 1;
> > -     } else if (ctx->cap->type == INTF_DP) {
> > -             hsync_polarity = p->hsync_polarity;
> > -             vsync_polarity = p->vsync_polarity;
> > -     } else {
> > -             hsync_polarity = 0;
> > -             vsync_polarity = 0;
> > -     }
> >       polarity_ctl = (den_polarity << 2) | /*  DEN Polarity  */
> > -             (vsync_polarity << 1) | /* VSYNC Polarity */
> > -             (hsync_polarity << 0);  /* HSYNC Polarity */
> > +             (p->vsync_polarity << 1) | /* VSYNC Polarity */
> > +             (p->hsync_polarity << 0);  /* HSYNC Polarity */
> >
> >       if (!DPU_FORMAT_IS_YUV(fmt))
> >               panel_format = (fmt->bits[C0_G_Y] |
> >



-- 
With best wishes
Dmitry

[PATCH 04/11] drm/msm/dpu: allow using lm mixer base stage

[Arnaud Vrac]

The dpu backend already handles applying alpha to the base stage, so we
can use it to render the bottom plane in all cases. This allows mixing
one additional plane with the hardware mixer.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 14b5cfe306113..148921ed62f85 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -881,7 +881,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
 	r_pipe->sspp = NULL;
 
-	pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
+	pstate->stage = DPU_STAGE_BASE + pstate->base.normalized_zpos;
 	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
 		DPU_ERROR("> %d plane stages assigned\n",
 			  pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);

-- 
2.40.0

Re: [PATCH 04/11] drm/msm/dpu: allow using lm mixer base stage

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> The dpu backend already handles applying alpha to the base stage, so we
> can use it to render the bottom plane in all cases. This allows mixing
> one additional plane with the hardware mixer.
> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>

This might require additional changes. First, for the STAGE_BASE pipe 
in the source split mode (iow using two LMs) should programmed with 
respect to the right LM's x offset (rather than usual left top-left LM). 
See  mdss_mdp_pipe_position_update().

Also this might need some interaction with CTL_MIXER_BORDER_OUT being 
set or not. If I remember correctly, if there bottom plane is not 
fullscreen or if there are no planes at all, we should set 
CTL_MIXER_BORDER_OUT (which takes STAGE_BASE) and start assigning them 
from STAGE0. If not, we can use STAGE_BASE.

> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 14b5cfe306113..148921ed62f85 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -881,7 +881,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>   	r_pipe->sspp = NULL;
>   
> -	pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
> +	pstate->stage = DPU_STAGE_BASE + pstate->base.normalized_zpos;
>   	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
>   		DPU_ERROR("> %d plane stages assigned\n",
>   			  pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
> 

-- 
With best wishes
Dmitry

Re: [PATCH 04/11] drm/msm/dpu: allow using lm mixer base stage

[Arnaud Vrac]

Le jeu. 20 avr. 2023 à 00:43, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> a écrit :
>
> On 19/04/2023 17:41, Arnaud Vrac wrote:
> > The dpu backend already handles applying alpha to the base stage, so we
> > can use it to render the bottom plane in all cases. This allows mixing
> > one additional plane with the hardware mixer.
> >
> > Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
>
> This might require additional changes. First, for the STAGE_BASE pipe
> in the source split mode (iow using two LMs) should programmed with
> respect to the right LM's x offset (rather than usual left top-left LM).
> See  mdss_mdp_pipe_position_update().

Ok, I did test with 2 LMs and it seems to be working, I'll investigate.

>
> Also this might need some interaction with CTL_MIXER_BORDER_OUT being
> set or not. If I remember correctly, if there bottom plane is not
> fullscreen or if there are no planes at all, we should set
> CTL_MIXER_BORDER_OUT (which takes STAGE_BASE) and start assigning them
> from STAGE0. If not, we can use STAGE_BASE.

I also tested with both fullscreen and non-fullscreen primary plane,
and no plane. I'll check this.

>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index 14b5cfe306113..148921ed62f85 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -881,7 +881,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
> >       r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> >       r_pipe->sspp = NULL;
> >
> > -     pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
> > +     pstate->stage = DPU_STAGE_BASE + pstate->base.normalized_zpos;
> >       if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
> >               DPU_ERROR("> %d plane stages assigned\n",
> >                         pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
> >
>
> --
> With best wishes
> Dmitry
>

Re: [PATCH 04/11] drm/msm/dpu: allow using lm mixer base stage

[Dmitry Baryshkov]

On 20/04/2023 10:26, Arnaud Vrac wrote:
> Le jeu. 20 avr. 2023 à 00:43, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> a écrit :
>>
>> On 19/04/2023 17:41, Arnaud Vrac wrote:
>>> The dpu backend already handles applying alpha to the base stage, so we
>>> can use it to render the bottom plane in all cases. This allows mixing
>>> one additional plane with the hardware mixer.
>>>
>>> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
>>
>> This might require additional changes. First, for the STAGE_BASE pipe
>> in the source split mode (iow using two LMs) should programmed with
>> respect to the right LM's x offset (rather than usual left top-left LM).
>> See  mdss_mdp_pipe_position_update().
> 
> Ok, I did test with 2 LMs and it seems to be working, I'll investigate.

The only reference I have here is the fbdev driver, see [1]. The newer 
SDE driver doesn't handle STAGE_BASE vs STAGE0 (and DPU inherited that 
design). Maybe this got fixed in hw at some point.

[1] 
https://git.codelinaro.org/clo/la/kernel/msm-4.19/-/blob/LE.UM.4.4.1.r2-17500-QRB5165.0/drivers/video/fbdev/msm/mdss_mdp_pipe.c#L1789

I think, it only concerns the src_split + multirect cases, where the 
rectangle base point is on the right LM.

> 
>>
>> Also this might need some interaction with CTL_MIXER_BORDER_OUT being
>> set or not. If I remember correctly, if there bottom plane is not
>> fullscreen or if there are no planes at all, we should set
>> CTL_MIXER_BORDER_OUT (which takes STAGE_BASE) and start assigning them
>> from STAGE0. If not, we can use STAGE_BASE.
> 
> I also tested with both fullscreen and non-fullscreen primary plane,
> and no plane. I'll check this.

Yes, the DPU driver always enables the MIXER_BORDER_OUT.

> 
>>
>>> ---
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> index 14b5cfe306113..148921ed62f85 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> @@ -881,7 +881,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>>>        r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>>>        r_pipe->sspp = NULL;
>>>
>>> -     pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
>>> +     pstate->stage = DPU_STAGE_BASE + pstate->base.normalized_zpos;
>>>        if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
>>>                DPU_ERROR("> %d plane stages assigned\n",
>>>                          pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
>>>
>>
>> --
>> With best wishes
>> Dmitry
>>

-- 
With best wishes
Dmitry

[PATCH 05/11] drm/msm/dpu: allow using all lm mixer stages

[Arnaud Vrac]

The max_mixer_blendstages hw catalog property represents the number of
planes that can be blended by the lm mixer, excluding the base stage, so
adjust the check for the number of currently assigned planes accordingly.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 148921ed62f85..128ecdc145260 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -882,9 +882,9 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	r_pipe->sspp = NULL;
 
 	pstate->stage = DPU_STAGE_BASE + pstate->base.normalized_zpos;
-	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
-		DPU_ERROR("> %d plane stages assigned\n",
-			  pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
+	if (pstate->stage > DPU_STAGE_BASE + pdpu->catalog->caps->max_mixer_blendstages) {
+		DPU_ERROR("> %d plane mixer stages assigned\n",
+			  pdpu->catalog->caps->max_mixer_blendstages);
 		return -EINVAL;
 	}
 

-- 
2.40.0

Re: [PATCH 05/11] drm/msm/dpu: allow using all lm mixer stages

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> The max_mixer_blendstages hw catalog property represents the number of
> planes that can be blended by the lm mixer, excluding the base stage, so
> adjust the check for the number of currently assigned planes accordingly.
> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

[PATCH 06/11] drm/msm/dpu: support cursor sspp hw blocks

[Arnaud Vrac]

Cursor SSPP must be assigned to the last mixer stage, so we assign an
immutable zpos property with a value higher than primary/overlay planes,
to ensure it will always be on top.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   | 19 ++++++++++++++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 26 +++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 0e7a68714e9e1..6cce0f6cfcb01 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -738,13 +738,22 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 	for (i = 0; i < catalog->sspp_count; i++) {
 		enum drm_plane_type type;
 
-		if ((catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR))
-			&& cursor_planes_idx < max_crtc_count)
-			type = DRM_PLANE_TYPE_CURSOR;
-		else if (primary_planes_idx < max_crtc_count)
+		if (catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR)) {
+			if (cursor_planes_idx < max_crtc_count) {
+				type = DRM_PLANE_TYPE_CURSOR;
+			} else if (catalog->sspp[i].type == SSPP_TYPE_CURSOR) {
+				/* Cursor SSPP can only be used in the last
+				 * mixer stage, so it doesn't make sense to
+				 * assign two of those to the same CRTC */
+				continue;
+			} else {
+				type = DRM_PLANE_TYPE_OVERLAY;
+			}
+		} else if (primary_planes_idx < max_crtc_count) {
 			type = DRM_PLANE_TYPE_PRIMARY;
-		else
+		} else {
 			type = DRM_PLANE_TYPE_OVERLAY;
+		}
 
 		DPU_DEBUG("Create plane type %d with features %lx (cur %lx)\n",
 			  type, catalog->sspp[i].features,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 128ecdc145260..5a7bb8543866c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -881,7 +881,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
 	r_pipe->sspp = NULL;
 
-	pstate->stage = DPU_STAGE_BASE + pstate->base.normalized_zpos;
+	if (pipe_hw_caps->type == SSPP_TYPE_CURSOR) {
+		/* enforce cursor sspp to use the last mixer stage */
+		pstate->stage = DPU_STAGE_BASE +
+			pdpu->catalog->caps->max_mixer_blendstages;
+	} else {
+		pstate->stage = DPU_STAGE_BASE + pstate->base.normalized_zpos;
+	}
+
 	if (pstate->stage > DPU_STAGE_BASE + pdpu->catalog->caps->max_mixer_blendstages) {
 		DPU_ERROR("> %d plane mixer stages assigned\n",
 			  pdpu->catalog->caps->max_mixer_blendstages);
@@ -1463,6 +1470,7 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
 	struct msm_drm_private *priv = dev->dev_private;
 	struct dpu_kms *kms = to_dpu_kms(priv->kms);
 	struct dpu_hw_sspp *pipe_hw;
+	const uint64_t *format_modifiers;
 	uint32_t num_formats;
 	uint32_t supported_rotations;
 	int ret = -EINVAL;
@@ -1489,15 +1497,27 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
 	format_list = pipe_hw->cap->sblk->format_list;
 	num_formats = pipe_hw->cap->sblk->num_formats;
 
+	if (pipe_hw->cap->type == SSPP_TYPE_CURSOR)
+		format_modifiers = NULL;
+	else
+		format_modifiers = supported_format_modifiers;
+
 	ret = drm_universal_plane_init(dev, plane, 0xff, &dpu_plane_funcs,
 				format_list, num_formats,
-				supported_format_modifiers, type, NULL);
+				format_modifiers, type, NULL);
 	if (ret)
 		goto clean_plane;
 
 	pdpu->catalog = kms->catalog;
 
-	ret = drm_plane_create_zpos_property(plane, 0, 0, DPU_ZPOS_MAX);
+	if (pipe_hw->cap->type == SSPP_TYPE_CURSOR) {
+		/* cursor SSPP can only be used in the last mixer stage,
+		 * enforce it by maxing out the cursor plane zpos */
+		ret = drm_plane_create_zpos_immutable_property(plane, DPU_ZPOS_MAX);
+	} else {
+		ret = drm_plane_create_zpos_property(plane, 0, 0, DPU_ZPOS_MAX - 1);
+	}
+
 	if (ret)
 		DPU_ERROR("failed to install zpos property, rc = %d\n", ret);
 

-- 
2.40.0

Re: [PATCH 06/11] drm/msm/dpu: support cursor sspp hw blocks

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> Cursor SSPP must be assigned to the last mixer stage, so we assign an
> immutable zpos property with a value higher than primary/overlay planes,
> to ensure it will always be on top.

The commit does more than is described in the commit message. Let's do 
it step by step. Please split into several patches. Also see below.

> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   | 19 ++++++++++++++-----
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 26 +++++++++++++++++++++++---
>   2 files changed, 37 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 0e7a68714e9e1..6cce0f6cfcb01 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -738,13 +738,22 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
>   	for (i = 0; i < catalog->sspp_count; i++) {
>   		enum drm_plane_type type;
>   
> -		if ((catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR))
> -			&& cursor_planes_idx < max_crtc_count)
> -			type = DRM_PLANE_TYPE_CURSOR;
> -		else if (primary_planes_idx < max_crtc_count)
> +		if (catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR)) {
> +			if (cursor_planes_idx < max_crtc_count) {
> +				type = DRM_PLANE_TYPE_CURSOR;
> +			} else if (catalog->sspp[i].type == SSPP_TYPE_CURSOR) {
> +				/* Cursor SSPP can only be used in the last
> +				 * mixer stage, so it doesn't make sense to
> +				 * assign two of those to the same CRTC */
> +				continue;
> +			} else {
> +				type = DRM_PLANE_TYPE_OVERLAY;
> +			}
> +		} else if (primary_planes_idx < max_crtc_count) {
>   			type = DRM_PLANE_TYPE_PRIMARY;
> -		else
> +		} else {
>   			type = DRM_PLANE_TYPE_OVERLAY;
> +		}

Ack. I'm not sure how compositors will cope if we have two planes with 
immutable zpos set to the same value. Also I'd prefer to have this as a 
separate commit.

>   
>   		DPU_DEBUG("Create plane type %d with features %lx (cur %lx)\n",
>   			  type, catalog->sspp[i].features,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 128ecdc145260..5a7bb8543866c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -881,7 +881,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>   	r_pipe->sspp = NULL;
>   
> -	pstate->stage = DPU_STAGE_BASE + pstate->base.normalized_zpos;
> +	if (pipe_hw_caps->type == SSPP_TYPE_CURSOR) {
> +		/* enforce cursor sspp to use the last mixer stage */

I'd add here 'we know that it is the last plane in the stack because of 
zpos property ranges'

> +		pstate->stage = DPU_STAGE_BASE +
> +			pdpu->catalog->caps->max_mixer_blendstages;
> +	} else {
> +		pstate->stage = DPU_STAGE_BASE + pstate->base.normalized_zpos;
> +	}
> +
>   	if (pstate->stage > DPU_STAGE_BASE + pdpu->catalog->caps->max_mixer_blendstages) {
>   		DPU_ERROR("> %d plane mixer stages assigned\n",
>   			  pdpu->catalog->caps->max_mixer_blendstages);
> @@ -1463,6 +1470,7 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>   	struct msm_drm_private *priv = dev->dev_private;
>   	struct dpu_kms *kms = to_dpu_kms(priv->kms);
>   	struct dpu_hw_sspp *pipe_hw;
> +	const uint64_t *format_modifiers;
>   	uint32_t num_formats;
>   	uint32_t supported_rotations;
>   	int ret = -EINVAL;
> @@ -1489,15 +1497,27 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>   	format_list = pipe_hw->cap->sblk->format_list;
>   	num_formats = pipe_hw->cap->sblk->num_formats;
>   
> +	if (pipe_hw->cap->type == SSPP_TYPE_CURSOR)
> +		format_modifiers = NULL;
> +	else
> +		format_modifiers = supported_format_modifiers;
> +
>   	ret = drm_universal_plane_init(dev, plane, 0xff, &dpu_plane_funcs,
>   				format_list, num_formats,
> -				supported_format_modifiers, type, NULL);
> +				format_modifiers, type, NULL);


Separate commit please

>   	if (ret)
>   		goto clean_plane;
>   
>   	pdpu->catalog = kms->catalog;
>   
> -	ret = drm_plane_create_zpos_property(plane, 0, 0, DPU_ZPOS_MAX);
> +	if (pipe_hw->cap->type == SSPP_TYPE_CURSOR) {
> +		/* cursor SSPP can only be used in the last mixer stage,
> +		 * enforce it by maxing out the cursor plane zpos */
> +		ret = drm_plane_create_zpos_immutable_property(plane, DPU_ZPOS_MAX);
> +	} else {
> +		ret = drm_plane_create_zpos_property(plane, 0, 0, DPU_ZPOS_MAX - 1);
> +	}
> +
>   	if (ret)
>   		DPU_ERROR("failed to install zpos property, rc = %d\n", ret);
>   
> 

-- 
With best wishes
Dmitry

[PATCH 07/11] drm/msm/dpu: add sspp cursor blocks to msm8998 hw catalog

[Arnaud Vrac]

Now that cursor sspp blocks can be used for cursor planes, enable them
on msm8998. The dma sspp blocks that were assigned to cursor planes can
now be used for overlay planes instead.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h    |  8 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     | 34 ++++++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
index b07e8a9941f79..7de393b0f91d7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
@@ -90,10 +90,14 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
 		sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
 	SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x1ac, DMA_MSM8998_MASK,
 		sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
-	SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
+	SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_MSM8998_MASK,
 		sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
-	SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
+	SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_MSM8998_MASK,
 		sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
+	SSPP_BLK("sspp_12", SSPP_CURSOR0, 0x34000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
+		msm8998_cursor_sblk_0, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
+	SSPP_BLK("sspp_13", SSPP_CURSOR1, 0x36000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
+		msm8998_cursor_sblk_1, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
 };
 
 static const struct dpu_lm_cfg msm8998_lm[] = {
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 8d5d782a43398..f34fa704936bc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -242,6 +242,22 @@ static const uint32_t wb2_formats[] = {
 	DRM_FORMAT_XBGR4444,
 };
 
+static const uint32_t cursor_formats[] = {
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_RGBA8888,
+	DRM_FORMAT_BGRA8888,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_ARGB1555,
+	DRM_FORMAT_ABGR1555,
+	DRM_FORMAT_RGBA5551,
+	DRM_FORMAT_BGRA5551,
+	DRM_FORMAT_ARGB4444,
+	DRM_FORMAT_ABGR4444,
+	DRM_FORMAT_RGBA4444,
+	DRM_FORMAT_BGRA4444,
+};
+
 /*************************************************************
  * SSPP sub blocks config
  *************************************************************/
@@ -300,6 +316,19 @@ static const uint32_t wb2_formats[] = {
 	.virt_num_formats = ARRAY_SIZE(plane_formats), \
 	}
 
+#define _CURSOR_SBLK(num) \
+	{ \
+	.maxdwnscale = SSPP_UNITY_SCALE, \
+	.maxupscale = SSPP_UNITY_SCALE, \
+	.smart_dma_priority = 0, \
+	.src_blk = {.name = STRCAT("sspp_src_", num), \
+		.id = DPU_SSPP_SRC, .base = 0x00, .len = 0x150,}, \
+	.format_list = cursor_formats, \
+	.num_formats = ARRAY_SIZE(cursor_formats), \
+	.virt_format_list = cursor_formats, \
+	.virt_num_formats = ARRAY_SIZE(cursor_formats), \
+	}
+
 static const struct dpu_sspp_sub_blks msm8998_vig_sblk_0 =
 				_VIG_SBLK("0", 0, DPU_SSPP_SCALER_QSEED3);
 static const struct dpu_sspp_sub_blks msm8998_vig_sblk_1 =
@@ -309,6 +338,11 @@ static const struct dpu_sspp_sub_blks msm8998_vig_sblk_2 =
 static const struct dpu_sspp_sub_blks msm8998_vig_sblk_3 =
 				_VIG_SBLK("3", 0, DPU_SSPP_SCALER_QSEED3);
 
+static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_0 =
+				_CURSOR_SBLK("12");
+static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_1 =
+				_CURSOR_SBLK("13");
+
 static const struct dpu_rotation_cfg dpu_rot_sc7280_cfg_v2 = {
 	.rot_maxheight = 1088,
 	.rot_num_formats = ARRAY_SIZE(rotation_v2_formats),

-- 
2.40.0

Re: [PATCH 07/11] drm/msm/dpu: add sspp cursor blocks to msm8998 hw catalog

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> Now that cursor sspp blocks can be used for cursor planes, enable them
> on msm8998. The dma sspp blocks that were assigned to cursor planes can
> now be used for overlay planes instead.

While the change is correct, there is more about it. Composers, using 
universal planes, will see this plane too. They have no obligations to 
use it only for the cursor. At the minimum could you please extend the 
plane_atomic_check to check for the plane dimensions for the CURSOR pipes?

For this change:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h    |  8 +++--
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     | 34 ++++++++++++++++++++++
>   2 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> index b07e8a9941f79..7de393b0f91d7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> @@ -90,10 +90,14 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
>   		sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
>   	SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x1ac, DMA_MSM8998_MASK,
>   		sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> -	SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> +	SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_MSM8998_MASK,
>   		sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> -	SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> +	SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_MSM8998_MASK,
>   		sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> +	SSPP_BLK("sspp_12", SSPP_CURSOR0, 0x34000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> +		msm8998_cursor_sblk_0, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> +	SSPP_BLK("sspp_13", SSPP_CURSOR1, 0x36000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> +		msm8998_cursor_sblk_1, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
>   };
>   
>   static const struct dpu_lm_cfg msm8998_lm[] = {
> 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 8d5d782a43398..f34fa704936bc 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -242,6 +242,22 @@ static const uint32_t wb2_formats[] = {
>   	DRM_FORMAT_XBGR4444,
>   };
>   
> +static const uint32_t cursor_formats[] = {
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_RGBA8888,
> +	DRM_FORMAT_BGRA8888,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_ARGB1555,
> +	DRM_FORMAT_ABGR1555,
> +	DRM_FORMAT_RGBA5551,
> +	DRM_FORMAT_BGRA5551,
> +	DRM_FORMAT_ARGB4444,
> +	DRM_FORMAT_ABGR4444,
> +	DRM_FORMAT_RGBA4444,
> +	DRM_FORMAT_BGRA4444,
> +};
> +
>   /*************************************************************
>    * SSPP sub blocks config
>    *************************************************************/
> @@ -300,6 +316,19 @@ static const uint32_t wb2_formats[] = {
>   	.virt_num_formats = ARRAY_SIZE(plane_formats), \
>   	}
>   
> +#define _CURSOR_SBLK(num) \
> +	{ \
> +	.maxdwnscale = SSPP_UNITY_SCALE, \
> +	.maxupscale = SSPP_UNITY_SCALE, \
> +	.smart_dma_priority = 0, \
> +	.src_blk = {.name = STRCAT("sspp_src_", num), \
> +		.id = DPU_SSPP_SRC, .base = 0x00, .len = 0x150,}, \
> +	.format_list = cursor_formats, \
> +	.num_formats = ARRAY_SIZE(cursor_formats), \
> +	.virt_format_list = cursor_formats, \
> +	.virt_num_formats = ARRAY_SIZE(cursor_formats), \
> +	}
> +
>   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_0 =
>   				_VIG_SBLK("0", 0, DPU_SSPP_SCALER_QSEED3);
>   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_1 =
> @@ -309,6 +338,11 @@ static const struct dpu_sspp_sub_blks msm8998_vig_sblk_2 =
>   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_3 =
>   				_VIG_SBLK("3", 0, DPU_SSPP_SCALER_QSEED3);
>   
> +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_0 =
> +				_CURSOR_SBLK("12");
> +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_1 =
> +				_CURSOR_SBLK("13");
> +
>   static const struct dpu_rotation_cfg dpu_rot_sc7280_cfg_v2 = {
>   	.rot_maxheight = 1088,
>   	.rot_num_formats = ARRAY_SIZE(rotation_v2_formats),
> 

-- 
With best wishes
Dmitry

Re: [PATCH 07/11] drm/msm/dpu: add sspp cursor blocks to msm8998 hw catalog

[Arnaud Vrac]

Le jeu. 20 avr. 2023 à 01:10, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> a écrit :
>
> On 19/04/2023 17:41, Arnaud Vrac wrote:
> > Now that cursor sspp blocks can be used for cursor planes, enable them
> > on msm8998. The dma sspp blocks that were assigned to cursor planes can
> > now be used for overlay planes instead.
>
> While the change is correct, there is more about it. Composers, using
> universal planes, will see this plane too. They have no obligations to
> use it only for the cursor. At the minimum could you please extend the
> plane_atomic_check to check for the plane dimensions for the CURSOR pipes?

Hum, I had assumed the generic atomic checks would already do this,
but it's not the case. I'll add the check when the pipe is of type
SSPP_CURSOR in another patch coming before, thanks.

>
> For this change:
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> >
> > Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> > ---
> >   .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h    |  8 +++--
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     | 34 ++++++++++++++++++++++
> >   2 files changed, 40 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> > index b07e8a9941f79..7de393b0f91d7 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> > @@ -90,10 +90,14 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> >               sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> >       SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x1ac, DMA_MSM8998_MASK,
> >               sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> > -     SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > +     SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_MSM8998_MASK,
> >               sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> > -     SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > +     SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_MSM8998_MASK,
> >               sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> > +     SSPP_BLK("sspp_12", SSPP_CURSOR0, 0x34000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > +             msm8998_cursor_sblk_0, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> > +     SSPP_BLK("sspp_13", SSPP_CURSOR1, 0x36000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > +             msm8998_cursor_sblk_1, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
> >   };
> >
> >   static const struct dpu_lm_cfg msm8998_lm[] = {
> > 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 8d5d782a43398..f34fa704936bc 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > @@ -242,6 +242,22 @@ static const uint32_t wb2_formats[] = {
> >       DRM_FORMAT_XBGR4444,
> >   };
> >
> > +static const uint32_t cursor_formats[] = {
> > +     DRM_FORMAT_ARGB8888,
> > +     DRM_FORMAT_ABGR8888,
> > +     DRM_FORMAT_RGBA8888,
> > +     DRM_FORMAT_BGRA8888,
> > +     DRM_FORMAT_XRGB8888,
> > +     DRM_FORMAT_ARGB1555,
> > +     DRM_FORMAT_ABGR1555,
> > +     DRM_FORMAT_RGBA5551,
> > +     DRM_FORMAT_BGRA5551,
> > +     DRM_FORMAT_ARGB4444,
> > +     DRM_FORMAT_ABGR4444,
> > +     DRM_FORMAT_RGBA4444,
> > +     DRM_FORMAT_BGRA4444,
> > +};
> > +
> >   /*************************************************************
> >    * SSPP sub blocks config
> >    *************************************************************/
> > @@ -300,6 +316,19 @@ static const uint32_t wb2_formats[] = {
> >       .virt_num_formats = ARRAY_SIZE(plane_formats), \
> >       }
> >
> > +#define _CURSOR_SBLK(num) \
> > +     { \
> > +     .maxdwnscale = SSPP_UNITY_SCALE, \
> > +     .maxupscale = SSPP_UNITY_SCALE, \
> > +     .smart_dma_priority = 0, \
> > +     .src_blk = {.name = STRCAT("sspp_src_", num), \
> > +             .id = DPU_SSPP_SRC, .base = 0x00, .len = 0x150,}, \
> > +     .format_list = cursor_formats, \
> > +     .num_formats = ARRAY_SIZE(cursor_formats), \
> > +     .virt_format_list = cursor_formats, \
> > +     .virt_num_formats = ARRAY_SIZE(cursor_formats), \
> > +     }
> > +
> >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_0 =
> >                               _VIG_SBLK("0", 0, DPU_SSPP_SCALER_QSEED3);
> >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_1 =
> > @@ -309,6 +338,11 @@ static const struct dpu_sspp_sub_blks msm8998_vig_sblk_2 =
> >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_3 =
> >                               _VIG_SBLK("3", 0, DPU_SSPP_SCALER_QSEED3);
> >
> > +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_0 =
> > +                             _CURSOR_SBLK("12");
> > +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_1 =
> > +                             _CURSOR_SBLK("13");
> > +
> >   static const struct dpu_rotation_cfg dpu_rot_sc7280_cfg_v2 = {
> >       .rot_maxheight = 1088,
> >       .rot_num_formats = ARRAY_SIZE(rotation_v2_formats),
> >
>
> --
> With best wishes
> Dmitry
>

Re: [PATCH 07/11] drm/msm/dpu: add sspp cursor blocks to msm8998 hw catalog

[Dmitry Baryshkov]

On Thu, 20 Apr 2023 at 10:06, Arnaud Vrac <avrac@freebox.fr> wrote:
>
> Le jeu. 20 avr. 2023 à 01:10, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> a écrit :
> >
> > On 19/04/2023 17:41, Arnaud Vrac wrote:
> > > Now that cursor sspp blocks can be used for cursor planes, enable them
> > > on msm8998. The dma sspp blocks that were assigned to cursor planes can
> > > now be used for overlay planes instead.
> >
> > While the change is correct, there is more about it. Composers, using
> > universal planes, will see this plane too. They have no obligations to
> > use it only for the cursor. At the minimum could you please extend the
> > plane_atomic_check to check for the plane dimensions for the CURSOR pipes?
>
> Hum, I had assumed the generic atomic checks would already do this,

Atomic will have these checks for the legacy cursor API (using the
mode_config.cursor_width/cursor_height that you have added). But I
don't think there is a generic API for telling the core 'this plane is
slightly limited'.

Fortunately, once the virtual planes land and are taught about the
SSPP_CURSOR peculiarities, it should not matter, since the driver will
know that it should not select these pipes in the inappropriate cases.

> but it's not the case. I'll add the check when the pipe is of type
> SSPP_CURSOR in another patch coming before, thanks.
>
> >
> > For this change:
> >
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >
> > >
> > > Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> > > ---
> > >   .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h    |  8 +++--
> > >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     | 34 ++++++++++++++++++++++
> > >   2 files changed, 40 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> > > index b07e8a9941f79..7de393b0f91d7 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> > > @@ -90,10 +90,14 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> > >               sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> > >       SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x1ac, DMA_MSM8998_MASK,
> > >               sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> > > -     SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > > +     SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_MSM8998_MASK,
> > >               sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> > > -     SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > > +     SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_MSM8998_MASK,
> > >               sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> > > +     SSPP_BLK("sspp_12", SSPP_CURSOR0, 0x34000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > > +             msm8998_cursor_sblk_0, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> > > +     SSPP_BLK("sspp_13", SSPP_CURSOR1, 0x36000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > > +             msm8998_cursor_sblk_1, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
> > >   };
> > >
> > >   static const struct dpu_lm_cfg msm8998_lm[] = {
> > > 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 8d5d782a43398..f34fa704936bc 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > @@ -242,6 +242,22 @@ static const uint32_t wb2_formats[] = {
> > >       DRM_FORMAT_XBGR4444,
> > >   };
> > >
> > > +static const uint32_t cursor_formats[] = {
> > > +     DRM_FORMAT_ARGB8888,
> > > +     DRM_FORMAT_ABGR8888,
> > > +     DRM_FORMAT_RGBA8888,
> > > +     DRM_FORMAT_BGRA8888,
> > > +     DRM_FORMAT_XRGB8888,
> > > +     DRM_FORMAT_ARGB1555,
> > > +     DRM_FORMAT_ABGR1555,
> > > +     DRM_FORMAT_RGBA5551,
> > > +     DRM_FORMAT_BGRA5551,
> > > +     DRM_FORMAT_ARGB4444,
> > > +     DRM_FORMAT_ABGR4444,
> > > +     DRM_FORMAT_RGBA4444,
> > > +     DRM_FORMAT_BGRA4444,
> > > +};
> > > +
> > >   /*************************************************************
> > >    * SSPP sub blocks config
> > >    *************************************************************/
> > > @@ -300,6 +316,19 @@ static const uint32_t wb2_formats[] = {
> > >       .virt_num_formats = ARRAY_SIZE(plane_formats), \
> > >       }
> > >
> > > +#define _CURSOR_SBLK(num) \
> > > +     { \
> > > +     .maxdwnscale = SSPP_UNITY_SCALE, \
> > > +     .maxupscale = SSPP_UNITY_SCALE, \
> > > +     .smart_dma_priority = 0, \
> > > +     .src_blk = {.name = STRCAT("sspp_src_", num), \
> > > +             .id = DPU_SSPP_SRC, .base = 0x00, .len = 0x150,}, \
> > > +     .format_list = cursor_formats, \
> > > +     .num_formats = ARRAY_SIZE(cursor_formats), \
> > > +     .virt_format_list = cursor_formats, \
> > > +     .virt_num_formats = ARRAY_SIZE(cursor_formats), \
> > > +     }
> > > +
> > >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_0 =
> > >                               _VIG_SBLK("0", 0, DPU_SSPP_SCALER_QSEED3);
> > >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_1 =
> > > @@ -309,6 +338,11 @@ static const struct dpu_sspp_sub_blks msm8998_vig_sblk_2 =
> > >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_3 =
> > >                               _VIG_SBLK("3", 0, DPU_SSPP_SCALER_QSEED3);
> > >
> > > +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_0 =
> > > +                             _CURSOR_SBLK("12");
> > > +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_1 =
> > > +                             _CURSOR_SBLK("13");
> > > +
> > >   static const struct dpu_rotation_cfg dpu_rot_sc7280_cfg_v2 = {
> > >       .rot_maxheight = 1088,
> > >       .rot_num_formats = ARRAY_SIZE(rotation_v2_formats),
> > >
> >
> > --
> > With best wishes
> > Dmitry
> >



-- 
With best wishes
Dmitry

[PATCH 08/11] drm/msm/dpu: fix cursor block register bit offset in msm8998 hw catalog

[Arnaud Vrac]

This matches the value for both fbdev and sde implementations in the
downstream msm-4.4 repository.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
index 7de393b0f91d7..5ae1d41e3fa92 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
@@ -39,8 +39,8 @@ static const struct dpu_mdp_cfg msm8998_mdp[] = {
 	.clk_ctrls[DPU_CLK_CTRL_DMA1] = { .reg_off = 0x2b4, .bit_off = 8 },
 	.clk_ctrls[DPU_CLK_CTRL_DMA2] = { .reg_off = 0x2c4, .bit_off = 8 },
 	.clk_ctrls[DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2c4, .bit_off = 12 },
-	.clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { .reg_off = 0x3a8, .bit_off = 15 },
-	.clk_ctrls[DPU_CLK_CTRL_CURSOR1] = { .reg_off = 0x3b0, .bit_off = 15 },
+	.clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { .reg_off = 0x3a8, .bit_off = 16 },
+	.clk_ctrls[DPU_CLK_CTRL_CURSOR1] = { .reg_off = 0x3b0, .bit_off = 16 },
 	},
 };
 

-- 
2.40.0

Re: [PATCH 08/11] drm/msm/dpu: fix cursor block register bit offset in msm8998 hw catalog

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> This matches the value for both fbdev and sde implementations in the
> downstream msm-4.4 repository.
> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

[PATCH 09/11] drm/msm/dpu: set max cursor width to 512x512

[Arnaud Vrac]

Override the default max cursor size reported to userspace of 64x64.
MSM8998 hw cursor planes support 512x512 size, and other chips use DMA
SSPPs.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 6cce0f6cfcb01..2dd19b7aca0f8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1014,6 +1014,9 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 	dpu_kms = to_dpu_kms(kms);
 	dev = dpu_kms->dev;
 
+	dev->mode_config.cursor_width = 512;
+	dev->mode_config.cursor_height = 512;
+
 	rc = dpu_kms_global_obj_init(dpu_kms);
 	if (rc)
 		return rc;

-- 
2.40.0

Re: [PATCH 09/11] drm/msm/dpu: set max cursor width to 512x512

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> Override the default max cursor size reported to userspace of 64x64.
> MSM8998 hw cursor planes support 512x512 size, and other chips use DMA
> SSPPs.
> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++
>   1 file changed, 3 insertions(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

[PATCH 10/11] drm/msm/dpu: tweak lm pairings in msm8998 hw catalog

[Arnaud Vrac]

Change lm blocks pairs so that lm blocks with the same features are
paired together:

LM_0 and LM_1 with PP and DSPP
LM_2 and LM_5 with PP
LM_3 and LM_4

This matches the sdm845 configuration and allows using pp or dspp when 2
lm blocks are needed in the topology. In the previous config the
reservation code could never find an lm pair without a matching feature
set.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
index 5ae1d41e3fa92..90db622eff4fa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
@@ -102,17 +102,17 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
 
 static const struct dpu_lm_cfg msm8998_lm[] = {
 	LM_BLK("lm_0", LM_0, 0x44000, MIXER_MSM8998_MASK,
-		&msm8998_lm_sblk, PINGPONG_0, LM_2, DSPP_0),
+		&msm8998_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
 	LM_BLK("lm_1", LM_1, 0x45000, MIXER_MSM8998_MASK,
-		&msm8998_lm_sblk, PINGPONG_1, LM_5, DSPP_1),
+		&msm8998_lm_sblk, PINGPONG_1, LM_0, DSPP_1),
 	LM_BLK("lm_2", LM_2, 0x46000, MIXER_MSM8998_MASK,
-		&msm8998_lm_sblk, PINGPONG_2, LM_0, 0),
+		&msm8998_lm_sblk, PINGPONG_2, LM_5, 0),
 	LM_BLK("lm_3", LM_3, 0x47000, MIXER_MSM8998_MASK,
 		&msm8998_lm_sblk, PINGPONG_MAX, 0, 0),
 	LM_BLK("lm_4", LM_4, 0x48000, MIXER_MSM8998_MASK,
 		&msm8998_lm_sblk, PINGPONG_MAX, 0, 0),
 	LM_BLK("lm_5", LM_5, 0x49000, MIXER_MSM8998_MASK,
-		&msm8998_lm_sblk, PINGPONG_3, LM_1, 0),
+		&msm8998_lm_sblk, PINGPONG_3, LM_2, 0),
 };
 
 static const struct dpu_pingpong_cfg msm8998_pp[] = {

-- 
2.40.0

Re: [PATCH 10/11] drm/msm/dpu: tweak lm pairings in msm8998 hw catalog

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> Change lm blocks pairs so that lm blocks with the same features are
> paired together:
> 
> LM_0 and LM_1 with PP and DSPP
> LM_2 and LM_5 with PP
> LM_3 and LM_4
> 
> This matches the sdm845 configuration and allows using pp or dspp when 2
> lm blocks are needed in the topology. In the previous config the
> reservation code could never find an lm pair without a matching feature
> set.

And this matches the hardcoded configuration in msm-4.4

Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)


-- 
With best wishes
Dmitry

Re: [PATCH 10/11] drm/msm/dpu: tweak lm pairings in msm8998 hw catalog

[Abhinav Kumar]

On 4/19/2023 7:41 AM, Arnaud Vrac wrote:
> Change lm blocks pairs so that lm blocks with the same features are
> paired together:
> 
> LM_0 and LM_1 with PP and DSPP
> LM_2 and LM_5 with PP
> LM_3 and LM_4
> 
> This matches the sdm845 configuration and allows using pp or dspp when 2
> lm blocks are needed in the topology. In the previous config the
> reservation code could never find an lm pair without a matching feature
> set.
> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>

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

[PATCH 11/11] drm/msm/dpu: do not use mixer that supports dspp when not required

[Arnaud Vrac]

This avoids using lm blocks that support DSPP when not needed, to
keep those resources available.

Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f4dda88a73f7d..4b393d46c743f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -362,7 +362,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(struct dpu_rm *rm,
 	*pp_idx = idx;
 
 	if (!reqs->topology.num_dspp)
-		return true;
+		return !lm_cfg->dspp;
 
 	idx = lm_cfg->dspp - DSPP_0;
 	if (idx < 0 || idx >= ARRAY_SIZE(rm->dspp_blks)) {

-- 
2.40.0

Re: [PATCH 11/11] drm/msm/dpu: do not use mixer that supports dspp when not required

[Dmitry Baryshkov]

On 19/04/2023 17:41, Arnaud Vrac wrote:
> This avoids using lm blocks that support DSPP when not needed, to
> keep those resources available.

This will break some of the platforms. Consider qcm2290 which has a 
single LM with DSPP. So, _dpu_rm_check_lm_and_get_connected_blks should 
be performed in two steps: first skip non-DSPP-enabled LMs when DSPP is 
not required. Then, if the LM (pair) is not found, look for any suitable 
LM(pair).

> 
> Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index f4dda88a73f7d..4b393d46c743f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -362,7 +362,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(struct dpu_rm *rm,
>   	*pp_idx = idx;
>   
>   	if (!reqs->topology.num_dspp)
> -		return true;
> +		return !lm_cfg->dspp;
>   
>   	idx = lm_cfg->dspp - DSPP_0;
>   	if (idx < 0 || idx >= ARRAY_SIZE(rm->dspp_blks)) {
> 

-- 
With best wishes
Dmitry

Re: [PATCH 11/11] drm/msm/dpu: do not use mixer that supports dspp when not required

[Arnaud Vrac]

Le jeu. 20 avr. 2023 à 01:18, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> a écrit :
>
> On 19/04/2023 17:41, Arnaud Vrac wrote:
> > This avoids using lm blocks that support DSPP when not needed, to
> > keep those resources available.
>
> This will break some of the platforms. Consider qcm2290 which has a
> single LM with DSPP. So, _dpu_rm_check_lm_and_get_connected_blks should
> be performed in two steps: first skip non-DSPP-enabled LMs when DSPP is
> not required. Then, if the LM (pair) is not found, look for any suitable
> LM(pair).

Good point, I'll add the change.

>
> >
> > Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> > index f4dda88a73f7d..4b393d46c743f 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> > @@ -362,7 +362,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(struct dpu_rm *rm,
> >       *pp_idx = idx;
> >
> >       if (!reqs->topology.num_dspp)
> > -             return true;
> > +             return !lm_cfg->dspp;
> >
> >       idx = lm_cfg->dspp - DSPP_0;
> >       if (idx < 0 || idx >= ARRAY_SIZE(rm->dspp_blks)) {
> >
>
> --
> With best wishes
> Dmitry
>

Re: (subset) [PATCH 00/11] drm/msm/dpu: tweaks for better hardware resources allocation

[Abhinav Kumar]

On Wed, 19 Apr 2023 16:41:07 +0200, Arnaud Vrac wrote:
> This series include misc fixes related to hardware resource allocations
> in the msm dpu driver, some specifically for msm8998 (including hw
> catalog fixes and cursor sspp support for cursor planes, instead of
> using Smart DMA pipes).
> 
> This series has been tested on msm8998 with additional patches to enable
> hdmi support.
> 
> [...]

Applied, thanks!

[01/11] drm/msm/dpu: tweak msm8998 hw catalog values
        https://gitlab.freedesktop.org/abhinavk/msm/-/commit/3f23a52fc2b8
[10/11] drm/msm/dpu: tweak lm pairings in msm8998 hw catalog
        https://gitlab.freedesktop.org/abhinavk/msm/-/commit/686eb89b1036

Best regards,
-- 
Abhinav Kumar <quic_abhinavk@quicinc.com>