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

["Michel Dänzer" <michel@daenzer.net>]

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