Nouveau - May 2019 - [PATCH] drm/nouveau: fix duplication of nv50_head_atom struct

nv50_head_atomic_duplicate_state() makes a copy of nv50_head_atom
struct. This patch adds copying of struct member named "or", which
previously was left uninitialized in the duplicated structure.

Due to this bug, incorrect nhsync and nvsync values were sometimes used.
In my particular case, that lead to a mismatch between the output
resolution of the graphics device (GeForce GT 630 OEM) and the reported
input signal resolution on the display. xrandr reported 1680x1050, but
the display reported 1280x1024. As a result of this mismatch, the output
on the display looked like it was cropped (only part of the output was
actually visible on the display).

git bisect pointed to commit 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle
SetControlOutputResource from head"), which added the member "or"
to
nv50_head_atom structure, but forgot to copy it in
nv50_head_atomic_duplicate_state().

Fixes: 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle SetControlOutputResource
from head")
Signed-off-by: Peteris Rudzusiks <peteris.rudzusiks at gmail.com>
---
 drivers/gpu/drm/nouveau/dispnv50/head.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 2e7a0c347ddb..adce62f4e18f 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -413,6 +413,7 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
 	asyh->ovly = armh->ovly;
 	asyh->dither = armh->dither;
 	asyh->procamp = armh->procamp;
+	asyh->or = armh->or;
 	asyh->dp = armh->dp;
 	asyh->clr.mask = 0;
 	asyh->set.mask = 0;
-- 
2.17.1

On Sun, 12 May 2019 at 04:23, Peteris Rudzusiks
<peteris.rudzusiks at gmail.com> wrote:
>
> nv50_head_atomic_duplicate_state() makes a copy of nv50_head_atom
> struct. This patch adds copying of struct member named "or",
which
> previously was left uninitialized in the duplicated structure.
>
> Due to this bug, incorrect nhsync and nvsync values were sometimes used.
> In my particular case, that lead to a mismatch between the output
> resolution of the graphics device (GeForce GT 630 OEM) and the reported
> input signal resolution on the display. xrandr reported 1680x1050, but
> the display reported 1280x1024. As a result of this mismatch, the output
> on the display looked like it was cropped (only part of the output was
> actually visible on the display).
>
> git bisect pointed to commit 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50:
handle
> SetControlOutputResource from head"), which added the member
"or" to
> nv50_head_atom structure, but forgot to copy it in
> nv50_head_atomic_duplicate_state().
>
> Fixes: 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle
SetControlOutputResource from head")
> Signed-off-by: Peteris Rudzusiks <peteris.rudzusiks at gmail.com>
Oops, nice catch.  Thank you for this, I've merged it in my tree and
will get it upstream ASAP.

Thanks,
Ben.
> ---
>  drivers/gpu/drm/nouveau/dispnv50/head.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 2e7a0c347ddb..adce62f4e18f 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -413,6 +413,7 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
>         asyh->ovly = armh->ovly;
>         asyh->dither = armh->dither;
>         asyh->procamp = armh->procamp;
> +       asyh->or = armh->or;
>         asyh->dp = armh->dp;
>         asyh->clr.mask = 0;
>         asyh->set.mask = 0;
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

On Tue, May 14, 2019 at 04:55:05PM +1000, Ben Skeggs
wrote:
> On Sun, 12 May 2019 at 04:23, Peteris Rudzusiks
> <peteris.rudzusiks at gmail.com> wrote:
> >
> > nv50_head_atomic_duplicate_state() makes a copy of nv50_head_atom
> > struct. This patch adds copying of struct member named "or",
which
> > previously was left uninitialized in the duplicated structure.
> >
> > Due to this bug, incorrect nhsync and nvsync values were sometimes
used.
> > In my particular case, that lead to a mismatch between the output
> > resolution of the graphics device (GeForce GT 630 OEM) and the
reported
> > input signal resolution on the display. xrandr reported 1680x1050, but
> > the display reported 1280x1024. As a result of this mismatch, the
output
> > on the display looked like it was cropped (only part of the output was
> > actually visible on the display).
> >
> > git bisect pointed to commit 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50:
handle
> > SetControlOutputResource from head"), which added the member
"or" to
> > nv50_head_atom structure, but forgot to copy it in
> > nv50_head_atomic_duplicate_state().
> >
> > Fixes: 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle
SetControlOutputResource from head")
> > Signed-off-by: Peteris Rudzusiks <peteris.rudzusiks at
gmail.com>
> Oops, nice catch.  Thank you for this, I've merged it in my tree and
> will get it upstream ASAP.
> 
> Thanks,
> Ben.
> 
Hi Ben,

Thank you for taking the time to review and merge this patch.

I'm new to the Linux kernel development process, so I am not sure what
happens next. Does inclusion in your tree imply that this fix will end
up in some (most likely - next) mainline kernel? Will it also be
backported to 4.19 LTS branch?

This bug affects all kernel versions starting from v4.18. Probably not
that many systems though.

Cheers,
Peteris
> > ---
> >  drivers/gpu/drm/nouveau/dispnv50/head.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
> > index 2e7a0c347ddb..adce62f4e18f 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> > @@ -413,6 +413,7 @@ nv50_head_atomic_duplicate_state(struct drm_crtc
*crtc)
> >         asyh->ovly = armh->ovly;
> >         asyh->dither = armh->dither;
> >         asyh->procamp = armh->procamp;
> > +       asyh->or = armh->or;
> >         asyh->dp = armh->dp;
> >         asyh->clr.mask = 0;
> >         asyh->set.mask = 0;
> > --
> > 2.17.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel