[PATCH i-g-t v4 4/4] tests/vmwgfx: Fix and extend the reference counting test

[Zack Rusin]

On Tue, Jul 30, 2024 at 1:58 PM Kamil Konieczny
<kamil.konieczny at linux.intel.com> wrote:
>
> Hi Zack,
> On 2024-07-29 at 00:37:56 -0400, Zack Rusin wrote:
> > Fix a few issues related to buffer management in the reference counting
> > test. Extend it to include various prime related reference counting
> > issues.
>
> Please describe also that you updated licence and copyright.

In general we don't really do that for the same reason why you wouldn't do:
/* add a to b */
c = a + b;
i.e. "Updating copyrights." comment isn't very valuable, because one
can see that copyrights are being updated. I'm happy to do it
differently for igt, and respin v5 with the updated commit message.

> > @@ -154,6 +134,7 @@ bool vmw_save_data_as_png(struct vmw_surface *surface, void *data,
> >       /* Can separate this into another function as it grows */
> >       switch (surface->params.base.format) {
> >       case SVGA3D_R8G8B8A8_UNORM:
> > +     case SVGA3D_B8G8R8X8_UNORM:
>
> This looks like a fix.

It's an adjustment for the rest of the changes in that file. It's not
valuable without the tests that are being added.

> > @@ -471,7 +456,7 @@ struct vmw_surface *vmw_ioctl_create_surface_full(
> >       arg.req.base.autogen_filter = autogen_filter;
> >       arg.req.base.drm_surface_flags |= surface_flags;
> >       arg.req.base.buffer_handle = buffer_handle;
> > -     if (buffer_handle != SVGA3D_INVALID_ID) {
> > +     if (buffer_handle == SVGA3D_INVALID_ID) {
>
> This also looks like a fix.

Same as above. It's only needed for the tests that this change is adding.

> I do not see why you cannot
> split this patch into a fixes, extending already existing
> tests and adding new ones but it is up to you.

Even if those were fixes we're not going to be backporting them or
running the tests without the rest of the changes. The reviewers
already reviewed the changes. So all the splitting would do is add
more work for everyone with zero practical benefits.

z