Re: [PATCH] drm/i915: Support HDMI EDID injection

[Marius Vlad <marius.c.vlad@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2686 bytes --]

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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx