| Message ID | 20190511170831.25645-1-peteris.rudzusiks@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/nouveau: fix duplication of nv50_head_atom struct | expand |
On Sun, 12 May 2019 at 04:23, Peteris Rudzusiks <peteris.rudzusiks@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@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@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@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@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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, May 14, 2019 at 3:57 PM Peteris Rudzusiks <peteris.rudzusiks@gmail.com> wrote: > > 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@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@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. Ben submits a pull request to Dave Airlie (drm maintainer), and Dave submits one to Linus for inclusion in the "official" upstream repository. Dave just sent a pull request to Linus, who usually picks these up within a few days (exceptions apply). Once in the mainline tree, the "Fixes" tag is likely to cause it to get picked up for stable. You can also nominate it for stable kernel branch inclusion explicitly (there are instructions somewhere, but basically you send an email to some list saying "please include commit ABC in kernels XYZ"). What Ubuntu ships is, ultimately, up to Ubuntu. They will, however, frequently follow the stable kernel branches, and listen to the list above as well. Hope this helps, -ilia
On Wed, May 15, 2019 at 11:29:40PM -0400, Ilia Mirkin wrote: > On Tue, May 14, 2019 at 3:57 PM Peteris Rudzusiks > <peteris.rudzusiks@gmail.com> wrote: > > > > 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@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@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. > > Ben submits a pull request to Dave Airlie (drm maintainer), and Dave > submits one to Linus for inclusion in the "official" upstream > repository. Dave just sent a pull request to Linus, who usually picks > these up within a few days (exceptions apply). > > Once in the mainline tree, the "Fixes" tag is likely to cause it to > get picked up for stable. You can also nominate it for stable kernel > branch inclusion explicitly (there are instructions somewhere, but > basically you send an email to some list saying "please include commit > ABC in kernels XYZ"). > > What Ubuntu ships is, ultimately, up to Ubuntu. They will, however, > frequently follow the stable kernel branches, and listen to the list > above as well. > > Hope this helps, > > -ilia Thanks for explaing this. I'll wait and see if this patch gets included in stable releases without explicitly asking for it. Regards, Peteris
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;
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@gmail.com> --- drivers/gpu/drm/nouveau/dispnv50/head.c | 1 + 1 file changed, 1 insertion(+)