On Thu, 2015-03-19 at 00:12 +0000, Konduru, Chandra wrote: > > > -----Original Message----- > > From: Conselvan De Oliveira, Ander > > Sent: Friday, March 13, 2015 2:49 AM > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Konduru, Chandra; Conselvan De Oliveira, Ander > > Subject: [PATCH 04/19] drm/i915: Allocate a crtc_state also when the crtc is > > being disabled > > > > For consistency, allocate a new crtc_state for a crtc that is being disabled. > > Previously only the enabled value of the current state would change. > > > > Signed-off-by: Ander Conselvan de Oliveira > > <ander.conselvan.de.oliveira@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++-------- > > --- > > 1 file changed, 25 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index b61e3f6..62b9021 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -11188,14 +11188,21 @@ intel_modeset_compute_config(struct drm_crtc > > *crtc, > > unsigned *prepare_pipes, > > unsigned *disable_pipes) > > { > > + struct drm_device *dev = crtc->dev; > > struct intel_crtc_state *pipe_config = NULL; > > + struct intel_crtc *intel_crtc; > > int ret = 0; > > > > intel_modeset_affected_pipes(crtc, modeset_pipes, > > prepare_pipes, disable_pipes); > > > > - if ((*modeset_pipes) == 0) > > - return NULL; > > + for_each_intel_crtc_masked(dev, *disable_pipes, intel_crtc) { > > + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc); > > + if (IS_ERR(pipe_config)) > > + return pipe_config; > > + > > + pipe_config->base.enable = false; > > + } > > > > /* > > * Note this needs changes when we start tracking multiple modes @@ - > > 11203,18 +11210,25 @@ intel_modeset_compute_config(struct drm_crtc *crtc, > > * (i.e. one pipe_config for each crtc) rather than just the one > > * for this crtc. > > */ > > - ret = intel_modeset_pipe_config(crtc, fb, mode, state); > > - if (ret) > > - return ERR_PTR(ret); > > + for_each_intel_crtc_masked(dev, *modeset_pipes, intel_crtc) { > > + /* FIXME: For now we still expect modeset_pipes has at most > > + * one bit set. */ > > + if (WARN_ON(&intel_crtc->base != crtc)) > > + continue; > > > > - pipe_config = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc)); > > - if (IS_ERR(pipe_config)) > > - return pipe_config; > > + ret = intel_modeset_pipe_config(crtc, fb, mode, state); > > + if (ret) > > + return ERR_PTR(ret); > > + > > + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc); > > + if (IS_ERR(pipe_config)) > > + return pipe_config; > > > > - intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, > > - "[modeset]"); > > + intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, > > + "[modeset]"); > > + } > > > > - return pipe_config; > > + return intel_atomic_get_crtc_state(state, to_intel_crtc(crtc)); > > } > > Instead of calling 3 separate intel_atomic_get_crtc_state() > Can you have something like: > intel_modeset_compute_config() > { > pipe_config = intel_atomic_get_crtc_state(state, crtc); > > for each disabled pipe { > use pipe_config; > } > > for each mode_set pipe { > use pipe_config; > } > > return pipe_config; > } > > > Or the way currently done is to cover where disable_pipes != modeset_pipes? > By the way, when can it happen? Yep, disable_pipes can be different than modeset_pipes if we the mode set "steals" a connector. For instance, we could have pipe A driving HDMI-1 and then mode set to pipe B to drive HDMI-1. Pipe B will steal the encoder from pipe A, and cause it to be disable. In that case disable_pipes will have the bit for pipe A set, while modeset_pipes will have the bit for pipe B set. Ander > > > > > static int __intel_set_mode_setup_plls(struct drm_device *dev, > > -- > > 2.1.0 > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx