All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Damien Lespiau <damien.lespiau@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 06/12] drm/i915/hdmi: Change the write_infoframe vfunc to take a buffer and a type
Date: Wed, 7 Aug 2013 13:58:24 +0300	[thread overview]
Message-ID: <20130807105824.GU5004@intel.com> (raw)
In-Reply-To: <1375817544-14565-7-git-send-email-damien.lespiau@intel.com>

On Tue, Aug 06, 2013 at 08:32:18PM +0100, Damien Lespiau wrote:
> First step in the move to the shared infoframe infrastructure, let's
> move the different infoframe helpers and the write_infoframe() vfunc to
> a type (enum hdmi_infoframe_type) and a buffer + len instead of using
> our struct dip_infoframe.
> 
> v2: constify the infoframe pointer and don't mix signs (Ville Syrjälä)
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Thierry Reding <thierry.reding at avionic-design.de>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h  |   4 +-
>  drivers/gpu/drm/i915/intel_hdmi.c | 106 ++++++++++++++++++++------------------
>  2 files changed, 59 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 474797b..273acfd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -26,6 +26,7 @@
>  #define __INTEL_DRV_H__
>  
>  #include <linux/i2c.h>
> +#include <linux/hdmi.h>
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  #include <drm/drm_crtc.h>
> @@ -463,7 +464,8 @@ struct intel_hdmi {
>  	enum hdmi_force_audio force_audio;
>  	bool rgb_quant_range_selectable;
>  	void (*write_infoframe)(struct drm_encoder *encoder,
> -				struct dip_infoframe *frame);
> +				enum hdmi_infoframe_type type,
> +				const uint8_t *frame, ssize_t len);
>  	void (*set_infoframes)(struct drm_encoder *encoder,
>  			       struct drm_display_mode *adjusted_mode);
>  };
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index e82cd81..ee67e23 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -29,6 +29,7 @@
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
> +#include <linux/hdmi.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_edid.h>
> @@ -81,74 +82,75 @@ void intel_dip_infoframe_csum(struct dip_infoframe *frame)
>  	frame->checksum = 0x100 - sum;
>  }
>  
> -static u32 g4x_infoframe_index(struct dip_infoframe *frame)
> +static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
>  {
> -	switch (frame->type) {
> -	case DIP_TYPE_AVI:
> +	switch (type) {
> +	case HDMI_INFOFRAME_TYPE_AVI:
>  		return VIDEO_DIP_SELECT_AVI;
> -	case DIP_TYPE_SPD:
> +	case HDMI_INFOFRAME_TYPE_SPD:
>  		return VIDEO_DIP_SELECT_SPD;
>  	default:
> -		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
> +		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
>  		return 0;
>  	}
>  }
>  
> -static u32 g4x_infoframe_enable(struct dip_infoframe *frame)
> +static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
>  {
> -	switch (frame->type) {
> -	case DIP_TYPE_AVI:
> +	switch (type) {
> +	case HDMI_INFOFRAME_TYPE_AVI:
>  		return VIDEO_DIP_ENABLE_AVI;
> -	case DIP_TYPE_SPD:
> +	case HDMI_INFOFRAME_TYPE_SPD:
>  		return VIDEO_DIP_ENABLE_SPD;
>  	default:
> -		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
> +		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
>  		return 0;
>  	}
>  }
>  
> -static u32 hsw_infoframe_enable(struct dip_infoframe *frame)
> +static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
>  {
> -	switch (frame->type) {
> -	case DIP_TYPE_AVI:
> +	switch (type) {
> +	case HDMI_INFOFRAME_TYPE_AVI:
>  		return VIDEO_DIP_ENABLE_AVI_HSW;
> -	case DIP_TYPE_SPD:
> +	case HDMI_INFOFRAME_TYPE_SPD:
>  		return VIDEO_DIP_ENABLE_SPD_HSW;
>  	default:
> -		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
> +		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
>  		return 0;
>  	}
>  }
>  
> -static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame,
> +static u32 hsw_infoframe_data_reg(enum hdmi_infoframe_type type,
>  				  enum transcoder cpu_transcoder)
>  {
> -	switch (frame->type) {
> -	case DIP_TYPE_AVI:
> +	switch (type) {
> +	case HDMI_INFOFRAME_TYPE_AVI:
>  		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
> -	case DIP_TYPE_SPD:
> +	case HDMI_INFOFRAME_TYPE_SPD:
>  		return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
>  	default:
> -		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
> +		DRM_DEBUG_DRIVER("unknown info frame type %d\n", type);
>  		return 0;
>  	}
>  }
>  
>  static void g4x_write_infoframe(struct drm_encoder *encoder,
> -				struct dip_infoframe *frame)
> +				enum hdmi_infoframe_type type,
> +				const uint8_t *frame, ssize_t len)
>  {
>  	uint32_t *data = (uint32_t *)frame;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 val = I915_READ(VIDEO_DIP_CTL);
> -	unsigned i, len = DIP_HEADER_SIZE + frame->len;
> +	int i;
>  
>  	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
>  
>  	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
> -	val |= g4x_infoframe_index(frame);
> +	val |= g4x_infoframe_index(type);
>  
> -	val &= ~g4x_infoframe_enable(frame);
> +	val &= ~g4x_infoframe_enable(type);
>  
>  	I915_WRITE(VIDEO_DIP_CTL, val);
>  
> @@ -162,7 +164,7 @@ static void g4x_write_infoframe(struct drm_encoder *encoder,
>  		I915_WRITE(VIDEO_DIP_DATA, 0);
>  	mmiowb();
>  
> -	val |= g4x_infoframe_enable(frame);
> +	val |= g4x_infoframe_enable(type);
>  	val &= ~VIDEO_DIP_FREQ_MASK;
>  	val |= VIDEO_DIP_FREQ_VSYNC;
>  
> @@ -171,22 +173,22 @@ static void g4x_write_infoframe(struct drm_encoder *encoder,
>  }
>  
>  static void ibx_write_infoframe(struct drm_encoder *encoder,
> -				struct dip_infoframe *frame)
> +				enum hdmi_infoframe_type type,
> +				const uint8_t *frame, ssize_t len)
>  {
>  	uint32_t *data = (uint32_t *)frame;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
> -	unsigned i, len = DIP_HEADER_SIZE + frame->len;
> +	int i, reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
>  	u32 val = I915_READ(reg);
>  
>  	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
>  
>  	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
> -	val |= g4x_infoframe_index(frame);
> +	val |= g4x_infoframe_index(type);
>  
> -	val &= ~g4x_infoframe_enable(frame);
> +	val &= ~g4x_infoframe_enable(type);
>  
>  	I915_WRITE(reg, val);
>  
> @@ -200,7 +202,7 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
>  		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
>  	mmiowb();
>  
> -	val |= g4x_infoframe_enable(frame);
> +	val |= g4x_infoframe_enable(type);
>  	val &= ~VIDEO_DIP_FREQ_MASK;
>  	val |= VIDEO_DIP_FREQ_VSYNC;
>  
> @@ -209,25 +211,25 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
>  }
>  
>  static void cpt_write_infoframe(struct drm_encoder *encoder,
> -				struct dip_infoframe *frame)
> +				enum hdmi_infoframe_type type,
> +				const uint8_t *frame, ssize_t len)
>  {
>  	uint32_t *data = (uint32_t *)frame;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
> -	unsigned i, len = DIP_HEADER_SIZE + frame->len;
> +	int i, reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
>  	u32 val = I915_READ(reg);
>  
>  	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
>  
>  	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
> -	val |= g4x_infoframe_index(frame);
> +	val |= g4x_infoframe_index(type);
>  
>  	/* The DIP control register spec says that we need to update the AVI
>  	 * infoframe without clearing its enable bit */
> -	if (frame->type != DIP_TYPE_AVI)
> -		val &= ~g4x_infoframe_enable(frame);
> +	if (type != HDMI_INFOFRAME_TYPE_AVI)
> +		val &= ~g4x_infoframe_enable(type);
>  
>  	I915_WRITE(reg, val);
>  
> @@ -241,7 +243,7 @@ static void cpt_write_infoframe(struct drm_encoder *encoder,
>  		I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
>  	mmiowb();
>  
> -	val |= g4x_infoframe_enable(frame);
> +	val |= g4x_infoframe_enable(type);
>  	val &= ~VIDEO_DIP_FREQ_MASK;
>  	val |= VIDEO_DIP_FREQ_VSYNC;
>  
> @@ -250,22 +252,22 @@ static void cpt_write_infoframe(struct drm_encoder *encoder,
>  }
>  
>  static void vlv_write_infoframe(struct drm_encoder *encoder,
> -				     struct dip_infoframe *frame)
> +				enum hdmi_infoframe_type type,
> +				const uint8_t *frame, ssize_t len)
>  {
>  	uint32_t *data = (uint32_t *)frame;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -	int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
> -	unsigned i, len = DIP_HEADER_SIZE + frame->len;
> +	int i, reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
>  	u32 val = I915_READ(reg);
>  
>  	WARN(!(val & VIDEO_DIP_ENABLE), "Writing DIP with CTL reg disabled\n");
>  
>  	val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
> -	val |= g4x_infoframe_index(frame);
> +	val |= g4x_infoframe_index(type);
>  
> -	val &= ~g4x_infoframe_enable(frame);
> +	val &= ~g4x_infoframe_enable(type);
>  
>  	I915_WRITE(reg, val);
>  
> @@ -279,7 +281,7 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
>  		I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
>  	mmiowb();
>  
> -	val |= g4x_infoframe_enable(frame);
> +	val |= g4x_infoframe_enable(type);
>  	val &= ~VIDEO_DIP_FREQ_MASK;
>  	val |= VIDEO_DIP_FREQ_VSYNC;
>  
> @@ -288,21 +290,24 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
>  }
>  
>  static void hsw_write_infoframe(struct drm_encoder *encoder,
> -				struct dip_infoframe *frame)
> +				enum hdmi_infoframe_type type,
> +				const uint8_t *frame, ssize_t len)
>  {
>  	uint32_t *data = (uint32_t *)frame;
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>  	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
> -	u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->config.cpu_transcoder);
> -	unsigned int i, len = DIP_HEADER_SIZE + frame->len;
> +	u32 data_reg;
> +	int i;
>  	u32 val = I915_READ(ctl_reg);
>  
> +	data_reg = hsw_infoframe_data_reg(type,
> +					  intel_crtc->config.cpu_transcoder);
>  	if (data_reg == 0)
>  		return;
>  
> -	val &= ~hsw_infoframe_enable(frame);
> +	val &= ~hsw_infoframe_enable(type);
>  	I915_WRITE(ctl_reg, val);
>  
>  	mmiowb();
> @@ -315,7 +320,7 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>  		I915_WRITE(data_reg + i, 0);
>  	mmiowb();
>  
> -	val |= hsw_infoframe_enable(frame);
> +	val |= hsw_infoframe_enable(type);
>  	I915_WRITE(ctl_reg, val);
>  	POSTING_READ(ctl_reg);
>  }
> @@ -326,7 +331,8 @@ static void intel_set_infoframe(struct drm_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>  
>  	intel_dip_infoframe_csum(frame);
> -	intel_hdmi->write_infoframe(encoder, frame);
> +	intel_hdmi->write_infoframe(encoder, frame->type, (uint8_t *)frame,
> +				    DIP_HEADER_SIZE + frame->len);
>  }
>  
>  static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

  reply	other threads:[~2013-08-07 10:58 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-06 19:32 Port the i915 HDMI infoframe code to the common infrastructure v2 Damien Lespiau
2013-08-06 19:32 ` [PATCH 01/12] video/hdmi: Replace the payload length by their defines Damien Lespiau
2013-08-07  2:06   ` Alex Deucher
2013-08-07  8:13     ` Daniel Vetter
2013-08-06 19:32 ` [PATCH 02/12] video/hdmi: Introduce a generic hdmi_infoframe union Damien Lespiau
2013-08-06 19:32 ` [PATCH 03/12] video/hdmi: Add a macro to return the size of a full infoframe Damien Lespiau
2013-08-06 19:32 ` [PATCH 04/12] video/hmdi: Clear the whole incoming buffer, not just the infoframe size Damien Lespiau
2013-08-07 10:56   ` [Intel-gfx] " Ville Syrjälä
2013-08-07 11:02     ` Damien Lespiau
2013-08-06 19:32 ` [PATCH 05/12] drm: Don't generate invalid AVI infoframes for CEA modes Damien Lespiau
2013-08-06 19:32 ` [PATCH 06/12] drm/i915/hdmi: Change the write_infoframe vfunc to take a buffer and a type Damien Lespiau
2013-08-07 10:58   ` Ville Syrjälä [this message]
2013-08-06 19:32 ` [PATCH 07/12] drm/i915/hdmi: Port the infoframe code to the common hdmi helpers Damien Lespiau
2013-08-07 11:00   ` Ville Syrjälä
2013-08-06 19:32 ` [PATCH 08/12] drm/i915/sdvo: Port the infoframe code to the shared infrastructure Damien Lespiau
2013-08-06 19:32 ` [PATCH 09/12] drm/i915: Remove the now obsolete infoframe definitions Damien Lespiau
2013-08-06 19:32 ` [PATCH 10/12] drm: Handle the DBLCLK flag in the common infoframe helper Damien Lespiau
2013-08-06 19:32 ` [PATCH 11/12] drm: Set aspect ratio fields in the AVI infoframe even for non CEA modes Damien Lespiau
2013-08-06 19:32 ` [PATCH 12/12] drm/i915/hmdi: Rename set_infoframe() to write_infoframe() Damien Lespiau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130807105824.GU5004@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=damien.lespiau@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.