From patchwork Thu Jun 18 16:09:15 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 6639031 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 67D6EC0020 for ; Thu, 18 Jun 2015 16:09:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 55CB720804 for ; Thu, 18 Jun 2015 16:09:33 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 4161D207D1 for ; Thu, 18 Jun 2015 16:09:32 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 219576E15D; Thu, 18 Jun 2015 09:09:31 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from pandora.arm.linux.org.uk (pandora.arm.linux.org.uk [78.32.30.218]) by gabe.freedesktop.org (Postfix) with ESMTP id 98CB46E15D for ; Thu, 18 Jun 2015 09:09:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=05Uwb5CbpZj2K6MNETn17hZITRp5xiB8EZ3TbJGNEwg=; b=CMxhYlZT2BEzXdOIOW0eRRPEam0sod22LTFLKIa/+o6NjGA+4FN+2g6xjsRYrNH4i12DNQYL/dJ0pUVCnFVmTZYZCo/qY4A2KroAz/OgIN/b/WnkiaOuRYQuvD5rxspUT0BSFYZR8fTqjT2WrEK1Rlf/FQyh3ch4CgWpFFGeC00=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:55216) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1Z5cNT-0003DJ-7h; Thu, 18 Jun 2015 17:09:19 +0100 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1Z5cNP-0004QO-RQ; Thu, 18 Jun 2015 17:09:15 +0100 Date: Thu, 18 Jun 2015 17:09:15 +0100 From: Russell King - ARM Linux To: Doug Anderson 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-Disposition: inline In-Reply-To: <20150618155545.GQ7557@n2100.arm.linux.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: Fabio Estevam , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Yakir Yang , Andy Yan , Thierry Reding X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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);