Re: [PATCH 2/6] backlight/generic-bl: remove DRIVER1 state
On Wed, 25 Apr 2018, Daniel Vetterwrote: > Nothing in the entire tree ever sets this, which means this is dead > code. Remove it. > > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han > Acked-by: Daniel Thompson > Signed-off-by: Daniel Vetter Reviewed-by: Jani Nikula > --- > drivers/video/backlight/generic_bl.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/video/backlight/generic_bl.c > b/drivers/video/backlight/generic_bl.c > index 67dfb939a514..4dea91acea13 100644 > --- a/drivers/video/backlight/generic_bl.c > +++ b/drivers/video/backlight/generic_bl.c > @@ -21,9 +21,6 @@ static int genericbl_intensity; > static struct backlight_device *generic_backlight_device; > static struct generic_bl_info *bl_machinfo; > > -/* Flag to signal when the battery is low */ > -#define GENERICBL_BATTLOW BL_CORE_DRIVER1 > - > static int genericbl_send_intensity(struct backlight_device *bd) > { > int intensity = bd->props.brightness; > @@ -34,8 +31,6 @@ static int genericbl_send_intensity(struct backlight_device > *bd) > intensity = 0; > if (bd->props.state & BL_CORE_SUSPENDED) > intensity = 0; > - if (bd->props.state & GENERICBL_BATTLOW) > - intensity &= bl_machinfo->limit_mask; > > bl_machinfo->set_bl_intensity(intensity); -- Jani Nikula, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] backlight/generic-bl: remove DRIVER1 state
Nothing in the entire tree ever sets this, which means this is dead code. Remove it. Cc: Lee JonesCc: Daniel Thompson Cc: Jingoo Han Acked-by: Daniel Thompson Signed-off-by: Daniel Vetter --- drivers/video/backlight/generic_bl.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/video/backlight/generic_bl.c b/drivers/video/backlight/generic_bl.c index 67dfb939a514..4dea91acea13 100644 --- a/drivers/video/backlight/generic_bl.c +++ b/drivers/video/backlight/generic_bl.c @@ -21,9 +21,6 @@ static int genericbl_intensity; static struct backlight_device *generic_backlight_device; static struct generic_bl_info *bl_machinfo; -/* Flag to signal when the battery is low */ -#define GENERICBL_BATTLOW BL_CORE_DRIVER1 - static int genericbl_send_intensity(struct backlight_device *bd) { int intensity = bd->props.brightness; @@ -34,8 +31,6 @@ static int genericbl_send_intensity(struct backlight_device *bd) intensity = 0; if (bd->props.state & BL_CORE_SUSPENDED) intensity = 0; - if (bd->props.state & GENERICBL_BATTLOW) - intensity &= bl_machinfo->limit_mask; bl_machinfo->set_bl_intensity(intensity); -- 2.17.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/6] backlight/generic-bl: remove DRIVER1 state
On 17 January 2018 at 16:37, Daniel Thompsonwrote: > > > On 17/01/18 14:36, Emil Velikov wrote: >> >> On 17 January 2018 at 14:01, Daniel Vetter wrote: >>> >>> Nothing in the entire tree ever sets this, which means this is dead >>> code. Remove it. >>> >>> Cc: Lee Jones >>> Cc: Daniel Thompson >>> Cc: Jingoo Han >>> Signed-off-by: Daniel Vetter >>> --- >>> drivers/video/backlight/generic_bl.c | 5 - >> >> >> Fly-by comment, while waiting for coffee to kick-in. >> I think this patch should be after pandora/others have stopped abusing >> BL_CORE_DRIVER1. > > > You sure? > > I can't see why the use or disuse of BL_CORE_DRIVER1 in this driver should > influence another independent driver. > Right my bad. Got mislead by the driver name. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/6] backlight/generic-bl: remove DRIVER1 state
On 17/01/18 17:13, Daniel Vetter wrote: On Wed, Jan 17, 2018 at 04:44:00PM +, Daniel Thompson wrote: On 17/01/18 14:01, Daniel Vetter wrote: Nothing in the entire tree ever sets this, which means this is dead code. Remove it. Cc: Lee JonesCc: Daniel Thompson Cc: Jingoo Han Signed-off-by: Daniel Vetter Not sure whether to ack this one or not. There is nothing wrong with the change but having taken a closer look the driver seems like it exists mostly to allow mach-XXX code to plug in function pointers and we don't do that sort of thing any more. I think the entire driver is dead code! Well I can also supply a patch to outright nuke the code, but figuring out whether that's the right thing to do is definitely way above may pay grade :-) I don't believe that for a second. ;-) However after a bit more thought I think its best to take this patch as is and we can remove generic-bl after. Whilst I'm confident this code is not used, nuking it after your cleanups would result in a simpler revert if I were wrong. So... Acked-by: Daniel Thompson I only really stitched these together after a long discussion with Meghana about why backlight seems to have 3+ different ways to enable/disable a backlight. Just trying to help a bit with getting the backlight_enable/disable stuff going, so that long-term, at least for newer drivers, we have one blessed way to do that. btw that kind of display pm simplification matches what we've done when implementing atomic modesetting about 3 years ago: We've smashed all the various power states drm (and fbdev/fbcon) knew about into a simple "is it on?" boolean. Todays digital hw doesn't really know anything in-between. Ofc there's tons of components to switch on/off to get the entire display pipe up, and they might want different autosuspend delays to optimize the overall system, but that's orthogonal (well, driver internal implementation detail) really. Cheers, Daniel Daniel. --- drivers/video/backlight/generic_bl.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/video/backlight/generic_bl.c b/drivers/video/backlight/generic_bl.c index 67dfb939a514..4dea91acea13 100644 --- a/drivers/video/backlight/generic_bl.c +++ b/drivers/video/backlight/generic_bl.c @@ -21,9 +21,6 @@ static int genericbl_intensity; static struct backlight_device *generic_backlight_device; static struct generic_bl_info *bl_machinfo; -/* Flag to signal when the battery is low */ -#define GENERICBL_BATTLOW BL_CORE_DRIVER1 - static int genericbl_send_intensity(struct backlight_device *bd) { int intensity = bd->props.brightness; @@ -34,8 +31,6 @@ static int genericbl_send_intensity(struct backlight_device *bd) intensity = 0; if (bd->props.state & BL_CORE_SUSPENDED) intensity = 0; -if (bd->props.state & GENERICBL_BATTLOW) - intensity &= bl_machinfo->limit_mask; bl_machinfo->set_bl_intensity(intensity); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/6] backlight/generic-bl: remove DRIVER1 state
On Wed, Jan 17, 2018 at 04:44:00PM +, Daniel Thompson wrote: > On 17/01/18 14:01, Daniel Vetter wrote: > > Nothing in the entire tree ever sets this, which means this is dead > > code. Remove it. > > > > Cc: Lee Jones> > Cc: Daniel Thompson > > Cc: Jingoo Han > > Signed-off-by: Daniel Vetter > > Not sure whether to ack this one or not. > > There is nothing wrong with the change but having taken a closer look the > driver seems like it exists mostly to allow mach-XXX code to plug in > function pointers and we don't do that sort of thing any more. > > I think the entire driver is dead code! Well I can also supply a patch to outright nuke the code, but figuring out whether that's the right thing to do is definitely way above may pay grade :-) I only really stitched these together after a long discussion with Meghana about why backlight seems to have 3+ different ways to enable/disable a backlight. Just trying to help a bit with getting the backlight_enable/disable stuff going, so that long-term, at least for newer drivers, we have one blessed way to do that. btw that kind of display pm simplification matches what we've done when implementing atomic modesetting about 3 years ago: We've smashed all the various power states drm (and fbdev/fbcon) knew about into a simple "is it on?" boolean. Todays digital hw doesn't really know anything in-between. Ofc there's tons of components to switch on/off to get the entire display pipe up, and they might want different autosuspend delays to optimize the overall system, but that's orthogonal (well, driver internal implementation detail) really. Cheers, Daniel > > > Daniel. > > > > --- drivers/video/backlight/generic_bl.c | 5 - 1 file changed, 5 > > deletions(-) > > > > diff --git a/drivers/video/backlight/generic_bl.c > > b/drivers/video/backlight/generic_bl.c index > > 67dfb939a514..4dea91acea13 100644 --- > > a/drivers/video/backlight/generic_bl.c +++ > > b/drivers/video/backlight/generic_bl.c @@ -21,9 +21,6 @@ static int > > genericbl_intensity; static struct backlight_device > > *generic_backlight_device; static struct generic_bl_info *bl_machinfo; > > -/* Flag to signal when the battery is low */ -#define > > GENERICBL_BATTLOW BL_CORE_DRIVER1 - static int > > genericbl_send_intensity(struct backlight_device *bd) { int intensity > > = bd->props.brightness; @@ -34,8 +31,6 @@ static int > > genericbl_send_intensity(struct backlight_device *bd) intensity = 0; > > if (bd->props.state & BL_CORE_SUSPENDED) intensity = 0; - if > > (bd->props.state & GENERICBL_BATTLOW) - intensity &= > > bl_machinfo->limit_mask; bl_machinfo->set_bl_intensity(intensity); > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/6] backlight/generic-bl: remove DRIVER1 state
On 17/01/18 14:01, Daniel Vetter wrote: Nothing in the entire tree ever sets this, which means this is dead code. Remove it. Cc: Lee JonesCc: Daniel Thompson Cc: Jingoo Han Signed-off-by: Daniel Vetter Not sure whether to ack this one or not. There is nothing wrong with the change but having taken a closer look the driver seems like it exists mostly to allow mach-XXX code to plug in function pointers and we don't do that sort of thing any more. I think the entire driver is dead code! Daniel. --- drivers/video/backlight/generic_bl.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/video/backlight/generic_bl.c b/drivers/video/backlight/generic_bl.c index 67dfb939a514..4dea91acea13 100644 --- a/drivers/video/backlight/generic_bl.c +++ b/drivers/video/backlight/generic_bl.c @@ -21,9 +21,6 @@ static int genericbl_intensity; static struct backlight_device *generic_backlight_device; static struct generic_bl_info *bl_machinfo; -/* Flag to signal when the battery is low */ -#define GENERICBL_BATTLOW BL_CORE_DRIVER1 - static int genericbl_send_intensity(struct backlight_device *bd) { int intensity = bd->props.brightness; @@ -34,8 +31,6 @@ static int genericbl_send_intensity(struct backlight_device *bd) intensity = 0; if (bd->props.state & BL_CORE_SUSPENDED) intensity = 0; - if (bd->props.state & GENERICBL_BATTLOW) - intensity &= bl_machinfo->limit_mask; bl_machinfo->set_bl_intensity(intensity); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/6] backlight/generic-bl: remove DRIVER1 state
On 17/01/18 14:36, Emil Velikov wrote: On 17 January 2018 at 14:01, Daniel Vetterwrote: Nothing in the entire tree ever sets this, which means this is dead code. Remove it. Cc: Lee Jones Cc: Daniel Thompson Cc: Jingoo Han Signed-off-by: Daniel Vetter --- drivers/video/backlight/generic_bl.c | 5 - Fly-by comment, while waiting for coffee to kick-in. I think this patch should be after pandora/others have stopped abusing BL_CORE_DRIVER1. You sure? I can't see why the use or disuse of BL_CORE_DRIVER1 in this driver should influence another independent driver. Daniel. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/6] backlight/generic-bl: remove DRIVER1 state
On 17 January 2018 at 14:01, Daniel Vetterwrote: > Nothing in the entire tree ever sets this, which means this is dead > code. Remove it. > > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han > Signed-off-by: Daniel Vetter > --- > drivers/video/backlight/generic_bl.c | 5 - Fly-by comment, while waiting for coffee to kick-in. I think this patch should be after pandora/others have stopped abusing BL_CORE_DRIVER1. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] backlight/generic-bl: remove DRIVER1 state
Nothing in the entire tree ever sets this, which means this is dead code. Remove it. Cc: Lee JonesCc: Daniel Thompson Cc: Jingoo Han Signed-off-by: Daniel Vetter --- drivers/video/backlight/generic_bl.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/video/backlight/generic_bl.c b/drivers/video/backlight/generic_bl.c index 67dfb939a514..4dea91acea13 100644 --- a/drivers/video/backlight/generic_bl.c +++ b/drivers/video/backlight/generic_bl.c @@ -21,9 +21,6 @@ static int genericbl_intensity; static struct backlight_device *generic_backlight_device; static struct generic_bl_info *bl_machinfo; -/* Flag to signal when the battery is low */ -#define GENERICBL_BATTLOW BL_CORE_DRIVER1 - static int genericbl_send_intensity(struct backlight_device *bd) { int intensity = bd->props.brightness; @@ -34,8 +31,6 @@ static int genericbl_send_intensity(struct backlight_device *bd) intensity = 0; if (bd->props.state & BL_CORE_SUSPENDED) intensity = 0; - if (bd->props.state & GENERICBL_BATTLOW) - intensity &= bl_machinfo->limit_mask; bl_machinfo->set_bl_intensity(intensity); -- 2.15.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel