[RFC v2 5/5] drm/i915: Use generic HDMI infoframe helpers
Paulo Zanoni
przanoni at gmail.com
Thu Dec 6 08:55:23 PST 2012
Hi
2012/12/6 Paulo Zanoni <przanoni at gmail.com>:
> Hi
>
> 2012/12/5 Thierry Reding <thierry.reding at avionic-design.de>:
>> Use the generic HDMI infoframe helpers to get rid of the duplicate
>> implementation in the i915 driver.
>
> A few comments:
>
> - I've compiled your patches and now "intel_infoframes -d" tells my my
> infoframes are full of zeroes. So there's a bug somewhere... I have to
> say that the i915 infoframe registers are quite complicated and it
> took me a long time to fix a lot of its problems, so please don't
> break it and read below for a suggestion :)
>
> - Why do we need that kconfig stuff? Why not just put all the code
> inside drivers/gpu/drm?
>
> - I really like having "common code between drivers merged", but I
> really don't see how the i915 driver is benefiting from this change.
> We're just basically complicating intel_hdmi.c to use a new struct
> that doesn't fit our needs due to the lack of the ECC byte. My idea:
> Instead of calling hdmi_avi_infoframe_pack, why don't we just
> implement intel_hdmi_avi_infoframe_pack that converts a "struct
> hdmi_avi_infoframe" into our own good-old "struct dip_infoframe" that
> has the cool ECC value in the right place? This would probably
> drastically reduce your chances of introducing bugs in our code :)
Just to be a little bit more clear about this paragraph since Daniel
asked me a few questions:
For the changes inside intel_hdmi.c, I'd really like your patch to not
touch any of the "write_infoframe" or "set_infoframes" callbacks. I
think you can do everything by just patching intel_set_infoframe,
intel_hdmi_set_avi_infoframe and intel_hdmi_set_spd_infoframe. In one
of those functions you could call something like
"intel_hdmi_infoframe_pack(struct hdmi_avi_infoframe, struct
dip_infoframe)" and then proceed. This way you don 't need to touch
the code that actually writes stuff to the hardware.
Also, it seems intel_sdvo.c can perfectly use the generic struct from
your patch (the one that doesn't contain the ECC value), so maybe we
could even try to move "struct dip_infoframe" to inside intel_hdmi.c,
making sure intel_sdvo.c can't even use it.
>
> - Instead of "converting our own structs into structs that don't
> exactly fit our usage", what I would really like to see is generic
> _optional_ drm functions to help me fill the infoframe data, like the
> things I did in the past. Examples:
> (i) I want a function that reads the EDID and tells me whether the
> monitor is going to respect my "underscan" settings (AVI infoframe
> field S) or not.
> (ii) I want unified overscan/underscan properties between all the
> drivers: we need properties to allow requesting specific
> overscan/underscan properties and also properties to allow the driver
> to force underscan in case the monitor doesn't want to cooperate (as
> far as I remember, radeon has these properties, but we need to have
> the behavior explicitly documented so all the drivers can try to
> implement the exact same thing, and then we also need to ask
> user-space guys to add support for these things in their GUIs).
> (iii) I want a function that helps me properly setting/changing the
> correct pixel and picture aspect ratios. it seems the only difference
> between some modes (with different VICs) is the pixel/aspect ratio,
> how do we expose this to the user-space and let it select the one it
> wants? How does the driver know exactly which one is the selected one?
> (iv) I want code that enables user-space to use those modes with
> variable pixel repetition rates and select exactly the one it wants to
> use with the correct pixel repetition.
> (v) I'd like a quirk list to recognize monitors/devices that don't
> work correctly when they receive infoframes, or when the VIC is 0,
> etc.
> (vi) I want magical functions that help me setting all those
> color-related stuff I did not study yet.
> (vii) Ideally, I'd like to accomplish all the above by looking at
> "struct drm_display_mode" and maybe passing it to helper functions
> that I can _optionally_ use if I want.
> (viii) I don't want the infoframe code to be a "mid layer" that gets
> in the way, I want it to be a set of optional Lego bricks I can use if
> I need.
> (ix) There's certainly more things I'm missing here, but I believe you
> got the idea :)
>
> Of course, implementing all I described will take a few months :)
And to make it clear: the list above doesn't contain any "requirements
to get your code merged", they're just suggestions, since you're
already touching this code and you need to take these problems into
consideration while designing things. We can merge the patch first and
then later worry about fixing the items on the list :)
>
> Thanks,
> Paulo
>
>>
>> Signed-off-by: Thierry Reding <thierry.reding at avionic-design.de>
>> ---
>> drivers/gpu/drm/Kconfig | 2 +
>> drivers/gpu/drm/i915/intel_drv.h | 62 +--------
>> drivers/gpu/drm/i915/intel_hdmi.c | 268 +++++++++++++++++++++-----------------
>> drivers/gpu/drm/i915/intel_sdvo.c | 22 ++--
>> 4 files changed, 159 insertions(+), 195 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 94a4623..5225012 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -144,6 +144,8 @@ config DRM_I915
>> select INPUT if ACPI
>> select ACPI_VIDEO if ACPI
>> select ACPI_BUTTON if ACPI
>> + select DRM_HDMI
>> + select HDMI
>> help
>> Choose this option if you have a system that has "Intel Graphics
>> Media Accelerator" or "HD Graphics" integrated graphics,
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 522061c..70a7440 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -276,63 +276,6 @@ struct cxsr_latency {
>> #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
>> #define to_intel_plane(x) container_of(x, struct intel_plane, base)
>>
>> -#define DIP_HEADER_SIZE 5
>> -
>> -#define DIP_TYPE_AVI 0x82
>> -#define DIP_VERSION_AVI 0x2
>> -#define DIP_LEN_AVI 13
>> -#define DIP_AVI_PR_1 0
>> -#define DIP_AVI_PR_2 1
>> -
>> -#define DIP_TYPE_SPD 0x83
>> -#define DIP_VERSION_SPD 0x1
>> -#define DIP_LEN_SPD 25
>> -#define DIP_SPD_UNKNOWN 0
>> -#define DIP_SPD_DSTB 0x1
>> -#define DIP_SPD_DVDP 0x2
>> -#define DIP_SPD_DVHS 0x3
>> -#define DIP_SPD_HDDVR 0x4
>> -#define DIP_SPD_DVC 0x5
>> -#define DIP_SPD_DSC 0x6
>> -#define DIP_SPD_VCD 0x7
>> -#define DIP_SPD_GAME 0x8
>> -#define DIP_SPD_PC 0x9
>> -#define DIP_SPD_BD 0xa
>> -#define DIP_SPD_SCD 0xb
>> -
>> -struct dip_infoframe {
>> - uint8_t type; /* HB0 */
>> - uint8_t ver; /* HB1 */
>> - uint8_t len; /* HB2 - body len, not including checksum */
>> - uint8_t ecc; /* Header ECC */
>> - uint8_t checksum; /* PB0 */
>> - union {
>> - struct {
>> - /* PB1 - Y 6:5, A 4:4, B 3:2, S 1:0 */
>> - uint8_t Y_A_B_S;
>> - /* PB2 - C 7:6, M 5:4, R 3:0 */
>> - uint8_t C_M_R;
>> - /* PB3 - ITC 7:7, EC 6:4, Q 3:2, SC 1:0 */
>> - uint8_t ITC_EC_Q_SC;
>> - /* PB4 - VIC 6:0 */
>> - uint8_t VIC;
>> - /* PB5 - YQ 7:6, CN 5:4, PR 3:0 */
>> - uint8_t YQ_CN_PR;
>> - /* PB6 to PB13 */
>> - uint16_t top_bar_end;
>> - uint16_t bottom_bar_start;
>> - uint16_t left_bar_end;
>> - uint16_t right_bar_start;
>> - } __attribute__ ((packed)) avi;
>> - struct {
>> - uint8_t vn[8];
>> - uint8_t pd[16];
>> - uint8_t sdi;
>> - } __attribute__ ((packed)) spd;
>> - uint8_t payload[27];
>> - } __attribute__ ((packed)) body;
>> -} __attribute__((packed));
>> -
>> struct intel_hdmi {
>> u32 sdvox_reg;
>> int ddc_bus;
>> @@ -340,8 +283,8 @@ struct intel_hdmi {
>> bool has_hdmi_sink;
>> bool has_audio;
>> enum hdmi_force_audio force_audio;
>> - void (*write_infoframe)(struct drm_encoder *encoder,
>> - struct dip_infoframe *frame);
>> + void (*write_infoframe)(struct drm_encoder *encoder, const void *frame,
>> + size_t len);
>> void (*set_infoframes)(struct drm_encoder *encoder,
>> struct drm_display_mode *adjusted_mode);
>> };
>> @@ -430,7 +373,6 @@ extern struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
>> extern bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
>> const struct drm_display_mode *mode,
>> struct drm_display_mode *adjusted_mode);
>> -extern void intel_dip_infoframe_csum(struct dip_infoframe *avi_if);
>> extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg,
>> bool is_sdvob);
>> extern void intel_dvo_init(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 5c279b4..a5492b1 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -29,9 +29,11 @@
>> #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>
>> +#include <drm/drm_hdmi.h>
>> #include "intel_drv.h"
>> #include <drm/i915_drm.h>
>> #include "i915_drv.h"
>> @@ -66,102 +68,112 @@ static struct intel_hdmi *intel_attached_hdmi(struct drm_connector *connector)
>> return enc_to_intel_hdmi(&intel_attached_encoder(connector)->base);
>> }
>>
>> -void intel_dip_infoframe_csum(struct dip_infoframe *frame)
>> +static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
>> {
>> - uint8_t *data = (uint8_t *)frame;
>> - uint8_t sum = 0;
>> - unsigned i;
>> -
>> - frame->checksum = 0;
>> - frame->ecc = 0;
>> -
>> - for (i = 0; i < frame->len + DIP_HEADER_SIZE; i++)
>> - sum += data[i];
>> -
>> - frame->checksum = 0x100 - sum;
>> -}
>> -
>> -static u32 g4x_infoframe_index(struct dip_infoframe *frame)
>> -{
>> - 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, enum pipe pipe)
>> +static u32 hsw_infoframe_data_reg(enum hdmi_infoframe_type type, enum pipe pipe)
>> {
>> - switch (frame->type) {
>> - case DIP_TYPE_AVI:
>> + switch (type) {
>> + case HDMI_INFOFRAME_TYPE_AVI:
>> return HSW_TVIDEO_DIP_AVI_DATA(pipe);
>> - case DIP_TYPE_SPD:
>> + case HDMI_INFOFRAME_TYPE_SPD:
>> return HSW_TVIDEO_DIP_SPD_DATA(pipe);
>> 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)
>> +static u32 intel_dip_pack(const u8 *data, size_t size)
>> {
>> - uint32_t *data = (uint32_t *)frame;
>> + u32 value = 0;
>> + size_t i;
>> +
>> + for (i = size; i > 0; i--)
>> + value = (value << 8) | data[i - 1];
>> +
>> + return value;
>> +}
>> +
>> +static void g4x_dip_write(struct drm_i915_private *dev_priv,
>> + unsigned long offset, const void *dip, size_t len)
>> +{
>> + u32 data;
>> + size_t i;
>> +
>> + /* Write infoframe header and zero out ECC at byte 4 */
>> + data = intel_dip_pack(dip, 3);
>> + I915_WRITE(offset, data);
>> +
>> + for (i = 3; i < len; i += 4) {
>> + size_t rem = min_t(size_t, len - i, 4);
>> + data = intel_dip_pack(dip + i, rem);
>> + I915_WRITE(offset, data);
>> + }
>> +
>> + /* Write every possible data byte to force correct ECC calculation. */
>> + for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
>> + I915_WRITE(offset, 0);
>> +}
>> +
>> +static void g4x_write_infoframe(struct drm_encoder *encoder, const void *frame,
>> + size_t len)
>> +{
>> + enum hdmi_infoframe_type type = ((const u8 *)frame)[0];
>> 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;
>>
>> 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);
>>
>> mmiowb();
>> - for (i = 0; i < len; i += 4) {
>> - I915_WRITE(VIDEO_DIP_DATA, *data);
>> - data++;
>> - }
>> - /* Write every possible data byte to force correct ECC calculation. */
>> - for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
>> - I915_WRITE(VIDEO_DIP_DATA, 0);
>> + g4x_dip_write(dev_priv, VIDEO_DIP_DATA, frame, len);
>> mmiowb();
>>
>> - val |= g4x_infoframe_enable(frame);
>> + val |= g4x_infoframe_enable(type);
>> val &= ~VIDEO_DIP_FREQ_MASK;
>> val |= VIDEO_DIP_FREQ_VSYNC;
>>
>> @@ -169,37 +181,31 @@ static void g4x_write_infoframe(struct drm_encoder *encoder,
>> POSTING_READ(VIDEO_DIP_CTL);
>> }
>>
>> -static void ibx_write_infoframe(struct drm_encoder *encoder,
>> - struct dip_infoframe *frame)
>> +static void ibx_write_infoframe(struct drm_encoder *encoder, const void *frame,
>> + size_t len)
>> {
>> - uint32_t *data = (uint32_t *)frame;
>> + enum hdmi_infoframe_type type = ((const u8 *)frame)[0];
>> 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;
>> 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);
>>
>> mmiowb();
>> - for (i = 0; i < len; i += 4) {
>> - I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
>> - data++;
>> - }
>> - /* Write every possible data byte to force correct ECC calculation. */
>> - for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
>> - I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
>> + g4x_dip_write(dev_priv, TVIDEO_DIP_DATA(intel_crtc->pipe), frame,
>> + len);
>> mmiowb();
>>
>> - val |= g4x_infoframe_enable(frame);
>> + val |= g4x_infoframe_enable(type);
>> val &= ~VIDEO_DIP_FREQ_MASK;
>> val |= VIDEO_DIP_FREQ_VSYNC;
>>
>> @@ -207,40 +213,33 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
>> POSTING_READ(reg);
>> }
>>
>> -static void cpt_write_infoframe(struct drm_encoder *encoder,
>> - struct dip_infoframe *frame)
>> +static void cpt_write_infoframe(struct drm_encoder *encoder, const void *frame,
>> + size_t len)
>> {
>> - uint32_t *data = (uint32_t *)frame;
>> + enum hdmi_infoframe_type type = ((const u8 *)frame)[0];
>> 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;
>> 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);
>>
>> mmiowb();
>> - for (i = 0; i < len; i += 4) {
>> - I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
>> - data++;
>> - }
>> - /* Write every possible data byte to force correct ECC calculation. */
>> - for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
>> - I915_WRITE(TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
>> + g4x_dip_write(dev_priv, TVIDEO_DIP_DATA(intel_crtc->pipe), frame, len);
>> mmiowb();
>>
>> - val |= g4x_infoframe_enable(frame);
>> + val |= g4x_infoframe_enable(type);
>> val &= ~VIDEO_DIP_FREQ_MASK;
>> val |= VIDEO_DIP_FREQ_VSYNC;
>>
>> @@ -248,37 +247,31 @@ static void cpt_write_infoframe(struct drm_encoder *encoder,
>> POSTING_READ(reg);
>> }
>>
>> -static void vlv_write_infoframe(struct drm_encoder *encoder,
>> - struct dip_infoframe *frame)
>> +static void vlv_write_infoframe(struct drm_encoder *encoder, const void *frame,
>> + size_t len)
>> {
>> - uint32_t *data = (uint32_t *)frame;
>> + enum hdmi_infoframe_type type = ((const u8 *)frame)[0];
>> 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;
>> 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);
>>
>> mmiowb();
>> - for (i = 0; i < len; i += 4) {
>> - I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
>> - data++;
>> - }
>> - /* Write every possible data byte to force correct ECC calculation. */
>> - for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
>> - I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), 0);
>> + g4x_dip_write(dev_priv, VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), frame,
>> + len);
>> mmiowb();
>>
>> - val |= g4x_infoframe_enable(frame);
>> + val |= g4x_infoframe_enable(type);
>> val &= ~VIDEO_DIP_FREQ_MASK;
>> val |= VIDEO_DIP_FREQ_VSYNC;
>>
>> @@ -286,76 +279,105 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
>> POSTING_READ(reg);
>> }
>>
>> -static void hsw_write_infoframe(struct drm_encoder *encoder,
>> - struct dip_infoframe *frame)
>> +static void hsw_write_infoframe(struct drm_encoder *encoder, const void *frame,
>> + size_t len)
>> {
>> - uint32_t *data = (uint32_t *)frame;
>> + enum hdmi_infoframe_type type = ((const u8 *)frame)[0];
>> 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->pipe);
>> - u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->pipe);
>> - unsigned int i, len = DIP_HEADER_SIZE + frame->len;
>> + u32 data_reg = hsw_infoframe_data_reg(type, intel_crtc->pipe);
>> u32 val = I915_READ(ctl_reg);
>> + unsigned long offset;
>> + u32 data;
>> + size_t i;
>>
>> if (data_reg == 0)
>> return;
>>
>> - val &= ~hsw_infoframe_enable(frame);
>> + val &= ~hsw_infoframe_enable(type);
>> I915_WRITE(ctl_reg, val);
>>
>> mmiowb();
>> - for (i = 0; i < len; i += 4) {
>> - I915_WRITE(data_reg + i, *data);
>> - data++;
>> +
>> + /* Write infoframe header and zero out ECC at byte 4 */
>> + data = intel_dip_pack(frame, 3);
>> + I915_WRITE(data_reg, data);
>> +
>> + for (i = 3, offset = 4; i < len; i += 4, offset += 4) {
>> + size_t rem = min_t(size_t, len - i, 4);
>> + data = intel_dip_pack(frame + i, rem);
>> + I915_WRITE(data_reg + offset, data);
>> }
>> +
>> /* Write every possible data byte to force correct ECC calculation. */
>> - for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
>> - I915_WRITE(data_reg + i, 0);
>> + for (; i < VIDEO_DIP_DATA_SIZE; i += 4, offset += 4)
>> + I915_WRITE(data_reg + offset, 0);
>> +
>> mmiowb();
>>
>> - val |= hsw_infoframe_enable(frame);
>> + val |= hsw_infoframe_enable(type);
>> I915_WRITE(ctl_reg, val);
>> POSTING_READ(ctl_reg);
>> }
>>
>> +/*
>> static void intel_set_infoframe(struct drm_encoder *encoder,
>> - struct dip_infoframe *frame)
>> + const struct hdmi_infoframe *frame)
>> {
>> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>> + u8 buffer[32];
>> + ssize_t len;
>>
>> - intel_dip_infoframe_csum(frame);
>> - intel_hdmi->write_infoframe(encoder, frame);
>> + len = hdmi_infoframe_pack(frame, buffer, sizeof(buffer));
>> + if (len < 0)
>> + return;
>> +
>> + intel_hdmi->write_infoframe(encoder, buffer, len);
>> }
>> +*/
>>
>> static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>> struct drm_display_mode *adjusted_mode)
>> {
>> - struct dip_infoframe avi_if = {
>> - .type = DIP_TYPE_AVI,
>> - .ver = DIP_VERSION_AVI,
>> - .len = DIP_LEN_AVI,
>> - };
>> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>> + u8 buffer[HDMI_AVI_INFOFRAME_SIZE];
>> + struct hdmi_avi_infoframe frame;
>> + ssize_t len;
>> +
>> + len = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode);
>> + if (len < 0)
>> + return;
>>
>> if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>> - avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;
>> + frame.pixel_repeat = 2;
>> +
>> + len = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer));
>> + if (len < 0)
>> + return;
>>
>> - intel_set_infoframe(encoder, &avi_if);
>> + intel_hdmi->write_infoframe(encoder, buffer, len);
>> }
>>
>> static void intel_hdmi_set_spd_infoframe(struct drm_encoder *encoder)
>> {
>> - struct dip_infoframe spd_if;
>> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>> + u8 buffer[HDMI_SPD_INFOFRAME_SIZE];
>> + struct hdmi_spd_infoframe frame;
>> + ssize_t len;
>> +
>> + len = hdmi_spd_infoframe_init(&frame, "Intel", "Integrated gfx");
>> + if (len < 0)
>> + return;
>> +
>> + frame.sdi = HDMI_SPD_SDI_PC;
>>
>> - memset(&spd_if, 0, sizeof(spd_if));
>> - spd_if.type = DIP_TYPE_SPD;
>> - spd_if.ver = DIP_VERSION_SPD;
>> - spd_if.len = DIP_LEN_SPD;
>> - strcpy(spd_if.body.spd.vn, "Intel");
>> - strcpy(spd_if.body.spd.pd, "Integrated gfx");
>> - spd_if.body.spd.sdi = DIP_SPD_PC;
>> + len = hdmi_spd_infoframe_pack(&frame, buffer, sizeof(buffer));
>> + if (len < 0)
>> + return;
>>
>> - intel_set_infoframe(encoder, &spd_if);
>> + intel_hdmi->write_infoframe(encoder, buffer, len);
>> }
>>
>> static void g4x_set_infoframes(struct drm_encoder *encoder,
>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>> index 4b07401..192d791 100644
>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
>> @@ -29,6 +29,7 @@
>> #include <linux/slab.h>
>> #include <linux/delay.h>
>> #include <linux/export.h>
>> +#include <linux/hdmi.h>
>> #include <drm/drmP.h>
>> #include <drm/drm_crtc.h>
>> #include <drm/drm_edid.h>
>> @@ -935,20 +936,17 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
>>
>> static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
>> {
>> - struct dip_infoframe avi_if = {
>> - .type = DIP_TYPE_AVI,
>> - .ver = DIP_VERSION_AVI,
>> - .len = DIP_LEN_AVI,
>> - };
>> - uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)];
>> + uint8_t sdvo_data[HDMI_AVI_INFOFRAME_SIZE];
>> + struct hdmi_avi_infoframe frame;
>> + ssize_t len;
>>
>> - intel_dip_infoframe_csum(&avi_if);
>> + len = hdmi_avi_infoframe_init(&frame);
>> + if (len < 0)
>> + return false;
>>
>> - /* sdvo spec says that the ecc is handled by the hw, and it looks like
>> - * we must not send the ecc field, either. */
>> - memcpy(sdvo_data, &avi_if, 3);
>> - sdvo_data[3] = avi_if.checksum;
>> - memcpy(&sdvo_data[4], &avi_if.body, sizeof(avi_if.body.avi));
>> + len = hdmi_avi_infoframe_pack(&frame, sdvo_data, sizeof(sdvo_data));
>> + if (len < 0)
>> + return false;
>>
>> return intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_AVI_IF,
>> SDVO_HBUF_TX_VSYNC,
>> --
>> 1.8.0.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Paulo Zanoni
--
Paulo Zanoni
More information about the dri-devel
mailing list