[Intel-gfx] [PATCH v2 14/14] drm/i915/skl: Do not allow scaling when crtc is disabled.

[Ville Syrjälä]

On Tue, Nov 03, 2015 at 01:00:32PM +0100, Maarten Lankhorst wrote:
> Op 03-11-15 om 11:40 schreef Ville Syrjälä:
> > On Tue, Nov 03, 2015 at 11:06:53AM +0100, Maarten Lankhorst wrote:
> >> Op 03-11-15 om 10:09 schreef Ville Syrjälä:
> >>> On Tue, Nov 03, 2015 at 08:31:53AM +0100, Maarten Lankhorst wrote:
> >>>> This fixes a warning when the crtc is turned off. In that case fb
> >>>> will be NULL, and crtc_clock will be 0. Because the crtc is no longer
> >>>> active this is not a bug, and shouldn't trigger the WARN_ON.
> >>> Mm. We want to do scaling checks and whatnot during DPMS. So this should
> >>> check .enabled, no?
> >> Not sure what the right decision would be here..
> >>
> >>      * skl max scale is lower of:
> >>      *    close to 3 but not 3, -1 is for that purpose
> >>      *            or
> >>      *    cdclk/crtc_clock
> >>
> >> So when multiple pipes are enabled potentially 3x scaling is allowed, but if you dpms them all off
> >> cdclk might get set to 0. This means a previous valid amount of scaling might suddenly become invalid.
> >>
> >> Maybe the fix is keeping 2 cdclk's, one for scaling and one for setting.
> > I think we should keep around the min required cdclk in the crtc state.
> > And we recalculate that one every time anything changes. Then we can
> > just compare it against the current cdclk after plane/crtc states have
> > otherwise been computed to see if we need to change the current cdclk.
> >
> What would the use be here? In that case the above formula reduces to 1<<16.
> If you want to treat dpms off the same as dpms on then you need the separate cdclk's..

I don't understand what you're saying. We need to calculate the required
cdclk based on dpms on. And that is what we must check against the
hardware limit. If we then want to optimize the dpms off case we can
just do 'cdclk = state->active ? state->cdclk : 0'. We probably want to
do that when all pipes are in dpms off. But if we have some pipes in
dpms off and some in dpms on, we may want to skip the 'active' check
for all pipes (ie. behave as all pipes are dpms on), just to avoid flicker
when some of the remaining pipes transition from dpms off to dpms on.

-- 
Ville Syrjälä
Intel OTC