linux-kernel - [PATCH] drm/bridge: dw-hdmi: Set sink_is_hdmi and sink_has_audio in mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20161030135625.11117-1-chewi@gentoo.org>
Date:   Sun, 30 Oct 2016 13:56:25 +0000
From:   James Le Cuirot <chewi@...too.org>
To:     Russell King <rmk+kernel@....linux.org.uk>,
        David Airlie <airlied@...ux.ie>
Cc:     Daniel Vetter <daniel.vetter@...ll.ch>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        James Le Cuirot <chewi@...too.org>
Subject: [PATCH] drm/bridge: dw-hdmi: Set sink_is_hdmi and sink_has_audio in mode_set

These were previously set in dw_hdmi_connector_get_modes but this
isn't called when the EDID is overridden. This can be seen in
drm_helper_probe_single_connector_modes. Using the
drm_kms_helper.edid_firmware parameter therefore always resulted in
silence, even when providing the very same EDID that had previously
been read from /sys.

I saw that other drivers set similar properties in mode_set rather
than get_modes. radeon_audio_hdmi_mode_set is one such example. It
calls radeon_connector_edid to retrieve the EDID so I drew inspiration
from this for the fix.

I have tested this with a Utilite Pro on 4.9-rc3. I tried overriding
the EDID with my own, not overriding the EDID, hotplugging the display
after booting, and overriding the EDID with 1920x1080.bin. The latter
has no audio parameters so no sound was heard as expected.

Signed-off-by: James Le Cuirot <chewi@...too.org>
---

Notes:
    I do have some questions.
    
    I don't know the significance of the mutex lock. I put my code inside
    it because I am modifying the hdmi properties. Is this necessary?
    Should it go before or after the lock instead?
    
    I'm also wondering whether I should initially set both properties to
    false in case the EDID is missing but the driver didn't do this
    previously. Perhaps it should have?
    
    I have only contributed one line to the kernel before, many years ago,
    so please be gentle. :)

 drivers/gpu/drm/bridge/dw-hdmi.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index ab7023e..a30b691 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1383,12 +1383,23 @@ static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
 				    struct drm_display_mode *mode)
 {
 	struct dw_hdmi *hdmi = bridge->driver_private;
+	struct drm_property_blob *edid_blob;
+	struct edid *edid;
 
 	mutex_lock(&hdmi->mutex);
 
 	/* Store the display mode for plugin/DKMS poweron events */
 	memcpy(&hdmi->previous_mode, mode, sizeof(hdmi->previous_mode));
 
+	edid_blob = hdmi->connector.edid_blob_ptr;
+	if (edid_blob) {
+		edid = (struct edid *) edid_blob->data;
+		if (edid) {
+			hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
+			hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
+		}
+	}
+
 	mutex_unlock(&hdmi->mutex);
 }
 
@@ -1445,8 +1456,6 @@ 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 */
-- 
2.10.1