From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: hdmi: add CEC notifier to intel_hdmi Date: Wed, 16 May 2018 17:07:20 +0300 Message-ID: <20180516140720.GD23723@intel.com> References: <1526395342-15481-1-git-send-email-narmstrong@baylibre.com> <1526395342-15481-3-git-send-email-narmstrong@baylibre.com> <20180515153521.GB23723@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Neil Armstrong Cc: felixe@google.com, seanpaul@google.com, airlied@linux.ie, sadolfsson@google.com, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, fparent@baylibre.com, hans.verkuil@cisco.com, bleung@google.com, darekm@google.com, lee.jones@linaro.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org T24gV2VkLCBNYXkgMTYsIDIwMTggYXQgMDk6NDA6MTdBTSArMDIwMCwgTmVpbCBBcm1zdHJvbmcg d3JvdGU6Cj4gT24gMTYvMDUvMjAxOCAwOTozMSwgTmVpbCBBcm1zdHJvbmcgd3JvdGU6Cj4gPiBI aSBWaWxsZSwKPiA+IAo+ID4gT24gMTUvMDUvMjAxOCAxNzozNSwgVmlsbGUgU3lyasOkbMOkIHdy b3RlOgo+ID4+IE9uIFR1ZSwgTWF5IDE1LCAyMDE4IGF0IDA0OjQyOjE5UE0gKzAyMDAsIE5laWwg QXJtc3Ryb25nIHdyb3RlOgo+ID4+PiBUaGlzIHBhdGNocyBhZGRzIHRoZSBjZWNfbm90aWZpZXIg ZmVhdHVyZSB0byB0aGUgaW50ZWxfaGRtaSBwYXJ0Cj4gPj4+IG9mIHRoZSBpOTE1IERSTSBkcml2 ZXIuIEl0IHVzZXMgdGhlIEhETUkgRFJNIGNvbm5lY3RvciBuYW1lIHRvIGRpZmZlcmVudGlhdGUK PiA+Pj4gYmV0d2VlbiBlYWNoIEhETUkgcG9ydHMuCj4gPj4+IFRoZSBjaGFuZ2VzIHdpbGwgYWxs b3cgdGhlIGk5MTUgSERNSSBjb2RlIHRvIG5vdGlmeSBFRElEIGFuZCBIUEQgY2hhbmdlcwo+ID4+ PiB0byBhbiBldmVudHVhbCBDRUMgYWRhcHRlci4KPiA+Pj4KPiA+Pj4gU2lnbmVkLW9mZi1ieTog TmVpbCBBcm1zdHJvbmcgPG5hcm1zdHJvbmdAYmF5bGlicmUuY29tPgo+ID4+PiAtLS0KPiA+Pj4g IGRyaXZlcnMvZ3B1L2RybS9pOTE1L0tjb25maWcgICAgICB8ICAxICsKPiA+Pj4gIGRyaXZlcnMv Z3B1L2RybS9pOTE1L2ludGVsX2Rydi5oICB8ICAyICsrCj4gPj4+ICBkcml2ZXJzL2dwdS9kcm0v aTkxNS9pbnRlbF9oZG1pLmMgfCAxMiArKysrKysrKysrKysKPiA+Pj4gIDMgZmlsZXMgY2hhbmdl ZCwgMTUgaW5zZXJ0aW9ucygrKQo+ID4+Pgo+ID4gCj4gPiBbLi5dCj4gPiAKPiA+Pj4gIH0KPiA+ Pj4gIAo+ID4+PiBAQCAtMjAzMSw2ICsyMDM3LDggQEAgc3RhdGljIHZvaWQgY2h2X2hkbWlfcHJl X2VuYWJsZShzdHJ1Y3QgaW50ZWxfZW5jb2RlciAqZW5jb2RlciwKPiA+Pj4gIAo+ID4+PiAgc3Rh dGljIHZvaWQgaW50ZWxfaGRtaV9kZXN0cm95KHN0cnVjdCBkcm1fY29ubmVjdG9yICpjb25uZWN0 b3IpCj4gPj4+ICB7Cj4gPj4+ICsJaWYgKGludGVsX2F0dGFjaGVkX2hkbWkoY29ubmVjdG9yKS0+ bm90aWZpZXIpCj4gPj4+ICsJCWNlY19ub3RpZmllcl9wdXQoaW50ZWxfYXR0YWNoZWRfaGRtaShj b25uZWN0b3IpLT5ub3RpZmllcik7Cj4gPj4+ICAJa2ZyZWUodG9faW50ZWxfY29ubmVjdG9yKGNv bm5lY3RvciktPmRldGVjdF9lZGlkKTsKPiA+Pj4gIAlkcm1fY29ubmVjdG9yX2NsZWFudXAoY29u bmVjdG9yKTsKPiA+Pj4gIAlrZnJlZShjb25uZWN0b3IpOwo+ID4+PiBAQCAtMjM1OCw2ICsyMzY2 LDEwIEBAIHZvaWQgaW50ZWxfaGRtaV9pbml0X2Nvbm5lY3RvcihzdHJ1Y3QgaW50ZWxfZGlnaXRh bF9wb3J0ICppbnRlbF9kaWdfcG9ydCwKPiA+Pj4gIAkJdTMyIHRlbXAgPSBJOTE1X1JFQUQoUEVH X0JBTkRfR0FQX0RBVEEpOwo+ID4+PiAgCQlJOTE1X1dSSVRFKFBFR19CQU5EX0dBUF9EQVRBLCAo dGVtcCAmIH4weGYpIHwgMHhkKTsKPiA+Pj4gIAl9Cj4gPj4+ICsKPiA+Pj4gKwlpbnRlbF9oZG1p LT5ub3RpZmllciA9IGNlY19ub3RpZmllcl9nZXRfY29ubihkZXYtPmRldiwgY29ubmVjdG9yLT5u YW1lKTsKPiA+Pgo+ID4+IFdoYXQgYXJlIHdlIGRvaW5nIHdpdGggdGhlIGNvbm5lY3RvciBuYW1l IGhlcmU/IFRob3NlIGFyZSBub3QgYWN0dWFsbHkKPiA+PiBndWFyYW50ZWVkIHRvIGJlIHN0YWJs ZSwgYW5kIGFueSBjaGFuZ2UgaW4gdGhlIGNvbm5lY3RvciBwcm9iZSBvcmRlcgo+ID4+IG1heSBj YXVzZSB0aGUgbmFtZSB0byBjaGFuZ2UuCj4gPiAKPiA+IFRoZSBpOTE1IGRyaXZlciBjYW4gZXhw b3NlIG11bHRpcGxlIEhETUkgY29ubmVjdG9ycywgYnV0IGVhY2ggY29ubmVjdGVkIHdpbGwKPiA+ IGhhdmUgYSBkaWZmZXJlbnQgRURJRCBhbmQgQ0VDIHBoeXNpY2FsIGFkZHJlc3MsIHNvIHdlIHdp bGwgbmVlZCBhIGRpZmZlcmVudCBub3RpZmllcgo+ID4gZm9yIGVhY2ggY29ubmVjdG9yLgo+ID4g Cj4gPiBUaGUgY29ubmVjdG9yIG5hbWUgaXMgRFJNIGRlcGVuZGVudCwgYnV0IHVzZXItc3BhY2Ug YWN0dWFsbHkgdXNlcyB0aGlzIG5hbWUgZm9yCj4gPiBvcGVyYXRpb25zLCBzbyBJIHN1cHBvc2Vk IGl0IHdhcyBraW5kIG9mIHN0YWJsZS4KPiA+IAo+ID4gQW55d2F5LCBpcyB0aGVyZSBhbm90aGVy IHN0YWJsZSBpZC9uYW1lL3doYXRldmVyIHRoYXQgY2FuIGJlIHVzZWQgdG8gbWFrZSBhIG5hbWUg dG8KPiA+IGRpc3Rpbmd1aXNoIHRoZSBIRE1JIGNvbm5lY3RvcnMgPwo+IAo+IFdvdWxkICJIRE1J ICVjIiwgcG9ydF9uYW1lKHBvcnQpIGJlIE9LIHRvIHVzZSA/CgpZZWFoLCBzb21ldGhpbmcgbGlr ZSBzZWVtcyBsaWtlIGEgcmVhc29uYWJsZSBhcHByb2FjaC4gVGhhdCBkb2VzIG1lYW4Kd2UgaGF2 ZSB0byBiZSBhIGxpdHRsZSBjYXJlZnVsIHdpdGggY2hhbmdpbmcgZW51bSBwb3J0IG9yIHBvcnRf bmFtZSgpCmluIHRoZSBmdXR1cmUsIGJ1dCBJIGd1ZXNzIHdlIGNvdWxkIGp1c3QgYWRkIGEgc21h bGwgZnVuY3Rpb24gdG8KZ2VuZXJhdGVkIHRoZSBuYW1lL2lkIHNwZWNpZmljYWxseSBmb3IgdGhp cyB0aGluZy4KCldlJ3JlIGFsc28gZ29pbmcgdG8gaGF2ZSB0byB0aGluayB3aGF0IHRvIGRvIHdp dGggZW51bSBwb3J0IGFuZApwb3J0X25hbWUoKSBvbiBJQ0wrIHRob3VnaC4gVGhlcmUgd2Ugd29u J3QganVzdCBoYXZlIEEtRiBidXQgYWxzbwpUQzEtVEM8bj4uIEhtbS4gTG9va3MgbGlrZSB3aGF0 IHdlIGhhdmUgZm9yIHRob3NlIHBvcnRzIGluIG91cgpjb2RlYmFzZSBBVE0gaXMgYSBiaXQgd29u a3kgc2luY2Ugd2UgaGF2ZW4ndCBldmVuIGNoYW5nZWQKcG9ydF9uYW1lKCkgdG8gZ2l2ZSB1cyB0 aGUgVEM8bj4gdHlwZSBuYW1lcy4KCkFsc28gd2UgbWlnaHQgbm90IHdhbnQgIkhETUkiIGluIHRo ZSBpZGVudGlmaWVyIHNpbmNlIHRoZSBwaHlzaWNhbApwb3J0IGlzIG5vdCBIRE1JIHNwZWNpZmlj LiBUaGVyZSBhcmUgZGlmZmVyZW50IHRoaW5ncyB3ZSBjb3VsZCBjYWxsCnRoZXNlIGJ1dCBJIHRo aW5rIHdlIGNvdWxkIGp1c3QgZ28gd2l0aCBhIGdlbmVyaWMgIlBvcnQgQS1GIiBhbmQKIlBvcnQg VEMxLVRDPG4+IiBhcHByb2FjaC4gSSB0aGluayBzb21ldGhpbmcgbGlrZSB0aGF0IHNob3VsZCB3 b3JrCmZpbmUgZm9yIGN1cnJlbnQgYW5kIHVwY29taW5nIGhhcmR3YXJlLiBBbmQgaW4gdGhlb3J5 IHRoYXQgY291bGQKdGhlbiBiZSB1c2VkIGZvciBvdGhlciB0aGluZ3MgYXMgd2VsbCB3aGljaCBu ZWVkIGEgc3RhYmxlCmlkZW50aWZpZXIuCgpPaCwgYW5kIG5vdyBJIHJlY2FsbCB0aGF0IGlucHV0 IHN1YnN5c3RlbSBhdCBsZWFzdCBoYXMgc29tZSBraW5kCm9mICJwaHlzaWNhbCBwYXRoIiBwcm9w ZXJ0eSBvbiBkZXZpY2VzLiBTbyBJIGd1ZXNzIHdoYXQgd2UncmUKZGljdXNzaW5nIGlzIGEgc29t ZXdoYXQgc2ltaWxhciBpZGVhLiBJIHRoaW5rIGlucHV0IGRyaXZlcnMKZ2VuZXJhbGx5IGluY2x1 ZGUgdGhlIHBjaS91c2IvZXRjLiBkZXZpY2UgaW4gdGhlIHBhdGggYXMgd2VsbC4KRXZlbiB0aG91 Z2ggd2UgY3VycmVudGx5IG9ubHkgZXZlciBoYXZlIHRoZSBvbmUgcGNpIGRldmljZSBpdAp3b3Vs ZCBzZWVtIGxpa2UgZGVjZW50IGlkZWEgdG8gaW5jbHVkZSB0aGF0IGluZm9ybWF0aW9uIGluIG91 cgppZGVudGlmaWVycyBhcyB3ZWxsLiBPciB3aGF0IGRvIHlvdSB0aGluaz8KCi0tIApWaWxsZSBT eXJqw6Rsw6QKSW50ZWwKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Au b3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRl dmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mga18.intel.com ([134.134.136.126]:12966 "EHLO mga18.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751510AbeEPOH0 (ORCPT ); Wed, 16 May 2018 10:07:26 -0400 Date: Wed, 16 May 2018 17:07:20 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Neil Armstrong Cc: airlied@linux.ie, hans.verkuil@cisco.com, lee.jones@linaro.org, olof@lixom.net, seanpaul@google.com, sadolfsson@google.com, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, fparent@baylibre.com, felixe@google.com, bleung@google.com, darekm@google.com, linux-media@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: hdmi: add CEC notifier to intel_hdmi Message-ID: <20180516140720.GD23723@intel.com> References: <1526395342-15481-1-git-send-email-narmstrong@baylibre.com> <1526395342-15481-3-git-send-email-narmstrong@baylibre.com> <20180515153521.GB23723@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: linux-media-owner@vger.kernel.org List-ID: On Wed, May 16, 2018 at 09:40:17AM +0200, Neil Armstrong wrote: > On 16/05/2018 09:31, Neil Armstrong wrote: > > Hi Ville, > > > > On 15/05/2018 17:35, Ville Syrjälä wrote: > >> On Tue, May 15, 2018 at 04:42:19PM +0200, Neil Armstrong wrote: > >>> This patchs adds the cec_notifier feature to the intel_hdmi part > >>> of the i915 DRM driver. It uses the HDMI DRM connector name to differentiate > >>> between each HDMI ports. > >>> The changes will allow the i915 HDMI code to notify EDID and HPD changes > >>> to an eventual CEC adapter. > >>> > >>> Signed-off-by: Neil Armstrong > >>> --- > >>> drivers/gpu/drm/i915/Kconfig | 1 + > >>> drivers/gpu/drm/i915/intel_drv.h | 2 ++ > >>> drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++++++++ > >>> 3 files changed, 15 insertions(+) > >>> > > > > [..] > > > >>> } > >>> > >>> @@ -2031,6 +2037,8 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder, > >>> > >>> static void intel_hdmi_destroy(struct drm_connector *connector) > >>> { > >>> + if (intel_attached_hdmi(connector)->notifier) > >>> + cec_notifier_put(intel_attached_hdmi(connector)->notifier); > >>> kfree(to_intel_connector(connector)->detect_edid); > >>> drm_connector_cleanup(connector); > >>> kfree(connector); > >>> @@ -2358,6 +2366,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, > >>> u32 temp = I915_READ(PEG_BAND_GAP_DATA); > >>> I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd); > >>> } > >>> + > >>> + intel_hdmi->notifier = cec_notifier_get_conn(dev->dev, connector->name); > >> > >> What are we doing with the connector name here? Those are not actually > >> guaranteed to be stable, and any change in the connector probe order > >> may cause the name to change. > > > > The i915 driver can expose multiple HDMI connectors, but each connected will > > have a different EDID and CEC physical address, so we will need a different notifier > > for each connector. > > > > The connector name is DRM dependent, but user-space actually uses this name for > > operations, so I supposed it was kind of stable. > > > > Anyway, is there another stable id/name/whatever that can be used to make a name to > > distinguish the HDMI connectors ? > > Would "HDMI %c", port_name(port) be OK to use ? Yeah, something like seems like a reasonable approach. That does mean we have to be a little careful with changing enum port or port_name() in the future, but I guess we could just add a small function to generated the name/id specifically for this thing. We're also going to have to think what to do with enum port and port_name() on ICL+ though. There we won't just have A-F but also TC1-TC. Hmm. Looks like what we have for those ports in our codebase ATM is a bit wonky since we haven't even changed port_name() to give us the TC type names. Also we might not want "HDMI" in the identifier since the physical port is not HDMI specific. There are different things we could call these but I think we could just go with a generic "Port A-F" and "Port TC1-TC" approach. I think something like that should work fine for current and upcoming hardware. And in theory that could then be used for other things as well which need a stable identifier. Oh, and now I recall that input subsystem at least has some kind of "physical path" property on devices. So I guess what we're dicussing is a somewhat similar idea. I think input drivers generally include the pci/usb/etc. device in the path as well. Even though we currently only ever have the one pci device it would seem like decent idea to include that information in our identifiers as well. Or what do you think? -- Ville Syrjälä Intel