Re: [RFC v2 3/5] drm: Add HDMI infoframe helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 05, 2012 at 05:45:42PM +0100, Thierry Reding wrote:
> Add a generic helper to fill in an HDMI AVI infoframe with data
> extracted from a DRM display mode.
> 
> Signed-off-by: Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx>

A few quick comments below.

> ---
> Changes in v2:
> - reuse CEA modes defined in drm_edid_modes.h
> 
>  drivers/gpu/drm/Kconfig    |  7 +++++
>  drivers/gpu/drm/Makefile   |  1 +
>  drivers/gpu/drm/drm_hdmi.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_hdmi.h     | 19 +++++++++++++
>  4 files changed, 98 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_hdmi.c
>  create mode 100644 include/drm/drm_hdmi.h
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 983201b..94a4623 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -69,6 +69,13 @@ config DRM_KMS_CMA_HELPER
>  	help
>  	  Choose this if you need the KMS CMA helper functions
>  
> +config DRM_HDMI
> +	bool
> +	depends on DRM
> +	select HDMI
> +	help
> +	  Choose this if you need the HDMI helper functions
> +
>  config DRM_TDFX
>  	tristate "3dfx Banshee/Voodoo3+"
>  	depends on DRM && PCI
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6f58c81..4a0b781 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -16,6 +16,7 @@ drm-y       :=	drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
>  
>  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> +drm-$(CONFIG_DRM_HDMI) += drm_hdmi.o
>  drm-$(CONFIG_PCI) += ati_pcigart.o
>  
>  drm-usb-y   := drm_usb.o
> diff --git a/drivers/gpu/drm/drm_hdmi.c b/drivers/gpu/drm/drm_hdmi.c
> new file mode 100644
> index 0000000..821ca56
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_hdmi.c
> @@ -0,0 +1,71 @@
> +/*
> + * Copyright (C) 2012 Avionic Design GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/hdmi.h>
> +
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_hdmi.h>
> +
> +#include "drm_edid_modes.h"

This creates a 2nd copy of that massive table. Imo we could just shovel
the infoframe helpers into the drm_edid.c file - allowing different
helpers to be disabled isn't that useful, since most drivers will want
them pretty much all anyway. Or at least most of them.

> +
> +static inline unsigned int
> +drm_mode_cea_vic(const struct drm_display_mode *mode)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < drm_num_cea_modes; i++)
> +		if (drm_mode_equal(mode, &edid_cea_modes[i]))
> +			return i + 1;
> +
> +	return 0;
> +}

Same function in drm_edid will land through drm-intel tree in drm-next
rsn. I'll send that pull request somewhen next week probably.

http://cgit.freedesktop.org/~danvet/drm-intel/commit/?id=374a868a726eb8a1cb28ba88805e51ce34222f8d

> +
> +static inline enum hdmi_picture_aspect
> +drm_display_mode_get_aspect(const struct drm_display_mode *mode)
> +{
> +	enum hdmi_picture_aspect aspect = HDMI_PICTURE_ASPECT_NONE;
> +
> +	if ((mode->hdisplay * 9) / 16 == mode->vdisplay)
> +		aspect = HDMI_PICTURE_ASPECT_16_9;
> +	else if ((mode->hdisplay * 3) / 4 == mode->vdisplay)
> +		aspect = HDMI_PICTURE_ASPECT_4_3;
> +
> +	return aspect;
> +}
> +
> +/**
> + * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
> + *                                              data from a DRM display mode
> + * @frame: HDMI AVI infoframe
> + * @mode: DRM display mode
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int
> +drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> +					 const struct drm_display_mode *mode)
> +{
> +	int err;
> +
> +	if (!frame || !mode)
> +		return -EINVAL;
> +
> +	err = hdmi_avi_infoframe_init(frame);
> +	if (err < 0)
> +		return err;
> +
> +	frame->video_code = drm_mode_cea_vic(mode);
> +	if (!frame->video_code)
> +		return 0;
> +
> +	frame->picture_aspect = drm_display_mode_get_aspect(mode);
> +	frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;

Note that the intel avi infoframe in intel_hdmi_set_avi_infoframe also
sets the pixel repeat for double clocked modes with:

	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
		avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;

Cheers, Daniel

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
> diff --git a/include/drm/drm_hdmi.h b/include/drm/drm_hdmi.h
> new file mode 100644
> index 0000000..e20462d
> --- /dev/null
> +++ b/include/drm/drm_hdmi.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2012 Avionic Design GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _DRM_HDMI_H_
> +#define _DRM_HDMI_H_
> +
> +struct hdmi_avi_infoframe;
> +struct drm_display_mode;
> +
> +int
> +drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> +					 const struct drm_display_mode *mode);
> +
> +#endif
> -- 
> 1.8.0.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux