[PATCH v3] backlight: lp855x: Switch to atomic PWM API

List overview All Threads
Download

newer

older

Re: [PATCH] drm/amd/display: Fix...

dri/drm/kms question with regards...

Maíra Canal

30 Oct 2021 30 Oct '21

1:35 p.m.

Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and replace it for the atomic PWM API.

Signed-off-by: Maíra Canal maira.canal@usp.br --- V1 -> V2: Initialize variable and simply conditional loop V2 -> V3: Fix assignment of NULL variable --- drivers/video/backlight/lp855x_bl.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c index e94932c69f54..627a31d05691 100644 --- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp)

static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) { - unsigned int period = lp->pdata->period_ns; - unsigned int duty = br * period / max_br; - struct pwm_device *pwm; + struct pwm_device *pwm = NULL; + struct pwm_state state;

/* request pwm device with the consumer name */ if (!lp->pwm) { @@ -245,18 +244,16 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)

lp->pwm = pwm;

- /* - * FIXME: pwm_apply_args() should be removed when switching to - * the atomic PWM API. - */ - pwm_apply_args(pwm); + pwm_init_state(lp->pwm, &state); + state.period = lp->pdata->period_ns; }

- pwm_config(lp->pwm, duty, period); - if (duty) - pwm_enable(lp->pwm); - else - pwm_disable(lp->pwm); + pwm_get_state(lp->pwm, &state); + + state.duty_cycle = br * state.period / max_br; + state.enabled = state.duty_cycle; + + pwm_apply_state(lp->pwm, &state); }

static int lp855x_bl_update_status(struct backlight_device *bl)

-- 2.31.1

Show replies by date

kernel test robot

30 Oct 30 Oct

11:43 p.m.

Hi "Maíra,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on lee-backlight/for-backlight-next] [also build test ERROR on v5.15-rc7 next-20211029] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Ma-ra-Canal/backlight-lp855x-Switch... base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git for-backlight-next config: i386-randconfig-a002-20211031 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/f4d53f4d51c78636fc4fd34aecbdcbd1f83f... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Ma-ra-Canal/backlight-lp855x-Switch-to-atomic-PWM-API/20211030-213551 git checkout f4d53f4d51c78636fc4fd34aecbdcbd1f83f656e # save the attached .config to linux build tree mkdir build_dir make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com

All errors (new ones prefixed by >>):

ld: drivers/video/backlight/lp855x_bl.o: in function `lp855x_pwm_ctrl':

...

...
drivers/video/backlight/lp855x_bl.c:253: undefined reference to `__udivdi3'

vim +253 drivers/video/backlight/lp855x_bl.c

233 234 static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) 235 { 236 struct pwm_device *pwm = NULL; 237 struct pwm_state state; 238 239 /* request pwm device with the consumer name */ 240 if (!lp->pwm) { 241 pwm = devm_pwm_get(lp->dev, lp->chipname); 242 if (IS_ERR(pwm)) 243 return; 244 245 lp->pwm = pwm; 246 247 pwm_init_state(lp->pwm, &state); 248 state.period = lp->pdata->period_ns; 249 } 250 251 pwm_get_state(lp->pwm, &state); 252

...

253 state.duty_cycle = br * state.period / max_br;

254 state.enabled = state.duty_cycle; 255 256 pwm_apply_state(lp->pwm, &state); 257 } 258

--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

kernel test robot

31 Oct 31 Oct

3:20 a.m.

Hi "Maíra,

Thank you for the patch! Yet something to improve:

url: https://github.com/0day-ci/linux/commits/Ma-ra-Canal/backlight-lp855x-Switch... base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git for-backlight-next config: m68k-randconfig-r005-20211028 (attached as .config) compiler: m68k-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/f4d53f4d51c78636fc4fd34aecbdcbd1f83f... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Ma-ra-Canal/backlight-lp855x-Switch-to-atomic-PWM-API/20211030-213551 git checkout f4d53f4d51c78636fc4fd34aecbdcbd1f83f656e # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com

All errors (new ones prefixed by >>):

m68k-linux-ld: arch/m68k/kernel/machine_kexec.o: in function `machine_kexec': machine_kexec.c:(.text+0x154): undefined reference to `m68k_mmutype' m68k-linux-ld: machine_kexec.c:(.text+0x15c): undefined reference to `m68k_cputype' m68k-linux-ld: arch/m68k/kernel/relocate_kernel.o:(.m68k_fixup+0x0): undefined reference to `M68K_FIXUP_MEMOFFSET' m68k-linux-ld: arch/m68k/kernel/relocate_kernel.o:(.m68k_fixup+0x8): undefined reference to `M68K_FIXUP_MEMOFFSET' m68k-linux-ld: arch/m68k/kernel/uboot.o: in function `process_uboot_commandline': uboot.c:(.init.text+0x2e): undefined reference to `_init_sp' m68k-linux-ld: drivers/video/backlight/lp855x_bl.o: in function `lp855x_bl_update_status':

...

...
lp855x_bl.c:(.text+0x15c): undefined reference to `__udivdi3'

--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Geert Uytterhoeven

2 Nov 2 Nov

8:39 a.m.

Hi Maíra,

On Sat, Oct 30, 2021 at 3:35 PM Maíra Canal maira.canal@usp.br wrote:

...

Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and replace it for the atomic PWM API.

Signed-off-by: Maíra Canal maira.canal@usp.br

Thanks for your patch!

...

--- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp)

static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) {

  unsigned int period = lp->pdata->period_ns;

  unsigned int duty = br * period / max_br;

```
  struct pwm_device *pwm;
```

```
  struct pwm_device *pwm = NULL;
```

  struct pwm_state state;

  /* request pwm device with the consumer name */
  if (!lp->pwm) {

@@ -245,18 +244,16 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)

            lp->pwm = pwm;

```
          /*
```

           * FIXME: pwm_apply_args() should be removed when switching to

```
           * the atomic PWM API.
```
```
           */
```
```
          pwm_apply_args(pwm);
```

          pwm_init_state(lp->pwm, &state);

          state.period = lp->pdata->period_ns;
  }

```
  pwm_config(lp->pwm, duty, period);
```
```
  if (duty)
```
```
          pwm_enable(lp->pwm);
```
```
  else
```
```
          pwm_disable(lp->pwm);
```

```
  pwm_get_state(lp->pwm, &state);
```

  state.duty_cycle = br * state.period / max_br;

Above is a 64-by-32 division, which should not be open-coded, as that will cause link failures on 32-bit platform (cfr. "undefined reference to `__udivdi3'", as reported by the kernel test robot). Please use div_u64() instead.

...

  state.enabled = state.duty_cycle;
  pwm_apply_state(lp->pwm, &state);
}

static int lp855x_bl_update_status(struct backlight_device *bl)

Gr{oetje,eeting}s,

Geert

-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds

Maíra Canal

3 Nov 3 Nov

12:32 p.m.

Em ter., 2 de nov. de 2021 às 05:39, Geert Uytterhoeven geert@linux-m68k.org escreveu:

...

Hi Maíra,

On Sat, Oct 30, 2021 at 3:35 PM Maíra Canal maira.canal@usp.br wrote:

...
Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and replace it for the atomic PWM API.

Signed-off-by: Maíra Canal maira.canal@usp.br

Thanks for your patch!

...
--- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp)

static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) {
  unsigned int period = lp->pdata->period_ns;
  unsigned int duty = br * period / max_br;
  struct pwm_device *pwm;
  struct pwm_device *pwm = NULL;
  struct pwm_state state;

  /* request pwm device with the consumer name */
  if (!lp->pwm) {
@@ -245,18 +244,16 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
            lp->pwm = pwm;
          /*
           * FIXME: pwm_apply_args() should be removed when switching to
           * the atomic PWM API.
           */
          pwm_apply_args(pwm);
          pwm_init_state(lp->pwm, &state);
          state.period = lp->pdata->period_ns;
  }
  pwm_config(lp->pwm, duty, period);
  if (duty)
          pwm_enable(lp->pwm);
  else
          pwm_disable(lp->pwm);
  pwm_get_state(lp->pwm, &state);
  state.duty_cycle = br * state.period / max_br;
Above is a 64-by-32 division, which should not be open-coded, as that will cause link failures on 32-bit platform (cfr. "undefined reference to `__udivdi3'", as reported by the kernel test robot). Please use div_u64() instead.

Hi Geert,

Thank you for the suggestion! I fixed this problem a bit differently and submitted the v4. I made use of the PWM API and applied the pwm_set_relative_duty_cycle function, which solved this division problem.

...

...
  state.enabled = state.duty_cycle;
  pwm_apply_state(lp->pwm, &state);
}

static int lp855x_bl_update_status(struct backlight_device *bl)
Gr{oetje,eeting}s,
                    Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds

1287

Age (days ago)

1291

Last active (days ago)

dri-devel@lists.freedesktop.org

4 comments

3 participants

tags (0)

participants (3)

Geert Uytterhoeven
kernel test robot
Maíra Canal