From: Lyude <lyude@redhat.com> To: intel-gfx@lists.freedesktop.org Cc: Lyude <lyude@redhat.com>, "Chris Wilson" <chris@chris-wilson.co.uk>, "Daniel Vetter" <daniel@ffwll.ch>, "Jani Nikula" <jani.nikula@intel.com>, "Ville Syrjälä" <ville.syrjala@linux.intel.com>, "Daniel Vetter" <daniel.vetter@intel.com>, "Jani Nikula" <jani.nikula@linux.intel.com>, "David Airlie" <airlied@linux.ie>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: [PATCH] drm/i915/dp: Stop enabling limited color ranges for everything Date: Wed, 4 Jan 2017 20:11:36 -0500 [thread overview] Message-ID: <20170105011137.20209-1-lyude@redhat.com> (raw) Until now, it seems we've been erroneously enabling limited color ranges for the vast majority of DisplayPort monitors. I noticed this after writing a frame dump comparison test for the Chamelium and noticing that every i915 device I had was failing, while amdgpu machines were fine: https://people.freedesktop.org/~lyudess/archive/01-03-2017/ It looks like this is because the DisplayPort spec tells us to use limited color ranges whenever we detect a CEA mode in use. However, from the looks of it there's another rather confusing part of the spec that got missed: source devices are allowed to use the full range of values for pixels -even- if the sink device declares that it's using a CEA mode. It's up to the sink device to limit the pixel range to the CEA ranges if it needs. Signed-off-by: Lyude <lyude@redhat.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> --- I'm really surprised that this bug would have been around as long as it looks like it has been without anyone noticing it, so I figured I'd just send a patch to the mailing list so you guys can point out whether or not this is really the correct thing to do. drivers/gpu/drm/i915/intel_dp.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d9bc19b..6642abd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1649,12 +1649,15 @@ intel_dp_compute_config(struct intel_encoder *encoder, found: if (intel_dp->color_range_auto) { /* + * We are required to use the limited CEA RGB range when a CEA + * mode is declared in the EDID. However, limiting the pixel + * value range is up to the sink, not the source. So, just + * don't enable limited color ranges. * See: * CEA-861-E - 5.1 Default Encoding Parameters * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry */ - pipe_config->limited_color_range = - bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1; + pipe_config->limited_color_range = false; } else { pipe_config->limited_color_range = intel_dp->limited_color_range; -- 2.9.3
WARNING: multiple messages have this Message-ID (diff)
From: Lyude <lyude@redhat.com> To: intel-gfx@lists.freedesktop.org Cc: Lyude <lyude@redhat.com>, Jani Nikula <jani.nikula@intel.com>, linux-kernel@vger.kernel.org, David Airlie <airlied@linux.ie>, dri-devel@lists.freedesktop.org, Daniel Vetter <daniel.vetter@intel.com> Subject: [PATCH] drm/i915/dp: Stop enabling limited color ranges for everything Date: Wed, 4 Jan 2017 20:11:36 -0500 [thread overview] Message-ID: <20170105011137.20209-1-lyude@redhat.com> (raw) Until now, it seems we've been erroneously enabling limited color ranges for the vast majority of DisplayPort monitors. I noticed this after writing a frame dump comparison test for the Chamelium and noticing that every i915 device I had was failing, while amdgpu machines were fine: https://people.freedesktop.org/~lyudess/archive/01-03-2017/ It looks like this is because the DisplayPort spec tells us to use limited color ranges whenever we detect a CEA mode in use. However, from the looks of it there's another rather confusing part of the spec that got missed: source devices are allowed to use the full range of values for pixels -even- if the sink device declares that it's using a CEA mode. It's up to the sink device to limit the pixel range to the CEA ranges if it needs. Signed-off-by: Lyude <lyude@redhat.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> --- I'm really surprised that this bug would have been around as long as it looks like it has been without anyone noticing it, so I figured I'd just send a patch to the mailing list so you guys can point out whether or not this is really the correct thing to do. drivers/gpu/drm/i915/intel_dp.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d9bc19b..6642abd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1649,12 +1649,15 @@ intel_dp_compute_config(struct intel_encoder *encoder, found: if (intel_dp->color_range_auto) { /* + * We are required to use the limited CEA RGB range when a CEA + * mode is declared in the EDID. However, limiting the pixel + * value range is up to the sink, not the source. So, just + * don't enable limited color ranges. * See: * CEA-861-E - 5.1 Default Encoding Parameters * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry */ - pipe_config->limited_color_range = - bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1; + pipe_config->limited_color_range = false; } else { pipe_config->limited_color_range = intel_dp->limited_color_range; -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next reply other threads:[~2017-01-05 1:12 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-01-05 1:11 Lyude [this message] 2017-01-05 1:11 ` [PATCH] drm/i915/dp: Stop enabling limited color ranges for everything Lyude 2017-01-05 8:41 ` Jani Nikula 2017-01-05 8:41 ` Jani Nikula 2017-01-05 8:52 ` Daniel Vetter 2017-01-05 8:52 ` Daniel Vetter 2017-01-05 9:41 ` Peter Frühberger 2017-01-05 10:04 ` Daniel Stone 2017-01-05 10:04 ` Daniel Stone 2017-01-05 14:49 ` Lyude Paul 2017-01-05 14:49 ` Lyude Paul
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20170105011137.20209-1-lyude@redhat.com \ --to=lyude@redhat.com \ --cc=airlied@linux.ie \ --cc=chris@chris-wilson.co.uk \ --cc=daniel.vetter@intel.com \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=jani.nikula@intel.com \ --cc=jani.nikula@linux.intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=ville.syrjala@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.