From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Damien Lespiau <damien.lespiau@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/8] drm/i915/hdmi: Change the write_infoframe vfunc to take a buffer and a type
Date: Mon, 5 Aug 2013 20:40:42 +0300 [thread overview]
Message-ID: <20130805174042.GY5004@intel.com> (raw)
In-Reply-To: <1375464180-7259-5-git-send-email-damien.lespiau@intel.com>
On Fri, Aug 02, 2013 at 06:22:56PM +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.
>
> 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>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 4 +-
> drivers/gpu/drm/i915/intel_hdmi.c | 103 +++++++++++++++++++++-----------------
> 2 files changed, 59 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 474797b..f8c21ac 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,
> + uint8_t *frame, ssize_t len);
While at it, could we sprinkle a bit of constness around the infoframe
code and make this 'const uint8_t *frame'?
> 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..d3225df 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,
> + 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;
> + unsigned i;
'len' is now signed, but before it was unsigned, and 'i' is still
unsigned. A bit of consistency with the types might be nice.
>
> 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,23 @@ 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,
> + 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;
> + unsigned i;
> 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 +203,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 +212,26 @@ 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,
> + 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;
> + unsigned i;
> 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 +245,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 +254,23 @@ 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,
> + 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;
> + unsigned i;
> 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 +284,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 +293,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,
> + 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;
> + unsigned 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 +323,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 +334,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
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2013-08-05 17:40 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-02 17:22 Port the i915 HDMI infoframe code to the common infrastructure Damien Lespiau
2013-08-02 17:22 ` [PATCH 1/8] video/hdmi: Replace the payload length by their defines Damien Lespiau
2013-08-05 17:11 ` Ville Syrjälä
2013-08-02 17:22 ` [PATCH 2/8] video/hdmi: Introduce a generic hdmi_infoframe union Damien Lespiau
2013-08-05 17:30 ` Ville Syrjälä
2013-08-02 17:22 ` [PATCH 3/8] video/hdmi: Add a macro to return the size of a full infoframe Damien Lespiau
2013-08-05 17:31 ` Ville Syrjälä
2013-08-02 17:22 ` [PATCH 4/8] drm/i915/hdmi: Change the write_infoframe vfunc to take a buffer and a type Damien Lespiau
2013-08-05 17:40 ` Ville Syrjälä [this message]
2013-08-02 17:22 ` [PATCH 5/8] drm/i915/hdmi: Port the infoframe code to the common hdmi helpers Damien Lespiau
2013-08-05 13:12 ` Ville Syrjälä
2013-08-06 19:17 ` Damien Lespiau
2013-08-02 17:22 ` [PATCH 6/8] drm/i915/hmdi: Rename set_infoframe() to write_infoframe() Damien Lespiau
2013-08-05 17:50 ` Ville Syrjälä
2013-08-06 17:49 ` Damien Lespiau
2013-08-06 18:01 ` Daniel Vetter
2013-08-06 18:18 ` Damien Lespiau
2013-08-02 17:22 ` [PATCH 7/8] drm/i915/sdvo: Port the infoframe code to the shared infrastructure Damien Lespiau
2013-08-05 18:08 ` Ville Syrjälä
2013-08-02 17:23 ` [PATCH 8/8] drm/i915: Remove the now obsolete infoframe definitions Damien Lespiau
2013-08-05 18:27 ` Ville Syrjälä
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=20130805174042.GY5004@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=damien.lespiau@intel.com \
--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.