diff mbox

[v2,2/3] fbcon: use the cursor blink interval provided by vt

Message ID 20150519214011.GA27047@ulmo.nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding May 19, 2015, 9:40 p.m. UTC
On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
> > vt now provides a cursor blink interval via vc_data. Use this
> > interval instead of the currently hardcoded 200 msecs. Store it in
> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> >
> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> > Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> This patch hit next-20150519 in the form of commit 27a4c827c34a
> (fbcon: use the cursor blink interval provided by vt) and has caused
> boot failure on a handful of ARM platforms when booting a MMC root
> filesystem.  This error was spotted by the kernelci.org bot on
> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> some tegra platforms too.
> 
> Thierry spotted this commit as a potential cause, and both Daniel and
> I have reverted and boot tested on exynos5 and tegra respectively and
> the boot panics disappear.

FWIW, if I apply the below on top of next-20150519 things seem to be
back to normal as well:

Comments

Kevin Hilman May 19, 2015, 9:45 p.m. UTC | #1
On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
>> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
>> > vt now provides a cursor blink interval via vc_data. Use this
>> > interval instead of the currently hardcoded 200 msecs. Store it in
>> > fbcon_ops to avoid locking the console in cursor_timer_handler().
>> >
>> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
>> > Acked-by: Pavel Machek <pavel@ucw.cz>
>>
>> This patch hit next-20150519 in the form of commit 27a4c827c34a
>> (fbcon: use the cursor blink interval provided by vt) and has caused
>> boot failure on a handful of ARM platforms when booting a MMC root
>> filesystem.  This error was spotted by the kernelci.org bot on
>> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
>> some tegra platforms too.
>>
>> Thierry spotted this commit as a potential cause, and both Daniel and
>> I have reverted and boot tested on exynos5 and tegra respectively and
>> the boot panics disappear.
>
> FWIW, if I apply the below on top of next-20150519 things seem to be
> back to normal as well:
>
> diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> index 05b1d1a71ef9..658c34bb9076 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
>                 return;
>
>         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> -       fbcon_del_cursor_timer(info);
> -       if (!(vc->vc_cursor_type & 0x10))
> +       if (vc->vc_cursor_type & 0x10)
> +               fbcon_del_cursor_timer(info);
> +       else
>                 fbcon_add_cursor_timer(info);
>
>         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;

Applying this on next-20150519 makes my exynos board happily boot again as well.

Tested-by: Kevin Hilman <khilman@linaro.org>

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding May 19, 2015, 9:52 p.m. UTC | #2
On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
> >> > vt now provides a cursor blink interval via vc_data. Use this
> >> > interval instead of the currently hardcoded 200 msecs. Store it in
> >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> >> >
> >> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> >> > Acked-by: Pavel Machek <pavel@ucw.cz>
> >>
> >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> >> (fbcon: use the cursor blink interval provided by vt) and has caused
> >> boot failure on a handful of ARM platforms when booting a MMC root
> >> filesystem.  This error was spotted by the kernelci.org bot on
> >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> >> some tegra platforms too.
> >>
> >> Thierry spotted this commit as a potential cause, and both Daniel and
> >> I have reverted and boot tested on exynos5 and tegra respectively and
> >> the boot panics disappear.
> >
> > FWIW, if I apply the below on top of next-20150519 things seem to be
> > back to normal as well:
> >
> > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > index 05b1d1a71ef9..658c34bb9076 100644
> > --- a/drivers/video/console/fbcon.c
> > +++ b/drivers/video/console/fbcon.c
> > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> >                 return;
> >
> >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > -       fbcon_del_cursor_timer(info);
> > -       if (!(vc->vc_cursor_type & 0x10))
> > +       if (vc->vc_cursor_type & 0x10)
> > +               fbcon_del_cursor_timer(info);
> > +       else
> >                 fbcon_add_cursor_timer(info);
> >
> >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> 
> Applying this on next-20150519 makes my exynos board happily boot again as well.
> 
> Tested-by: Kevin Hilman <khilman@linaro.org>

Excellent. Greg, Scot, any opinions on whether or not this is the right
thing to do? It restores a bit that looks suspiciously like it snuck in
in the original (at least it isn't documented in the commit message).

Greg, feel free to squash this in if everybody agrees this is good to
go. If you prefer a patch on top let me know and I'll come up with a
proper commit message.

Thierry
Greg KH May 19, 2015, 11:41 p.m. UTC | #3
On Tue, May 19, 2015 at 11:52:29PM +0200, Thierry Reding wrote:
> On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > <thierry.reding@gmail.com> wrote:
> > > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> > >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
> > >> > vt now provides a cursor blink interval via vc_data. Use this
> > >> > interval instead of the currently hardcoded 200 msecs. Store it in
> > >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> > >> >
> > >> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> > >> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > >>
> > >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> > >> (fbcon: use the cursor blink interval provided by vt) and has caused
> > >> boot failure on a handful of ARM platforms when booting a MMC root
> > >> filesystem.  This error was spotted by the kernelci.org bot on
> > >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> > >> some tegra platforms too.
> > >>
> > >> Thierry spotted this commit as a potential cause, and both Daniel and
> > >> I have reverted and boot tested on exynos5 and tegra respectively and
> > >> the boot panics disappear.
> > >
> > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > back to normal as well:
> > >
> > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > index 05b1d1a71ef9..658c34bb9076 100644
> > > --- a/drivers/video/console/fbcon.c
> > > +++ b/drivers/video/console/fbcon.c
> > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > >                 return;
> > >
> > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > -       fbcon_del_cursor_timer(info);
> > > -       if (!(vc->vc_cursor_type & 0x10))
> > > +       if (vc->vc_cursor_type & 0x10)
> > > +               fbcon_del_cursor_timer(info);
> > > +       else
> > >                 fbcon_add_cursor_timer(info);
> > >
> > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > 
> > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > 
> > Tested-by: Kevin Hilman <khilman@linaro.org>
> 
> Excellent. Greg, Scot, any opinions on whether or not this is the right
> thing to do? It restores a bit that looks suspiciously like it snuck in
> in the original (at least it isn't documented in the commit message).
> 
> Greg, feel free to squash this in if everybody agrees this is good to
> go. If you prefer a patch on top let me know and I'll come up with a
> proper commit message.

Please send a real patch and I'll apply it on top, as I can't rebase my
public tree.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scot Doyle May 20, 2015, 12:36 a.m. UTC | #4
On Tue, 19 May 2015, Thierry Reding wrote:
> On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > <thierry.reding@gmail.com> wrote:

...

> > >
> > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > back to normal as well:
> > >
> > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > index 05b1d1a71ef9..658c34bb9076 100644
> > > --- a/drivers/video/console/fbcon.c
> > > +++ b/drivers/video/console/fbcon.c
> > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > >                 return;
> > >
> > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > -       fbcon_del_cursor_timer(info);
> > > -       if (!(vc->vc_cursor_type & 0x10))
> > > +       if (vc->vc_cursor_type & 0x10)
> > > +               fbcon_del_cursor_timer(info);
> > > +       else
> > >                 fbcon_add_cursor_timer(info);
> > >
> > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > 
> > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > 
> > Tested-by: Kevin Hilman <khilman@linaro.org>
> 
> Excellent. Greg, Scot, any opinions on whether or not this is the right
> thing to do? It restores a bit that looks suspiciously like it snuck in
> in the original (at least it isn't documented in the commit message).
> 
> Greg, feel free to squash this in if everybody agrees this is good to
> go. If you prefer a patch on top let me know and I'll come up with a
> proper commit message.
> 
> Thierry

Hi all, sorry for the trouble.

The timer delete was to prevent blink stutter when updating the interval. 
Since the stutter isn't so noticable when changing from the default 200ms, 
and since most people seem to prefer leaving the fbcon code alone if 
possible, I agree with Thierry's approach.

Tested-by: Scot Doyle <lkml14@scotdoyle.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 05b1d1a71ef9..658c34bb9076 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -1310,8 +1310,9 @@  static void fbcon_cursor(struct vc_data *vc, int mode)
                return;
 
        ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-       fbcon_del_cursor_timer(info);
-       if (!(vc->vc_cursor_type & 0x10))
+       if (vc->vc_cursor_type & 0x10)
+               fbcon_del_cursor_timer(info);
+       else
                fbcon_add_cursor_timer(info);
 
        ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;