On Wed, 25 Sept 2024 at 22:39, Jessica Zhang <quic_jesszhan@xxxxxxxxxxx> wrote:
>
>
>
> 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@xxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxx>
> >> [quic_abhinavk@xxxxxxxxxxx: Refactored resource allocation for CDM]
> >> Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> >> [quic_jesszhan@xxxxxxxxxxx: Changed to grabbing exising global state]
> >> Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
> >> ---
> >> 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.
I'd prefer for this to be more natural rather than just checking for
the DP && DP_YUV420 && WB && WB_FMT_YUV. In the worst case, count CDM
requests and then in RM check them against the catalog. But I had a
more logical (although more intrusive) implementation on my mind:
struct msm_display_topology {
struct {
u32 num_intf;
u32 num_wb;
u32 num_dsc;
bool needs_cdm;
} outputs[MAX_OUTPUTS];
u32 num_lm;
};
WDYT?
>
> Thanks,
>
> Jessica Zhang
>
> >
> >> }
> >>
> > --
> > With best wishes
> > Dmitry
>
--
With best wishes
Dmitry
