Re: [PATCH] drm/atomic-helper: implementatations for legacy interfaces

Sean Paul
2014-11-06T18:31:07Z

On Wed, Nov 5, 2014 at 5:01 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Nov 05, 2014 at 02:48:48PM -0500, Sean Paul wrote:
>> > + if (!crtc && crtc != set->crtc)
>>
>> I think this should be an ||
>
> Hm. My idea idea was actually something along the lines of
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4f80885de3f6..077c792c46e0 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1271,7 +1271,14 @@ static int update_output_state(struct drm_atomic_state *state,
> struct drm_crtc *crtc = state->crtcs[i];
> struct drm_crtc_state *crtc_state = state->crtc_states[i];
>
> - if (!crtc && crtc != set->crtc)
> + if (!crtc)
> + continue;
> +
> + /* Don't update ->enable for the CRTC in the set_config request,
> + * since a mismatch would indicate a bug in the upper layers.
> + * The actual modeset code later on will catch any
> + * inconsistencies here. */
> + if (crtc == set->crtc)
> continue;
>
> crtc_state->enable =
>
>
> I.e. that we don't recompute the new enable state for set->crtc so that we
> can catch bug in the helper function or core drm code which maps the
> legacy ->set_config to the atomic interface.
>
> Still r-b with that change applied, or want to take a deeper look again?

Fixup looks good to me, please add my R-b.

Sean

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org