Hi 2012/11/22 Thierry Reding <thierry.reding at avionic-design.de>: > On Wed, Nov 21, 2012 at 01:39:48PM -0200, Paulo Zanoni wrote: >> From: Paulo Zanoni <paulo.r.zanoni at 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 at avionic-design.de> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at 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
