From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: FW: [PATCH] drm/i915: Reject HDMI 12bpc if the sink doesn't indicate support Date: Mon, 13 Mar 2017 12:53:18 +0200 Message-ID: <20170313105318.GG31595@intel.com> References: <20170111113810.GJ31595@intel.com> <20170213175818.24958-1-ville.syrjala@linux.intel.com> <7756d09d-a33f-c7bf-cc73-12e6f772ed97@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id B7ABD6E3FB for ; Mon, 13 Mar 2017 10:53:21 +0000 (UTC) Content-Disposition: inline In-Reply-To: <7756d09d-a33f-c7bf-cc73-12e6f772ed97@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Sharma, Shashank" Cc: Intel Graphics Development , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gTW9uLCBNYXIgMTMsIDIwMTcgYXQgMTI6MjI6NTNQTSArMDIwMCwgU2hhcm1hLCBTaGFzaGFu ayB3cm90ZToKPiBSZWdhcmRzCj4gCj4gU2hhc2hhbmsKPiAKPiA+IEZyb206IFZpbGxlIFN5cmrD pGzDpCA8dmlsbGUuc3lyamFsYUBsaW51eC5pbnRlbC5jb20+Cj4gPgo+ID4gQ2hlY2sgdGhhdCB0 aGUgc2luayByZWFsbHkgZGVjbGFyZWQgMTJicGMgc3VwcG9ydCBiZWZvcmUgd2UgZW5hYmxlIGl0 Lgo+ID4gVGhpcyBzaG91bGQgbm90IGFjdHVhbGx5IG5ldmVyIGhhcHBlbiBzaW5jZSBpdCdzIG1h bmRhdG9yeSBmb3IgSERNSSBzaW5rcyB0byBzdXBwb3J0IDEyYnBjIGlmIHRoZXkgc3VwcG9ydCBh bnkgZGVlcCBjb2xvciBtb2Rlcy4gQnV0IHJlYWxpdHkgZGlzYWdyZWVzIHdpdGggdGhlIHRoZW9y eSBhbmQgdGhlcmUgYXJlIGFjdHVhbGx5IHNpbmtzIGluIHRoZSB3aWxkIHRoYXQgdmlvbGF0ZSB0 aGUgc3BlYy4KPiA+Cj4gPiB2MjogRml4IHRoZSBvdXRwdXRfdHlwZXMgY2hlY2sKPiA+ICAgICAg VXBkYXRlIGNvbW1pdCBtZXNzYWdlIHRvIHN0YXRlIHRoYXQgdGhlc2UgdGhpbmdzIGFyZSBpbiBm YWN0IHJlYWwKPiA+Cj4gPiBDYzogc3RhYmxlQHZnZXIua2VybmVsLm9yZwo+ID4gQ2M6IE5pY2hv bGFzIFNpZWxpY2tpIDxuaWNob2xhcy5zaWVsaWNraUBnbWFpbC5jb20+Cj4gPiBCdWd6aWxsYTog aHR0cHM6Ly9idWdzLmZyZWVkZXNrdG9wLm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTkyNTAKPiA+IFNp Z25lZC1vZmYtYnk6IFZpbGxlIFN5cmrDpGzDpCA8dmlsbGUuc3lyamFsYUBsaW51eC5pbnRlbC5j b20+Cj4gPiAtLS0KPiA+ICAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfaGRtaS5jIHwgMjQg KysrKysrKysrKysrKysrKysrKysrLS0tCj4gPiAgIDEgZmlsZSBjaGFuZ2VkLCAyMSBpbnNlcnRp b25zKCspLCAzIGRlbGV0aW9ucygtKQo+ID4KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9k cm0vaTkxNS9pbnRlbF9oZG1pLmMgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9oZG1pLmMK PiA+IGluZGV4IGE1ODBkZTgwZDJiNS4uMmJmNTkxNTI4NGFhIDEwMDY0NAo+ID4gLS0tIGEvZHJp dmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfaGRtaS5jCj4gPiArKysgYi9kcml2ZXJzL2dwdS9kcm0v aTkxNS9pbnRlbF9oZG1pLmMKPiA+IEBAIC0xMjk4LDE2ICsxMjk4LDM0IEBAIGludGVsX2hkbWlf bW9kZV92YWxpZChzdHJ1Y3QgZHJtX2Nvbm5lY3RvciAqY29ubmVjdG9yLAo+ID4gICAKPiA+ICAg c3RhdGljIGJvb2wgaGRtaV8xMmJwY19wb3NzaWJsZShzdHJ1Y3QgaW50ZWxfY3J0Y19zdGF0ZSAq Y3J0Y19zdGF0ZSkgIHsKPiA+IC0Jc3RydWN0IGRybV9kZXZpY2UgKmRldiA9IGNydGNfc3RhdGUt PmJhc2UuY3J0Yy0+ZGV2Owo+ID4gKwlzdHJ1Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3ByaXYg PQo+ID4gKwkJdG9faTkxNShjcnRjX3N0YXRlLT5iYXNlLmNydGMtPmRldik7Cj4gPiArCXN0cnVj dCBkcm1fYXRvbWljX3N0YXRlICpzdGF0ZSA9IGNydGNfc3RhdGUtPmJhc2Uuc3RhdGU7Cj4gPiAr CXN0cnVjdCBkcm1fY29ubmVjdG9yX3N0YXRlICpjb25uZWN0b3Jfc3RhdGU7Cj4gPiArCXN0cnVj dCBkcm1fY29ubmVjdG9yICpjb25uZWN0b3I7Cj4gPiArCWludCBpOwo+ID4gICAKPiA+IC0JaWYg KEhBU19HTUNIX0RJU1BMQVkodG9faTkxNShkZXYpKSkKPiA+ICsJaWYgKEhBU19HTUNIX0RJU1BM QVkoZGV2X3ByaXYpKQo+ID4gICAJCXJldHVybiBmYWxzZTsKPiA+ICAgCj4gPiAgIAkvKgo+ID4g ICAJICogSERNSSAxMmJwYyBhZmZlY3RzIHRoZSBjbG9ja3MsIHNvIGl0J3Mgb25seSBwb3NzaWJs ZQo+ID4gICAJICogd2hlbiBub3QgY2xvbmluZyB3aXRoIG90aGVyIGVuY29kZXIgdHlwZXMuCj4g PiAgIAkgKi8KPiA+IC0JcmV0dXJuIGNydGNfc3RhdGUtPm91dHB1dF90eXBlcyA9PSAxIDw8IElO VEVMX09VVFBVVF9IRE1JOwo+ID4gKwlpZiAoY3J0Y19zdGF0ZS0+b3V0cHV0X3R5cGVzICE9IDEg PDwgSU5URUxfT1VUUFVUX0hETUkpCj4gPiArCQlyZXR1cm4gZmFsc2U7Cj4gPiArCj4gVGhpcyBm dW5jdGlvbiBpcyBjYWxsZWQgZnJvbSBvbmx5IG9uZSBwbGFjZSAoIGZvciBub3cpLCBhbmQgdGhh dCBhbHJlYWR5IAo+IGhhcyBhIHBpcGVfY29uZmlnLT5oYXNfaGRtaV9zaW5rIGNoZWNrLgo+IERv ZXMgaXQgbWFrZXMgc2Vuc2UgdG8gaGF2ZSBvbmx5IG9uZSBvZiB0aGUgY2hlY2tzID8gSSBjYW4g dW5kZXJzdGFuZCAKPiB0aGF0IHRoaXMgbWlnaHQgYmUgdG8gY29tcGxlCgpoYXNfaGRtaV9zaW5r IGlzIG5vdCB0aGUgc2FtZSB0aGluZy4gSXQganVzdCBzYXlzIHdlJ3JlIHRhbGtpbmcgdG8gYXQK bGVhc3Qgb25lIEhETUkgc2luayBhbmQgdGh1cyBjYW4gc2VuZCBpbmZvZnJhbWVzL2F1ZGlvLiBv dXRwdXRfdHlwZXMgb24gCnRoZSBvdGhlciBoYW5kIGxpc3RzIGFsbCB0aGUgZGlmZmVyZW50IHBv cnQgdHlwZXMgd2UncmUgY2xvbmluZyB3aXRoLgpTbyB5b3UgY2FuIGRvIEhETUkrVkdBIGZvciBp bnN0YW5jZSBhbmQgc3RpbGwgdGhlIEhETUkgZ29lcyB0byBhIHJlYWwKSERNSSBzaW5rIHNvIHdl J2xsIGhhdmUgaGFzX2hkbWlfc2luaz09dHJ1ZS4gQnV0IHNpbmNlIFZHQSBjYW4ndCBkZWFsCndp dGggdGhlIDEuNXggY2xvY2sgd2UgY2FuJ3QgZG8gZGVlcCBjb2xvciB3aGVuIGNsb25pbmcuCgo+ ID4gKwlmb3JfZWFjaF9jb25uZWN0b3JfaW5fc3RhdGUoc3RhdGUsIGNvbm5lY3RvciwgY29ubmVj dG9yX3N0YXRlLCBpKSB7Cj4gPiArCQljb25zdCBzdHJ1Y3QgZHJtX2Rpc3BsYXlfaW5mbyAqaW5m byA9ICZjb25uZWN0b3ItPmRpc3BsYXlfaW5mbzsKPiA+ICsKPiA+ICsJCWlmIChjb25uZWN0b3Jf c3RhdGUtPmNydGMgIT0gY3J0Y19zdGF0ZS0+YmFzZS5jcnRjKQo+ID4gKwkJCWNvbnRpbnVlOwo+ ID4gKwo+ID4gKwkJaWYgKChpbmZvLT5lZGlkX2hkbWlfZGNfbW9kZXMgJiBEUk1fRURJRF9IRE1J X0RDXzM2KSA9PSAwKQo+ID4gKwkJCXJldHVybiBmYWxzZTsKPiBEbyB3ZSB3YW50IHRvIGhhdmUg YSBpbmZvLT5icGMgY2hlY2sgdG9vLCBvciB0aGV5IGFyZSBtb3JlIG9yIGxlc3Mgc2FtZSA/CgpU aGF0IHdhcyBhbHJlYWR5IGNoZWNrZWQgYXQgdGhlIHN0YXJ0IG9mIHRoZSBzdGF0ZSBjb21wdXRh dGlvbi4gU28Kc3RhdGUtPnBpcGVfYnBwIGFscmVhZHkgYWNjb3VudHMgZm9yIHRoYXQuIEhvd2V2 ZXIgYXMgaW5mby0+YnBjIGlzIGp1c3QKdGhlIG1heCBicGMgdGhlIHNpbmsgY2FuIGRvIGl0J3Mg bm90IHN1ZmZpY2llbnQgdG8gZ3VhcmFudGVlIGl0IGNhbiBkb28KYWxsIGxvd2VyIGJwYyB2YWx1 ZXMuCgo+IC0gU2hhc2hhbmsKPiA+ICsJfQo+ID4gKwo+ID4gKwlyZXR1cm4gdHJ1ZTsKPiA+ICAg fQo+ID4gICAKPiA+ICAgYm9vbCBpbnRlbF9oZG1pX2NvbXB1dGVfY29uZmlnKHN0cnVjdCBpbnRl bF9lbmNvZGVyICplbmNvZGVyLAo+ID4gLS0KPiA+IDIuMTAuMgo+ID4KPiA+IF9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCj4gPiBkcmktZGV2ZWwgbWFpbGlu ZyBsaXN0Cj4gPiBkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCj4gPiBodHRwczovL2xp c3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAoKLS0gClZpbGxl IFN5cmrDpGzDpApJbnRlbCBPVEMKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRl c2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8v aW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com ([192.55.52.93]:28856 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751568AbdCMKxW (ORCPT ); Mon, 13 Mar 2017 06:53:22 -0400 Date: Mon, 13 Mar 2017 12:53:18 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: "Sharma, Shashank" Cc: Nicholas Sielicki , stable@vger.kernel.org, Intel Graphics Development Subject: Re: FW: [PATCH] drm/i915: Reject HDMI 12bpc if the sink doesn't indicate support Message-ID: <20170313105318.GG31595@intel.com> References: <20170111113810.GJ31595@intel.com> <20170213175818.24958-1-ville.syrjala@linux.intel.com> <7756d09d-a33f-c7bf-cc73-12e6f772ed97@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <7756d09d-a33f-c7bf-cc73-12e6f772ed97@intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Mon, Mar 13, 2017 at 12:22:53PM +0200, Sharma, Shashank wrote: > Regards > > Shashank > > > From: Ville Syrj�l� > > > > Check that the sink really declared 12bpc support before we enable it. > > This should not actually never happen since it's mandatory for HDMI sinks to support 12bpc if they support any deep color modes. But reality disagrees with the theory and there are actually sinks in the wild that violate the spec. > > > > v2: Fix the output_types check > > Update commit message to state that these things are in fact real > > > > Cc: stable@vger.kernel.org > > Cc: Nicholas Sielicki > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99250 > > Signed-off-by: Ville Syrj�l� > > --- > > drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++++++++++--- > > 1 file changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > index a580de80d2b5..2bf5915284aa 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -1298,16 +1298,34 @@ intel_hdmi_mode_valid(struct drm_connector *connector, > > > > static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) { > > - struct drm_device *dev = crtc_state->base.crtc->dev; > > + struct drm_i915_private *dev_priv = > > + to_i915(crtc_state->base.crtc->dev); > > + struct drm_atomic_state *state = crtc_state->base.state; > > + struct drm_connector_state *connector_state; > > + struct drm_connector *connector; > > + int i; > > > > - if (HAS_GMCH_DISPLAY(to_i915(dev))) > > + if (HAS_GMCH_DISPLAY(dev_priv)) > > return false; > > > > /* > > * HDMI 12bpc affects the clocks, so it's only possible > > * when not cloning with other encoder types. > > */ > > - return crtc_state->output_types == 1 << INTEL_OUTPUT_HDMI; > > + if (crtc_state->output_types != 1 << INTEL_OUTPUT_HDMI) > > + return false; > > + > This function is called from only one place ( for now), and that already > has a pipe_config->has_hdmi_sink check. > Does it makes sense to have only one of the checks ? I can understand > that this might be to comple has_hdmi_sink is not the same thing. It just says we're talking to at least one HDMI sink and thus can send infoframes/audio. output_types on the other hand lists all the different port types we're cloning with. So you can do HDMI+VGA for instance and still the HDMI goes to a real HDMI sink so we'll have has_hdmi_sink==true. But since VGA can't deal with the 1.5x clock we can't do deep color when cloning. > > + for_each_connector_in_state(state, connector, connector_state, i) { > > + const struct drm_display_info *info = &connector->display_info; > > + > > + if (connector_state->crtc != crtc_state->base.crtc) > > + continue; > > + > > + if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0) > > + return false; > Do we want to have a info->bpc check too, or they are more or less same ? That was already checked at the start of the state computation. So state->pipe_bpp already accounts for that. However as info->bpc is just the max bpc the sink can do it's not sufficient to guarantee it can doo all lower bpc values. > - Shashank > > + } > > + > > + return true; > > } > > > > bool intel_hdmi_compute_config(struct intel_encoder *encoder, > > -- > > 2.10.2 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrj�l� Intel OTC