On Tue, 2015-03-17 at 18:11 +0000, Konduru, Chandra wrote: > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel Vetter > > Sent: Tuesday, March 17, 2015 7:17 AM > > To: Konduru, Chandra > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Conselvan De Oliveira, Ander; Vetter, Daniel > > Subject: Re: [PATCH 13/21] drm/i915: Preserve scaler state when > > clearing crtc_state > > > > On Sat, Mar 14, 2015 at 10:55:38PM -0700, Chandra Konduru wrote: > > > crtc_state is cleared during mode set which wipes out complete scaler > > > state too. This is causing issues. To fix, ensure scaler state is > > > preserved because it contains not only crtc scaler usage, but also > > > planes using scalers on this crtc. > > > > > > Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx> > > > > I guess this is going to conflict with Ander's crtc state series. But since you're > > signed up to review that I guess you will know what needs to be changed here. > > Hopefully Ander's series will just take care of this. > > Planes may have changed its scaling needs and updated scaler_users > is in crtc_state. Wiping out crtc state will wipeout staged scaler_state > and cause incorrect behavior. So preserving computed (or staged) > scaler state into crtc state. > > Ander, > Can you pls check for any issue here? As is your patch shouldn't cause any trouble. The problem we have is that the legacy modeset path assumes the new pipe_config is kzalloc'd. I need to inspect all that code and check what really depends on the state being zeroed, but I haven't got to it yet. Once that's done, we could just duplicate the whole crtc state and be done with it. Ander > > > -Daniel > > > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index e07c24e..9c3c6b1 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -10617,11 +10617,14 @@ static void > > > clear_intel_crtc_state(struct intel_crtc_state *crtc_state) { > > > struct drm_crtc_state tmp_state; > > > + struct intel_crtc_scaler_state scaler_state; > > > > > > - /* Clear only the intel specific part of the crtc state */ > > > + /* Clear only the intel specific part of the crtc state excluding > > > +scalers */ > > > tmp_state = crtc_state->base; > > > + scaler_state = crtc_state->scaler_state; > > > memset(crtc_state, 0, sizeof *crtc_state); > > > crtc_state->base = tmp_state; > > > + crtc_state->scaler_state = scaler_state; > > > } > > > > > > static int > > > -- > > > 1.7.9.5 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx