1. Patchew
  2. linux
  3. [PATCH v1 0/5] add DSC 1.2 dpu supports
  4. View patch

[PATCH v1 3/5] drm/msm/dpu: save dpu topology configuration

Kuogee Hsieh posted 5 patches 2 years, 8 months ago
There is a newer version of this series
[PATCH v1 3/5] drm/msm/dpu: save dpu topology configuration
Posted by Kuogee Hsieh 2 years, 8 months ago 
At current implementation, topology configuration is thrown away after
dpu_rm_reserve(). This patch save the topology so that it can be used
for DSC related calculation later.

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 32 ++++++++++++++---------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index ecb87bc..2fdacf1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -542,13 +542,13 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
 	return (num_dsc > 0) && (num_dsc > intf_count);
 }
 
-static struct msm_display_topology dpu_encoder_get_topology(
+static void dpu_encoder_get_topology(
 			struct dpu_encoder_virt *dpu_enc,
 			struct dpu_kms *dpu_kms,
 			struct drm_display_mode *mode,
-			struct drm_crtc_state *crtc_state)
+			struct drm_crtc_state *crtc_state,
+			struct msm_display_topology *topology)
 {
-	struct msm_display_topology topology = {0};
 	int i, intf_count = 0;
 
 	for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
@@ -567,16 +567,16 @@ static struct msm_display_topology dpu_encoder_get_topology(
 	 * Add dspps to the reservation requirements if ctm is requested
 	 */
 	if (intf_count == 2)
-		topology.num_lm = 2;
+		topology->num_lm = 2;
 	else if (!dpu_kms->catalog->caps->has_3d_merge)
-		topology.num_lm = 1;
+		topology->num_lm = 1;
 	else
-		topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
+		topology->num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
 
 	if (crtc_state->ctm)
-		topology.num_dspp = topology.num_lm;
+		topology->num_dspp = topology->num_lm;
 
-	topology.num_intf = intf_count;
+	topology->num_intf = intf_count;
 
 	if (dpu_enc->dsc) {
 		/*
@@ -585,12 +585,10 @@ static struct msm_display_topology dpu_encoder_get_topology(
 		 * this is power optimal and can drive up to (including) 4k
 		 * screens
 		 */
-		topology.num_dsc = 2;
-		topology.num_lm = 2;
-		topology.num_intf = 1;
+		topology->num_dsc = 2;
+		topology->num_lm = 2;
+		topology->num_intf = 1;
 	}
-
-	return topology;
 }
 
 static int dpu_encoder_virt_atomic_check(
@@ -602,7 +600,7 @@ static int dpu_encoder_virt_atomic_check(
 	struct msm_drm_private *priv;
 	struct dpu_kms *dpu_kms;
 	struct drm_display_mode *adj_mode;
-	struct msm_display_topology topology;
+	struct msm_display_topology *topology;
 	struct dpu_global_state *global_state;
 	int i = 0;
 	int ret = 0;
@@ -639,7 +637,9 @@ static int dpu_encoder_virt_atomic_check(
 		}
 	}
 
-	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
+	topology = &dpu_enc->topology;
+	memset(topology, 0, sizeof (*topology));
+	dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, topology);
 
 	/*
 	 * Release and Allocate resources on every modeset
@@ -650,7 +650,7 @@ static int dpu_encoder_virt_atomic_check(
 
 		if (!crtc_state->active_changed || crtc_state->enable)
 			ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
-					drm_enc, crtc_state, topology);
+					drm_enc, crtc_state, *topology);
 	}
 
 	trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v1 3/5] drm/msm/dpu: save dpu topology configuration
Posted by Dmitry Baryshkov 2 years, 8 months ago 
On 21/04/2023 02:25, Kuogee Hsieh wrote:
> At current implementation, topology configuration is thrown away after
> dpu_rm_reserve(). This patch save the topology so that it can be used
> for DSC related calculation later.

Please take a look at 
https://patchwork.freedesktop.org/patch/527960/?series=115283&rev=2 .

> 
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 32 ++++++++++++++---------------
>   1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ecb87bc..2fdacf1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -542,13 +542,13 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
>   	return (num_dsc > 0) && (num_dsc > intf_count);
>   }
>   
> -static struct msm_display_topology dpu_encoder_get_topology(
> +static void dpu_encoder_get_topology(
>   			struct dpu_encoder_virt *dpu_enc,
>   			struct dpu_kms *dpu_kms,
>   			struct drm_display_mode *mode,
> -			struct drm_crtc_state *crtc_state)
> +			struct drm_crtc_state *crtc_state,
> +			struct msm_display_topology *topology)
>   {
> -	struct msm_display_topology topology = {0};
>   	int i, intf_count = 0;
>   
>   	for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
> @@ -567,16 +567,16 @@ static struct msm_display_topology dpu_encoder_get_topology(
>   	 * Add dspps to the reservation requirements if ctm is requested
>   	 */
>   	if (intf_count == 2)
> -		topology.num_lm = 2;
> +		topology->num_lm = 2;
>   	else if (!dpu_kms->catalog->caps->has_3d_merge)
> -		topology.num_lm = 1;
> +		topology->num_lm = 1;
>   	else
> -		topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
> +		topology->num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
>   
>   	if (crtc_state->ctm)
> -		topology.num_dspp = topology.num_lm;
> +		topology->num_dspp = topology->num_lm;
>   
> -	topology.num_intf = intf_count;
> +	topology->num_intf = intf_count;
>   
>   	if (dpu_enc->dsc) {
>   		/*
> @@ -585,12 +585,10 @@ static struct msm_display_topology dpu_encoder_get_topology(
>   		 * this is power optimal and can drive up to (including) 4k
>   		 * screens
>   		 */
> -		topology.num_dsc = 2;
> -		topology.num_lm = 2;
> -		topology.num_intf = 1;
> +		topology->num_dsc = 2;
> +		topology->num_lm = 2;
> +		topology->num_intf = 1;
>   	}
> -
> -	return topology;
>   }
>   
>   static int dpu_encoder_virt_atomic_check(
> @@ -602,7 +600,7 @@ static int dpu_encoder_virt_atomic_check(
>   	struct msm_drm_private *priv;
>   	struct dpu_kms *dpu_kms;
>   	struct drm_display_mode *adj_mode;
> -	struct msm_display_topology topology;
> +	struct msm_display_topology *topology;
>   	struct dpu_global_state *global_state;
>   	int i = 0;
>   	int ret = 0;
> @@ -639,7 +637,9 @@ static int dpu_encoder_virt_atomic_check(
>   		}
>   	}
>   
> -	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
> +	topology = &dpu_enc->topology;
> +	memset(topology, 0, sizeof (*topology));
> +	dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, topology);
>   
>   	/*
>   	 * Release and Allocate resources on every modeset
> @@ -650,7 +650,7 @@ static int dpu_encoder_virt_atomic_check(
>   
>   		if (!crtc_state->active_changed || crtc_state->enable)
>   			ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
> -					drm_enc, crtc_state, topology);
> +					drm_enc, crtc_state, *topology);
>   	}
>   
>   	trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);

-- 
With best wishes
Dmitry
Re: [PATCH v1 3/5] drm/msm/dpu: save dpu topology configuration
Posted by Abhinav Kumar 2 years, 8 months ago 

On 4/20/2023 5:12 PM, Dmitry Baryshkov wrote:
> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>> At current implementation, topology configuration is thrown away after
>> dpu_rm_reserve(). This patch save the topology so that it can be used
>> for DSC related calculation later.
> 
> Please take a look at 
> https://patchwork.freedesktop.org/patch/527960/?series=115283&rev=2 .

Let the review of this series go on and lets try to get the acks on the 
non-topology related pieces. I think 2/5 patches in this series are 
conflicting in the design. We will resolve that in a weeks time. 
Meanwhile, I think we can keep the reviews / versions going on the rest.

I think we can move patch 5 of this series to patch 3. That way we get 
acks on patches 1-3 and patches 4 & 5 which deal with topology are dealt 
with together with virtual planes.

I will review virtual planes next week and we will decide the best 
course of action. Moving resource allocation to CRTC needs to be thought 
of a bit deeper for DSC as that one is directly tied to encoder.

> 
>>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 32 
>> ++++++++++++++---------------
>>   1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index ecb87bc..2fdacf1 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -542,13 +542,13 @@ bool dpu_encoder_use_dsc_merge(struct 
>> drm_encoder *drm_enc)
>>       return (num_dsc > 0) && (num_dsc > intf_count);
>>   }
>> -static struct msm_display_topology dpu_encoder_get_topology(
>> +static void dpu_encoder_get_topology(
>>               struct dpu_encoder_virt *dpu_enc,
>>               struct dpu_kms *dpu_kms,
>>               struct drm_display_mode *mode,
>> -            struct drm_crtc_state *crtc_state)
>> +            struct drm_crtc_state *crtc_state,
>> +            struct msm_display_topology *topology)
>>   {
>> -    struct msm_display_topology topology = {0};
>>       int i, intf_count = 0;
>>       for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
>> @@ -567,16 +567,16 @@ static struct msm_display_topology 
>> dpu_encoder_get_topology(
>>        * Add dspps to the reservation requirements if ctm is requested
>>        */
>>       if (intf_count == 2)
>> -        topology.num_lm = 2;
>> +        topology->num_lm = 2;
>>       else if (!dpu_kms->catalog->caps->has_3d_merge)
>> -        topology.num_lm = 1;
>> +        topology->num_lm = 1;
>>       else
>> -        topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
>> +        topology->num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 
>> : 1;
>>       if (crtc_state->ctm)
>> -        topology.num_dspp = topology.num_lm;
>> +        topology->num_dspp = topology->num_lm;
>> -    topology.num_intf = intf_count;
>> +    topology->num_intf = intf_count;
>>       if (dpu_enc->dsc) {
>>           /*
>> @@ -585,12 +585,10 @@ static struct msm_display_topology 
>> dpu_encoder_get_topology(
>>            * this is power optimal and can drive up to (including) 4k
>>            * screens
>>            */
>> -        topology.num_dsc = 2;
>> -        topology.num_lm = 2;
>> -        topology.num_intf = 1;
>> +        topology->num_dsc = 2;
>> +        topology->num_lm = 2;
>> +        topology->num_intf = 1;
>>       }
>> -
>> -    return topology;
>>   }
>>   static int dpu_encoder_virt_atomic_check(
>> @@ -602,7 +600,7 @@ static int dpu_encoder_virt_atomic_check(
>>       struct msm_drm_private *priv;
>>       struct dpu_kms *dpu_kms;
>>       struct drm_display_mode *adj_mode;
>> -    struct msm_display_topology topology;
>> +    struct msm_display_topology *topology;
>>       struct dpu_global_state *global_state;
>>       int i = 0;
>>       int ret = 0;
>> @@ -639,7 +637,9 @@ static int dpu_encoder_virt_atomic_check(
>>           }
>>       }
>> -    topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, 
>> crtc_state);
>> +    topology = &dpu_enc->topology;
>> +    memset(topology, 0, sizeof (*topology));
>> +    dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, 
>> topology);
>>       /*
>>        * Release and Allocate resources on every modeset
>> @@ -650,7 +650,7 @@ static int dpu_encoder_virt_atomic_check(
>>           if (!crtc_state->active_changed || crtc_state->enable)
>>               ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
>> -                    drm_enc, crtc_state, topology);
>> +                    drm_enc, crtc_state, *topology);
>>       }
>>       trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.