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

[Daniel Stone]

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?

Cheers,
Daniel