Re: [PATCH v2 05/22] drm/msm/dpu: move resource allocation to CRTC

[Jessica Zhang <quic_jesszhan@quicinc.com>]

On 9/24/2024 4:13 PM, Dmitry Baryshkov wrote:
> On Tue, Sep 24, 2024 at 03:59:21PM GMT, Jessica Zhang wrote:
>> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>
>> All resource allocation is centered around the LMs. Then other blocks
>> (except DSCs) are allocated basing on the LMs that was selected, and LM
>> powers up the CRTC rather than the encoder.
>>
>> Moreover if at some point the driver supports encoder cloning,
>> allocating resources from the encoder will be incorrect, as all clones
>> will have different encoder IDs, while LMs are to be shared by these
>> encoders.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> [quic_abhinavk@quicinc.com: Refactored resource allocation for CDM]
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> [quic_jesszhan@quicinc.com: Changed to grabbing exising global state]
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    |  86 ++++++++++++
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 201 +++++++++++-----------------
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  19 +++
>>   3 files changed, 183 insertions(+), 123 deletions(-)
>>
>> @@ -544,159 +542,117 @@ void dpu_encoder_helper_split_config(
>>   	}
>>   }
>>   
>> -bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
>> +void dpu_encoder_update_topology(struct drm_encoder *drm_enc,
>> +				 struct msm_display_topology *topology,
>> +				 struct drm_atomic_state *state,
>> +				 const struct drm_display_mode *adj_mode)
>>   {
>>   	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
>> -	int i, intf_count = 0, num_dsc = 0;
>> +	struct drm_connector *connector;
>> +	struct drm_connector_state *conn_state;
>> +	struct msm_display_info *disp_info;
>> +	struct drm_framebuffer *fb;
>> +	struct msm_drm_private *priv;
>> +	int i;
>>   
>>   	for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
>>   		if (dpu_enc->phys_encs[i])
>> -			intf_count++;
>> +			topology->num_intf++;
>>   
>> -	/* See dpu_encoder_get_topology, we only support 2:2:1 topology */
>> +	/* We only support 2 DSC mode (with 2 LM and 1 INTF) */
>>   	if (dpu_enc->dsc)
>> -		num_dsc = 2;
>> +		topology->num_dsc += 2;
>>   
>> -	return (num_dsc > 0) && (num_dsc > intf_count);
>> -}
>> +	connector = drm_atomic_get_new_connector_for_encoder(state, drm_enc);
>> +	if (!connector)
>> +		return;
>> +	conn_state = drm_atomic_get_new_connector_state(state, connector);
>> +	if (!conn_state)
>> +		return;
>>   
>> -struct drm_dsc_config *dpu_encoder_get_dsc_config(struct drm_encoder *drm_enc)
>> -{
>> -	struct msm_drm_private *priv = drm_enc->dev->dev_private;
>> -	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
>> -	int index = dpu_enc->disp_info.h_tile_instance[0];
>> +	disp_info = &dpu_enc->disp_info;
>>   
>> -	if (dpu_enc->disp_info.intf_type == INTF_DSI)
>> -		return msm_dsi_get_dsc_config(priv->dsi[index]);
>> +	priv = drm_enc->dev->dev_private;
>>   
>> -	return NULL;
>> +	/*
>> +	 * Use CDM only for writeback or DP at the moment as other interfaces cannot handle it.
>> +	 * If writeback itself cannot handle cdm for some reason it will fail in its atomic_check()
>> +	 * earlier.
>> +	 */
>> +	if (disp_info->intf_type == INTF_WB && conn_state->writeback_job) {
>> +		fb = conn_state->writeback_job->fb;
>> +
>> +		if (fb && MSM_FORMAT_IS_YUV(msm_framebuffer_format(fb)))
>> +			topology->needs_cdm = true;
>> +	} else if (disp_info->intf_type == INTF_DP) {
>> +		if (msm_dp_is_yuv_420_enabled(priv->dp[disp_info->h_tile_instance[0]], adj_mode))
>> +			topology->needs_cdm = true;
>> +	}
> 
> Just to note, the needs_cdm is not enough once you introduce CWB
> support. E.g. DP/YUV420 + WB/YUV case requires two CDM blocks (which we
> don't have), but this doesn't get reflected in the topology.

Hi Dmitry,

Ack. I can add something to make atomic_check fail if the input FB is 
YUV format and CWB is enabled.

Thanks,

Jessica Zhang

> 
>>   }
>>   
> -- 
> With best wishes
> Dmitry