On Wed, May 20, 2015 at 06:04:26PM +0200, maarten.lankhorst@xxxxxxxxxxxxxxx wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx>
>
> It makes more sense there, since these are computation steps that can
> fail.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
