From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marius Vlad Subject: Re: [PATCH] drm/i915: Support HDMI EDID injection Date: Tue, 2 Feb 2016 18:00:03 +0200 Message-ID: <20160202160002.GA10873@mcvlad-wk.rb.intel.com> References: <1454418061-10178-1-git-send-email-marius.c.vlad@intel.com> <87h9hr2kmy.fsf@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0815485223==" Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTP id CDA907A09D for ; Wed, 3 Feb 2016 08:24:09 -0800 (PST) In-Reply-To: <87h9hr2kmy.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0815485223== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="zhXaljGHf11kAtnf" Content-Disposition: inline --zhXaljGHf11kAtnf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Feb 02, 2016 at 05:32:21PM +0200, Jani Nikula wrote: > On Tue, 02 Feb 2016, Marius Vlad wrote: > > Use the drm_property_blob data for user-supplied EDID blobs. > > > > Signed-off-by: Marius Vlad > > --- > > drivers/gpu/drm/i915/intel_hdmi.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/i= ntel_hdmi.c > > index 8698a64..a10f3d9 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -1336,7 +1336,8 @@ intel_hdmi_unset_edid(struct drm_connector *conne= ctor) > > intel_hdmi->has_audio =3D false; > > intel_hdmi->rgb_quant_range_selectable =3D false; > > =20 > > - kfree(to_intel_connector(connector)->detect_edid); > > + if (!connector->override_edid) > > + kfree(to_intel_connector(connector)->detect_edid); > > to_intel_connector(connector)->detect_edid =3D NULL; > > } > > =20 > > @@ -1355,6 +1356,9 @@ intel_hdmi_set_edid(struct drm_connector *connect= or, bool force) > > intel_gmbus_get_adapter(dev_priv, > > intel_hdmi->ddc_bus)); > > =20 > > + if (!edid && connector->override_edid) > > + edid =3D (struct edid *) connector->edid_blob_ptr->data; > > + >=20 > If the user goes on to update the edid by hand, ->detect_edid will end > up pointing at released memory. You should probably kmemdup the edid > (like some other drivers do, git grep for edid_blob_ptr), even though > that will lead to using a stale edid until intel_hdmi_set_edid is called > again. My initial approach was doing just that. Then I noticed I might get away without doing any kind of allocations. I've some tests that work both ways. In intel_hdmi_unset_edid(), kfree(detect_edid) is only called when connector->override_edid is not set, in this way it won't deallocate random data. >=20 > The other question is, why do you base the decision to use override edid > on whether we can get the actual edid or not? In case edid is not set (failed to communicate with the display) and the override_edid is set (which is set by edid_write() in drm_debugfs() meaing a user supplied a EDID blob) I can assume that the user did injected a HDMI EDID, and use data from drm_property_blob. Seems to be the only place before looking for CEA extensions in HDMI supplied EDID. >=20 > /me thinks this is all really messy at the drm level, including the > handling of edid firmware. >=20 > BR, > Jani. >=20 >=20 >=20 > > intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); > > } >=20 > --=20 > Jani Nikula, Intel Open Source Technology Center --zhXaljGHf11kAtnf Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJWsNKCAAoJELmLWIAQzyE+nBsH/ittStwjrT7guj0EY80MkU8r Ccdgysh9ziv3jMJMsf+/rsBtYWScFVLDBe6VkQZgTo0C9Gm3SeKkRsbuKI+249M+ Aeagdf2UWLNyJPAoLlZl0GP08gh9DN5RvOVj3UBDu2ytKnY4mFXChdX/370ssQ3K mxkLfTTCqbLPY/Kzbl/zTmiPjU2sJd33CEQvrkiXxXcu3HurbsgFtt3vm2fDZfe5 Xj0XjRmU0Aoesb/cl4kcMf1gZG2cm4OzM/uJaGHy27JS9mYNCMeLVGY1Hl7ADfgj WL+Gcf0z9fCWTEuAaQIAaIlD+voYsSJF2LcP4IdMxixwMVHc3sSIQtEGRC79HKs= =IJ7+ -----END PGP SIGNATURE----- --zhXaljGHf11kAtnf-- --===============0815485223== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============0815485223==--