Re: [Intel-gfx] [PATCH v5 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()

2023-11-29 Thread Sean Young
On Fri, Nov 24, 2023 at 02:31:18PM +0100, Thierry Reding wrote:
> On Sat, Nov 18, 2023 at 04:16:17PM +, Sean Young wrote:
> > In order to introduce a pwm api which can be used from atomic context,
> > we will need two functions for applying pwm changes:
> > 
> > int pwm_apply_cansleep(struct pwm *, struct pwm_state *);
> > int pwm_apply_atomic(struct pwm *, struct pwm_state *);
> > 
> > This commit just deals with renaming pwm_apply_state(), a following
> > commit will introduce the pwm_apply_atomic() function.
> 
> Sorry, I still don't agree with that _cansleep suffix. I think it's the
> wrong terminology. Just because something can sleep doesn't mean that it
> ever will. "Might sleep" is much more accurate because it says exactly
> what might happen and indicates what we're guarding against.

Sorry, I forgot about this in the last round. I've renamed it _might_sleep
in v6 which I'll post shortly.


Sean


Re: [Intel-gfx] [PATCH v5 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()

2023-11-24 Thread Thierry Reding
On Sat, Nov 18, 2023 at 04:16:17PM +, Sean Young wrote:
> In order to introduce a pwm api which can be used from atomic context,
> we will need two functions for applying pwm changes:
> 
>   int pwm_apply_cansleep(struct pwm *, struct pwm_state *);
>   int pwm_apply_atomic(struct pwm *, struct pwm_state *);
> 
> This commit just deals with renaming pwm_apply_state(), a following
> commit will introduce the pwm_apply_atomic() function.

Sorry, I still don't agree with that _cansleep suffix. I think it's the
wrong terminology. Just because something can sleep doesn't mean that it
ever will. "Might sleep" is much more accurate because it says exactly
what might happen and indicates what we're guarding against.

Thierry


signature.asc
Description: PGP signature


Re: [Intel-gfx] [PATCH v5 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()

2023-11-23 Thread Lee Jones
On Sat, 18 Nov 2023, Sean Young wrote:

> In order to introduce a pwm api which can be used from atomic context,
> we will need two functions for applying pwm changes:
> 
>   int pwm_apply_cansleep(struct pwm *, struct pwm_state *);
>   int pwm_apply_atomic(struct pwm *, struct pwm_state *);
> 
> This commit just deals with renaming pwm_apply_state(), a following
> commit will introduce the pwm_apply_atomic() function.
> 
> Acked-by: Hans de Goede 
> Acked-by: Jani Nikula 
> Signed-off-by: Sean Young 
> ---
>  Documentation/driver-api/pwm.rst  |  8 +++---
>  .../gpu/drm/i915/display/intel_backlight.c|  6 ++--
>  drivers/gpu/drm/solomon/ssd130x.c |  2 +-
>  drivers/hwmon/pwm-fan.c   |  8 +++---
>  drivers/input/misc/da7280.c   |  4 +--
>  drivers/input/misc/pwm-beeper.c   |  4 +--
>  drivers/input/misc/pwm-vibra.c|  8 +++---

>  drivers/leds/leds-pwm.c   |  2 +-
>  drivers/leds/rgb/leds-pwm-multicolor.c|  4 +--

Acked-by: Lee Jones 

>  drivers/media/rc/pwm-ir-tx.c  |  4 +--
>  drivers/platform/x86/lenovo-yogabook.c|  2 +-
>  drivers/pwm/core.c| 18 ++--
>  drivers/pwm/pwm-twl-led.c |  2 +-
>  drivers/pwm/pwm-vt8500.c  |  2 +-
>  drivers/pwm/sysfs.c   | 10 +++
>  drivers/regulator/pwm-regulator.c |  4 +--

>  drivers/video/backlight/lm3630a_bl.c  |  2 +-
>  drivers/video/backlight/lp855x_bl.c   |  2 +-
>  drivers/video/backlight/pwm_bl.c  | 12 

Acked-by: Lee Jones 

>  drivers/video/fbdev/ssd1307fb.c   |  2 +-
>  include/linux/pwm.h   | 28 +--
>  21 files changed, 67 insertions(+), 67 deletions(-)

[...]

-- 
Lee Jones [李琼斯]


[Intel-gfx] [PATCH v5 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()

2023-11-18 Thread Sean Young
In order to introduce a pwm api which can be used from atomic context,
we will need two functions for applying pwm changes:

int pwm_apply_cansleep(struct pwm *, struct pwm_state *);
int pwm_apply_atomic(struct pwm *, struct pwm_state *);

This commit just deals with renaming pwm_apply_state(), a following
commit will introduce the pwm_apply_atomic() function.

Acked-by: Hans de Goede 
Acked-by: Jani Nikula 
Signed-off-by: Sean Young 
---
 Documentation/driver-api/pwm.rst  |  8 +++---
 .../gpu/drm/i915/display/intel_backlight.c|  6 ++--
 drivers/gpu/drm/solomon/ssd130x.c |  2 +-
 drivers/hwmon/pwm-fan.c   |  8 +++---
 drivers/input/misc/da7280.c   |  4 +--
 drivers/input/misc/pwm-beeper.c   |  4 +--
 drivers/input/misc/pwm-vibra.c|  8 +++---
 drivers/leds/leds-pwm.c   |  2 +-
 drivers/leds/rgb/leds-pwm-multicolor.c|  4 +--
 drivers/media/rc/pwm-ir-tx.c  |  4 +--
 drivers/platform/x86/lenovo-yogabook.c|  2 +-
 drivers/pwm/core.c| 18 ++--
 drivers/pwm/pwm-twl-led.c |  2 +-
 drivers/pwm/pwm-vt8500.c  |  2 +-
 drivers/pwm/sysfs.c   | 10 +++
 drivers/regulator/pwm-regulator.c |  4 +--
 drivers/video/backlight/lm3630a_bl.c  |  2 +-
 drivers/video/backlight/lp855x_bl.c   |  2 +-
 drivers/video/backlight/pwm_bl.c  | 12 
 drivers/video/fbdev/ssd1307fb.c   |  2 +-
 include/linux/pwm.h   | 28 +--
 21 files changed, 67 insertions(+), 67 deletions(-)

diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst
index bb264490a87a1..5f6d1540dcd7e 100644
--- a/Documentation/driver-api/pwm.rst
+++ b/Documentation/driver-api/pwm.rst
@@ -41,7 +41,7 @@ the getter, devm_pwm_get() and devm_fwnode_pwm_get(), also 
exist.
 
 After being requested, a PWM has to be configured using::
 
-   int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state);
+   int pwm_apply_cansleep(struct pwm_device *pwm, struct pwm_state *state);
 
 This API controls both the PWM period/duty_cycle config and the
 enable/disable state.
@@ -57,13 +57,13 @@ If supported by the driver, the signal can be optimized, 
for example to improve
 EMI by phase shifting the individual channels of a chip.
 
 The pwm_config(), pwm_enable() and pwm_disable() functions are just wrappers
-around pwm_apply_state() and should not be used if the user wants to change
+around pwm_apply_cansleep() and should not be used if the user wants to change
 several parameter at once. For example, if you see pwm_config() and
 pwm_{enable,disable}() calls in the same function, this probably means you
-should switch to pwm_apply_state().
+should switch to pwm_apply_cansleep().
 
 The PWM user API also allows one to query the PWM state that was passed to the
-last invocation of pwm_apply_state() using pwm_get_state(). Note this is
+last invocation of pwm_apply_cansleep() using pwm_get_state(). Note this is
 different to what the driver has actually implemented if the request cannot be
 satisfied exactly with the hardware in use. There is currently no way for
 consumers to get the actually implemented settings.
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c 
b/drivers/gpu/drm/i915/display/intel_backlight.c
index 2e8f17c045222..cf516190cde8f 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -274,7 +274,7 @@ static void ext_pwm_set_backlight(const struct 
drm_connector_state *conn_state,
struct intel_panel *panel = 
&to_intel_connector(conn_state->connector)->panel;
 
pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
-   pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
+   pwm_apply_cansleep(panel->backlight.pwm, &panel->backlight.pwm_state);
 }
 
 static void
@@ -427,7 +427,7 @@ static void ext_pwm_disable_backlight(const struct 
drm_connector_state *old_conn
intel_backlight_set_pwm_level(old_conn_state, level);
 
panel->backlight.pwm_state.enabled = false;
-   pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
+   pwm_apply_cansleep(panel->backlight.pwm, &panel->backlight.pwm_state);
 }
 
 void intel_backlight_disable(const struct drm_connector_state *old_conn_state)
@@ -749,7 +749,7 @@ static void ext_pwm_enable_backlight(const struct 
intel_crtc_state *crtc_state,
 
pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
panel->backlight.pwm_state.enabled = true;
-   pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
+   pwm_apply_cansleep(panel->backlight.pwm, &panel->backlight.pwm_state);
 }
 
 static void __intel_backlight_enable(const struct intel_crtc_state