All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org,
	Antony Chen <antonychen@qnap.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/edid: Reset more of the display info
Date: Wed, 25 Apr 2018 17:09:10 +0300	[thread overview]
Message-ID: <20180425140910.GF23723@intel.com> (raw)
In-Reply-To: <20180424193629.GC25142@phenom.ffwll.local>

On Tue, Apr 24, 2018 at 09:36:29PM +0200, Daniel Vetter wrote:
> On Tue, Apr 24, 2018 at 05:26:30PM +0300, Ville Syrjälä wrote:
> > On Tue, Apr 24, 2018 at 04:18:37PM +0200, Daniel Vetter wrote:
> > > On Tue, Apr 24, 2018 at 04:02:50PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > We're currently failing to reset everything in display_info.hdmi
> > > > which will potentially cause us to use stale information when
> > > > swapping monitors. Eg. if the user replaces a HDMI 2.0 monitor
> > > > with a HDMI 1.x monitor we will continue to think that the monitor
> > > > supports scrambling. That will lead to a black screen since the
> > > > HDMI 1.x monitor won't understand the scrambled signal.
> > > > 
> > > > Fix the problem by clearing display_info.hdmi fully. And while at
> > > > eliminate some duplicated code by calling drm_reset_display_info()
> > > > in drm_add_display_info().
> > > > 
> > > > Cc: Antony Chen <antonychen@qnap.com>
> > > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105655
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_edid.c | 11 +++--------
> > > >  1 file changed, 3 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > index 134069f36482..39f1db4acda4 100644
> > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > @@ -4451,6 +4451,7 @@ drm_reset_display_info(struct drm_connector *connector)
> > > >  	info->max_tmds_clock = 0;
> > > >  	info->dvi_dual = false;
> > > >  	info->has_hdmi_infoframe = false;
> > > > +	memset(&info->hdmi, 0, sizeof(info->hdmi));
> > > >  
> > > >  	info->non_desktop = 0;
> > > >  }
> > > > @@ -4462,17 +4463,11 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
> > > >  
> > > >  	u32 quirks = edid_get_quirks(edid);
> > > >  
> > > > +	drm_reset_display_info(connector);
> > > > +
> > > 
> > > Strictly speaking this is a separate bugfix, for the case where you
> > > immediately go from one output to a different one. Keith already fixed the
> > > case where at least somewhere in between you go to the disconnected state
> > > in:
> > > 
> > > commit 170178fe99dd212bf25e70c89bc4b6e195564ffc
> > > Author: Keith Packard <keithp@keithp.com>
> > > Date:   Wed Dec 13 00:44:26 2017 -0800
> > > 
> > >     drm: Update edid-derived drm_display_info fields at edid property set [v2]
> > > 
> > > With the above explained in the commit message:
> > 
> > The drm_reset_display_info() call is just the "deduplicate the code"
> > part. The memset() is the bugfix, and applies in all cases.
> 
> Hm ... I guess my brain wasn't fully working.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> as-is.

Cool. Thanks.

Pushed to drm-misc-fixes with
Cc: stable@vger.kernel.org
Tested-by: Antony Chen <antonychen@qnap.com>

> -Daniel
> 
> > 
> > > 
> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > 
> > > >  	info->width_mm = edid->width_cm * 10;
> > > >  	info->height_mm = edid->height_cm * 10;
> > > >  
> > > > -	/* driver figures it out in this case */
> > > > -	info->bpc = 0;
> > > > -	info->color_formats = 0;
> > > > -	info->cea_rev = 0;
> > > > -	info->max_tmds_clock = 0;
> > > > -	info->dvi_dual = false;
> > > > -	info->has_hdmi_infoframe = false;
> > > > -
> > > >  	info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
> > > >  
> > > >  	DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
> > > > -- 
> > > > 2.16.1
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > -- 
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2018-04-25 14:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-24 13:02 [PATCH] drm/edid: Reset more of the display info Ville Syrjala
2018-04-24 14:18 ` Daniel Vetter
2018-04-24 14:26   ` Ville Syrjälä
2018-04-24 19:36     ` Daniel Vetter
2018-04-25 11:03       ` [Intel-gfx] " Antony Chen
2018-04-25 14:10         ` Ville Syrjälä
2018-04-25 14:09       ` Ville Syrjälä [this message]
2018-04-24 14:22 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-04-24 17:46 ` ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180425140910.GF23723@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=antonychen@qnap.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.