From mboxrd@z Thu Jan 1 00:00:00 1970 From: linux@armlinux.org.uk (Russell King - ARM Linux) Date: Thu, 9 Nov 2017 09:31:22 +0000 Subject: [PATCH] drm/bridge: dw-hdmi: fix EDID parsing In-Reply-To: <20171109082317.predibr5vv2d33i2@phenom.ffwll.local> References: <20171109082317.predibr5vv2d33i2@phenom.ffwll.local> Message-ID: <20171109093122.GA12318@n2100.armlinux.org.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Nov 09, 2017 at 09:23:18AM +0100, Daniel Vetter wrote: > On Tue, Nov 07, 2017 at 11:27:21AM +0000, Russell King wrote: > > Parsing the EDID for HDMI and audio information in the get_modes() > > callback is incorrect - this only parses the EDID read from the > > connector, not any override or firmware provided EDID. > > > > The correct place to parse the EDID for these parameters is the > > fill_modes() callback, after we've called the helper. Move the parsing > > there. This caused problems for Lu?s Mendes. > > > > Cc: > > Reported-by: Lu?s Mendes > > Tested-by: Lu?s Mendes > > Signed-off-by: Russell King > > --- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 28 ++++++++++++++++++++++++---- > > 1 file changed, 24 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > index 9fe407f49986..2516a1c18a10 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > @@ -1905,10 +1905,7 @@ 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); > > - cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid); > > ret = drm_add_edid_modes(connector, edid); > > /* Store the ELD */ > > drm_edid_to_eld(connector, edid); > > @@ -1920,6 +1917,29 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) > > return ret; > > } > > > > +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); > > + int ret; > > + > > + ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY); > > + > > + if (connector->edid_blob_ptr) { > > + struct edid *edid = (void *)connector->edid_blob_ptr->data; > > + > > + hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); > > + hdmi->sink_has_audio = drm_detect_monitor_audio(edid); > > + cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid); > > + } else { > > + hdmi->sink_is_hdmi = false; > > + hdmi->sink_has_audio = false; > > + } > > + > > + return ret; > > +} > > + > > static void dw_hdmi_connector_force(struct drm_connector *connector) > > { > > struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, > > @@ -1933,7 +1953,7 @@ static void dw_hdmi_connector_force(struct drm_connector *connector) > > } > > > > static const struct drm_connector_funcs dw_hdmi_connector_funcs = { > > - .fill_modes = drm_helper_probe_single_connector_modes, > > + .fill_modes = dw_hdmi_connector_fill_modes, > > Papering over helper functions shouldn't be necessary, except the helper > functions not handling the override edid is a known issue. Jani Nikula is > working on a proper fix, please coordinate with him. So, what you're basically saying is that fixing real bugs that affect users is not something that DRM people want. That's fine, I'll ignore people who come to me for help with DRM bugs in future then because it's obviously a dead loss. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux Subject: Re: [PATCH] drm/bridge: dw-hdmi: fix EDID parsing Date: Thu, 9 Nov 2017 09:31:22 +0000 Message-ID: <20171109093122.GA12318@n2100.armlinux.org.uk> References: <20171109082317.predibr5vv2d33i2@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20171109082317.predibr5vv2d33i2@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Daniel Vetter Cc: Archit Taneja , David Airlie , dri-devel@lists.freedesktop.org, Andrzej Hajda , Jani Nikula , =?iso-8859-1?Q?Lu=EDs?= Mendes , linux-arm-kernel@lists.infradead.org, Laurent Pinchart List-Id: dri-devel@lists.freedesktop.org On Thu, Nov 09, 2017 at 09:23:18AM +0100, Daniel Vetter wrote: > On Tue, Nov 07, 2017 at 11:27:21AM +0000, Russell King wrote: > > Parsing the EDID for HDMI and audio information in the get_modes() > > callback is incorrect - this only parses the EDID read from the > > connector, not any override or firmware provided EDID. > > = > > The correct place to parse the EDID for these parameters is the > > fill_modes() callback, after we've called the helper. Move the parsing > > there. This caused problems for Lu=EDs Mendes. > > = > > Cc: > > Reported-by: Lu=EDs Mendes > > Tested-by: Lu=EDs Mendes > > Signed-off-by: Russell King > > --- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 28 +++++++++++++++++++++++= +---- > > 1 file changed, 24 insertions(+), 4 deletions(-) > > = > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/dr= m/bridge/synopsys/dw-hdmi.c > > index 9fe407f49986..2516a1c18a10 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > @@ -1905,10 +1905,7 @@ static int dw_hdmi_connector_get_modes(struct dr= m_connector *connector) > > dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n", > > edid->width_cm, edid->height_cm); > > = > > - hdmi->sink_is_hdmi =3D drm_detect_hdmi_monitor(edid); > > - hdmi->sink_has_audio =3D drm_detect_monitor_audio(edid); > > drm_mode_connector_update_edid_property(connector, edid); > > - cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid); > > ret =3D drm_add_edid_modes(connector, edid); > > /* Store the ELD */ > > drm_edid_to_eld(connector, edid); > > @@ -1920,6 +1917,29 @@ static int dw_hdmi_connector_get_modes(struct dr= m_connector *connector) > > return ret; > > } > > = > > +static int dw_hdmi_connector_fill_modes(struct drm_connector *connecto= r, > > + uint32_t maxX, uint32_t maxY) > > +{ > > + struct dw_hdmi *hdmi =3D container_of(connector, struct dw_hdmi, > > + connector); > > + int ret; > > + > > + ret =3D drm_helper_probe_single_connector_modes(connector, maxX, maxY= ); > > + > > + if (connector->edid_blob_ptr) { > > + struct edid *edid =3D (void *)connector->edid_blob_ptr->data; > > + > > + hdmi->sink_is_hdmi =3D drm_detect_hdmi_monitor(edid); > > + hdmi->sink_has_audio =3D drm_detect_monitor_audio(edid); > > + cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid); > > + } else { > > + hdmi->sink_is_hdmi =3D false; > > + hdmi->sink_has_audio =3D false; > > + } > > + > > + return ret; > > +} > > + > > static void dw_hdmi_connector_force(struct drm_connector *connector) > > { > > struct dw_hdmi *hdmi =3D container_of(connector, struct dw_hdmi, > > @@ -1933,7 +1953,7 @@ static void dw_hdmi_connector_force(struct drm_co= nnector *connector) > > } > > = > > static const struct drm_connector_funcs dw_hdmi_connector_funcs =3D { > > - .fill_modes =3D drm_helper_probe_single_connector_modes, > > + .fill_modes =3D dw_hdmi_connector_fill_modes, > = > Papering over helper functions shouldn't be necessary, except the helper > functions not handling the override edid is a known issue. Jani Nikula is > working on a proper fix, please coordinate with him. So, what you're basically saying is that fixing real bugs that affect users is not something that DRM people want. That's fine, I'll ignore people who come to me for help with DRM bugs in future then because it's obviously a dead loss. -- = RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up