From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161051AbcHEIHg (ORCPT ); Fri, 5 Aug 2016 04:07:36 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:32956 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759178AbcHEIGN (ORCPT ); Fri, 5 Aug 2016 04:06:13 -0400 Date: Fri, 5 Aug 2016 10:06:08 +0200 From: Daniel Vetter To: Russell King - ARM Linux Cc: Jose Abreu , dri-devel@lists.freedesktop.org, Carlos Palminha , Archit Taneja , David Airlie , Fabio Estevam , Daniel Vetter , Takashi Iwai , Vladimir Zapolskiy , Thierry Reding , linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3 v3] drm: bridge/dw-hdmi: Move edid reading to .detect() callback Message-ID: <20160805080608.GZ6232@phenom.ffwll.local> Mail-Followup-To: Russell King - ARM Linux , Jose Abreu , dri-devel@lists.freedesktop.org, Carlos Palminha , Archit Taneja , David Airlie , Fabio Estevam , Takashi Iwai , Vladimir Zapolskiy , Thierry Reding , linux-kernel@vger.kernel.org References: <1bbffb944fb9ce0363f5544db31baf2a911c1c55.1470306826.git.joabreu@synopsys.com> <20160804104738.GR1041@n2100.armlinux.org.uk> <57A349E8.5040400@synopsys.com> <20160804143102.GU1041@n2100.armlinux.org.uk> <57A357D1.1070900@synopsys.com> <20160804150443.GW1041@n2100.armlinux.org.uk> <57A377AE.5060208@synopsys.com> <20160804230125.GZ1041@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160804230125.GZ1041@n2100.armlinux.org.uk> X-Operating-System: Linux phenom 4.6.0-1-amd64 User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 05, 2016 at 12:01:25AM +0100, Russell King - ARM Linux wrote: > On Thu, Aug 04, 2016 at 06:13:18PM +0100, Jose Abreu wrote: > > Hi Russell, > > > > So, I didn't use framebuffer console but used X instead and it is > > working as it should. I think we can drop this patch. I am now > > making interoperability with DVI and I am facing the following > > scenario: > > - I start the driver > > - An EDID is sent which tells the driver that HDMI is NOT > > supported; > > - The driver configures itself to a DVI mode; > > > > Until this point everything is working as it should. But: > > > > - Now I send an EDID which tells the driver that HDMI is > > supported; > > - As the EDID has the same preferred mode the user will not > > reconfigure the mode and there will be no change to HDMI mode. > > The EDID should still be read, but as you say, userspace doesn't take > any action because it sees that the mode parameters are still the same, > as you have identified. > > > The missing change to HDMI mode will cause the test to fail. The > > workaround that I am using is to reconfigure to another video > > mode and then configure to the preferred one but I think this > > could be fixed in the driver, right? > > This one is extremely awkward - to fix it, we would need to check to > see whether we reconfigure the hardware each time we read the EDID, > and I don't think that's a particularly nice thing to do. > > I'd like to hear what other DRM developers think about this issue. I pondored whether we should have a counter on each connector for probe state, which helpers would increment whenever something changes, like: - connector_status (as tracked by probe helpers) - anything in the edid changes (when setting it drm_mode_connector_update_edid_property) - other changes (like sink state changes in dpcd or whatever) That way userspace would be able to reliably spot such changes and do a new modeset. The other side of this is making sure that the kernel doesn't optimize away a modeset in this case. The problem is that current helpers (both legacy and atomic) only look at the output routing and the display mode and nothing else to decide whether a full modeset is required. At least with atomic that's easy to fix: - When the connector state changes we simply need to set the connectors_changed boolean in the corresponding CRTC state. - We can use the probe lifecycle counter from above to do that: If it's changed in atomic_check, then we set connectors_changed to force a full modeset. I guess with just the kernel changes (and not even upgrading userspace) we'd probably fix many cases already. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch