Re: [Nouveau] [PATCH v2] bl: fix backlight regression

[Pierre Moreau]

A few words about why this change is needed would be nice, as well as what
changed since the v1 (well, looking back to your original patch, absolutely
everything changed :-D).

On 2018-02-19 — 17:09, Karol Herbst wrote:
> fixes d9c0aadc5aa241df26ce8301d34a8418919fb5ae

The formatting does not follow the kernel guidelines, see
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=91ab883eb21325ad80f3473633f794c78ac87f51#n183
Also, you might want to use the commit hash from Linus’ tree, rather than from
Ben’s out-of-tree module.

> 
> Suggested-by: Ben Skeggs 
> Signed-off-by: Karol Herbst 
> ---
>  drm/nouveau/nouveau_backlight.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
> index 380f3402..55172655 100644
> --- a/drm/nouveau/nouveau_backlight.c
> +++ b/drm/nouveau/nouveau_backlight.c
> @@ -134,7 +134,7 @@ nv50_get_intensity(struct backlight_device *bd)
>   struct nouveau_encoder *nv_encoder = bl_get_data(bd);
>   struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
>   struct nvif_object *device = >client.device.object;
> - int or = nv_encoder->or;
> + int or = ffs(nv_encoder->dcb->or) - 1;

Since you have that change in a few places, wouldn’t it make sense to have it
in an helper function, that given an nv_encoder object would give you back the
correct or value? The function wouldn’t be doing much, I agree, but it would be
less error-prone to forgetting the -1 or using ffs in some cases.

Pierre

>   u32 div = 1025;
>   u32 val;
>  
> @@ -149,7 +149,7 @@ nv50_set_intensity(struct backlight_device *bd)
>   struct nouveau_encoder *nv_encoder = bl_get_data(bd);
>   struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
>   struct nvif_object *device = >client.device.object;
> - int or = nv_encoder->or;
> + int or = ffs(nv_encoder->dcb->or) - 1;
>   u32 div = 1025;
>   u32 val = (bd->props.brightness * div) / 100;
>  
> @@ -170,7 +170,7 @@ nva3_get_intensity(struct backlight_device *bd)
>   struct nouveau_encoder *nv_encoder = bl_get_data(bd);
>   struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
>   struct nvif_object *device = >client.device.object;
> - int or = nv_encoder->or;
> + int or = ffs(nv_encoder->dcb->or) - 1;
>   u32 div, val;
>  
>   div  = nvif_rd32(device, NV50_PDISP_SOR_PWM_DIV(or));
> @@ -188,7 +188,7 @@ nva3_set_intensity(struct backlight_device *bd)
>   struct nouveau_encoder *nv_encoder = bl_get_data(bd);
>   struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
>   struct nvif_object *device = >client.device.object;
> - int or = nv_encoder->or;
> + int or = ffs(nv_encoder->dcb->or) - 1;
>   u32 div, val;
>  
>   div = nvif_rd32(device, NV50_PDISP_SOR_PWM_DIV(or));
> @@ -228,7 +228,7 @@ nv50_backlight_init(struct drm_connector *connector)
>   return -ENODEV;
>   }
>  
> - if (!nvif_rd32(device, NV50_PDISP_SOR_PWM_CTL(nv_encoder->or)))
> + if (!nvif_rd32(device, NV50_PDISP_SOR_PWM_CTL(ffs(nv_encoder->dcb->or) 
> - 1)))
>   return 0;
>  
>   if (drm->client.device.info.chipset <= 0xa0 ||
> -- 
> 2.14.3
> 
> ___
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau


signature.asc
Description: PGP signature
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH v2] bl: fix backlight regression

[Karol Herbst]

fixes d9c0aadc5aa241df26ce8301d34a8418919fb5ae

Suggested-by: Ben Skeggs 
Signed-off-by: Karol Herbst 
---
 drm/nouveau/nouveau_backlight.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index 380f3402..55172655 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -134,7 +134,7 @@ nv50_get_intensity(struct backlight_device *bd)
struct nouveau_encoder *nv_encoder = bl_get_data(bd);
struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
struct nvif_object *device = >client.device.object;
-   int or = nv_encoder->or;
+   int or = ffs(nv_encoder->dcb->or) - 1;
u32 div = 1025;
u32 val;
 
@@ -149,7 +149,7 @@ nv50_set_intensity(struct backlight_device *bd)
struct nouveau_encoder *nv_encoder = bl_get_data(bd);
struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
struct nvif_object *device = >client.device.object;
-   int or = nv_encoder->or;
+   int or = ffs(nv_encoder->dcb->or) - 1;
u32 div = 1025;
u32 val = (bd->props.brightness * div) / 100;
 
@@ -170,7 +170,7 @@ nva3_get_intensity(struct backlight_device *bd)
struct nouveau_encoder *nv_encoder = bl_get_data(bd);
struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
struct nvif_object *device = >client.device.object;
-   int or = nv_encoder->or;
+   int or = ffs(nv_encoder->dcb->or) - 1;
u32 div, val;
 
div  = nvif_rd32(device, NV50_PDISP_SOR_PWM_DIV(or));
@@ -188,7 +188,7 @@ nva3_set_intensity(struct backlight_device *bd)
struct nouveau_encoder *nv_encoder = bl_get_data(bd);
struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
struct nvif_object *device = >client.device.object;
-   int or = nv_encoder->or;
+   int or = ffs(nv_encoder->dcb->or) - 1;
u32 div, val;
 
div = nvif_rd32(device, NV50_PDISP_SOR_PWM_DIV(or));
@@ -228,7 +228,7 @@ nv50_backlight_init(struct drm_connector *connector)
return -ENODEV;
}
 
-   if (!nvif_rd32(device, NV50_PDISP_SOR_PWM_CTL(nv_encoder->or)))
+   if (!nvif_rd32(device, NV50_PDISP_SOR_PWM_CTL(ffs(nv_encoder->dcb->or) 
- 1)))
return 0;
 
if (drm->client.device.info.chipset <= 0xa0 ||
-- 
2.14.3

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau