[PATCH] drm: tilcdc: Calculate the vrefresh if it is not set by userspace

Kevin Hao kexin.hao at windriver.com
Wed Oct 11 02:57:29 UTC 2017


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 at 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 at vger.kernel.org> # v4.11+
> > Signed-off-by: Kevin Hao <kexin.hao at windriver.com>
> > ---
> >  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;
> >  }
> >  
> > 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20171011/a4a68640/attachment.sig>


More information about the dri-devel mailing list