| Message ID | s5hk3ymp2bl.wl%tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 02, 2012 at 06:14:54PM +0200, Takashi Iwai wrote: > Some SNB/IVY Laptops with 1600x900 HD+ panel (e.g. Dell Latitude E6420 > and HP ProBook 65xx) are known to show the whole black/white garbage > or flickering screens while starting up or changing the mode. > This can be worked around by disabling LVDS off while changing the > mode. > > Since this doesn't give any visible drawback on machines I've tested, > enable it generically for HD+ panels for SCH-split case. > > Tested-by: Giacomo Comes <comes@naic.edu> > Cc: <stable@vger.kernel.org> > Signed-off-by: Takashi Iwai <tiwai@suse.de> As part of a larger rework effort for the modeset stuff I've stumbled over this, too - I see detiled garbage on some mode switches. http://cgit.freedesktop.org/~danvet/drm/commit/?h=modeset-rework&id=bf8528171b2c2bb94aeca8316e26e017e42643d3 So my question: why so complicated, and what's the justification for the funky hdisplay/vdisplay condiditons? Cheers, Daniel > --- > drivers/gpu/drm/i915/intel_lvds.c | 25 +++++++++++++++++++------ > 1 file changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 08eb04c..cf1c150 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -407,13 +407,26 @@ out: > static void intel_lvds_prepare(struct drm_encoder *encoder) > { > struct intel_lvds *intel_lvds = to_intel_lvds(encoder); > + bool do_disable = false; > > - /* > - * Prior to Ironlake, we must disable the pipe if we want to adjust > - * the panel fitter. However at all other times we can just reset > - * the registers regardless. > - */ > - if (!HAS_PCH_SPLIT(encoder->dev) && intel_lvds->pfit_dirty) > + if (HAS_PCH_SPLIT(encoder->dev)) { > + /* Some HD+ panels require the pipe-off while mode changing; > + * otherwise you'll get B/W garbage or sustaining flickering > + * screen > + */ > + if (intel_lvds->fixed_mode->hdisplay >= 1600 && > + intel_lvds->fixed_mode->vdisplay >= 900) > + do_disable = true; > + } else { > + /* > + * Prior to Ironlake, we must disable the pipe if we want to > + * adjust the panel fitter. However at all other times we can > + * just reset the registers regardless. > + */ > + do_disable = intel_lvds->pfit_dirty; > + } > + > + if (do_disable) > intel_lvds_disable(intel_lvds); > } > > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
At Mon, 2 Jul 2012 18:53:29 +0200, Daniel Vetter wrote: > > On Mon, Jul 02, 2012 at 06:14:54PM +0200, Takashi Iwai wrote: > > Some SNB/IVY Laptops with 1600x900 HD+ panel (e.g. Dell Latitude E6420 > > and HP ProBook 65xx) are known to show the whole black/white garbage > > or flickering screens while starting up or changing the mode. > > This can be worked around by disabling LVDS off while changing the > > mode. > > > > Since this doesn't give any visible drawback on machines I've tested, > > enable it generically for HD+ panels for SCH-split case. > > > > Tested-by: Giacomo Comes <comes@naic.edu> > > Cc: <stable@vger.kernel.org> > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > As part of a larger rework effort for the modeset stuff I've stumbled over > this, too - I see detiled garbage on some mode switches. > > http://cgit.freedesktop.org/~danvet/drm/commit/?h=modeset-rework&id=bf8528171b2c2bb94aeca8316e26e017e42643d3 > > So my question: why so complicated, and what's the justification for the > funky hdisplay/vdisplay condiditons? It's just for avoiding the possible regressions by this change. We definitely need a fix for HD+ panels ASAP, while other panels work in the current code. Introducing the extra disable code affecting all laptops sounds too risky for 3.5 kernel, as we can't cover all cases. But for HD+ panels, we did fairly good coverage, as there are still little models. If you find this limitation nonsense and we are brave enough, let's do it without complication. thanks, Takashi > > Cheers, Daniel > > > --- > > drivers/gpu/drm/i915/intel_lvds.c | 25 +++++++++++++++++++------ > > 1 file changed, 19 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > > index 08eb04c..cf1c150 100644 > > --- a/drivers/gpu/drm/i915/intel_lvds.c > > +++ b/drivers/gpu/drm/i915/intel_lvds.c > > @@ -407,13 +407,26 @@ out: > > static void intel_lvds_prepare(struct drm_encoder *encoder) > > { > > struct intel_lvds *intel_lvds = to_intel_lvds(encoder); > > + bool do_disable = false; > > > > - /* > > - * Prior to Ironlake, we must disable the pipe if we want to adjust > > - * the panel fitter. However at all other times we can just reset > > - * the registers regardless. > > - */ > > - if (!HAS_PCH_SPLIT(encoder->dev) && intel_lvds->pfit_dirty) > > + if (HAS_PCH_SPLIT(encoder->dev)) { > > + /* Some HD+ panels require the pipe-off while mode changing; > > + * otherwise you'll get B/W garbage or sustaining flickering > > + * screen > > + */ > > + if (intel_lvds->fixed_mode->hdisplay >= 1600 && > > + intel_lvds->fixed_mode->vdisplay >= 900) > > + do_disable = true; > > + } else { > > + /* > > + * Prior to Ironlake, we must disable the pipe if we want to > > + * adjust the panel fitter. However at all other times we can > > + * just reset the registers regardless. > > + */ > > + do_disable = intel_lvds->pfit_dirty; > > + } > > + > > + if (do_disable) > > intel_lvds_disable(intel_lvds); > > } > > > > -- > > 1.7.10.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Mail: daniel@ffwll.ch > Mobile: +41 (0)79 365 57 48 >
On Mon, Jul 2, 2012 at 7:51 PM, Takashi Iwai <tiwai@suse.de> wrote: > It's just for avoiding the possible regressions by this change. > We definitely need a fix for HD+ panels ASAP, while other panels work > in the current code. Introducing the extra disable code affecting all > laptops sounds too risky for 3.5 kernel, as we can't cover all cases. > But for HD+ panels, we did fairly good coverage, as there are still > little models. > > If you find this limitation nonsense and we are brave enough, let's > do it without complication. Hm, I've hoped I could chicken out and to the non-limited thing for 3.6 ... as you say, it's a bit late to do such a change in after -rc5. So I guess the question is: How much does not having this patch break panels, i.e. can some mad vt-switching restore them or are they bricked for good until reset? -Daniel
At Mon, 2 Jul 2012 19:55:17 +0200, Daniel Vetter wrote: > > On Mon, Jul 2, 2012 at 7:51 PM, Takashi Iwai <tiwai@suse.de> wrote: > > It's just for avoiding the possible regressions by this change. > > We definitely need a fix for HD+ panels ASAP, while other panels work > > in the current code. Introducing the extra disable code affecting all > > laptops sounds too risky for 3.5 kernel, as we can't cover all cases. > > But for HD+ panels, we did fairly good coverage, as there are still > > little models. > > > > If you find this limitation nonsense and we are brave enough, let's > > do it without complication. > > Hm, I've hoped I could chicken out and to the non-limited thing for > 3.6 ... as you say, it's a bit late to do such a change in after -rc5. > So I guess the question is: How much does not having this patch break > panels, i.e. can some mad vt-switching restore them or are they > bricked for good until reset? On HP laptops I've tested, the screen can be partially recovered from B/W garbages when you once disable LVDS and enable again. But then the screen starts flickering. And much worse, this flickering remains even after the cold boot! I have to put the machine rest for a while to restore the screen completely. thanks, Takashi
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 08eb04c..cf1c150 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -407,13 +407,26 @@ out: static void intel_lvds_prepare(struct drm_encoder *encoder) { struct intel_lvds *intel_lvds = to_intel_lvds(encoder); + bool do_disable = false; - /* - * Prior to Ironlake, we must disable the pipe if we want to adjust - * the panel fitter. However at all other times we can just reset - * the registers regardless. - */ - if (!HAS_PCH_SPLIT(encoder->dev) && intel_lvds->pfit_dirty) + if (HAS_PCH_SPLIT(encoder->dev)) { + /* Some HD+ panels require the pipe-off while mode changing; + * otherwise you'll get B/W garbage or sustaining flickering + * screen + */ + if (intel_lvds->fixed_mode->hdisplay >= 1600 && + intel_lvds->fixed_mode->vdisplay >= 900) + do_disable = true; + } else { + /* + * Prior to Ironlake, we must disable the pipe if we want to + * adjust the panel fitter. However at all other times we can + * just reset the registers regardless. + */ + do_disable = intel_lvds->pfit_dirty; + } + + if (do_disable) intel_lvds_disable(intel_lvds); }