Message ID | 20190520082216.26273-28-daniel.vetter@ffwll.ch (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | fbcon notifier begone! | expand |
Hi Daniel. > With the recursion broken in the previous patch we can drop the > FBINFO_MISC_USEREVENT flag around calls to fb_blank - recursion > prevention was it's only job. > When grepping for FBINFO_MISC_USEREVENT I get a few hits not addressed in the patch below: drivers/video/fbdev/core/fbcon.c: if (!(info->flags & FBINFO_MISC_USEREVENT)) drivers/video/fbdev/core/fbmem.c: if (!ret && (flags & FBINFO_MISC_USEREVENT)) { drivers/video/fbdev/core/fbmem.c: info->flags &= ~FBINFO_MISC_USEREVENT; drivers/video/fbdev/core/fbmem.c: info->flags |= FBINFO_MISC_USEREVENT; drivers/video/fbdev/core/fbmem.c: info->flags &= ~FBINFO_MISC_USEREVENT; drivers/video/fbdev/core/fbmem.c: info->flags |= FBINFO_MISC_USEREVENT; drivers/video/fbdev/core/fbmem.c: info->flags &= ~FBINFO_MISC_USEREVENT; drivers/video/fbdev/core/fbsysfs.c: fb_info->flags |= FBINFO_MISC_USEREVENT; drivers/video/fbdev/core/fbsysfs.c: fb_info->flags &= ~FBINFO_MISC_USEREVENT; drivers/video/fbdev/core/fbsysfs.c: fb_info->flags |= FBINFO_MISC_USEREVENT; drivers/video/fbdev/core/fbsysfs.c: fb_info->flags &= ~FBINFO_MISC_USEREVENT; drivers/video/fbdev/ps3fb.c: info->flags |= FBINFO_MISC_USEREVENT; drivers/video/fbdev/ps3fb.c: info->flags &= ~FBINFO_MISC_USEREVENT; drivers/video/fbdev/sh_mobile_lcdcfb.c: * FBINFO_MISC_USEREVENT flag is set. Since we do not want to fake a include/linux/fb.h:#define FBINFO_MISC_USEREVENT 0x10000 /* event request The use in ps3fb looks like a candidate for removal and this file is not touch in this patch series, so I guess I did not miss it. As I did not apply the full series maybe some of the other users was already taken care of. Sam > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Yisheng Xie <ysxie@foxmail.com> > Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl> > Cc: Peter Rosin <peda@axentia.se> > Cc: Mikulas Patocka <mpatocka@redhat.com> > Cc: Rob Clark <robdclark@gmail.com> > --- > drivers/video/fbdev/core/fbcon.c | 5 ++--- > drivers/video/fbdev/core/fbmem.c | 3 --- > drivers/video/fbdev/core/fbsysfs.c | 2 -- > 3 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > index f85d794a3bee..c1a7476e980f 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -2382,9 +2382,8 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch) > fbcon_cursor(vc, blank ? CM_ERASE : CM_DRAW); > ops->cursor_flash = (!blank); > > - if (!(info->flags & FBINFO_MISC_USEREVENT)) > - if (fb_blank(info, blank)) > - fbcon_generic_blank(vc, info, blank); > + if (fb_blank(info, blank)) > + fbcon_generic_blank(vc, info, blank); > } > > if (!blank) > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > index 7f95c7e80155..65a075ccac4a 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1194,10 +1194,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, > case FBIOBLANK: > console_lock(); > lock_fb_info(info); > - info->flags |= FBINFO_MISC_USEREVENT; > ret = fb_blank(info, arg); > - info->flags &= ~FBINFO_MISC_USEREVENT; > - > /* might again call into fb_blank */ > fbcon_fb_blanked(info, arg); > unlock_fb_info(info); > diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c > index 252d4f52d2a5..882b471d619e 100644 > --- a/drivers/video/fbdev/core/fbsysfs.c > +++ b/drivers/video/fbdev/core/fbsysfs.c > @@ -310,9 +310,7 @@ static ssize_t store_blank(struct device *device, > > arg = simple_strtoul(buf, &last, 0); > console_lock(); > - fb_info->flags |= FBINFO_MISC_USEREVENT; > err = fb_blank(fb_info, arg); > - fb_info->flags &= ~FBINFO_MISC_USEREVENT; > /* might again call into fb_blank */ > fbcon_fb_blanked(fb_info, arg); > console_unlock(); > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, May 20, 2019 at 7:20 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Daniel. > > > With the recursion broken in the previous patch we can drop the > > FBINFO_MISC_USEREVENT flag around calls to fb_blank - recursion > > prevention was it's only job. > > > When grepping for FBINFO_MISC_USEREVENT I get a few hits not addressed > in the patch below: > > drivers/video/fbdev/core/fbcon.c: if (!(info->flags & FBINFO_MISC_USEREVENT)) > drivers/video/fbdev/core/fbmem.c: if (!ret && (flags & FBINFO_MISC_USEREVENT)) { > drivers/video/fbdev/core/fbmem.c: info->flags &= ~FBINFO_MISC_USEREVENT; > drivers/video/fbdev/core/fbmem.c: info->flags |= FBINFO_MISC_USEREVENT; > drivers/video/fbdev/core/fbmem.c: info->flags &= ~FBINFO_MISC_USEREVENT; > drivers/video/fbdev/core/fbmem.c: info->flags |= FBINFO_MISC_USEREVENT; > drivers/video/fbdev/core/fbmem.c: info->flags &= ~FBINFO_MISC_USEREVENT; > drivers/video/fbdev/core/fbsysfs.c: fb_info->flags |= FBINFO_MISC_USEREVENT; > drivers/video/fbdev/core/fbsysfs.c: fb_info->flags &= ~FBINFO_MISC_USEREVENT; > drivers/video/fbdev/core/fbsysfs.c: fb_info->flags |= FBINFO_MISC_USEREVENT; > drivers/video/fbdev/core/fbsysfs.c: fb_info->flags &= ~FBINFO_MISC_USEREVENT; > drivers/video/fbdev/ps3fb.c: info->flags |= FBINFO_MISC_USEREVENT; > drivers/video/fbdev/ps3fb.c: info->flags &= ~FBINFO_MISC_USEREVENT; > drivers/video/fbdev/sh_mobile_lcdcfb.c: * FBINFO_MISC_USEREVENT flag is set. Since we do not want to fake a > include/linux/fb.h:#define FBINFO_MISC_USEREVENT 0x10000 /* event request > > The use in ps3fb looks like a candidate for removal and this file is not > touch in this patch series, so I guess I did not miss it. > > As I did not apply the full series maybe some of the other users was > already taken care of. It's also used to break recursion around fb_set_par and fb_set_pan. Untangling that one would be possible, but also requires untangling some locking, so a lot more work. If you chase all the call paths then you'll noticed that the users still left have no overlap with the ones I'm removing here. -Daniel > > > Sam > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > Cc: Hans de Goede <hdegoede@redhat.com> > > Cc: Yisheng Xie <ysxie@foxmail.com> > > Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl> > > Cc: Peter Rosin <peda@axentia.se> > > Cc: Mikulas Patocka <mpatocka@redhat.com> > > Cc: Rob Clark <robdclark@gmail.com> > > --- > > drivers/video/fbdev/core/fbcon.c | 5 ++--- > > drivers/video/fbdev/core/fbmem.c | 3 --- > > drivers/video/fbdev/core/fbsysfs.c | 2 -- > > 3 files changed, 2 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > > index f85d794a3bee..c1a7476e980f 100644 > > --- a/drivers/video/fbdev/core/fbcon.c > > +++ b/drivers/video/fbdev/core/fbcon.c > > @@ -2382,9 +2382,8 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch) > > fbcon_cursor(vc, blank ? CM_ERASE : CM_DRAW); > > ops->cursor_flash = (!blank); > > > > - if (!(info->flags & FBINFO_MISC_USEREVENT)) > > - if (fb_blank(info, blank)) > > - fbcon_generic_blank(vc, info, blank); > > + if (fb_blank(info, blank)) > > + fbcon_generic_blank(vc, info, blank); > > } > > > > if (!blank) > > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > > index 7f95c7e80155..65a075ccac4a 100644 > > --- a/drivers/video/fbdev/core/fbmem.c > > +++ b/drivers/video/fbdev/core/fbmem.c > > @@ -1194,10 +1194,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, > > case FBIOBLANK: > > console_lock(); > > lock_fb_info(info); > > - info->flags |= FBINFO_MISC_USEREVENT; > > ret = fb_blank(info, arg); > > - info->flags &= ~FBINFO_MISC_USEREVENT; > > - > > /* might again call into fb_blank */ > > fbcon_fb_blanked(info, arg); > > unlock_fb_info(info); > > diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c > > index 252d4f52d2a5..882b471d619e 100644 > > --- a/drivers/video/fbdev/core/fbsysfs.c > > +++ b/drivers/video/fbdev/core/fbsysfs.c > > @@ -310,9 +310,7 @@ static ssize_t store_blank(struct device *device, > > > > arg = simple_strtoul(buf, &last, 0); > > console_lock(); > > - fb_info->flags |= FBINFO_MISC_USEREVENT; > > err = fb_blank(fb_info, arg); > > - fb_info->flags &= ~FBINFO_MISC_USEREVENT; > > /* might again call into fb_blank */ > > fbcon_fb_blanked(fb_info, arg); > > console_unlock(); > > -- > > 2.20.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Daniel. On Mon, May 20, 2019 at 07:29:52PM +0200, Daniel Vetter wrote: > On Mon, May 20, 2019 at 7:20 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > > > Hi Daniel. > > > > > With the recursion broken in the previous patch we can drop the > > > FBINFO_MISC_USEREVENT flag around calls to fb_blank - recursion > > > prevention was it's only job. > > > > > When grepping for FBINFO_MISC_USEREVENT I get a few hits not addressed > > in the patch below: > > > > drivers/video/fbdev/core/fbcon.c: if (!(info->flags & FBINFO_MISC_USEREVENT)) > > drivers/video/fbdev/core/fbmem.c: if (!ret && (flags & FBINFO_MISC_USEREVENT)) { > > drivers/video/fbdev/core/fbmem.c: info->flags &= ~FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/core/fbmem.c: info->flags |= FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/core/fbmem.c: info->flags &= ~FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/core/fbmem.c: info->flags |= FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/core/fbmem.c: info->flags &= ~FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/core/fbsysfs.c: fb_info->flags |= FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/core/fbsysfs.c: fb_info->flags &= ~FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/core/fbsysfs.c: fb_info->flags |= FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/core/fbsysfs.c: fb_info->flags &= ~FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/ps3fb.c: info->flags |= FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/ps3fb.c: info->flags &= ~FBINFO_MISC_USEREVENT; > > drivers/video/fbdev/sh_mobile_lcdcfb.c: * FBINFO_MISC_USEREVENT flag is set. Since we do not want to fake a > > include/linux/fb.h:#define FBINFO_MISC_USEREVENT 0x10000 /* event request > > > > The use in ps3fb looks like a candidate for removal and this file is not > > touch in this patch series, so I guess I did not miss it. > > > > As I did not apply the full series maybe some of the other users was > > already taken care of. > > It's also used to break recursion around fb_set_par and fb_set_pan. > Untangling that one would be possible, but also requires untangling > some locking, so a lot more work. If you chase all the call paths then > you'll noticed that the users still left have no overlap with the ones > I'm removing here. Now you spell it out it is obvious. I guess I never read more than fb_ and missed that we are dealing with different calls. Thanks for the quick feedback, and sorry for the noise. Sam
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index f85d794a3bee..c1a7476e980f 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -2382,9 +2382,8 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch) fbcon_cursor(vc, blank ? CM_ERASE : CM_DRAW); ops->cursor_flash = (!blank); - if (!(info->flags & FBINFO_MISC_USEREVENT)) - if (fb_blank(info, blank)) - fbcon_generic_blank(vc, info, blank); + if (fb_blank(info, blank)) + fbcon_generic_blank(vc, info, blank); } if (!blank) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 7f95c7e80155..65a075ccac4a 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1194,10 +1194,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, case FBIOBLANK: console_lock(); lock_fb_info(info); - info->flags |= FBINFO_MISC_USEREVENT; ret = fb_blank(info, arg); - info->flags &= ~FBINFO_MISC_USEREVENT; - /* might again call into fb_blank */ fbcon_fb_blanked(info, arg); unlock_fb_info(info); diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c index 252d4f52d2a5..882b471d619e 100644 --- a/drivers/video/fbdev/core/fbsysfs.c +++ b/drivers/video/fbdev/core/fbsysfs.c @@ -310,9 +310,7 @@ static ssize_t store_blank(struct device *device, arg = simple_strtoul(buf, &last, 0); console_lock(); - fb_info->flags |= FBINFO_MISC_USEREVENT; err = fb_blank(fb_info, arg); - fb_info->flags &= ~FBINFO_MISC_USEREVENT; /* might again call into fb_blank */ fbcon_fb_blanked(fb_info, arg); console_unlock();
With the recursion broken in the previous patch we can drop the FBINFO_MISC_USEREVENT flag around calls to fb_blank - recursion prevention was it's only job. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Cc: Hans de Goede <hdegoede@redhat.com> Cc: Yisheng Xie <ysxie@foxmail.com> Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl> Cc: Peter Rosin <peda@axentia.se> Cc: Mikulas Patocka <mpatocka@redhat.com> Cc: Rob Clark <robdclark@gmail.com> --- drivers/video/fbdev/core/fbcon.c | 5 ++--- drivers/video/fbdev/core/fbmem.c | 3 --- drivers/video/fbdev/core/fbsysfs.c | 2 -- 3 files changed, 2 insertions(+), 8 deletions(-)