[PATCH weston 10/68] compositor-drm: Make scanout view preparation more stringent

[Derek Foreman]

On 27/01/17 08:30 AM, Daniel Stone wrote:
> Hi,
>
> On 27 January 2017 at 14:25, Derek Foreman <derekf at osg.samsung.com> wrote:
>> On 09/12/16 01:57 PM, Daniel Stone wrote:
>>> +static int
>>> +drm_view_transform_supported(struct weston_view *ev)
>>> +{
>>> +       return !ev->transform.enabled ||
>>> +               (ev->transform.matrix.type <
>>> WESTON_MATRIX_TRANSFORM_ROTATE);
>>> +}
>>> +
>>>  static uint32_t
>>>  drm_output_check_scanout_format(struct drm_output *output,
>>
>> Not sure this function name is really ideal anymore?  Maybe it never was,
>> lots of stuff here I'm not sure one would all "format".
>
> Yeah, it's probably not the best, but I also didn't touch it here, so ... :)
>
>>> @@ -502,27 +509,40 @@ drm_output_prepare_scanout_view(struct drm_output
>>> *output,
>>>         struct gbm_bo *bo;
>>>         uint32_t format;
>>>
>>> +       /* Don't import buffers which span multiple outputs. */
>>> +       if (ev->output_mask != (1u << output->base.id))
>>> +               return NULL;
>>> +
>>
>> I suppose technically this is overly stringent, but I'd rather it be too
>> careful than broken.
>
> Yeah, it should if anything just serve as an early-out. We can never
> hoist multiple-output views to planes, because otherwise it falls out
> of the scene graph for the other outputs. So we later build up to
> using this unconditionally for plane import; these early patches are
> just about making the scanout/overlay/cursor test paths as similar as
> possible, to save any surprises further on down the line.
>
>>>         /* Make sure our view is exactly compatible with the output. */
>>>         if (ev->geometry.x != output->base.x ||
>>>             ev->geometry.y != output->base.y)
>>>                 return NULL;
>>> +       if (buffer->width != output->base.current_mode->width ||
>>> +           buffer->height != output->base.current_mode->height)
>>> +               return NULL;
>>> +
>>
>> Was there a reason to move this block?
>
> Just for symmetry with the others; certainly no functional effect.
>
>> I really don't have any concerns that would warrant another revision, this
>> looks good to me.  (Also looks like it could be trivially landed without
>> dependency on any other patch in this series, with no functional issues.)
>
> Thanks! I'm going to go ahead and just do that I think, and then if
> any of the cleanups apply to the atomic series, we could address them
> there perhaps?

Agreed!

> Cheers,
> Daniel
>