From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915/dp: Stop enabling limited color ranges for everything Date: Thu, 05 Jan 2017 10:41:07 +0200 Message-ID: <87bmvlhpb0.fsf@intel.com> References: <20170105011137.20209-1-lyude@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170105011137.20209-1-lyude@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: intel-gfx@lists.freedesktop.org Cc: Lyude , David Airlie , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCAwNSBKYW4gMjAxNywgTHl1ZGUgPGx5dWRlQHJlZGhhdC5jb20+IHdyb3RlOgo+IFVu dGlsIG5vdywgaXQgc2VlbXMgd2UndmUgYmVlbiBlcnJvbmVvdXNseSBlbmFibGluZyBsaW1pdGVk IGNvbG9yIHJhbmdlcwo+IGZvciB0aGUgdmFzdCBtYWpvcml0eSBvZiBEaXNwbGF5UG9ydCBtb25p dG9ycy4gSSBub3RpY2VkIHRoaXMgYWZ0ZXIKPiB3cml0aW5nIGEgZnJhbWUgZHVtcCBjb21wYXJp c29uIHRlc3QgZm9yIHRoZSBDaGFtZWxpdW0gYW5kIG5vdGljaW5nIHRoYXQKPiBldmVyeSBpOTE1 IGRldmljZSBJIGhhZCB3YXMgZmFpbGluZywgd2hpbGUgYW1kZ3B1IG1hY2hpbmVzIHdlcmUgZmlu ZToKPgo+IGh0dHBzOi8vcGVvcGxlLmZyZWVkZXNrdG9wLm9yZy9+bHl1ZGVzcy9hcmNoaXZlLzAx LTAzLTIwMTcvCj4KPiBJdCBsb29rcyBsaWtlIHRoaXMgaXMgYmVjYXVzZSB0aGUgRGlzcGxheVBv cnQgc3BlYyB0ZWxscyB1cyB0byB1c2UKPiBsaW1pdGVkIGNvbG9yIHJhbmdlcyB3aGVuZXZlciB3 ZSBkZXRlY3QgYSBDRUEgbW9kZSBpbiB1c2UuIEhvd2V2ZXIsIGZyb20KPiB0aGUgbG9va3Mgb2Yg aXQgdGhlcmUncyBhbm90aGVyIHJhdGhlciBjb25mdXNpbmcgcGFydCBvZiB0aGUgc3BlYyB0aGF0 Cj4gZ290IG1pc3NlZDogc291cmNlIGRldmljZXMgYXJlIGFsbG93ZWQgdG8gdXNlIHRoZSBmdWxs IHJhbmdlIG9mIHZhbHVlcwo+IGZvciBwaXhlbHMgLWV2ZW4tIGlmIHRoZSBzaW5rIGRldmljZSBk ZWNsYXJlcyB0aGF0IGl0J3MgdXNpbmcgYSBDRUEKPiBtb2RlLiBJdCdzIHVwIHRvIHRoZSBzaW5r IGRldmljZSB0byBsaW1pdCB0aGUgcGl4ZWwgcmFuZ2UgdG8gdGhlIENFQQo+IHJhbmdlcyBpZiBp dCBuZWVkcy4KCkRQIDEuMmEgNS4xLjEuMSBWaWRlbyBDb2xvcmltZXRyeQoKKiBXaGVuIGEgU291 cmNlIGRldmljZSBpcyB0cmFuc21pdHRpbmcgYW4gUkdCIHN0cmVhbSB3aXRoIGEgdmlkZW8gdGlt aW5nCiAgZm9ybWF0IGNhbGxlZCBvdXQgaW4gQ0VBLTg2MUMgU2VjdGlvbiA1IChleGNlcHQgNjQw IHggNDgwcCkgYXMgdXNpbmcKICBDRUEgcmFuZ2UgUkdCLCBpdCBzaG91bGQgdXNlIENFQSByYW5n ZSBSR0IuCgoqIFdoZW4gYSBTb3VyY2UgZGV2aWNlIGlzIHRyYW5zbWl0dGluZyBhIFJHQiBzdHJl YW0gd2l0aCBhIHZpZGVvIHRpbWluZwogIGZvcm1hdCBjYWxsZWQgb3V0IGluIENFQS04NjFDIFNl Y3Rpb24gNSBhcyB1c2luZyBDRUEgcmFuZ2UgUkdCLCBpdAogIG11c3QgdXNlIHRoZSBDRUEgcmFu Z2UgUkdCLgoKKiBIb3dldmVyLCBhIFNvdXJjZSBkZXZpY2UgbWF5IHRyYW5zbWl0IGFsbCBjb2Rl IHZhbHVlcyBmcm9tIHplcm8gdG8gdGhlCiAgbWF4aW11bSBldmVuIHdoZW4gaXQgZGVjbGFyZXMg dGhlIENFQSByYW5nZSBpbiB0aGUgTWFpbiBTdHJlYW0KICBBdHRyaWJ1dGVzLiBJdCBpcyB0aGUg cmVzcG9uc2liaWxpdHkgb2YgdGhlIFNpbmsgZGV2aWNlIHRvIGxpbWl0IHRoZQogIHBpeGVsIHZh bHVlIHJhbmdlIGFzIG5lZWRlZC4KClRoZSBzcGVjIGhhcyBhIGNvbmZsaWN0IGFib3V0IFNIT1VM RCBhbmQgTVVTVC4gTmV2ZXJ0aGVsZXNzLCB1c2luZyBDRUEKcmFuZ2UgaXMgYXQgbGVhc3QgcmVj b21tZW5kZWQuIFdoZW4gdXNpbmcgQ0VBIHJhbmdlLCB0aGUgQ0VBIHJhbmdlIE1VU1QKYmUgZGVj bGFyZWQgaW4gTWFpbiBTdHJlYW0gQXR0cmlidXRlcyBmb3IgQ0VBIG1vZGVzLCBidXQgZnVsbCBy YW5nZSBvZgp2YWx1ZXMgTUFZIGJlIHRyYW5zbWl0dGVkLgoKRFAgMS40IDUuMS4xLjEgVmlkZW8g Q29sb3JpbWV0cnkKCiogV2hlbiBhIFNvdXJjZSBkZXZpY2UgaXMgdHJhbnNtaXR0aW5nIGFuIFJH QiBzdHJlYW0gd2l0aCBhIHZpZGVvIHRpbWluZwogIGZvcm1hdCBjYWxsZWQgb3V0IGluIENFQS04 NjEtRiwgU2VjdGlvbiA1IChleGNlcHQgNjQweDQ4MHApIGFzIHVzaW5nCiAgQ0VBIHJhbmdlIFJH QiwgaXQgc2hvdWxkIHVzZSBDRUEgcmFuZ2UgUkdCLgoKKiBXaGVuIGEgU291cmNlIGRldmljZSBp cyB0cmFuc21pdHRpbmcgYW4gUkdCIHN0cmVhbSB3aXRoIGEgdmlkZW8gdGltaW5nCiAgZm9ybWF0 IGNhbGxlZCBvdXQgaW4gQ0VBLTg2MS1GLCB0aGUgZGV2aWNlIG1heSBwcm92aWRlIFJHQiBpbiB0 aGUgVkVTQQogIHJhbmdlLgoKKiBUaGUgU2luayBkZXZpY2Ugc2hhbGwgaGF2ZSB0aGUgcmVzcG9u c2liaWxpdHkgb2YgbGltaXRpbmcgdGhlIHBpeGVsCiAgdmFsdWUgcmFuZ2UsIGFzIG5lZWRlZC4K ClRoZSBjb25mbGljdCBoYXMgYmVlbiByZXNvbHZlZCwgdXNpbmcgQ0VBIHJhbmdlIGlzIG5vdwpy ZWNvbW1lbmRlZC4gSG93ZXZlciwgdGhlIHNvdXJjZSBNQVkgYWxzbyB1c2UgVkVTQSByYW5nZS4g SSB0YWtlIGl0IHRoYXQKbWVhbnMgdGhlIHNvdXJjZSBjb3VsZCBhbHNvIGRlY2xhcmUgVkVTQSBy YW5nZSBpbiBNYWluIFN0cmVhbQpBdHRyaWJ1dGVzLgoKLS0tCgpJIGFtIG5vdCBzdXJlIGlmIHdl J2QgYmUgYWJsZSB0byBkZWNsYXJlIENFQSByYW5nZSBpbiBNYWluIFN0cmVhbQpBdHRyaWJ1dGVz IGJ1dCB0cmFuc21pdCBWRVNBIHJhbmdlIGFueXdheS4gRmVlbHMga2luZCBvZiBzaWxseSBhbnl3 YXksCmFuZCBJSVVDIHdvdWxkIG9ubHkgYmUgYWNjb3JkaW5nIHRvIERQIDEuMmEuIEl0IGJvaWxz IGRvd24gdG8gQ0VBIHJhbmdlCnZzLiBWRVNBIHJhbmdlLgoKPiBJJ20gcmVhbGx5IHN1cnByaXNl ZCB0aGF0IHRoaXMgYnVnIHdvdWxkIGhhdmUgYmVlbiBhcm91bmQgYXMgbG9uZyBhcyBpdCBsb29r cwo+IGxpa2UgaXQgaGFzIGJlZW4gd2l0aG91dCBhbnlvbmUgbm90aWNpbmcgaXQsIHNvIEkgZmln dXJlZCBJJ2QganVzdCBzZW5kIGEgcGF0Y2gKPiB0byB0aGUgbWFpbGluZyBsaXN0IHNvIHlvdSBn dXlzIGNhbiBwb2ludCBvdXQgd2hldGhlciBvciBub3QgdGhpcyBpcyByZWFsbHkgdGhlCj4gY29y cmVjdCB0aGluZyB0byBkby4KCldlJ3ZlIGhhZCB0aGUgc2FtZSBkaXNjdXNzaW9uIHNldmVyYWwg dGltZXMgb3ZlciB3aXRoIEhETUkgZm9yIHN1cmUsIGJ1dApJIHRoaW5rIGFsc28gd2l0aCBEUC4g VGhlIGNvbmNsdXNpb24gaGFzIGFsd2F5cyBiZWVuIHRoYXQgbm8gbWF0dGVyCndoaWNoIHJhbmdl IHdlIGNob29zZSBhcyB0aGUgZGVmYXVsdCwgaXQncyBnb2luZyB0byBiZSB3cm9uZyBmb3Igc29t ZQpkaXNwbGF5cy4gQW5kIHdoZW4gaXQgdHVybnMgaW50byBhIGNvbnRlc3Qgb2Ygd2hvIGNvbXBs YWlucyB0aGUgbG91ZGVzdCwKaXQncyBhIG5vIGJyYWluZXIgdG8gZ28gYnkgc3BlYy4gSXQncyBu ZXZlciBiZWVuIGxlc3MgdGhhbiByZWNvbW1lbmRlZAp0byBnbyB3aXRoIENFQSByYW5nZSBmb3Ig Q0VBIG1vZGVzLCByZWdhcmRsZXNzIG9mIHRoZSBjb25mbGljdCBpbiBEUAoxLjJhLgoKTm8gbWF0 dGVyIHdoYXQgd2UgZG8gaGVyZSwgdGhlIHF1ZXN0aW9uIHJlbWFpbnMgd2hhdCB0byBkbyB3aXRo CkNoYW1lbGl1bS4gQ2hhbmdpbmcgdGhlIGNvbG9yIHJhbmdlIGlzIHJlYWxseSBhIHdvcmthcm91 bmQgZm9yCkNoYW1lbGl1bSwgbm90IGEgZml4LiBVc2luZyBDRUEgcmFuZ2UgaXMgcGVyZmVjdGx5 IGZpbmUgcGVyIERQIHNwZWMuCgoKQlIsCkphbmkuCgo+Cj4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1 L2ludGVsX2RwLmMgfCA3ICsrKysrLS0KPiAgMSBmaWxlIGNoYW5nZWQsIDUgaW5zZXJ0aW9ucygr KSwgMiBkZWxldGlvbnMoLSkKPgo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9p bnRlbF9kcC5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAuYwo+IGluZGV4IGQ5YmMx OWIuLjY2NDJhYmQgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAu Ywo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwLmMKPiBAQCAtMTY0OSwxMiAr MTY0OSwxNSBAQCBpbnRlbF9kcF9jb21wdXRlX2NvbmZpZyhzdHJ1Y3QgaW50ZWxfZW5jb2RlciAq ZW5jb2RlciwKPiAgZm91bmQ6Cj4gIAlpZiAoaW50ZWxfZHAtPmNvbG9yX3JhbmdlX2F1dG8pIHsK PiAgCQkvKgo+ICsJCSAqIFdlIGFyZSByZXF1aXJlZCB0byB1c2UgdGhlIGxpbWl0ZWQgQ0VBIFJH QiByYW5nZSB3aGVuIGEgQ0VBCj4gKwkJICogbW9kZSBpcyBkZWNsYXJlZCBpbiB0aGUgRURJRC4g SG93ZXZlciwgbGltaXRpbmcgdGhlIHBpeGVsCj4gKwkJICogdmFsdWUgcmFuZ2UgaXMgdXAgdG8g dGhlIHNpbmssIG5vdCB0aGUgc291cmNlLiBTbywganVzdAo+ICsJCSAqIGRvbid0IGVuYWJsZSBs aW1pdGVkIGNvbG9yIHJhbmdlcy4KPiAgCQkgKiBTZWU6Cj4gIAkJICogQ0VBLTg2MS1FIC0gNS4x IERlZmF1bHQgRW5jb2RpbmcgUGFyYW1ldGVycwo+ICAJCSAqIFZFU0EgRGlzcGxheVBvcnQgVmVy LjEuMmEgLSA1LjEuMS4xIFZpZGVvIENvbG9yaW1ldHJ5Cj4gIAkJICovCj4gLQkJcGlwZV9jb25m aWctPmxpbWl0ZWRfY29sb3JfcmFuZ2UgPQo+IC0JCQlicHAgIT0gMTggJiYgZHJtX21hdGNoX2Nl YV9tb2RlKGFkanVzdGVkX21vZGUpID4gMTsKPiArCQlwaXBlX2NvbmZpZy0+bGltaXRlZF9jb2xv cl9yYW5nZSA9IGZhbHNlOwo+ICAJfSBlbHNlIHsKPiAgCQlwaXBlX2NvbmZpZy0+bGltaXRlZF9j b2xvcl9yYW5nZSA9Cj4gIAkJCWludGVsX2RwLT5saW1pdGVkX2NvbG9yX3JhbmdlOwoKLS0gCkph bmkgTmlrdWxhLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9sb2d5IENlbnRlcgpfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBs aXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVz a3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760932AbdAEImI (ORCPT ); Thu, 5 Jan 2017 03:42:08 -0500 Received: from mga09.intel.com ([134.134.136.24]:7465 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753244AbdAEIle (ORCPT ); Thu, 5 Jan 2017 03:41:34 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,320,1477983600"; d="scan'208";a="805358604" From: Jani Nikula To: Lyude , intel-gfx@lists.freedesktop.org Cc: Lyude , Chris Wilson , Daniel Vetter , Ville =?utf-8?B?U3lyasOkbMOk?= , Daniel Vetter , David Airlie , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/i915/dp: Stop enabling limited color ranges for everything In-Reply-To: <20170105011137.20209-1-lyude@redhat.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20170105011137.20209-1-lyude@redhat.com> Date: Thu, 05 Jan 2017 10:41:07 +0200 Message-ID: <87bmvlhpb0.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 05 Jan 2017, Lyude wrote: > 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. DP 1.2a 5.1.1.1 Video Colorimetry * When a Source device is transmitting an RGB stream with a video timing format called out in CEA-861C Section 5 (except 640 x 480p) as using CEA range RGB, it should use CEA range RGB. * When a Source device is transmitting a RGB stream with a video timing format called out in CEA-861C Section 5 as using CEA range RGB, it must use the CEA range RGB. * However, a Source device may transmit all code values from zero to the maximum even when it declares the CEA range in the Main Stream Attributes. It is the responsibility of the Sink device to limit the pixel value range as needed. The spec has a conflict about SHOULD and MUST. Nevertheless, using CEA range is at least recommended. When using CEA range, the CEA range MUST be declared in Main Stream Attributes for CEA modes, but full range of values MAY be transmitted. DP 1.4 5.1.1.1 Video Colorimetry * When a Source device is transmitting an RGB stream with a video timing format called out in CEA-861-F, Section 5 (except 640x480p) as using CEA range RGB, it should use CEA range RGB. * When a Source device is transmitting an RGB stream with a video timing format called out in CEA-861-F, the device may provide RGB in the VESA range. * The Sink device shall have the responsibility of limiting the pixel value range, as needed. The conflict has been resolved, using CEA range is now recommended. However, the source MAY also use VESA range. I take it that means the source could also declare VESA range in Main Stream Attributes. --- I am not sure if we'd be able to declare CEA range in Main Stream Attributes but transmit VESA range anyway. Feels kind of silly anyway, and IIUC would only be according to DP 1.2a. It boils down to CEA range vs. VESA range. > 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. We've had the same discussion several times over with HDMI for sure, but I think also with DP. The conclusion has always been that no matter which range we choose as the default, it's going to be wrong for some displays. And when it turns into a contest of who complains the loudest, it's a no brainer to go by spec. It's never been less than recommended to go with CEA range for CEA modes, regardless of the conflict in DP 1.2a. No matter what we do here, the question remains what to do with Chamelium. Changing the color range is really a workaround for Chamelium, not a fix. Using CEA range is perfectly fine per DP spec. BR, Jani. > > 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; -- Jani Nikula, Intel Open Source Technology Center