[RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe

[RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe

[Rafał Miłecki]

---
 drivers/gpu/drm/radeon/r600_hdmi.c |   81 ++++++-----------------------------
 drivers/gpu/drm/radeon/radeon.h    |   41 ++++++++++++++++++
 2 files changed, 55 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
index c308432..b14c90a 100644
--- a/drivers/gpu/drm/radeon/r600_hdmi.c
+++ b/drivers/gpu/drm/radeon/r600_hdmi.c
@@ -134,78 +134,22 @@ static void r600_hdmi_infoframe_checksum(uint8_t packetType,
 }
 
 /*
- * build a HDMI Video Info Frame
+ * Upload a HDMI AVI Infoframe
  */
-static void r600_hdmi_videoinfoframe(
-	struct drm_encoder *encoder,
-	enum r600_hdmi_color_format color_format,
-	int active_information_present,
-	uint8_t active_format_aspect_ratio,
-	uint8_t scan_information,
-	uint8_t colorimetry,
-	uint8_t ex_colorimetry,
-	uint8_t quantization,
-	int ITC,
-	uint8_t picture_aspect_ratio,
-	uint8_t video_format_identification,
-	uint8_t pixel_repetition,
-	uint8_t non_uniform_picture_scaling,
-	uint8_t bar_info_data_valid,
-	uint16_t top_bar,
-	uint16_t bottom_bar,
-	uint16_t left_bar,
-	uint16_t right_bar
-)
+static void r600_hdmi_avi_infoframe(struct drm_encoder *encoder,
+				    struct radeon_avi_infoframe infoframe)
 {
 	struct drm_device *dev = encoder->dev;
 	struct radeon_device *rdev = dev->dev_private;
 	uint32_t offset = to_radeon_encoder(encoder)->hdmi_offset;
 
-	uint8_t frame[14];
-
-	frame[0x0] = 0;
-	frame[0x1] =
-		(scan_information & 0x3) |
-		((bar_info_data_valid & 0x3) << 2) |
-		((active_information_present & 0x1) << 4) |
-		((color_format & 0x3) << 5);
-	frame[0x2] =
-		(active_format_aspect_ratio & 0xF) |
-		((picture_aspect_ratio & 0x3) << 4) |
-		((colorimetry & 0x3) << 6);
-	frame[0x3] =
-		(non_uniform_picture_scaling & 0x3) |
-		((quantization & 0x3) << 2) |
-		((ex_colorimetry & 0x7) << 4) |
-		((ITC & 0x1) << 7);
-	frame[0x4] = (video_format_identification & 0x7F);
-	frame[0x5] = (pixel_repetition & 0xF);
-	frame[0x6] = (top_bar & 0xFF);
-	frame[0x7] = (top_bar >> 8);
-	frame[0x8] = (bottom_bar & 0xFF);
-	frame[0x9] = (bottom_bar >> 8);
-	frame[0xA] = (left_bar & 0xFF);
-	frame[0xB] = (left_bar >> 8);
-	frame[0xC] = (right_bar & 0xFF);
-	frame[0xD] = (right_bar >> 8);
+	/* Standard says about 0x82 0x02 but radeon uses 0x81 0x01...? */
+	r600_hdmi_infoframe_checksum(0x81, 0x01, 13, (u8 *)&infoframe);
 
-	r600_hdmi_infoframe_checksum(0x82, 0x02, 0x0D, frame);
-	/* Our header values (type, version, length) should be alright, Intel
-	 * is using the same. Checksum function also seems to be OK, it works
-	 * fine for audio infoframe. However calculated value is always lower
-	 * by 2 in comparison to fglrx. It breaks displaying anything in case
-	 * of TVs that strictly check the checksum. Hack it manually here to
-	 * workaround this issue. */
-	frame[0x0] += 2;
-
-	WREG32(HDMI0_AVI_INFO0 + offset,
-		frame[0x0] | (frame[0x1] << 8) | (frame[0x2] << 16) | (frame[0x3] << 24));
-	WREG32(HDMI0_AVI_INFO1 + offset,
-		frame[0x4] | (frame[0x5] << 8) | (frame[0x6] << 16) | (frame[0x7] << 24));
-	WREG32(HDMI0_AVI_INFO2 + offset,
-		frame[0x8] | (frame[0x9] << 8) | (frame[0xA] << 16) | (frame[0xB] << 24));
-	WREG32(HDMI0_AVI_INFO3 + offset,
-		frame[0xC] | (frame[0xD] << 8));
+	WREG32(HDMI0_AVI_INFO0 + offset, ((u32 *)&infoframe)[0]);
+	WREG32(HDMI0_AVI_INFO1 + offset, ((u32 *)&infoframe)[1]);
+	WREG32(HDMI0_AVI_INFO2 + offset, ((u32 *)&infoframe)[2]);
+	WREG32(HDMI0_AVI_INFO3 + offset, ((u32 *)&infoframe)[3]);
 }
 
 /*
@@ -316,6 +260,10 @@ void r600_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode *mod
 	struct radeon_device *rdev = dev->dev_private;
 	uint32_t offset = to_radeon_encoder(encoder)->hdmi_offset;
 
+	/* TODO: fill in more avi info */
+	struct radeon_avi_infoframe infoframe = { };
+	infoframe.color_format = 0;
+
 	if (ASIC_IS_DCE5(rdev))
 		return;
 
@@ -367,8 +315,7 @@ void r600_hdmi_setmode(struct drm_encoder *encoder, struct drm_display_mode *mod
 
 	WREG32(HDMI0_GC + offset, 0); /* unset HDMI0_GC_AVMUTE */
 
-	r600_hdmi_videoinfoframe(encoder, RGB, 0, 0, 0, 0,
-		0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+	r600_hdmi_avi_infoframe(encoder, infoframe);
 
 	r600_hdmi_update_ACR(encoder, mode->clock);
 
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index a7ae4ca..5a5c76f 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1874,6 +1874,47 @@ struct radeon_hdmi_acr {
 
 };
 
+struct radeon_avi_infoframe {
+	unsigned checksum: 8;
+
+	unsigned scan_information: 2;
+	unsigned bar_info_data_valid: 2;
+	unsigned active_information_present: 1;
+	unsigned color_format: 2;
+	unsigned :1;
+
+	unsigned active_format_aspect_ratio:4;
+	unsigned picture_aspect_ratio:2;
+	unsigned colorimetry:2;
+
+	unsigned non_uniform_picture_scaling:2;
+	unsigned quantization:2;
+	unsigned ex_colorimetry:3;
+	unsigned ITC:1;
+
+	unsigned video_format_identification: 7;
+	unsigned :1;
+
+	unsigned pixel_repetition: 4;
+	unsigned :4;
+
+	unsigned top_bar_lo: 8;
+
+	unsigned top_bar_hi: 8;
+
+	unsigned bottom_bar_lo: 8;
+
+	unsigned bottom_bar_hi: 8;
+
+	unsigned left_bar_lo: 8;
+
+	unsigned left_bar_hi: 8;
+
+	unsigned right_bar_lo: 8;
+
+	unsigned right_bar_hi: 8;
+} __packed;
+
 extern struct radeon_hdmi_acr r600_hdmi_acr(uint32_t clock);
 
 extern void r600_hdmi_enable(struct drm_encoder *encoder);
-- 
1.7.7

Re: [RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe

[Rafał Miłecki]

2012/5/6 Rafał Miłecki <zajec5@gmail.com>:
> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
> index c308432..b14c90a 100644
> --- a/drivers/gpu/drm/radeon/r600_hdmi.c
> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c
> @@ -134,78 +134,22 @@ static void r600_hdmi_infoframe_checksum(uint8_t packetType,
>  }
>
>  /*
> - * build a HDMI Video Info Frame
> + * Upload a HDMI AVI Infoframe
>  */
> -static void r600_hdmi_videoinfoframe(
> -       struct drm_encoder *encoder,
> -       enum r600_hdmi_color_format color_format,
> -       int active_information_present,
> -       uint8_t active_format_aspect_ratio,
> -       uint8_t scan_information,
> -       uint8_t colorimetry,
> -       uint8_t ex_colorimetry,
> -       uint8_t quantization,
> -       int ITC,
> -       uint8_t picture_aspect_ratio,
> -       uint8_t video_format_identification,
> -       uint8_t pixel_repetition,
> -       uint8_t non_uniform_picture_scaling,
> -       uint8_t bar_info_data_valid,
> -       uint16_t top_bar,
> -       uint16_t bottom_bar,
> -       uint16_t left_bar,
> -       uint16_t right_bar
> -)

In case someone wonders about the reason: I think it's really ugly to
have a function taking 18 arguments, 17 of them related to the
infoframe. It makes much more sense for me to use struct for that.
While working on that I though it's reasonable to prepare nice
bitfield __packed struct ready-to-be-written to the GPU registers.

-- 
Rafał
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe

[Dave Airlie]

On Sun, May 6, 2012 at 5:19 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> 2012/5/6 Rafał Miłecki <zajec5@gmail.com>:
>> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
>> index c308432..b14c90a 100644
>> --- a/drivers/gpu/drm/radeon/r600_hdmi.c
>> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c
>> @@ -134,78 +134,22 @@ static void r600_hdmi_infoframe_checksum(uint8_t packetType,
>>  }
>>
>>  /*
>> - * build a HDMI Video Info Frame
>> + * Upload a HDMI AVI Infoframe
>>  */
>> -static void r600_hdmi_videoinfoframe(
>> -       struct drm_encoder *encoder,
>> -       enum r600_hdmi_color_format color_format,
>> -       int active_information_present,
>> -       uint8_t active_format_aspect_ratio,
>> -       uint8_t scan_information,
>> -       uint8_t colorimetry,
>> -       uint8_t ex_colorimetry,
>> -       uint8_t quantization,
>> -       int ITC,
>> -       uint8_t picture_aspect_ratio,
>> -       uint8_t video_format_identification,
>> -       uint8_t pixel_repetition,
>> -       uint8_t non_uniform_picture_scaling,
>> -       uint8_t bar_info_data_valid,
>> -       uint16_t top_bar,
>> -       uint16_t bottom_bar,
>> -       uint16_t left_bar,
>> -       uint16_t right_bar
>> -)
>
> In case someone wonders about the reason: I think it's really ugly to
> have a function taking 18 arguments, 17 of them related to the
> infoframe. It makes much more sense for me to use struct for that.
> While working on that I though it's reasonable to prepare nice
> bitfield __packed struct ready-to-be-written to the GPU registers.

won't this screw up on other endian machines?

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe

[Daniel Vetter]

On Sun, May 06, 2012 at 05:22:59PM +0100, Dave Airlie wrote:
> On Sun, May 6, 2012 at 5:19 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> > 2012/5/6 Rafał Miłecki <zajec5@gmail.com>:
> >> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
> >> index c308432..b14c90a 100644
> >> --- a/drivers/gpu/drm/radeon/r600_hdmi.c
> >> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c
> >> @@ -134,78 +134,22 @@ static void r600_hdmi_infoframe_checksum(uint8_t packetType,
> >>  }
> >>
> >>  /*
> >> - * build a HDMI Video Info Frame
> >> + * Upload a HDMI AVI Infoframe
> >>  */
> >> -static void r600_hdmi_videoinfoframe(
> >> -       struct drm_encoder *encoder,
> >> -       enum r600_hdmi_color_format color_format,
> >> -       int active_information_present,
> >> -       uint8_t active_format_aspect_ratio,
> >> -       uint8_t scan_information,
> >> -       uint8_t colorimetry,
> >> -       uint8_t ex_colorimetry,
> >> -       uint8_t quantization,
> >> -       int ITC,
> >> -       uint8_t picture_aspect_ratio,
> >> -       uint8_t video_format_identification,
> >> -       uint8_t pixel_repetition,
> >> -       uint8_t non_uniform_picture_scaling,
> >> -       uint8_t bar_info_data_valid,
> >> -       uint16_t top_bar,
> >> -       uint16_t bottom_bar,
> >> -       uint16_t left_bar,
> >> -       uint16_t right_bar
> >> -)
> >
> > In case someone wonders about the reason: I think it's really ugly to
> > have a function taking 18 arguments, 17 of them related to the
> > infoframe. It makes much more sense for me to use struct for that.
> > While working on that I though it's reasonable to prepare nice
> > bitfield __packed struct ready-to-be-written to the GPU registers.
> 
> won't this screw up on other endian machines?

... and can we have this in a slightly generic way maybe? We have copies
of this in i915 and nouveau.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe

[Rafał Miłecki]

2012/5/6 Dave Airlie <airlied@gmail.com>:
> On Sun, May 6, 2012 at 5:19 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
>> 2012/5/6 Rafał Miłecki <zajec5@gmail.com>:
>>> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
>>> index c308432..b14c90a 100644
>>> --- a/drivers/gpu/drm/radeon/r600_hdmi.c
>>> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c
>>> @@ -134,78 +134,22 @@ static void r600_hdmi_infoframe_checksum(uint8_t packetType,
>>>  }
>>>
>>>  /*
>>> - * build a HDMI Video Info Frame
>>> + * Upload a HDMI AVI Infoframe
>>>  */
>>> -static void r600_hdmi_videoinfoframe(
>>> -       struct drm_encoder *encoder,
>>> -       enum r600_hdmi_color_format color_format,
>>> -       int active_information_present,
>>> -       uint8_t active_format_aspect_ratio,
>>> -       uint8_t scan_information,
>>> -       uint8_t colorimetry,
>>> -       uint8_t ex_colorimetry,
>>> -       uint8_t quantization,
>>> -       int ITC,
>>> -       uint8_t picture_aspect_ratio,
>>> -       uint8_t video_format_identification,
>>> -       uint8_t pixel_repetition,
>>> -       uint8_t non_uniform_picture_scaling,
>>> -       uint8_t bar_info_data_valid,
>>> -       uint16_t top_bar,
>>> -       uint16_t bottom_bar,
>>> -       uint16_t left_bar,
>>> -       uint16_t right_bar
>>> -)
>>
>> In case someone wonders about the reason: I think it's really ugly to
>> have a function taking 18 arguments, 17 of them related to the
>> infoframe. It makes much more sense for me to use struct for that.
>> While working on that I though it's reasonable to prepare nice
>> bitfield __packed struct ready-to-be-written to the GPU registers.
>
> won't this screw up on other endian machines?

Hm, maybe it can. Is there some easy to handle it correctly? Some trick like
__le8 foo: 3
__le8 bar: 1
maybe?

-- 
Rafał
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe

[Rafał Miłecki]

2012/5/6 Daniel Vetter <daniel@ffwll.ch>:
> On Sun, May 06, 2012 at 05:22:59PM +0100, Dave Airlie wrote:
>> On Sun, May 6, 2012 at 5:19 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
>> > 2012/5/6 Rafał Miłecki <zajec5@gmail.com>:
>> >> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
>> >> index c308432..b14c90a 100644
>> >> --- a/drivers/gpu/drm/radeon/r600_hdmi.c
>> >> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c
>> >> @@ -134,78 +134,22 @@ static void r600_hdmi_infoframe_checksum(uint8_t packetType,
>> >>  }
>> >>
>> >>  /*
>> >> - * build a HDMI Video Info Frame
>> >> + * Upload a HDMI AVI Infoframe
>> >>  */
>> >> -static void r600_hdmi_videoinfoframe(
>> >> -       struct drm_encoder *encoder,
>> >> -       enum r600_hdmi_color_format color_format,
>> >> -       int active_information_present,
>> >> -       uint8_t active_format_aspect_ratio,
>> >> -       uint8_t scan_information,
>> >> -       uint8_t colorimetry,
>> >> -       uint8_t ex_colorimetry,
>> >> -       uint8_t quantization,
>> >> -       int ITC,
>> >> -       uint8_t picture_aspect_ratio,
>> >> -       uint8_t video_format_identification,
>> >> -       uint8_t pixel_repetition,
>> >> -       uint8_t non_uniform_picture_scaling,
>> >> -       uint8_t bar_info_data_valid,
>> >> -       uint16_t top_bar,
>> >> -       uint16_t bottom_bar,
>> >> -       uint16_t left_bar,
>> >> -       uint16_t right_bar
>> >> -)
>> >
>> > In case someone wonders about the reason: I think it's really ugly to
>> > have a function taking 18 arguments, 17 of them related to the
>> > infoframe. It makes much more sense for me to use struct for that.
>> > While working on that I though it's reasonable to prepare nice
>> > bitfield __packed struct ready-to-be-written to the GPU registers.
>>
>> won't this screw up on other endian machines?
>
> ... and can we have this in a slightly generic way maybe? We have copies
> of this in i915 and nouveau.

More or less...

I know Intel defines struct dip_infoframe.

Unfortunately it uses things like:
	uint8_t type;		/* HB0 */
	uint8_t ver;		/* HB1 */
	uint8_t len;		/* HB2 - body len, not including checksum */
	uint8_t ecc;		/* Header ECC */
which we don't need in radeon. But maybe we could just ignore that
additional fields in radeon and live with just a little bigger struct.

However Intel still has fields like:
/* 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;

That means it still requires setting some fields with bitshifting and
ORing. If this is possible to make it endian-safe I've love to split
such a fields into separated-and-shorter ones.

-- 
Rafał
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe

[Michel Dänzer]

On Son, 2012-05-06 at 18:29 +0200, Rafał Miłecki wrote: 
> 2012/5/6 Dave Airlie <airlied@gmail.com>:
> > On Sun, May 6, 2012 at 5:19 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> >> 2012/5/6 Rafał Miłecki <zajec5@gmail.com>:
> >>> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
> >>> index c308432..b14c90a 100644
> >>> --- a/drivers/gpu/drm/radeon/r600_hdmi.c
> >>> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c
> >>> @@ -134,78 +134,22 @@ static void r600_hdmi_infoframe_checksum(uint8_t packetType,
> >>>  }
> >>>
> >>>  /*
> >>> - * build a HDMI Video Info Frame
> >>> + * Upload a HDMI AVI Infoframe
> >>>  */
> >>> -static void r600_hdmi_videoinfoframe(
> >>> -       struct drm_encoder *encoder,
> >>> -       enum r600_hdmi_color_format color_format,
> >>> -       int active_information_present,
> >>> -       uint8_t active_format_aspect_ratio,
> >>> -       uint8_t scan_information,
> >>> -       uint8_t colorimetry,
> >>> -       uint8_t ex_colorimetry,
> >>> -       uint8_t quantization,
> >>> -       int ITC,
> >>> -       uint8_t picture_aspect_ratio,
> >>> -       uint8_t video_format_identification,
> >>> -       uint8_t pixel_repetition,
> >>> -       uint8_t non_uniform_picture_scaling,
> >>> -       uint8_t bar_info_data_valid,
> >>> -       uint16_t top_bar,
> >>> -       uint16_t bottom_bar,
> >>> -       uint16_t left_bar,
> >>> -       uint16_t right_bar
> >>> -)
> >>
> >> In case someone wonders about the reason: I think it's really ugly to
> >> have a function taking 18 arguments, 17 of them related to the
> >> infoframe. It makes much more sense for me to use struct for that.
> >> While working on that I though it's reasonable to prepare nice
> >> bitfield __packed struct ready-to-be-written to the GPU registers.
> >
> > won't this screw up on other endian machines?
> 
> Hm, maybe it can. Is there some easy to handle it correctly? Some trick like
> __le8 foo: 3
> __le8 bar: 1
> maybe?

Not really. The memory layout of bitfields is basically completely up to
the C implementation, so IMHO they're just inadequate for describing
fixed memory layouts.


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe

[Jerome Glisse]

On Mon, May 7, 2012 at 3:38 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On Son, 2012-05-06 at 18:29 +0200, Rafał Miłecki wrote:
>> 2012/5/6 Dave Airlie <airlied@gmail.com>:
>> > On Sun, May 6, 2012 at 5:19 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
>> >> 2012/5/6 Rafał Miłecki <zajec5@gmail.com>:
>> >>> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
>> >>> index c308432..b14c90a 100644
>> >>> --- a/drivers/gpu/drm/radeon/r600_hdmi.c
>> >>> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c
>> >>> @@ -134,78 +134,22 @@ static void r600_hdmi_infoframe_checksum(uint8_t packetType,
>> >>>  }
>> >>>
>> >>>  /*
>> >>> - * build a HDMI Video Info Frame
>> >>> + * Upload a HDMI AVI Infoframe
>> >>>  */
>> >>> -static void r600_hdmi_videoinfoframe(
>> >>> -       struct drm_encoder *encoder,
>> >>> -       enum r600_hdmi_color_format color_format,
>> >>> -       int active_information_present,
>> >>> -       uint8_t active_format_aspect_ratio,
>> >>> -       uint8_t scan_information,
>> >>> -       uint8_t colorimetry,
>> >>> -       uint8_t ex_colorimetry,
>> >>> -       uint8_t quantization,
>> >>> -       int ITC,
>> >>> -       uint8_t picture_aspect_ratio,
>> >>> -       uint8_t video_format_identification,
>> >>> -       uint8_t pixel_repetition,
>> >>> -       uint8_t non_uniform_picture_scaling,
>> >>> -       uint8_t bar_info_data_valid,
>> >>> -       uint16_t top_bar,
>> >>> -       uint16_t bottom_bar,
>> >>> -       uint16_t left_bar,
>> >>> -       uint16_t right_bar
>> >>> -)
>> >>
>> >> In case someone wonders about the reason: I think it's really ugly to
>> >> have a function taking 18 arguments, 17 of them related to the
>> >> infoframe. It makes much more sense for me to use struct for that.
>> >> While working on that I though it's reasonable to prepare nice
>> >> bitfield __packed struct ready-to-be-written to the GPU registers.
>> >
>> > won't this screw up on other endian machines?
>>
>> Hm, maybe it can. Is there some easy to handle it correctly? Some trick like
>> __le8 foo: 3
>> __le8 bar: 1
>> maybe?
>
> Not really. The memory layout of bitfields is basically completely up to
> the C implementation, so IMHO they're just inadequate for describing
> fixed memory layouts.
>
>

Yes i agree please stay away from bitfields, i know it looks cool but
bitshift is cool too.

Cheers,
Jerome
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel