[PATCH] drm/i915: Support HDMI EDID injection

[PATCH] drm/i915: Support HDMI EDID injection

[Marius Vlad]

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;
+
 		intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
 	}
 
-- 
2.7.0.rc3

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

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

[Jani Nikula]

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);
>  	}

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Marius Vlad]

[-- 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

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

[Daniel Vetter]

On Mon, Feb 01, 2016 at 06:35:33PM +0200, Marius Vlad wrote:
> Allow HDMI EDID injection by making a copy of edid_blob_ptr. When disconnecting
> the connector, or forcing a disconnect, the copy will free'd by intel_hdmi_unset_edid().
> 
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8698a64..4725e8d1 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1355,6 +1355,16 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
>  				    intel_gmbus_get_adapter(dev_priv,
>  				    intel_hdmi->ddc_bus));
>  
> +		/* for injected EDIDs */
> +		if (!edid && connector->override_edid) {
> +			edid = kzalloc(connector->edid_blob_ptr->length,
> +					GFP_KERNEL);
> +			if (edid) {
> +				memcpy(edid, connector->edid_blob_ptr->data,
> +						connector->edid_blob_ptr->length);
> +			}
> +		}
> +

You're sure this does anything? When forcing the edid, ->detect isn't
called. And the problem with hdmi injection isn't the edid, but that our
audio detection code is wrong. See how has_audio is set in
intel_hdmi_set_edid is handled. Similarly we need to fixup the eld code to
correctly update drm_connector->eld.

Finally this needs a testcase which:
1. injects hdmi edid without audio, then sets mode.
2. injects hdmi edid with audio, then sets mode.
3. injects hdmi edid without audio, then sets mode.

Just to exercise the code paths. Bonus points if you check through alsa
that the hdmi audio state indeed matches our expectations (compare the eld
you can grab from alsa with what you injected to make sure you look at the
right screen).

But given all our other oustanding troubles around CI I'd suggest we
deprioritize this task for now. Please add this comment to JIRA so it
wont' get lost.
-Daniel

>  		intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>  	}
>  
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Tue, Feb 02, 2016 at 01:05:07PM +0200, Marius Vlad wrote:
> Use the drm_property_blob data for EDID when an blob EDID
> has been supplied over the debugfs interface.
> 
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>

v2 of patches must have an in-patch changelog of what (and why) stuff
changed.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8698a64..241305b 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1336,7 +1336,9 @@ 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);
> +	/* only free if we haven't injected EDID */
> +	if (!connector->override_edid)
> +		kfree(to_intel_connector(connector)->detect_edid);
>  	to_intel_connector(connector)->detect_edid = NULL;
>  }
>  
> @@ -1355,6 +1357,10 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
>  				    intel_gmbus_get_adapter(dev_priv,
>  				    intel_hdmi->ddc_bus));
>  
> +		/* for injected EDID */
> +		if (!edid && connector->override_edid)
> +			edid = (struct edid *) connector->edid_blob_ptr->data;
> +
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>  	}
>  
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Support HDMI EDID injection

[Abdiel Janulgue]

From: marius vlad <marius.c.vlad@intel.com>

Make a copy of drm_property_blob data for user-supplied EDID blobs.

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index a580de8..76b8023 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1463,6 +1463,16 @@ intel_hdmi_set_edid(struct drm_connector *connector)
 			    intel_gmbus_get_adapter(dev_priv,
 			    intel_hdmi->ddc_bus));
 
+	/*
+	 * Use the user-supplied EDID blob in case we don't have an
+	 * EDID from the display.
+	 */
+	if (!edid && connector->override_edid) {
+		edid = kmemdup(connector->edid_blob_ptr->data,
+			       connector->edid_blob_ptr->length,
+			       GFP_KERNEL);
+	}
+
 	intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
-- 
2.7.4

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

✗ Fi.CI.BAT: failure for drm/i915: Support HDMI EDID injection (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Support HDMI EDID injection (rev3)
URL   : https://patchwork.freedesktop.org/series/3007/
State : failure

== Summary ==

Series 3007v3 drm/i915: Support HDMI EDID injection
https://patchwork.freedesktop.org/api/1.0/series/3007/revisions/3/mbox/

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-FAIL (fi-snb-2520m)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> DMESG-FAIL (fi-snb-2520m)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-FAIL (fi-snb-2520m)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-FAIL (fi-snb-2520m)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> INCOMPLETE (fi-snb-2520m)

fi-bdw-5557u     total:252  pass:238  dwarn:3   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:252  pass:210  dwarn:3   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:230  dwarn:3   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:222  dwarn:3   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:218  dwarn:3   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:252  pass:233  dwarn:3   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:252  pass:233  dwarn:3   dfail:0   fail:0   skip:16 
fi-ilk-650       total:252  pass:199  dwarn:3   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:252  pass:231  dwarn:3   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:252  pass:231  dwarn:3   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:252  pass:231  dwarn:3   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:252  pass:239  dwarn:3   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:252  pass:232  dwarn:3   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:252  pass:230  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:252  pass:239  dwarn:3   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:204  pass:178  dwarn:0   dfail:4   fail:0   skip:21 
fi-snb-2600      total:252  pass:220  dwarn:3   dfail:0   fail:0   skip:29 

5bec901e7ea94b6e656c8b0813c45c90d37a5673 drm-tip: 2017y-02m-16d-04h-31m-41s UTC integration manifest
0cd3a6e drm/i915: Support HDMI EDID injection

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3837/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Thu, 16 Feb 2017, Abdiel Janulgue <abdiel.janulgue@linux.intel.com> wrote:
> From: marius vlad <marius.c.vlad@intel.com>
>
> Make a copy of drm_property_blob data for user-supplied EDID blobs.

I'd like to stop this approach from spreading by handling the EDID
overrides at a lower level [1]. Otherwise, we'll fill all drivers with
special casing.


BR,
Jani.


[1] https://patchwork.freedesktop.org/series/19764/

> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index a580de8..76b8023 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1463,6 +1463,16 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>  			    intel_gmbus_get_adapter(dev_priv,
>  			    intel_hdmi->ddc_bus));
>  
> +	/*
> +	 * Use the user-supplied EDID blob in case we don't have an
> +	 * EDID from the display.
> +	 */
> +	if (!edid && connector->override_edid) {
> +		edid = kmemdup(connector->edid_blob_ptr->data,
> +			       connector->edid_blob_ptr->length,
> +			       GFP_KERNEL);
> +	}
> +
>  	intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
>  
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Support HDMI EDID injection (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Support HDMI EDID injection (rev3)
URL   : https://patchwork.freedesktop.org/series/3007/
State : success

== Summary ==

Series 3007v3 drm/i915: Support HDMI EDID injection
https://patchwork.freedesktop.org/api/1.0/series/3007/revisions/3/mbox/

fi-bdw-5557u     total:252  pass:241  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:225  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:252  pass:202  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:252  pass:235  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:252  pass:230  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:252  pass:224  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:252  pass:223  dwarn:0   dfail:0   fail:0   skip:29 

89a932d98b6e1733011019c9872583a9c7c8fda3 drm-tip: 2017y-02m-16d-10h-06m-42s UTC integration manifest
fcb6c24 drm/i915: Support HDMI EDID injection

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3849/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx