Re: [PATCH] drm/bridge: dw-hdmi: Set sink_is_hdmi and sink_has_audio in mode_set

[Russell King - ARM Linux]

On Wed, Nov 02, 2016 at 09:47:50PM +0000, James Le Cuirot wrote:
> On Mon, 31 Oct 2016 16:37:36 +0000
> Russell King - ARM Linux <linux@xxxxxxxxxxxxxxx> wrote:
> > We also need to apply this to the ELD as well - and several other
> > drivers are similarly buggy, and are going to need similar fixes
> > (thanks for pointing this problem out!)
> 
> Are you sure that is necessary? If you take a look at
> drm_load_edid_firmware, you'll see that it already calls
> drm_edid_to_eld when the drm_kms_helper.edid_firmware parameter is
> given.

Hmm, thanks for pointing that out - you are correct that we only need
to call drm_edid_to_eld() in get_modes().

I've also been digging more into the CEA861B spec, and it seems that
keying the infoframes off the result of drm_detect_hdmi_monitor() is
not entirely correct - CEA861B allows version 2 AVI infoframes to be
sent if the EDID version is 3, and audio (along with audio infoframes)
where audio is reported as supported.

I've been fixing this in TDA998x primarily at the moment, because that
works.  I've been avoiding iMX6 stuff (where dw-hdmi is used) while
iMX stabilises after the merge window - the autobuilers have been
reporting boot failures on those platforms, although it looks like rc3
has solved them all now.

> I briefly tested while giving drm_kms_helper.edid_firmware and it
> works, though I did have to assign edid via edid_blob->data like I did
> in my own patch. edid_blob_ptr is not an edid struct.

Yes, found that with applying a similar approach to TDA998x, I'll fix
this dw-hdmi patch up for that once I've finished with TDA998x.
Nevertheless, your report is good news.

> >  drivers/gpu/drm/bridge/dw-hdmi.c | 30 +++++++++++++++++++++++++-----
> >  1 file changed, 25 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
> > index 77ab47341658..878568af2d41 100644
> > --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> > @@ -1413,6 +1413,30 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
> >  	mutex_unlock(&hdmi->mutex);
> >  }
> >  
> > +static int dw_hdmi_connector_fill_modes(struct drm_connector *connector,
> > +					uint32_t maxX, uint32_t maxY)
> > +{
> > +	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
> > +					     connector);
> > +	struct edid *edid;
> > +	int ret;
> > +
> > +	ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
> > +
> > +	edid = connector->edid_blob_ptr;
> > +	if (edid) {
> > +		hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
> > +		hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
> > +		/* Store the ELD */
> > +		drm_edid_to_eld(connector, edid);
> > +	} else {
> > +		hdmi->sink_is_hdmi = false;
> > +		hdmi->sink_has_audio = false;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> >  static enum drm_connector_status
> >  dw_hdmi_connector_detect(struct drm_connector *connector, bool force)
> >  {
> > @@ -1444,12 +1468,8 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
> >  		dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
> >  			edid->width_cm, edid->height_cm);
> >  
> > -		hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
> > -		hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
> >  		drm_mode_connector_update_edid_property(connector, edid);
> >  		ret = drm_add_edid_modes(connector, edid);
> > -		/* Store the ELD */
> > -		drm_edid_to_eld(connector, edid);
> >  		kfree(edid);
> >  	} else {
> >  		dev_dbg(hdmi->dev, "failed to get edid\n");
> > @@ -1496,7 +1516,7 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
> >  
> >  static const struct drm_connector_funcs dw_hdmi_connector_funcs = {
> >  	.dpms = drm_atomic_helper_connector_dpms,
> > -	.fill_modes = drm_helper_probe_single_connector_modes,
> > +	.fill_modes = dw_hdmi_connector_fill_modes,
> >  	.detect = dw_hdmi_connector_detect,
> >  	.destroy = dw_hdmi_connector_destroy,
> >  	.force = dw_hdmi_connector_force,
> > 
> > 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.