On Tue, Oct 10, 2017 at 04:58:15PM +0300, Jyri Sarha wrote:
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
> On 10/10/17 15:09, Kevin Hao wrote:
> > The check for vrefresh has been removed by commit 11abbc9f39e0
> > ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is
> > enabled"). But the userspace is really possible to request a display
> > mode with vrefresh uninitialized, then it would cause the following
> > calltrace:
> > [<c010fa20>] (unwind_backtrace) from [<c010c508>] (show_stack+0x20/0x24)
> > [<c010c508>] (show_stack) from [<c0959744>] (dump_stack+0x20/0x28)
> > [<c0959744>] (dump_stack) from [<c010c23c>] (__div0+0x20/0x28)
> > [<c010c23c>] (__div0) from [<c095805c>] (Ldiv0+0x8/0x14)
> > [<c095805c>] (Ldiv0) from [<c0615ccc>] (tilcdc_crtc_update_fb+0xa8/0x1d4)
> > [<c0615ccc>] (tilcdc_crtc_update_fb) from [<c0614b70>] (tilcdc_plane_atomic_update+0x54/0x5c)
> > [<c0614b70>] (tilcdc_plane_atomic_update) from [<c05c53a4>] (drm_atomic_helper_commit_planes+0x1b4/0x270)
> > [<c05c53a4>] (drm_atomic_helper_commit_planes) from [<c0617d48>] (tilcdc_commit+0x58/0x84)
> > [<c0617d48>] (tilcdc_commit) from [<c05e3f08>] (drm_atomic_commit+0x54/0x68)
> > [<c05e3f08>] (drm_atomic_commit) from [<c05c937c>] (drm_atomic_helper_set_config+0x68/0x9c)
> > [<c05c937c>] (drm_atomic_helper_set_config) from [<c05d843c>] (__drm_mode_set_config_internal+0x60/0xe0)
> > [<c05d843c>] (__drm_mode_set_config_internal) from [<c05d906c>] (drm_mode_setcrtc+0x3a8/0x4c0)
> > [<c05d906c>] (drm_mode_setcrtc) from [<c05d3c2c>] (drm_ioctl_kernel+0x78/0xb0)
> > [<c05d3c2c>] (drm_ioctl_kernel) from [<c05d3e60>] (drm_ioctl+0x1fc/0x328)
> > [<c05d3e60>] (drm_ioctl) from [<c0280008>] (vfs_ioctl+0x28/0x44)
> > [<c0280008>] (vfs_ioctl) from [<c0280a00>] (do_vfs_ioctl+0x890/0x8ec)
> > [<c0280a00>] (do_vfs_ioctl) from [<c0280abc>] (SyS_ioctl+0x60/0x7c)
> > [<c0280abc>] (SyS_ioctl) from [<c01078e0>] (ret_fast_syscall+0x0/0x48)
> >
> > So calculate the vrefresh in mode_fixup() to make sure there always
> > have a valid vrefresh.
> >
>
> The earlier check was there for the frame buffer updates when the
> display is not yet enabled and the hwmode's vrefresh is not valid yet.
> Now, with the lock protected enabled state variable check, this should
> never happen.
>
> Do you have a reliable way to reproduce the problem you are seeing?
I get the calltrace every time when I reboot my BeagleBone Black board.
The following is the xorg.conf I used:
root@beaglebone:~# cat /etc/X11/xorg.conf
Section "Monitor"
Identifier "Builtin Default Monitor"
EndSection
Section "Device"
Identifier "Builtin Default fbdev Device 0"
Driver "modesetting"
EndSection
Section "Screen"
Identifier "Builtin Default fbdev Screen 0"
DefaultDepth 16
Device "Builtin Default fbdev Device 0"
Monitor "Builtin Default Monitor"
EndSection
Section "ServerLayout"
Identifier "Builtin Default Layout"
Screen "Builtin Default fbdev Screen 0"
EndSection
> Could try adding a test for the vrefresh validity in the
> tilcdc_crtc_mode_valid() to see if that helps?
No, this will break the modeset driver. I have added the following code:
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index d2589f310437..4dbce75272a3 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -845,6 +845,11 @@ int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
return MODE_BAD;
}
+ if (!mode->vrefresh) {
+ DBG("Pruning mode: invalid vrefresh");
+ return MODE_BAD;
+ }
+
return MODE_OK;
}
And got the following error for Xorg:
[ 17.563] (II) modeset(0): Damage tracking initialized
[ 17.563] (II) modeset(0): Setting screen physical size to 169 x 127
[ 18.388] (EE) modeset(0): failed to set mode: Invalid argument
[ 21.907] (II) modeset(0): Disabling kernel dirty updates, not required.
It seems that function drmmode_set_mode_major() in hw/xfree86/drivers/modesetting/drmmode_display.c
always invoke the drmModeSetCrtc() with a zero vrefresh.
Thanks,
Kevin
>
> Adding the extra test in the mode validity check probably produces a
> failed mode set attempt, which I guess is Ok if the mode has zero
> vertical refresh time. But if this does not help, then the issue you are
> seeing may be an indication of a synchronization problem somewhere.
>
> Best regards,
> Jyri
>
> > Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled")
> > Cc: <stable@xxxxxxxxxxxxxxx> # v4.11+
> > Signed-off-by: Kevin Hao <kexin.hao@xxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > index d2589f310437..16b020a9044c 100644
> > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > @@ -690,6 +690,9 @@ static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc,
> > adjusted_mode->flags &= ~DRM_MODE_FLAG_PHSYNC;
> > }
> >
> > + if (!adjusted_mode->vrefresh)
> > + adjusted_mode->vrefresh = drm_mode_vrefresh(adjusted_mode);
> > +
> > return true;
> > }
> >
> >
>
>
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
