Op 07-07-15 om 11:26 schreef Daniel Vetter: > On Tue, Jul 07, 2015 at 09:08:15AM +0200, Maarten Lankhorst wrote: >> This must be done in advance, and during crtc_disable all scalers > "in advance" ... before what exactly? Yes I'm harping a bit about commit > messages today ;-) > >> can be force disabled. > Why does it matter that all scalers can be force disabled? > >> This means intel_atomic_setup_scalers is only called in 1 place now, >> during crtc_check. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > tbh I don't really understand what's the problem and what's the solution > implemented here. Judging from comments the trouble is that we don't > correctly recover the pfit state for skl scalers at hw readout time? > > I guess we should fix that for at lest the crtc level scaler (including > cross-checking of all scaler state) and in sanitize_plane force-disabling > any plane that is using a scaler. But really not much clue here. intel_atomic_setup_scalers should only be called for the new state, but it's called on the old state which doesn't need to be modified. The crtc_disable function can just disable all scalers since there's no point in doing a full recheck. Alternatively it could only disable the crtc scaler id if it's >= 0, but I think disabling all might be better for paranoia reasons. >> --- >> drivers/gpu/drm/i915/intel_atomic.c | 14 ++------ >> drivers/gpu/drm/i915/intel_display.c | 68 +++++++++++++++++++++++------------- >> drivers/gpu/drm/i915/intel_dp.c | 2 +- >> drivers/gpu/drm/i915/intel_drv.h | 2 +- >> 4 files changed, 48 insertions(+), 38 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c >> index 0aeced82201e..429677a111d5 100644 >> --- a/drivers/gpu/drm/i915/intel_atomic.c >> +++ b/drivers/gpu/drm/i915/intel_atomic.c >> @@ -272,17 +272,12 @@ int intel_atomic_setup_scalers(struct drm_device *dev, > This function is way too big. Yeah not your doing, but imo would be good > to split it up a bit. Especially since it already has a comment explaining > the high-level flow which would be a good pattern to structure the > subfunction extraction after. I think adding a pipe_to_scaler_id or something would remove the whole plane special casing block and make it a lot more readable. Not checking intel_plane->pipe would be a good thing too, not sure how that could even happen. _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
