Re: [PATCHv3 1/1] drm/imx: convey the pixelclk-active and de-active flags from DT to the ipu-di driver

From: Lothar WaÃmann
Date: Tue Jul 12 2016 - 07:54:47 EST

Hi,

On Wed, 25 May 2016 15:18:16 +0200 Lothar WaÃmann wrote:
> The 'de-active' and 'pixelclk-active' DT properties are evaluated
> by of_parse_display_timing() called from of_get_drm_display_mode(),
> but later lost in the conversion from videomode.flags to
> drm_display_mode.flags.
> Use an open coded version of of_get_drm_display_mode() to get access
> to these flags and make sure they are passed on to the ipu-di driver.
>
> Changes vs. v2:
> - removed patches which have already been applied
> - created a drm_bus_flags_from_videomode() helper to prevent code
> duplication as suggested by Philipp Zabel
>
> Changes vs. v1:
> - rebased on top of https://patchwork.kernel.org/patch/9113791/ as
> per Philipp Zabel's request
>
> GIT: [PATCHv2 1/3] drm/imx: imx-ldb: honor 'native-mode' property when
> GIT: [PATCHv2 2/3] drm/imx: convey the pixelclk-active and de-active flags
> GIT: [PATCHv2 3/3] drm/imx: remove dead code
> Signed-off-by: Lothar WaÃmann <LW@xxxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_modes.c | 20 +++++++++++++++++++-
> drivers/gpu/drm/imx/imx-ldb.c | 15 ++++++++++-----
> drivers/gpu/drm/imx/parallel-display.c | 16 +++++++++++++---
> include/drm/drm_modes.h | 5 +++--
> 4 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 7def3d5..7c97d5b 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -655,6 +655,21 @@ void drm_display_mode_to_videomode(const struct drm_display_mode *dmode,
> }
> EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
>
> +void drm_bus_flags_from_videomode(const struct videomode *vm, u32 *bus_flags)
> +{
> + *bus_flags = 0;
> + if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> + *bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
> + if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> + *bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> +
> + if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> + *bus_flags |= DRM_BUS_FLAG_DE_LOW;
> + if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> + *bus_flags |= DRM_BUS_FLAG_DE_HIGH;
> +}
> +EXPORT_SYMBOL_GPL(drm_bus_flags_from_videomode);
> +
> #ifdef CONFIG_OF
> /**
> * of_get_drm_display_mode - get a drm_display_mode from devicetree
> @@ -670,7 +685,8 @@ EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
> * 0 on success, a negative errno code when no of videomode node was found.
> */
> int of_get_drm_display_mode(struct device_node *np,
> - struct drm_display_mode *dmode, int index)
> + struct drm_display_mode *dmode, u32 *bus_flags,
> + int index)
> {
> struct videomode vm;
> int ret;
> @@ -680,6 +696,8 @@ int of_get_drm_display_mode(struct device_node *np,
> return ret;
>
> drm_display_mode_from_videomode(&vm, dmode);
> + if (bus_flags)
> + drm_bus_flags_from_videomode(&vm, bus_flags);
>
> pr_debug("%s: got %dx%d display mode from %s\n",
> of_node_full_name(np), vm.hactive, vm.vactive, np->name);
> diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
> index b2dc4df..bb9d745 100644
> --- a/drivers/gpu/drm/imx/imx-ldb.c
> +++ b/drivers/gpu/drm/imx/imx-ldb.c
> @@ -65,7 +65,8 @@ struct imx_ldb_channel {
> int edid_len;
> struct drm_display_mode mode;
> int mode_valid;
> - int bus_format;
> + u32 bus_format;
> + u32 bus_flags;
> };
>
> struct bus_mux {
> @@ -102,8 +103,10 @@ static int imx_ldb_connector_get_modes(struct drm_connector *connector)
> struct drm_display_info *di = &connector->display_info;
>
> num_modes = imx_ldb_ch->panel->funcs->get_modes(imx_ldb_ch->panel);
> - if (!imx_ldb_ch->bus_format && di->num_bus_formats)
> + if (!imx_ldb_ch->bus_format && di->num_bus_formats) {
> imx_ldb_ch->bus_format = di->bus_formats[0];
> + imx_ldb_ch->bus_flags = di->bus_flags;
> + }
> if (num_modes > 0)
> return num_modes;
> }
> @@ -202,7 +205,8 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
> break;
> }
>
> - imx_drm_set_bus_format(encoder, bus_format);
> + imx_drm_set_bus_config(encoder, bus_format, 2, 3,
> + imx_ldb_ch->bus_flags);
> }
>
> static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
> @@ -558,7 +562,7 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
>
> ret = of_property_read_u32(child, "reg", &i);
> if (ret || i < 0 || i > 1)
> - return -EINVAL;
> + return ret ?: -EINVAL;
>
> if (dual && i > 0) {
> dev_warn(dev, "dual-channel mode, ignoring second output\n");
> @@ -602,7 +606,8 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
> GFP_KERNEL);
> } else if (!channel->panel) {
> ret = of_get_drm_display_mode(child, &channel->mode,
> - OF_USE_NATIVE_MODE);
> + &channel->bus_flags,
> + OF_USE_NATIVE_MODE);
> if (!ret)
> channel->mode_valid = 1;
> }
> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
> index 2a81079..002d4d5 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -35,6 +35,7 @@ struct imx_parallel_display {
> void *edid;
> int edid_len;
> u32 bus_format;
> + u32 bus_flags;
> struct drm_display_mode mode;
> struct drm_panel *panel;
> };
> @@ -56,8 +57,10 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector)
> struct drm_display_info *di = &connector->display_info;
>
> num_modes = imxpd->panel->funcs->get_modes(imxpd->panel);
> - if (!imxpd->bus_format && di->num_bus_formats)
> + if (!imxpd->bus_format && di->num_bus_formats) {
> imxpd->bus_format = di->bus_formats[0];
> + imxpd->bus_flags = di->bus_flags;
> + }
> if (num_modes > 0)
> return num_modes;
> }
> @@ -69,10 +72,17 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector)
>
> if (np) {
> struct drm_display_mode *mode = drm_mode_create(connector->dev);
> + int ret;
>
> if (!mode)
> return -EINVAL;
> - of_get_drm_display_mode(np, &imxpd->mode, OF_USE_NATIVE_MODE);
> +
> + ret = of_get_drm_display_mode(np, &imxpd->mode,
> + &imxpd->bus_flags,
> + OF_USE_NATIVE_MODE);
> + if (ret)
> + return ret;
> +
> drm_mode_copy(mode, &imxpd->mode);
> mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> drm_mode_probed_add(connector, mode);
> @@ -104,7 +114,7 @@ static void imx_pd_encoder_prepare(struct drm_encoder *encoder)
> {
> struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
> imx_drm_set_bus_config(encoder, imxpd->bus_format, 2, 3,
> - imxpd->connector.display_info.bus_flags);
> + imxpd->bus_flags);
> }
>
> static void imx_pd_encoder_commit(struct drm_encoder *encoder)
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 625966a..a243533 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -432,7 +432,7 @@ struct drm_cmdline_mode;
> struct drm_display_mode *drm_mode_create(struct drm_device *dev);
> void drm_mode_destroy(struct drm_device *dev, struct drm_display_mode *mode);
> void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
> - const struct drm_display_mode *in);
> + const struct drm_display_mode *in);
> int drm_mode_convert_umode(struct drm_display_mode *out,
> const struct drm_mode_modeinfo *in);
> void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode);
> @@ -455,8 +455,9 @@ void drm_display_mode_from_videomode(const struct videomode *vm,
> struct drm_display_mode *dmode);
> void drm_display_mode_to_videomode(const struct drm_display_mode *dmode,
> struct videomode *vm);
> +void drm_bus_flags_from_videomode(const struct videomode *vm, u32 *bus_flags);
> int of_get_drm_display_mode(struct device_node *np,
> - struct drm_display_mode *dmode,
> + struct drm_display_mode *dmode, u32 *bus_flags,
> int index);
>
> void drm_mode_set_name(struct drm_display_mode *mode);
>
Ping?!


Lothar WaÃmann