All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: tilcdc: Calculate the vrefresh if it is not set by userspace
@ 2017-10-10 12:09 Kevin Hao
  2017-10-10 13:58 ` Jyri Sarha
  0 siblings, 1 reply; 4+ messages in thread
From: Kevin Hao @ 2017-10-10 12:09 UTC (permalink / raw)
  To: dri-devel; +Cc: Tomi Valkeinen, Jyri Sarha

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.

Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled")
Cc: <stable@vger.kernel.org> # v4.11+
Signed-off-by: Kevin Hao <kexin.hao@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;
 }
 
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm: tilcdc: Calculate the vrefresh if it is not set by userspace
  2017-10-10 12:09 [PATCH] drm: tilcdc: Calculate the vrefresh if it is not set by userspace Kevin Hao
@ 2017-10-10 13:58 ` Jyri Sarha
  2017-10-11  2:57   ` Kevin Hao
  0 siblings, 1 reply; 4+ messages in thread
From: Jyri Sarha @ 2017-10-10 13:58 UTC (permalink / raw)
  To: Kevin Hao, dri-devel; +Cc: Tomi Valkeinen


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?
Could try adding a test for the vrefresh validity in the
tilcdc_crtc_mode_valid() to see if that helps?

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@vger.kernel.org> # v4.11+
> Signed-off-by: Kevin Hao <kexin.hao@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;
>  }
>  
> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm: tilcdc: Calculate the vrefresh if it is not set by userspace
  2017-10-10 13:58 ` Jyri Sarha
@ 2017-10-11  2:57   ` Kevin Hao
  2017-10-12  9:36     ` Jyri Sarha
  0 siblings, 1 reply; 4+ messages in thread
From: Kevin Hao @ 2017-10-11  2:57 UTC (permalink / raw)
  To: Jyri Sarha; +Cc: Tomi Valkeinen, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 5720 bytes --]

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@vger.kernel.org> # v4.11+
> > Signed-off-by: Kevin Hao <kexin.hao@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;
> >  }
> >  
> > 
> 
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm: tilcdc: Calculate the vrefresh if it is not set by userspace
  2017-10-11  2:57   ` Kevin Hao
@ 2017-10-12  9:36     ` Jyri Sarha
  0 siblings, 0 replies; 4+ messages in thread
From: Jyri Sarha @ 2017-10-12  9:36 UTC (permalink / raw)
  To: Kevin Hao; +Cc: Tomi Valkeinen, dri-devel


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@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@vger.kernel.org> # v4.11+
>>> Signed-off-by: Kevin Hao <kexin.hao@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;
>>>  }
>>>  
>>>
>>
>>


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-10-12  9:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-10 12:09 [PATCH] drm: tilcdc: Calculate the vrefresh if it is not set by userspace Kevin Hao
2017-10-10 13:58 ` Jyri Sarha
2017-10-11  2:57   ` Kevin Hao
2017-10-12  9:36     ` Jyri Sarha

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.