On Tue, Nov 17, 2015 at 02:59:35PM +0100, Maarten Lankhorst wrote: > Op 03-11-15 om 14:11 schreef 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. > Yeah but this would break things, or we'd need to keep 2 cdclk freqs globally. > One with the real frequency, the other one used for calculations in atomic. > And only when all crtc's are disabled they would differ. > > In any case, when a plane is DPMS off this code won't be useful, visible = false > because the rectangle will get clipped to (0,0)x(0,0). And that must be changed. > > Saying no scaling is possible would be fine here I think, without side effects. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
