On Fri, Feb 22, 2013 at 12:56:45AM +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. > > 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 OOPS when trying to kfree an ERR_PTR. > > v6: Used drm_moode_copy and some other small cleanups as suggested > by Ville Syrjälä. > > Signed-off-by: Daniel Vetter <[email protected]> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 +- > drivers/gpu/drm/i915/intel_display.c | 83 > +++++++++++++++++++++--------------- > drivers/gpu/drm/i915/intel_drv.h | 7 +++ > 3 files changed, 58 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index e95337c..8fb14fb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -271,6 +271,8 @@ struct drm_i915_error_state { > struct intel_display_error_state *display; > }; > > +struct intel_crtc_config; > + > struct drm_i915_display_funcs { > bool (*fbc_enabled)(struct drm_device *dev); > void (*enable_fbc)(struct drm_crtc *crtc, unsigned long interval); > @@ -284,8 +286,6 @@ struct drm_i915_display_funcs { > struct drm_display_mode *mode); > void (*modeset_global_resources)(struct drm_device *dev); > int (*crtc_mode_set)(struct drm_crtc *crtc, > - struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode, > int x, int y, > struct drm_framebuffer *old_fb); > void (*crtc_enable)(struct drm_crtc *crtc); > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index b521198..5a3e231 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3915,15 +3915,15 @@ bool intel_connector_get_hw_state(struct > intel_connector *connector) > return encoder->get_hw_state(encoder, &pipe); > } > > -static bool intel_crtc_mode_fixup(struct drm_crtc *crtc, > - const struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode) > +static bool intel_crtc_compute_config(struct drm_crtc *crtc, > + struct intel_crtc_config *pipe_config) > { > struct drm_device *dev = crtc->dev; > + struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode; > > if (HAS_PCH_SPLIT(dev)) { > /* FDI link clock is fixed at 2.7G */ > - if (mode->clock * 3 > IRONLAKE_FDI_FREQ * 4) > + if (pipe_config->requested_mode.clock * 3 > IRONLAKE_FDI_FREQ * > 4) > return false; > } > > @@ -4609,14 +4609,15 @@ static void intel_set_pipe_timings(struct intel_crtc > *intel_crtc, > } > > static int i9xx_crtc_mode_set(struct drm_crtc *crtc, > - struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode, > int x, int y, > struct drm_framebuffer *fb) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_display_mode *adjusted_mode = > + &intel_crtc->config.adjusted_mode; > + struct drm_display_mode *mode = &intel_crtc->config.requested_mode; > int pipe = intel_crtc->pipe; > int plane = intel_crtc->plane; > int refclk, num_connectors = 0; > @@ -5579,14 +5580,15 @@ static uint32_t ironlake_compute_dpll(struct > intel_crtc *intel_crtc, > } > > static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > - struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode, > int x, int y, > struct drm_framebuffer *fb) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_display_mode *adjusted_mode = > + &intel_crtc->config.adjusted_mode; > + struct drm_display_mode *mode = &intel_crtc->config.requested_mode; > int pipe = intel_crtc->pipe; > int plane = intel_crtc->plane; > int num_connectors = 0; > @@ -5745,14 +5747,15 @@ static void haswell_modeset_global_resources(struct > drm_device *dev) > } > > static int haswell_crtc_mode_set(struct drm_crtc *crtc, > - struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode, > int x, int y, > struct drm_framebuffer *fb) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_display_mode *adjusted_mode = > + &intel_crtc->config.adjusted_mode; > + struct drm_display_mode *mode = &intel_crtc->config.requested_mode; > int pipe = intel_crtc->pipe; > int plane = intel_crtc->plane; > int num_connectors = 0; > @@ -5829,8 +5832,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, > } > > static int intel_crtc_mode_set(struct drm_crtc *crtc, > - struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode, > int x, int y, > struct drm_framebuffer *fb) > { > @@ -5839,6 +5840,9 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc, > struct drm_encoder_helper_funcs *encoder_funcs; > struct intel_encoder *encoder; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_display_mode *adjusted_mode = > + &intel_crtc->config.adjusted_mode; > + struct drm_display_mode *mode = &intel_crtc->config.requested_mode; > int pipe = intel_crtc->pipe; > int ret; > > @@ -5849,8 +5853,8 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc, > > drm_vblank_pre_modeset(dev, pipe); > > - ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode, > - x, y, fb); > + ret = dev_priv->display.crtc_mode_set(crtc, x, y, fb); > + > drm_vblank_post_modeset(dev, pipe); > > if (ret != 0) > @@ -7478,19 +7482,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); > > + drm_mode_copy(&pipe_config->adjusted_mode, mode); > + pipe_config->adjusted_mode.base.id = 0; > + drm_mode_copy(&pipe_config->requested_mode, mode); > + pipe_config->requested_mode.base.id = 0;
drm_mode_copy() preserves the destination mode's id, and since you kzalloc() the whole thing there's no need to zero the ids explicitly. That was pretty much why I suggeted it. The rest looks good to me. Reviewed-by: Ville Syrjälä <[email protected]> -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
