[PATCH 0/3] imx drm atomic mode setting fixups

[Ying Liu]

Hi Philipp,

On Wed, Jul 6, 2016 at 11:52 PM, Philipp Zabel <p.zabel at pengutronix.de> wrote:
> Hi,
>
> I have tested Liu Ying's imx drm atomic mode setting conversion series [1]

Thanks for the testing!

> with three different outputs (DPI + eDP bridge, LVDS + panel, HDMI)
> and noticed that in the LDB case bus_format handling doesn't work
> correctly if the format is determined by the panel instead of the
> device tree. In that case, the LVDS bus_format is not yet known at bind
> time, but is filled into the display_info structure by the panel driver
> in the get_modes callback. The LDB driver then has to translate from the
> LVDS bus format to the corresponding internal parallel bus format and
> let the crtc know about the format before crtc mode_set is called.
> The same issue occurs when the bus_format is determined by a bridge driver
> which implements its own connector. To solve this, and because crtc state
> seems to be the right place for potentially dynamic crtc settings, I have
> moved the bus_format, bus_flags, and di_*sync_pin configuration into an
> imx-drm specific crtc state structure and removed imx_drm_encoder again.

I didn't program imx_ldb_ch->imx_encoder.bus_format correctly
in imx_ldb_connector_get_modes() - I forgot to translate the bus format
from LVDS bus format to internal bus format.  Also, data width and
LVDS data mapping(SPWG/JEIDA) can be set to ldb->ldb_ctrl in
the function. So, one option to fix the issue you found is to simply
modify the function and respin the particular patch in my patch set.
Actually, I have got a fix tested with the format determined by the panel.

I don't have strong opinion on storing bus_format, bus_flags and
di_*sync_pin in imx_crtc_state.  But, very likely, they will not be
dynamically changed.  Setting them again and again at atomic_check
is somewhat wasting CPU cycles...

BTW, the imx-drm atomic conversion patch set has reached version 3
which uses the new non-blocking atomic commit helpers.

Regards,
Liu Ying

>
> I have also removed the now optional empty encoder mode_set callbacks
> and turned the container_of helper macros into inline functions.
>
> [1] https://lists.freedesktop.org/archives/dri-devel/2016-May/108218.html
>
> regards
> Philipp
>
> Philipp Zabel (3):
>   drm/imx: remove empty mode_set encoder callbacks
>   drm/imx: store internal bus configuration in crtc state
>   drm/imx: turn remaining container_of macros into inline functions
>
>  drivers/gpu/drm/imx/dw_hdmi-imx.c      |  38 +++++----
>  drivers/gpu/drm/imx/imx-drm.h          |   9 ++-
>  drivers/gpu/drm/imx/imx-ldb.c          | 142 +++++++++++++++++++++------------
>  drivers/gpu/drm/imx/imx-tve.c          |  45 ++++++++---
>  drivers/gpu/drm/imx/ipuv3-crtc.c       |  72 +++++++++++++----
>  drivers/gpu/drm/imx/ipuv3-plane.c      |   5 +-
>  drivers/gpu/drm/imx/parallel-display.c |  85 +++++++++++---------
>  7 files changed, 259 insertions(+), 137 deletions(-)
>
> --
> 2.8.1
>