[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <157e8219-7af2-c7ed-6d99-3caa6fbc11ba@quicinc.com>
Date: Wed, 31 May 2023 08:40:26 -0700
From: Kuogee Hsieh <quic_khsieh@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
<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>
CC: <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()
>> + 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?
>
> 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.
Do you have other suggestion?
>
Powered by blists - more mailing lists