On Thu, Feb 21, 2013 at 01:49:53AM +0100, Daniel Vetter wrote: > Currently only containing the requested and the adjusted mode. And > only crtc callbacks are converted somewhat to it, encoders will be > done on a as-needed basis (simply too much churn in one patch > otherwise). > > Future patches will add tons more useful stuff to this struct, > starting with the very simple.
Just a few bikesheds below. Otherwise the direction where this is going looks very nice. > v2: Store the pipe_config in the intel_crtc, so that the ->mode-set, > ->enable and also ->disable have easy access to it. > > v3: Store the pipe config in the right crtc ... > > v4: Rebased. > > v5: Fixup an BUG when trying to kfree an ERR_PTR. > > Signed-off-by: Daniel Vetter <[email protected]> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 +- > drivers/gpu/drm/i915/intel_display.c | 84 > +++++++++++++++++++++--------------- > drivers/gpu/drm/i915/intel_drv.h | 7 +++ > 3 files changed, 59 insertions(+), 36 deletions(-) > <snip> > @@ -7382,19 +7386,24 @@ static void intel_modeset_commit_output_state(struct > drm_device *dev) > } > } > > -static struct drm_display_mode * > -intel_modeset_adjusted_mode(struct drm_crtc *crtc, > - struct drm_display_mode *mode) > +static struct intel_crtc_config * > +intel_modeset_pipe_config(struct drm_crtc *crtc, > + struct drm_display_mode *mode) > { > struct drm_device *dev = crtc->dev; > - struct drm_display_mode *adjusted_mode; > struct drm_encoder_helper_funcs *encoder_funcs; > struct intel_encoder *encoder; > + struct intel_crtc_config *pipe_config; > > - adjusted_mode = drm_mode_duplicate(dev, mode); > - if (!adjusted_mode) > + pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL); > + if (!pipe_config) > return ERR_PTR(-ENOMEM); > > + pipe_config->adjusted_mode = *mode; > + pipe_config->adjusted_mode.base.id = 0; > + pipe_config->requested_mode = *mode; > + pipe_config->requested_mode.base.id = 0; drm_mode_copy() perhaps? > + > /* Pass our mode to the connectors and the CRTC to give them a chance to > * adjust it according to limitations or connector properties, and also > * a chance to reject the mode entirely. > @@ -7405,22 +7414,23 @@ intel_modeset_adjusted_mode(struct drm_crtc *crtc, > if (&encoder->new_crtc->base != crtc) > continue; > encoder_funcs = encoder->base.helper_private; > - if (!(encoder_funcs->mode_fixup(&encoder->base, mode, > - adjusted_mode))) { > + if (!(encoder_funcs->mode_fixup(&encoder->base, > + &pipe_config->requested_mode, > + &pipe_config->adjusted_mode))) { > DRM_DEBUG_KMS("Encoder fixup failed\n"); > goto fail; > } > } > > - if (!(intel_crtc_mode_fixup(crtc, mode, adjusted_mode))) { > + if (!(intel_crtc_compute_config(crtc, pipe_config))) { > DRM_DEBUG_KMS("CRTC fixup failed\n"); > goto fail; > } > DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id); > > - return adjusted_mode; > + return pipe_config; > fail: > - drm_mode_destroy(dev, adjusted_mode); > + kfree(pipe_config); > return ERR_PTR(-EINVAL); > } > > @@ -7686,7 +7696,8 @@ int intel_set_mode(struct drm_crtc *crtc, > { > struct drm_device *dev = crtc->dev; > drm_i915_private_t *dev_priv = dev->dev_private; > - struct drm_display_mode *adjusted_mode, *saved_mode, *saved_hwmode; > + struct drm_display_mode *saved_mode, *saved_hwmode; > + struct intel_crtc_config *pipe_config; > struct intel_crtc *intel_crtc; > unsigned disable_pipes, prepare_pipes, modeset_pipes; > int ret = 0; > @@ -7713,11 +7724,13 @@ int intel_set_mode(struct drm_crtc *crtc, > * Hence simply check whether any bit is set in modeset_pipes in all the > * pieces of code that are not yet converted to deal with mutliple crtcs > * changing their mode at the same time. */ > - adjusted_mode = NULL; > + pipe_config = NULL; The NULL assignment could be moved to where pipe_config is declared. > if (modeset_pipes) { > - adjusted_mode = intel_modeset_adjusted_mode(crtc, mode); > - if (IS_ERR(adjusted_mode)) { > - ret = PTR_ERR(adjusted_mode); > + pipe_config = intel_modeset_pipe_config(crtc, mode); > + if (IS_ERR(pipe_config)) { > + ret = PTR_ERR(pipe_config); > + pipe_config = NULL; > + > goto out; > } > } -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
