On 5/31/2023 10:53 AM, Dmitry Baryshkov wrote:
On Wed, 31 May 2023 at 20:29, Kuogee Hsieh <quic_khs...@quicinc.com> wrote:
On 5/31/2023 10:12 AM, Dmitry Baryshkov wrote:
On Wed, 31 May 2023 at 18:41, Kuogee Hsieh <quic_khs...@quicinc.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).
got it.
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?
I would think should be DP side. there is no reason to enable dsc on
both DSI and DP fro a bridge chip.
My above statement is not correct. For DSI-to-DP bridge,
drm_bridge_chain_get_first_bridge(drm_enc) return DSI bridge.
Is possible that DSC feature can be added to DSI-to-DP bridge?
If it is not possible, then we can rule out DSI-to-DP bridge case, then
use struct drm_bridge to retrieve DSC form controller will work.
Well, there can be. E.g. to lower the clock rates of the DSI link.
drm_bridge_chain_get_first_bridge(drm_enc) should return the bridge of
the controller.
In DSI-to-DP bridge chip case, this controller will be the bridge chip
who configured to perform protocol conversion between DSI and DP.
If DSC enabled should be at DP size which connect to panel.
Ok, so it returns the DSC configuration of the bridge's source side.
Now let's consider a panel bridge for the DSC-enabled DSI panel.
Should get_bridge_dsc() return a DSC config in this case?
Do you have other suggestion?
Let me think about it for a few days.
There is one option which is keep current
1) keep struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi
*msm_dsi) at dsi.c
2) use struct msm_display_info *disp_info saved at dpu_enc to locate
struct msm_dsi from priv->dsi[] list (see item #3)
3) info.dsc = msm_dsi_get_dsc_config(priv->dsi[info.h_tile_instance[0]]);
4) ballistically, keep original code but move info.dsc =
msm_dsi_get_dsc_config(priv->dsi[i]); to other place sush as
atomic_check() and atomic_enable().