[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA8EJponkEne2vVsNP=2Fxmv=Uc6i_LzAGBSEz9hPPotCEpGzg@mail.gmail.com>
Date: Wed, 31 May 2023 20:12:31 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Kuogee Hsieh <quic_khsieh@...cinc.com>
Cc: dri-devel@...ts.freedesktop.org, robdclark@...il.com,
sean@...rly.run, swboyd@...omium.org, dianders@...omium.org,
vkoul@...nel.org, daniel@...ll.ch, airlied@...il.com,
agross@...nel.org, andersson@...nel.org, quic_abhinavk@...cinc.com,
quic_jesszhan@...cinc.com, quic_sbillaka@...cinc.com,
marijn.suijten@...ainline.org, freedreno@...ts.freedesktop.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()
On Wed, 31 May 2023 at 18:41, Kuogee Hsieh <quic_khsieh@...cinc.com> wrote:
>
>
>
> >> + if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
> >
> > INTF_DSI
> >
> >> + struct drm_bridge *bridge;
> >> +
> >> + if (!dpu_enc->dsc) {
> >
> > This condition is not correct. We should be updating the DSC even if
> > there is one.
> >
> >> + bridge = drm_bridge_chain_get_first_bridge(drm_enc);
> >> + dpu_enc->dsc = msm_dsi_bridge_get_dsc_config(bridge);
> >
> > This approach will not work for the hot-pluggable outputs. The dpu_enc
> > is not a part of the state. It should not be touched before
> > atomic_commit actually commits changes.
> where can drm_dsc_config be stored?
I'd say, get it during atomic_check (and don't store it anywhere).
Then get it during atomic_enable (and save in dpu_enc).
> >
> > Also, I don't think I like the API. It makes it impossible for the
> > driver to check that the bridge is the actually our DSI bridge or not.
> > Once you add DP here, the code will explode.
> >
> > I think instead we should extend the drm_bridge API to be able to get
> > the DSC configuration from it directly. Additional care should be put
> > to design an assymetrical API. Theoretically a drm_bridge can be both
> > DSC source and DSC sink. Imagine a DSI-to-DP or DSI-to-HDMI bridge,
> > supporting DSC on the DSI side too.
>
> Form my understanding, a bridge contains two interfaces.
>
> Therefore I would think only one bridge for dsi-to-dp bridge? and this
> bridge should represent the bridge chip?
>
> I am thinking adding an ops function, get_bridge_dsc() to struct
> drm_bridge_funcs to retrieve drm_dsc_config.
So, for this DSI-to-DP bridge will get_bridge_dsc() return DSC
configuration for the DSI or for the DP side of the bridge?
>
> Do you have other suggestion?
Let me think about it for a few days.
--
With best wishes
Dmitry
Powered by blists - more mailing lists