[PATCH] drm/i915: set the AVI VIC of the HDMI mode

[PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We currently set "0" as the VIC value of the AVI InfoFrames. According
to the specs this should be fine and work for every mode, so to my
point of view we can't consider the current behavior as a bug. The
problem is that  we recently received a bug report (Kernel bug #50371)
from a user that has an AV receiver that gives a black screen for any
mode with VIC set to 0.

So in order to make at least some modes work for him, this patch sets
the correct VIC number when sending AVI InfoFrames. We add a generic
drm function to calculate the VIC number and then call it from the
i915 driver.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=50371
Cc: Thierry Reding <thierry.reding@avionic-design.de>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/drm_edid.c        |   19 +++++++++++++++++++
 drivers/gpu/drm/drm_modes.c       |    3 ++-
 drivers/gpu/drm/i915/intel_hdmi.c |    2 ++
 include/drm/drm_crtc.h            |    4 +++-
 4 files changed, 26 insertions(+), 2 deletions(-)

Patch applies on top of Daniel's drm-intel-next-queued. I'm not sure who exactly
is going to merge this (Dave or Daniel).

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fadcd44..c57fc46 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2062,3 +2062,22 @@ int drm_add_modes_noedid(struct drm_connector *connector,
 	return num_modes;
 }
 EXPORT_SYMBOL(drm_add_modes_noedid);
+
+/**
+ * drm_mode_vic - return the CEA-861 VIC of a given mode
+ * @mode: mode
+ *
+ * RETURNS:
+ * The VIC number, 0 in case it's not a CEA-861 mode.
+ */
+uint8_t drm_mode_cea_vic(const struct drm_display_mode *mode)
+{
+	uint8_t i;
+
+	for (i = 0; i < drm_num_cea_modes; i++)
+		if (drm_mode_equal(mode, &edid_cea_modes[i]))
+			return i + 1;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_mode_cea_vic);
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..9ef6750 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -768,7 +768,8 @@ EXPORT_SYMBOL(drm_mode_duplicate);
  * RETURNS:
  * True if the modes are equal, false otherwise.
  */
-bool drm_mode_equal(struct drm_display_mode *mode1, struct drm_display_mode *mode2)
+bool drm_mode_equal(const struct drm_display_mode *mode1,
+		    const struct drm_display_mode *mode2)
 {
 	/* do clock check convert to PICOS so fb modes get matched
 	 * the same */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 1dcfd5b..eaf70d6 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -340,6 +340,8 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
 		avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;
 
+	avi_if.body.avi.VIC = drm_mode_cea_vic(adjusted_mode);
+
 	intel_set_infoframe(encoder, &avi_if);
 }
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3fa18b7..245220e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -892,7 +892,8 @@ extern void drm_mode_config_init(struct drm_device *dev);
 extern void drm_mode_config_reset(struct drm_device *dev);
 extern void drm_mode_config_cleanup(struct drm_device *dev);
 extern void drm_mode_set_name(struct drm_display_mode *mode);
-extern bool drm_mode_equal(struct drm_display_mode *mode1, struct drm_display_mode *mode2);
+extern bool drm_mode_equal(const struct drm_display_mode *mode1,
+			   const struct drm_display_mode *mode2);
 extern int drm_mode_width(struct drm_display_mode *mode);
 extern int drm_mode_height(struct drm_display_mode *mode);
 
@@ -1053,6 +1054,7 @@ extern struct drm_display_mode *drm_gtf_mode_complex(struct drm_device *dev,
 				int GTF_2C, int GTF_K, int GTF_2J);
 extern int drm_add_modes_noedid(struct drm_connector *connector,
 				int hdisplay, int vdisplay);
+extern uint8_t drm_mode_cea_vic(const struct drm_display_mode *mode);
 
 extern int drm_edid_header_is_valid(const u8 *raw_edid);
 extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
-- 
1.7.10.4

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2244 bytes --]

On Wed, Nov 21, 2012 at 01:39:48PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> We currently set "0" as the VIC value of the AVI InfoFrames. According
> to the specs this should be fine and work for every mode, so to my
> point of view we can't consider the current behavior as a bug. The
> problem is that  we recently received a bug report (Kernel bug #50371)
> from a user that has an AV receiver that gives a black screen for any
> mode with VIC set to 0.
> 
> So in order to make at least some modes work for him, this patch sets
> the correct VIC number when sending AVI InfoFrames. We add a generic
> drm function to calculate the VIC number and then call it from the
> i915 driver.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=50371
> Cc: Thierry Reding <thierry.reding@avionic-design.de>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c        |   19 +++++++++++++++++++
>  drivers/gpu/drm/drm_modes.c       |    3 ++-
>  drivers/gpu/drm/i915/intel_hdmi.c |    2 ++
>  include/drm/drm_crtc.h            |    4 +++-
>  4 files changed, 26 insertions(+), 2 deletions(-)
> 
> Patch applies on top of Daniel's drm-intel-next-queued. I'm not sure who exactly
> is going to merge this (Dave or Daniel).
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index fadcd44..c57fc46 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2062,3 +2062,22 @@ int drm_add_modes_noedid(struct drm_connector *connector,
>  	return num_modes;
>  }
>  EXPORT_SYMBOL(drm_add_modes_noedid);
> +
> +/**
> + * drm_mode_vic - return the CEA-861 VIC of a given mode
> + * @mode: mode
> + *
> + * RETURNS:
> + * The VIC number, 0 in case it's not a CEA-861 mode.
> + */
> +uint8_t drm_mode_cea_vic(const struct drm_display_mode *mode)
> +{
> +	uint8_t i;
> +
> +	for (i = 0; i < drm_num_cea_modes; i++)
> +		if (drm_mode_equal(mode, &edid_cea_modes[i]))
> +			return i + 1;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_cea_vic);

Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can
reuse that in the HDMI infoframe series.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Daniel Vetter]

On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can
> reuse that in the HDMI infoframe series.

Wrt the infoframe series, I think it'd be awesome if you could convert
i915 and radeon (iirc the existing drivers with the "best" avi
infoframe support) over to the new code. This gives some nice
validation, both by testing on actual hw and that the interface is
sane, since it'll be used by 2-3 different drivers then.

I think the best way is to pick the infoframe implementation you best
like from one of these, move it into the helper, then improve it until
you're happy. And then convert over 1-2 other drivers. At least that's
been my approach for the recent dp helper refactoring.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Daniel Vetter]

On Wed, Nov 21, 2012 at 5:08 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
>> Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can
>> reuse that in the HDMI infoframe series.
>
> Wrt the infoframe series, I think it'd be awesome if you could convert
> i915 and radeon (iirc the existing drivers with the "best" avi
> infoframe support) over to the new code. This gives some nice
> validation, both by testing on actual hw and that the interface is
> sane, since it'll be used by 2-3 different drivers then.
>
> I think the best way is to pick the infoframe implementation you best
> like from one of these, move it into the helper, then improve it until
> you're happy. And then convert over 1-2 other drivers. At least that's
> been my approach for the recent dp helper refactoring.

And if you could use that opportunity to integrate the kerneldoc for
edid/eld and the new infoframe code into the drm docbook, that would
be rather awesome ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 3069 bytes --]

On Wed, Nov 21, 2012 at 01:39:48PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> We currently set "0" as the VIC value of the AVI InfoFrames. According
> to the specs this should be fine and work for every mode, so to my
> point of view we can't consider the current behavior as a bug. The
> problem is that  we recently received a bug report (Kernel bug #50371)
> from a user that has an AV receiver that gives a black screen for any
> mode with VIC set to 0.
> 
> So in order to make at least some modes work for him, this patch sets
> the correct VIC number when sending AVI InfoFrames. We add a generic
> drm function to calculate the VIC number and then call it from the
> i915 driver.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=50371
> Cc: Thierry Reding <thierry.reding@avionic-design.de>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c        |   19 +++++++++++++++++++
>  drivers/gpu/drm/drm_modes.c       |    3 ++-
>  drivers/gpu/drm/i915/intel_hdmi.c |    2 ++
>  include/drm/drm_crtc.h            |    4 +++-
>  4 files changed, 26 insertions(+), 2 deletions(-)
> 
> Patch applies on top of Daniel's drm-intel-next-queued. I'm not sure who exactly
> is going to merge this (Dave or Daniel).
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index fadcd44..c57fc46 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2062,3 +2062,22 @@ int drm_add_modes_noedid(struct drm_connector *connector,
>  	return num_modes;
>  }
>  EXPORT_SYMBOL(drm_add_modes_noedid);
> +
> +/**
> + * drm_mode_vic - return the CEA-861 VIC of a given mode

The name in the comment here doesn't match the actual function name.

> + * @mode: mode
> + *
> + * RETURNS:
> + * The VIC number, 0 in case it's not a CEA-861 mode.
> + */
> +uint8_t drm_mode_cea_vic(const struct drm_display_mode *mode)

I understand the reason for returning an uint8_t here, but ...

> +{
> +	uint8_t i;
> +
> +	for (i = 0; i < drm_num_cea_modes; i++)

... maybe unsigned int would be better for the iteration variable here.
Looking at drm_edid_modes.h, drm_num_cea_modes is actually signed, which
isn't necessary to store an array size, so maybe that should be changed
as well.

> +		if (drm_mode_equal(mode, &edid_cea_modes[i]))
> +			return i + 1;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_cea_vic);
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 59450f3..9ef6750 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -768,7 +768,8 @@ EXPORT_SYMBOL(drm_mode_duplicate);
>   * RETURNS:
>   * True if the modes are equal, false otherwise.
>   */
> -bool drm_mode_equal(struct drm_display_mode *mode1, struct drm_display_mode *mode2)
> +bool drm_mode_equal(const struct drm_display_mode *mode1,
> +		    const struct drm_display_mode *mode2)

I think this change warrants a separate commit.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 915 bytes --]

On Wed, Nov 21, 2012 at 05:08:12PM +0100, Daniel Vetter wrote:
> On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
> > Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can
> > reuse that in the HDMI infoframe series.
> 
> Wrt the infoframe series, I think it'd be awesome if you could convert
> i915 and radeon (iirc the existing drivers with the "best" avi
> infoframe support) over to the new code. This gives some nice
> validation, both by testing on actual hw and that the interface is
> sane, since it'll be used by 2-3 different drivers then.

I'll have to rely on somebody else to do the testing since I don't have
an HDMI capable hardware except Tegra to run this on. But yes, I had
planned to convert Tegra and at least one other driver for reference.
But I guess while at it I could just as well convert all of them.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1248 bytes --]

On Wed, Nov 21, 2012 at 05:09:31PM +0100, Daniel Vetter wrote:
> On Wed, Nov 21, 2012 at 5:08 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding
> > <thierry.reding@avionic-design.de> wrote:
> >> Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can
> >> reuse that in the HDMI infoframe series.
> >
> > Wrt the infoframe series, I think it'd be awesome if you could convert
> > i915 and radeon (iirc the existing drivers with the "best" avi
> > infoframe support) over to the new code. This gives some nice
> > validation, both by testing on actual hw and that the interface is
> > sane, since it'll be used by 2-3 different drivers then.
> >
> > I think the best way is to pick the infoframe implementation you best
> > like from one of these, move it into the helper, then improve it until
> > you're happy. And then convert over 1-2 other drivers. At least that's
> > been my approach for the recent dp helper refactoring.
> 
> And if you could use that opportunity to integrate the kerneldoc for
> edid/eld and the new infoframe code into the drm docbook, that would
> be rather awesome ;-)

I suck at documentation =), but I'll see what I can do.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Rafał Miłecki]

2012/11/22 Thierry Reding <thierry.reding@avionic-design.de>:
> On Wed, Nov 21, 2012 at 05:08:12PM +0100, Daniel Vetter wrote:
>> On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding
>> <thierry.reding@avionic-design.de> wrote:
>> > Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can
>> > reuse that in the HDMI infoframe series.
>>
>> Wrt the infoframe series, I think it'd be awesome if you could convert
>> i915 and radeon (iirc the existing drivers with the "best" avi
>> infoframe support) over to the new code. This gives some nice
>> validation, both by testing on actual hw and that the interface is
>> sane, since it'll be used by 2-3 different drivers then.
>
> I'll have to rely on somebody else to do the testing since I don't have
> an HDMI capable hardware except Tegra to run this on. But yes, I had
> planned to convert Tegra and at least one other driver for reference.
> But I guess while at it I could just as well convert all of them.

I'll take a look at radeon (I'll try to convert it to the new
functions) over weekend.

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

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1547 bytes --]

On Thu, Nov 22, 2012 at 09:00:24AM +0100, Rafał Miłecki wrote:
> 2012/11/22 Thierry Reding <thierry.reding@avionic-design.de>:
> > On Wed, Nov 21, 2012 at 05:08:12PM +0100, Daniel Vetter wrote:
> >> On Wed, Nov 21, 2012 at 4:47 PM, Thierry Reding
> >> <thierry.reding@avionic-design.de> wrote:
> >> > Oh great, so I copied that table for nothing. Thanks for Cc'ing, I can
> >> > reuse that in the HDMI infoframe series.
> >>
> >> Wrt the infoframe series, I think it'd be awesome if you could convert
> >> i915 and radeon (iirc the existing drivers with the "best" avi
> >> infoframe support) over to the new code. This gives some nice
> >> validation, both by testing on actual hw and that the interface is
> >> sane, since it'll be used by 2-3 different drivers then.
> >
> > I'll have to rely on somebody else to do the testing since I don't have
> > an HDMI capable hardware except Tegra to run this on. But yes, I had
> > planned to convert Tegra and at least one other driver for reference.
> > But I guess while at it I could just as well convert all of them.
> 
> I'll take a look at radeon (I'll try to convert it to the new
> functions) over weekend.

Okay, great! I think for radeon things should be the easiest since it
doesn't currently fill in anything but the colorspace field. Judging by
the bug report that Paulo mentioned this will probably not be enough for
some hardware, but should be enough according to the specification.
Having more data in the AVI infoframe shouldn't hurt, though.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Paulo Zanoni]

Hi

2012/11/22 Thierry Reding <thierry.reding@avionic-design.de>:
> On Wed, Nov 21, 2012 at 01:39:48PM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> We currently set "0" as the VIC value of the AVI InfoFrames. According
>> to the specs this should be fine and work for every mode, so to my
>> point of view we can't consider the current behavior as a bug. The
>> problem is that  we recently received a bug report (Kernel bug #50371)
>> from a user that has an AV receiver that gives a black screen for any
>> mode with VIC set to 0.
>>
>> So in order to make at least some modes work for him, this patch sets
>> the correct VIC number when sending AVI InfoFrames. We add a generic
>> drm function to calculate the VIC number and then call it from the
>> i915 driver.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=50371
>> Cc: Thierry Reding <thierry.reding@avionic-design.de>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c        |   19 +++++++++++++++++++
>>  drivers/gpu/drm/drm_modes.c       |    3 ++-
>>  drivers/gpu/drm/i915/intel_hdmi.c |    2 ++
>>  include/drm/drm_crtc.h            |    4 +++-
>>  4 files changed, 26 insertions(+), 2 deletions(-)
>>
>> Patch applies on top of Daniel's drm-intel-next-queued. I'm not sure who exactly
>> is going to merge this (Dave or Daniel).
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index fadcd44..c57fc46 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2062,3 +2062,22 @@ int drm_add_modes_noedid(struct drm_connector *connector,
>>       return num_modes;
>>  }
>>  EXPORT_SYMBOL(drm_add_modes_noedid);
>> +
>> +/**
>> + * drm_mode_vic - return the CEA-861 VIC of a given mode
>
> The name in the comment here doesn't match the actual function name.

Will fix.

>
>> + * @mode: mode
>> + *
>> + * RETURNS:
>> + * The VIC number, 0 in case it's not a CEA-861 mode.
>> + */
>> +uint8_t drm_mode_cea_vic(const struct drm_display_mode *mode)
>
> I understand the reason for returning an uint8_t here, but ...
>
>> +{
>> +     uint8_t i;
>> +
>> +     for (i = 0; i < drm_num_cea_modes; i++)
>
> ... maybe unsigned int would be better for the iteration variable here.
> Looking at drm_edid_modes.h, drm_num_cea_modes is actually signed, which
> isn't necessary to store an array size, so maybe that should be changed
> as well.

I used uint8_t because "i" is the thing we return. I don't think
there's a perfect solution to this problem and I don't really think it
matters too much.

>
>> +             if (drm_mode_equal(mode, &edid_cea_modes[i]))
>> +                     return i + 1;
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL(drm_mode_cea_vic);
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index 59450f3..9ef6750 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -768,7 +768,8 @@ EXPORT_SYMBOL(drm_mode_duplicate);
>>   * RETURNS:
>>   * True if the modes are equal, false otherwise.
>>   */
>> -bool drm_mode_equal(struct drm_display_mode *mode1, struct drm_display_mode *mode2)
>> +bool drm_mode_equal(const struct drm_display_mode *mode1,
>> +                 const struct drm_display_mode *mode2)
>
> I think this change warrants a separate commit.

I just noticed Dave's tree already has these things as const. So V2
will be based on Dave's drm-next tree.

>
> Thierry



-- 
Paulo Zanoni

Re: [PATCH] drm/i915: set the AVI VIC of the HDMI mode

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1045 bytes --]

On Fri, Nov 23, 2012 at 11:46:10AM -0200, Paulo Zanoni wrote:
> Hi
> 
> 2012/11/22 Thierry Reding <thierry.reding@avionic-design.de>:
> > On Wed, Nov 21, 2012 at 01:39:48PM -0200, Paulo Zanoni wrote:
[...]
> >> + * @mode: mode
> >> + *
> >> + * RETURNS:
> >> + * The VIC number, 0 in case it's not a CEA-861 mode.
> >> + */
> >> +uint8_t drm_mode_cea_vic(const struct drm_display_mode *mode)
> >
> > I understand the reason for returning an uint8_t here, but ...
> >
> >> +{
> >> +     uint8_t i;
> >> +
> >> +     for (i = 0; i < drm_num_cea_modes; i++)
> >
> > ... maybe unsigned int would be better for the iteration variable here.
> > Looking at drm_edid_modes.h, drm_num_cea_modes is actually signed, which
> > isn't necessary to store an array size, so maybe that should be changed
> > as well.
> 
> I used uint8_t because "i" is the thing we return. I don't think
> there's a perfect solution to this problem and I don't really think it
> matters too much.

Fair enough, uint8_t should work fine.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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