On Tue, Feb 02, 2016 at 05:32:21PM +0200, Jani Nikula wrote: > On Tue, 02 Feb 2016, Marius Vlad <marius.c.vlad@xxxxxxxxx> wrote: > > Use the drm_property_blob data for user-supplied EDID blobs. > > > > Signed-off-by: Marius Vlad <marius.c.vlad@xxxxxxxxx> > > --- > > 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/intel_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 *connector) > > intel_hdmi->has_audio = false; > > intel_hdmi->rgb_quant_range_selectable = false; > > > > - kfree(to_intel_connector(connector)->detect_edid); > > + if (!connector->override_edid) > > + kfree(to_intel_connector(connector)->detect_edid); > > to_intel_connector(connector)->detect_edid = NULL; > > } > > > > @@ -1355,6 +1356,9 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force) > > intel_gmbus_get_adapter(dev_priv, > > intel_hdmi->ddc_bus)); > > > > + if (!edid && connector->override_edid) > > + edid = (struct edid *) connector->edid_blob_ptr->data; > > + > > 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. > > 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. > > /me thinks this is all really messy at the drm level, including the > handling of edid firmware. > > BR, > Jani. > > > > > intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); > > } > > -- > Jani Nikula, Intel Open Source Technology Center
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
