Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756058AbbFRQJj (ORCPT ); Thu, 18 Jun 2015 12:09:39 -0400 Received: from pandora.arm.linux.org.uk ([78.32.30.218]:40736 "EHLO pandora.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754528AbbFRQJb (ORCPT ); Thu, 18 Jun 2015 12:09:31 -0400 Date: Thu, 18 Jun 2015 17:09:15 +0100 From: Russell King - ARM Linux To: Doug Anderson Cc: Philipp Zabel , Thierry Reding , Heiko Stuebner , David Airlie , Andy Yan , Yakir Yang , Fabio Estevam , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] drm: bridge/dw_hdmi: Filter modes > 165MHz for DVI Message-ID: <20150618160915.GA16638@n2100.arm.linux.org.uk> References: <1434582847-713-1-git-send-email-dianders@chromium.org> <20150617233040.GE7557@n2100.arm.linux.org.uk> <20150618085335.GF7557@n2100.arm.linux.org.uk> <20150618155545.GQ7557@n2100.arm.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150618155545.GQ7557@n2100.arm.linux.org.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: 3362 Lines: 87 On Thu, Jun 18, 2015 at 04:55:45PM +0100, Russell King - ARM Linux wrote: > On Thu, Jun 18, 2015 at 08:26:39AM -0700, Doug Anderson wrote: > > Perhaps you can try > > Something like that needs to be done, but let's get rid of the mdvi > thing in struct hdmi_vmode - it doesn't belong there, it isn't part > of the currently set video mode, but becomes a property of the > connected sink. > > I'd also prefer it to be called "is_dvi_sink", especially as its > function is changing from "is it a CEA mode" to "is the attached > device a DVI sink". > > Even better would be to call it "is_hdmi_sink" to maintain positive > logic with single-negation where required, rather than double- > negation in places. This is actually a _very_ important point. Changing the function of mdvi when it's used in multiple places throughout the driver is not on - it's too big a change: /*check csc whether needed activated in HDMI mode */ cscon = (is_color_space_conversion(hdmi) && !hdmi->hdmi_data.video_mode.mdvi); inv_val |= (vmode->mdvi ? HDMI_FC_INVIDCONF_DVI_MODEZ_DVI_MODE : HDMI_FC_INVIDCONF_DVI_MODEZ_HDMI_MODE); if (hdmi->cable_plugin && !hdmi->hdmi_data.video_mode.mdvi) hdmi_enable_overflow_interrupts(hdmi); It's unclear what the effect would be to change the meaning of mdvi from "this is a CEA mode" to "the attached device is DVI" in all these locations, and it's just not on to do this in a patch which merely says: If the monitor support audio, so we should support audio for it, even if the display resolution is No-CEA mode. In other words, doesn't even describe this change. In any case, this patch has been dropped from more recent audio driver series. So, what I'd like to see is a patch series which starts with the change below, and builds on that, with explainations why each change is needed. This is important, as this is shared IP, and we need to make sure that we don't regress non-Rockchip users of this IP. I'll try and do some work in this area if nothing crops up in the next month. drivers/gpu/drm/bridge/dw_hdmi.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c index 49cafb61d290..8834e8142ea6 100644 --- a/drivers/gpu/drm/bridge/dw_hdmi.c +++ b/drivers/gpu/drm/bridge/dw_hdmi.c @@ -119,6 +119,8 @@ struct dw_hdmi { u8 edid[HDMI_EDID_LEN]; bool cable_plugin; + bool sink_is_hdmi; + bool sink_has_audio; bool phy_enabled; struct drm_display_mode previous_mode; @@ -1402,6 +1404,9 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) edid = drm_get_edid(connector, hdmi->ddc); if (edid) { + hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); + hdmi->sink_has_audio = drm_detect_monitor_audio(edid); + dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n", edid->width_cm, edid->height_cm); -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/