| Message ID | 1454418061-10178-1-git-send-email-marius.c.vlad@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 02 Feb 2016, Marius Vlad <marius.c.vlad@intel.com> wrote: > Use the drm_property_blob data for user-supplied EDID blobs. > > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com> > --- > 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. The other question is, why do you base the decision to use override edid on whether we can get the actual edid or not? /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); > }
On Tue, Feb 02, 2016 at 05:32:21PM +0200, Jani Nikula wrote: > On Tue, 02 Feb 2016, Marius Vlad <marius.c.vlad@intel.com> wrote: > > Use the drm_property_blob data for user-supplied EDID blobs. > > > > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com> > > --- > > 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
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; + intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); }
Use the drm_property_blob data for user-supplied EDID blobs. Signed-off-by: Marius Vlad <marius.c.vlad@intel.com> --- drivers/gpu/drm/i915/intel_hdmi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)