Re: [Intel-gfx] [PATCH i-g-t v4 5/7] lib/igt_kms: Rework plane properties to be more atomic, v4.

[Maarten Lankhorst]

Op 03-10-17 om 14:05 schreef Mika Kahola:
> On Fri, 2017-09-29 at 11:59 +0200, Maarten Lankhorst wrote:
>> In the future I want to allow tests to commit more properties,
>> but for this to work I have to fix all properties to work better
>> with atomic commit. Instead of special casing each
>> property make a bitmask for all property changed flags, and try to
>> commit all properties.
>>
>> Changes since v1:
>> - Remove special dumping of src and crtc coordinates.
>> - Dump all modified coordinates.
>> Changes since v2:
>> - Move igt_plane_set_prop_changed up slightly.
>> Changes since v3:
>> - Fix wrong ordering of set_position in kms_plane_lowres causing a
>> test failure.
>>
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>  lib/igt_kms.c| 293 +--
>> 
>>  lib/igt_kms.h|  59 
>>  tests/kms_atomic_interruptible.c |  12 +-
>>  tests/kms_plane_lowres.c |   2 +-
>>  tests/kms_rotation_crc.c |   4 +-
>>  5 files changed, 160 insertions(+), 210 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 07d2074c2b1a..6e0052ebe7a7 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -192,11 +192,11 @@ const char
>> *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>>  
>>  /*
>>   * Retrieve all the properies specified in props_name and store them
>> into
>> - * plane->atomic_props_plane.
>> + * plane->props.
>>   */
>>  static void
>> -igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t
>> *plane,
>> -int num_props, const char **prop_names)
>> +igt_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
>> + int num_props, const char **prop_names)
>>  {
>>  drmModeObjectPropertiesPtr props;
>>  int i, j, fd;
>> @@ -214,7 +214,7 @@ igt_atomic_fill_plane_props(igt_display_t
>> *display, igt_plane_t *plane,
>>  if (strcmp(prop->name, prop_names[j]) != 0)
>>  continue;
>>  
>> -plane->atomic_props_plane[j] = props-
>>> props[i];
>> +plane->props[j] = props->props[i];
>>  break;
>>  }
>>  
>> @@ -1659,7 +1659,6 @@ void igt_display_init(igt_display_t *display,
>> int drm_fd)
>>  drmModeRes *resources;
>>  drmModePlaneRes *plane_resources;
>>  int i;
>> -int is_atomic = 0;
>>  
>>  memset(display, 0, sizeof(igt_display_t));
>>  
>> @@ -1679,7 +1678,9 @@ void igt_display_init(igt_display_t *display,
>> int drm_fd)
>>  igt_assert_f(display->pipes, "Failed to allocate memory for
>> %d pipes\n", display->n_pipes);
>>  
>>  drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
>> -is_atomic = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC,
>> 1);
>> +if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
>> +display->is_atomic = 1;
>> +
>>  plane_resources = drmModeGetPlaneResources(display->drm_fd);
>>  igt_assert(plane_resources);
>>  
>> @@ -1776,19 +1777,19 @@ void igt_display_init(igt_display_t *display,
>> int drm_fd)
>>  plane->type = type;
>>  plane->pipe = pipe;
>>  plane->drm_plane = drm_plane;
>> -plane->fence_fd = -1;
>> +plane->values[IGT_PLANE_IN_FENCE_FD] =
>> ~0ULL;
>>  
>> -if (is_atomic == 0) {
>> -display->is_atomic = 1;
>> -igt_atomic_fill_plane_props(display,
>> plane, IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
>> -}
>> +igt_fill_plane_props(display, plane,
>> IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
>>  
>>  get_plane_property(display->drm_fd,
>> drm_plane->plane_id,
>> "rotation",
>> -   
>>> rotation_property,
>> -   _value,
>> +   
>>> props[IGT_PLANE_ROTATION],
>> +   
>>> values[IGT_PLANE_ROTATION],
>> NULL);
>> -plane->rotation =
>> (igt_rotation_t)prop_value;
>> +
>> +/* Clear any residual framebuffer on first
>> commit. */
>> +igt_plane_set_prop_changed(plane,
>> IGT_PLANE_FB_ID);
>> +igt_plane_set_prop_changed(plane,
>> IGT_PLANE_CRTC_ID);
>>  }
>>  
>>  /*
>> @@ -1805,9 +1806,6 @@ void igt_display_init(igt_display_t *display,
>> int drm_fd)
>>  
>>  pipe->n_planes = n_planes;
>>  
>> -for_each_plane_on_pipe(display, i, plane)
>> -plane->fb_changed = true;
>> -
>>  pipe->mode_changed = true;
>>  }
>>  
>> @@ -2070,18 +2068,7 @@ bool igt_pipe_get_property(igt_pipe_t *pipe,
>> const 

Re: [Intel-gfx] [PATCH i-g-t v4 5/7] lib/igt_kms: Rework plane properties to be more atomic, v4.

[Mika Kahola]

On Fri, 2017-09-29 at 11:59 +0200, Maarten Lankhorst wrote:
> In the future I want to allow tests to commit more properties,
> but for this to work I have to fix all properties to work better
> with atomic commit. Instead of special casing each
> property make a bitmask for all property changed flags, and try to
> commit all properties.
> 
> Changes since v1:
> - Remove special dumping of src and crtc coordinates.
> - Dump all modified coordinates.
> Changes since v2:
> - Move igt_plane_set_prop_changed up slightly.
> Changes since v3:
> - Fix wrong ordering of set_position in kms_plane_lowres causing a
> test failure.
> 
> Signed-off-by: Maarten Lankhorst 
> ---
>  lib/igt_kms.c| 293 +--
> 
>  lib/igt_kms.h|  59 
>  tests/kms_atomic_interruptible.c |  12 +-
>  tests/kms_plane_lowres.c |   2 +-
>  tests/kms_rotation_crc.c |   4 +-
>  5 files changed, 160 insertions(+), 210 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 07d2074c2b1a..6e0052ebe7a7 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -192,11 +192,11 @@ const char
> *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>  
>  /*
>   * Retrieve all the properies specified in props_name and store them
> into
> - * plane->atomic_props_plane.
> + * plane->props.
>   */
>  static void
> -igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t
> *plane,
> - int num_props, const char **prop_names)
> +igt_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
> +  int num_props, const char **prop_names)
>  {
>   drmModeObjectPropertiesPtr props;
>   int i, j, fd;
> @@ -214,7 +214,7 @@ igt_atomic_fill_plane_props(igt_display_t
> *display, igt_plane_t *plane,
>   if (strcmp(prop->name, prop_names[j]) != 0)
>   continue;
>  
> - plane->atomic_props_plane[j] = props-
> >props[i];
> + plane->props[j] = props->props[i];
>   break;
>   }
>  
> @@ -1659,7 +1659,6 @@ void igt_display_init(igt_display_t *display,
> int drm_fd)
>   drmModeRes *resources;
>   drmModePlaneRes *plane_resources;
>   int i;
> - int is_atomic = 0;
>  
>   memset(display, 0, sizeof(igt_display_t));
>  
> @@ -1679,7 +1678,9 @@ void igt_display_init(igt_display_t *display,
> int drm_fd)
>   igt_assert_f(display->pipes, "Failed to allocate memory for
> %d pipes\n", display->n_pipes);
>  
>   drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> - is_atomic = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC,
> 1);
> + if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> + display->is_atomic = 1;
> +
>   plane_resources = drmModeGetPlaneResources(display->drm_fd);
>   igt_assert(plane_resources);
>  
> @@ -1776,19 +1777,19 @@ void igt_display_init(igt_display_t *display,
> int drm_fd)
>   plane->type = type;
>   plane->pipe = pipe;
>   plane->drm_plane = drm_plane;
> - plane->fence_fd = -1;
> + plane->values[IGT_PLANE_IN_FENCE_FD] =
> ~0ULL;
>  
> - if (is_atomic == 0) {
> - display->is_atomic = 1;
> - igt_atomic_fill_plane_props(display,
> plane, IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
> - }
> + igt_fill_plane_props(display, plane,
> IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
>  
>   get_plane_property(display->drm_fd,
> drm_plane->plane_id,
>      "rotation",
> -    
> >rotation_property,
> -    _value,
> +    
> >props[IGT_PLANE_ROTATION],
> +    
> >values[IGT_PLANE_ROTATION],
>      NULL);
> - plane->rotation =
> (igt_rotation_t)prop_value;
> +
> + /* Clear any residual framebuffer on first
> commit. */
> + igt_plane_set_prop_changed(plane,
> IGT_PLANE_FB_ID);
> + igt_plane_set_prop_changed(plane,
> IGT_PLANE_CRTC_ID);
>   }
>  
>   /*
> @@ -1805,9 +1806,6 @@ void igt_display_init(igt_display_t *display,
> int drm_fd)
>  
>   pipe->n_planes = n_planes;
>  
> - for_each_plane_on_pipe(display, i, plane)
> - plane->fb_changed = true;
> -
>   pipe->mode_changed = true;
>   }
>  
> @@ -2070,18 +2068,7 @@ bool igt_pipe_get_property(igt_pipe_t *pipe,
> const char *name,
>  
>  static uint32_t igt_plane_get_fb_id(igt_plane_t *plane)
>  {
> - if (plane->fb)
> -