On Wed, May 20, 2015 at 06:04:26PM +0200, maarten.lankho...@linux.intel.com 
wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.olive...@intel.com>
> 
> It makes more sense there, since these are computation steps that can
> fail.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>

I've noticed that a few of the patches in this series were originally
written by Ander, but seem to be missing his s-o-b line.  I think you
generally want to just append your line after his in that case.

One other cosmetic note farther down.

> ---
>  drivers/gpu/drm/i915/intel_display.c | 70 
> ++++++++++++++++++------------------
>  1 file changed, 35 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 81d5358efdde..e7aa8610cbdc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12085,37 +12085,6 @@ static void update_scanline_offset(struct intel_crtc 
> *crtc)
>               crtc->scanline_offset = 1;
>  }
>  
> -static int
> -intel_modeset_compute_config(struct drm_atomic_state *state)
> -{
> -     struct drm_crtc *crtc;
> -     struct drm_crtc_state *crtc_state;
> -     int ret, i;
> -
> -     ret = drm_atomic_helper_check_modeset(state->dev, state);
> -     if (ret)
> -             return ret;
> -
> -     for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -             if (!crtc_state->enable &&
> -                 WARN_ON(crtc_state->active))
> -                     crtc_state->active = false;
> -
> -             if (!crtc_state->enable)
> -                     continue;
> -
> -             ret = intel_modeset_pipe_config(crtc, state);
> -             if (ret)
> -                     return ret;
> -
> -             intel_dump_pipe_config(to_intel_crtc(crtc),
> -                                    to_intel_crtc_state(crtc_state),
> -                                    "[modeset]");
> -     }
> -
> -     return drm_atomic_helper_check_planes(state->dev, state);
> -}
> -
>  static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
>  {
>       struct drm_device *dev = state->dev;
> @@ -12191,6 +12160,41 @@ static int __intel_set_mode_checks(struct 
> drm_atomic_state *state)
>       return 0;
>  }
>  
> +static int
> +intel_modeset_compute_config(struct drm_atomic_state *state)
> +{
> +     struct drm_crtc *crtc;
> +     struct drm_crtc_state *crtc_state;
> +     int ret, i;
> +
> +     ret = drm_atomic_helper_check_modeset(state->dev, state);
> +     if (ret)
> +             return ret;
> +
> +     for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +             if (!crtc_state->enable &&
> +                 WARN_ON(crtc_state->active))
> +                     crtc_state->active = false;
> +
> +             if (!crtc_state->enable)
> +                     continue;
> +
> +             ret = intel_modeset_pipe_config(crtc, state);
> +             if (ret)
> +                     return ret;
> +
> +             intel_dump_pipe_config(to_intel_crtc(crtc),
> +                                    to_intel_crtc_state(crtc_state),
> +                                    "[modeset]");
> +     }
> +
> +     ret = drm_atomic_helper_check_planes(state->dev, state);
> +     if (ret)
> +             return ret;
> +
> +     return __intel_set_mode_checks(state);

Just a cosmetic note, but maybe we should rename this function now?
It's not called from __intel_set_mode anymore and it isn't really
'checks' (but rather setup that we intend to be done during the check
phase), so the whole name seems a bit misleading now.


Matt

> +}
> +
>  static int __intel_set_mode(struct drm_atomic_state *state)
>  {
>       struct drm_device *dev = state->dev;
> @@ -12200,10 +12204,6 @@ static int __intel_set_mode(struct drm_atomic_state 
> *state)
>       int ret = 0;
>       int i;
>  
> -     ret = __intel_set_mode_checks(state);
> -     if (ret < 0)
> -             return ret;
> -
>       ret = drm_atomic_helper_prepare_planes(dev, state);
>       if (ret)
>               return ret;
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to