Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1441286 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCHv3 1/1] drm/imx: convey the pixelclk-active and de-active  flags from DT to the ipu-di driver Lothar Waßmann <LW@KARO-electronics.de> - 2016-07-12 14:00 +0200
    Re: [PATCHv3 1/1] drm/imx: convey the pixelclk-active and de-active  flags from DT to the ipu-di driver Daniel Vetter <daniel@ffwll.ch> - 2016-07-12 14:50 +0200
      Re: [PATCHv3 1/1] drm/imx: convey the pixelclk-active and de-active  flags from DT to the ipu-di driver Philipp Zabel <p.zabel@pengutronix.de> - 2016-07-12 19:00 +0200
    Re: [PATCHv3 1/1] drm/imx: convey the pixelclk-active and de-active  flags from DT to the ipu-di driver Philipp Zabel <p.zabel@pengutronix.de> - 2016-07-12 18:50 +0200

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

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@KARO-electronics.de>
> ---
>  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

[toc] | [next] | [standalone]

#1441320

On Tue, Jul 12, 2016 at 01:54:28PM +0200, Lothar Waßmann wrote:
> 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

Should this be split up into 3 patches? At least it's good practice to
split up the drm core helper functions into it's own patch, instead of
hiding it behind something with the drm/imx prefix - no one will spot it
that way.

> Ping?!

If imx maintainers keep being asleep at the helm I can merge this through
drm-misc, but I'd like some more tested-by from other folks if possible.
Even better when there's a review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1441587

Am Dienstag, den 12.07.2016, 14:43 +0200 schrieb Daniel Vetter:
> On Tue, Jul 12, 2016 at 01:54:28PM +0200, Lothar Waßmann wrote:
> > 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
> 
> Should this be split up into 3 patches? At least it's good practice to
> split up the drm core helper functions into it's own patch, instead of
> hiding it behind something with the drm/imx prefix - no one will spot it
> that way.
> 
> > Ping?!
> 
> If imx maintainers keep being asleep at the helm I can merge this through
> drm-misc, but I'd like some more tested-by from other folks if possible.
> Even better when there's a review.

This clashes with the atomic modeset changes. If you don't disagree with
the core change, I can include it with the next imx-drm/next pull
request.

regards
Philipp

[toc] | [prev] | [next] | [standalone]

#1441553

Hi Lothar,

Am Dienstag, den 12.07.2016, 13:54 +0200 schrieb Lothar Waßmann:
> 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
[...]
> Ping?!

Sorry for the delay. I wanted to get the atomic modeset changes
underneath this because your patches are easier to rebase than the other
way around.
I've applied v4 patch 1 and 2, patch 3 has to be rebased onto
imx-drm/next.

regards
Philipp

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web