[Intel-gfx] [PATCH v3] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl

[Ville Syrjälä]

On Thu, Nov 09, 2017 at 08:02:40AM -0800, Lucas De Marchi wrote:
> On Thu, Nov 9, 2017 at 5:11 AM, Ville Syrjälä
> <ville.syrjala at linux.intel.com> wrote:
> > On Thu, Nov 09, 2017 at 02:58:04AM -0800, Lucas De Marchi wrote:
> >> Wa Display #1183 was recently added to workaround
> >> "Failures when enabling DPLL0 with eDP link rate 2.16
> >> or 4.32 GHz and CD clock frequency 308.57 or 617.14 MHz
> >> (CDCLK_CTL CD Frequency Select 10b or 11b) used in this
> >>  enabling or in previous enabling."
> >>
> >> This Workaround was designed to minimize the impact only
> >> to save the bad case with that link rates. But HW engineers
> >> indicated that it should be safe to apply broadly. Although
> >> they were expecting the DPLL0 link rate to be unchanged on
> >> runtime.
> >>
> >> We need to cover 2 cases: when we are in fact enabling DPLL0
> >> and when we are just changing the frequency. The workaround
> >> for those cases are similar but different enough to have them
> >> done in different places.
> >>
> >> This is based on previous patch by Rodrigo Vivi with suggestions
> >> from Ville Syrjälä.
> >
> > Still doesn't look like what I suggested.
> 
> I agree with your suggestion of moving stuff to skl_set_cdclk() to
> cover the case in which
> vco isn't changing. However see the paragraph I added above on why I
> need to do it
> differently. In short: the sequence on the WA for enabling and
> updating cdclck are different,
> with some code duplication unfortunately. I don't see you covering
> that case in your
> suggestion. Have I missed anything?

Even if we follow the spec literally I think we can do it all
in skl_set_cdclk(). I think the following should dtrt:

pcu start

if (...)
	disable_dpll0()

cdclk_sel = real

if (need_wa)
	divmux=1 

if (...)
	enable_dpll0()

if (need_wa) {
	cdclk_sel = 0
	cdclk_sel = real 
	divmux=0 
}

pcu done

-- 
Ville Syrjälä
Intel OTC