[PATCH] drm/atomic: refuse changing CRTC for planes directly

[Rob Clark]

On Wed, Aug 26, 2015 at 12:03 PM, Rob Clark <robdclark at gmail.com> wrote:
> On Wed, Aug 26, 2015 at 11:41 AM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>> Very strictly speaking this is possible if you have special hw and
>> genlocked CRTCs. In general switching a plane between two active CRTC
>> just won't work so well and is probably not tested at all. Just forbid
>> it.
>
> So, I expect msm should actually be able to do this w/ dual-dsi (where
> we are using two CRTC's, w/ synchronized flushes)..
>
> Probably someone who has a dual-dsi panel should actually test that to
> confirm.  But it seems like it should work.  Maybe we need something
> in 'struct drm_crtc' so core can realize that two CRTC's are locked
> together..

oh, and for most drivers, switching plane between CRTCs without an
off-cycle would probably also work for DSI cmd mode..

BR,
-R

>
>> I've put this into the core since I really couldn't come up with a
>> case where we don't want to enforce that. But if that ever happens it
>> would be easy to move this check into helpers.
>>
>> v2: don't bother with complexity and just outright disallow plane
>> switching without the intermediate OFF state. Simplifies drivers, we
>> don't have any hw that could do it anyway and current atomic userspace
>> (weston) works like this already anyway.
>>
>> Cc: Thierry Reding <thierry.reding at gmail.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> Cc: Daniel Stone <daniels at collabora.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>> ---
>>  drivers/gpu/drm/drm_atomic.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 434915448ea0..f27aae3fa765 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -663,6 +663,22 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>>         return 0;
>>  }
>>
>> +static bool
>> +plane_switching(struct drm_atomic_state *state,
>> +               struct drm_plane *plane,
>> +               struct drm_plane_state *plane_state)
>> +{
>> +       struct drm_crtc_state *crtc_state, *curr_crtc_state;
>> +
>> +       if (!plane->state->crtc || !plane_state->crtc)
>> +               return false;
>> +
>> +       if (plane->state->crtc == plane_state->crtc)
>> +               return false;
>> +
>> +       return true;
>> +}
>> +
>>  /**
>>   * drm_atomic_plane_check - check plane state
>>   * @plane: plane to check
>> @@ -734,6 +750,12 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
>>                 return -ENOSPC;
>>         }
>>
>> +       if (plane_switching(state->state, plane, state)) {
>> +               DRM_DEBUG_ATOMIC("[PLANE:%d] switching CRTC directly\n",
>> +                                plane->base.id);
>> +               return -EINVAL;
>> +       }
>> +
>>         return 0;
>>  }
>>
>> --
>> 2.5.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel