[PATCH] drm: tilcdc: Calculate the vrefresh if it is not set by userspace
Jyri Sarha
jsarha at ti.com
Thu Oct 12 09:36:14 UTC 2017
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 10/11/17 05:57, Kevin Hao wrote:
> 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.
>
I first thought this is a bug either in X-server or drmlib. But then it
says "Not used in a functional way." about the vrefresh in the struct
drm_display_mode definition.
Now I wonder if we should use the value in the driver at all. I think we
should recalculate it unconditionally.
Also the place where the value is calculated is currently wrong. The
mode fixup is only executed if the encoder needs VESA-compliant sync.
I'll make a new patch with a comment of what is going on.
Best regards,
Jyri
> 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;
>>> }
>>>
>>>
>>
>>
More information about the dri-devel
mailing list