On 2016-05-10 08:07, Philipp Zabel wrote:
> This patch allows panels to set pixel clock and data enable pin polarity
> other than the default of driving data at the rising pixel clock edge and
> active high display enable.
>
> Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>
> ---
> drivers/gpu/drm/imx/imx-drm-core.c | 9 ++++++---
> drivers/gpu/drm/imx/imx-drm.h | 5 +++--
> drivers/gpu/drm/imx/imx-tve.c | 4 +++-
> drivers/gpu/drm/imx/ipuv3-crtc.c | 9 ++++++---
> drivers/gpu/drm/imx/parallel-display.c | 4 ++--
> 5 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c
> b/drivers/gpu/drm/imx/imx-drm-core.c
> index 2453fb1..6e5891a 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -97,7 +97,7 @@ static struct imx_drm_crtc *imx_drm_find_crtc(struct
> drm_crtc *crtc)
> }
>
> int imx_drm_set_bus_format_pins(struct drm_encoder *encoder, u32 bus_format,
> - int hsync_pin, int vsync_pin)
> + int hsync_pin, int vsync_pin, u32 bus_flags)
> {
> struct imx_drm_crtc_helper_funcs *helper;
> struct imx_drm_crtc *imx_crtc;
> @@ -109,14 +109,17 @@ int imx_drm_set_bus_format_pins(struct
> drm_encoder *encoder, u32 bus_format,
> helper = &imx_crtc->imx_drm_helper_funcs;
> if (helper->set_interface_pix_fmt)
> return helper->set_interface_pix_fmt(encoder->crtc,
> - bus_format, hsync_pin, vsync_pin);
> + bus_format, hsync_pin, vsync_pin,
> + bus_flags);
> return 0;
> }
> EXPORT_SYMBOL_GPL(imx_drm_set_bus_format_pins);
>
> int imx_drm_set_bus_format(struct drm_encoder *encoder, u32 bus_format)
> {
> - return imx_drm_set_bus_format_pins(encoder, bus_format, 2, 3);
> + return imx_drm_set_bus_format_pins(encoder, bus_format, 2, 3,
> + DRM_BUS_FLAG_DE_HIGH |
> + DRM_BUS_FLAG_PIXDATA_POSEDGE);
> }
> EXPORT_SYMBOL_GPL(imx_drm_set_bus_format);
>
> diff --git a/drivers/gpu/drm/imx/imx-drm.h b/drivers/gpu/drm/imx/imx-drm.h
> index b0241b9..1400664 100644
> --- a/drivers/gpu/drm/imx/imx-drm.h
> +++ b/drivers/gpu/drm/imx/imx-drm.h
> @@ -19,7 +19,8 @@ struct imx_drm_crtc_helper_funcs {
> int (*enable_vblank)(struct drm_crtc *crtc);
> void (*disable_vblank)(struct drm_crtc *crtc);
> int (*set_interface_pix_fmt)(struct drm_crtc *crtc,
> - u32 bus_format, int hsync_pin, int vsync_pin);
> + u32 bus_format, int hsync_pin, int vsync_pin,
> + u32 bus_flags);
> const struct drm_crtc_helper_funcs *crtc_helper_funcs;
> const struct drm_crtc_funcs *crtc_funcs;
> };
> @@ -42,7 +43,7 @@ void imx_drm_mode_config_init(struct drm_device *drm);
> struct drm_gem_cma_object *imx_drm_fb_get_obj(struct drm_framebuffer *fb);
>
> int imx_drm_set_bus_format_pins(struct drm_encoder *encoder,
> - u32 bus_format, int hsync_pin, int vsync_pin);
> + u32 bus_format, int hsync_pin, int vsync_pin, u32 bus_flags);
> int imx_drm_set_bus_format(struct drm_encoder *encoder,
> u32 bus_format);
>
> diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
> index ae7a9fb..cb25582 100644
> --- a/drivers/gpu/drm/imx/imx-tve.c
> +++ b/drivers/gpu/drm/imx/imx-tve.c
> @@ -295,7 +295,9 @@ static void imx_tve_encoder_prepare(struct
> drm_encoder *encoder)
> switch (tve->mode) {
> case TVE_MODE_VGA:
> imx_drm_set_bus_format_pins(encoder, MEDIA_BUS_FMT_GBR888_1X24,
> - tve->hsync_pin, tve->vsync_pin);
> + tve->hsync_pin, tve->vsync_pin,
> + DRM_BUS_FLAG_DE_HIGH |
> + DRM_BUS_FLAG_PIXDATA_POSEDGE);
> break;
> case TVE_MODE_TVOUT:
> imx_drm_set_bus_format(encoder, MEDIA_BUS_FMT_YUV8_1X24);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c
> b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index dee8e8b..d8b58b0 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -66,6 +66,7 @@ struct ipu_crtc {
> struct ipu_flip_work *flip_work;
> int irq;
> u32 bus_format;
> + u32 bus_flags;
> int di_hsync_pin;
> int di_vsync_pin;
> };
> @@ -271,8 +272,9 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
> else
> sig_cfg.clkflags = 0;
>
> - sig_cfg.enable_pol = 1;
> - sig_cfg.clk_pol = 0;
> + sig_cfg.enable_pol = !(ipu_crtc->bus_flags & DRM_BUS_FLAG_DE_LOW);
> + sig_cfg.clk_pol = !!(ipu_crtc->bus_flags &
> + DRM_BUS_FLAG_PIXDATA_NEGEDGE);
I think this polarity is actually wrong...
My data sheet says: IPU_DI0_GENERAL di0_polarity_disp_clk:
- 1 The output clock is active high
- 0 The output clock is active low
What active high/low means in this context is totally unclear to me, so
I looked at the actual signals.
- 1 => The controller drives the data signal on rising edge
- 0 => The controller drives the data signal on falling edge
https://drive.google.com/file/d/0B8pBTl0mP98PVEpCRUJrNjJJakE/view
https://drive.google.com/file/d/0B8pBTl0mP98POTBvRW9ITVpQN00/view
Afaik, most displays sample on rising edge. Hence controllers should
drive on falling edge. This actually also matches the old default (0).
Since most display do not explicitly specify a polarity, it should
default to DRM_BUS_FLAG_PIXDATA_NEGEDGE (the flags are controller
centric, negedge means controller drives on falling edge, display
samples on positive/rising edge).
You should set clk_pol only if a display explicitly requests the
controller to drive on rising edge:
+ sig_cfg.clk_pol = !!(ipu_crtc->bus_flags &
+ DRM_BUS_FLAG_PIXDATA_POSEDGE);
...and call the function in the other places using the
DRM_BUS_FLAG_PIXDATA_NEGEDGE flag.
--
Stefan
> sig_cfg.bus_format = ipu_crtc->bus_format;
> sig_cfg.v_to_h_sync = 0;
> sig_cfg.hsync_pin = ipu_crtc->di_hsync_pin;
> @@ -396,11 +398,12 @@ static void ipu_disable_vblank(struct drm_crtc *crtc)
> }
>
> static int ipu_set_interface_pix_fmt(struct drm_crtc *crtc,
> - u32 bus_format, int hsync_pin, int vsync_pin)
> + u32 bus_format, int hsync_pin, int vsync_pin, u32 bus_flags)
> {
> struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
>
> ipu_crtc->bus_format = bus_format;
> + ipu_crtc->bus_flags = bus_flags;
> ipu_crtc->di_hsync_pin = hsync_pin;
> ipu_crtc->di_vsync_pin = vsync_pin;
>
> diff --git a/drivers/gpu/drm/imx/parallel-display.c
> b/drivers/gpu/drm/imx/parallel-display.c
> index 363e2c7..030235f 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -115,8 +115,8 @@ static void imx_pd_encoder_dpms(struct drm_encoder
> *encoder, int mode)
> static void imx_pd_encoder_prepare(struct drm_encoder *encoder)
> {
> struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
> -
> - imx_drm_set_bus_format(encoder, imxpd->bus_format);
> + imx_drm_set_bus_format_pins(encoder, imxpd->bus_format, 2, 3,
> + imxpd->connector.display_info.bus_flags);
> }
>
> static void imx_pd_encoder_commit(struct drm_encoder *encoder)
