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 <daniel.vetter at ffwll.ch>
> ---
> 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
