Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757592AbcKBWCm (ORCPT ); Wed, 2 Nov 2016 18:02:42 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:40176 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755971AbcKBWCk (ORCPT ); Wed, 2 Nov 2016 18:02:40 -0400 Date: Wed, 2 Nov 2016 22:02:22 +0000 From: Russell King - ARM Linux To: James Le Cuirot Cc: David Airlie , Daniel Vetter , Philipp Zabel , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/bridge: dw-hdmi: Set sink_is_hdmi and sink_has_audio in mode_set Message-ID: <20161102220222.GW1041@n2100.armlinux.org.uk> References: <20161030135625.11117-1-chewi@gentoo.org> <20161031163736.GL1041@n2100.armlinux.org.uk> <20161102214750.76d1e228@symphony.aura-online.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161102214750.76d1e228@symphony.aura-online.co.uk> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4214 Lines: 102 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 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.