[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <24a11f4c-d848-4f1b-afbd-35b135fa3105@quicinc.com>
Date: Wed, 25 Sep 2024 13:38:51 -0700
From: Jessica Zhang <quic_jesszhan@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: Rob Clark <robdclark@...il.com>, <quic_abhinavk@...cinc.com>,
Sean Paul
<sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
"David
Airlie" <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
"Maarten
Lankhorst" <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard
<mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, <quic_ebharadw@...cinc.com>,
<linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
Rob Clark <robdclark@...omium.org>,
Ville Syrjälä <ville.syrjala@...ux.intel.com>
Subject: Re: [PATCH v2 05/22] drm/msm/dpu: move resource allocation to CRTC
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@...aro.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@...aro.org>
>> [quic_abhinavk@...cinc.com: Refactored resource allocation for CDM]
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>> [quic_jesszhan@...cinc.com: Changed to grabbing exising global state]
>> Signed-off-by: Jessica Zhang <quic_jesszhan@...cinc.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
Powered by blists - more mailing lists