Re: [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

[Kuogee Hsieh]

> >   +    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?

>