Re: [Nouveau] [PATCH v2 1/2] drm/nouveau: Fix ida leak

2021-05-12 Thread Pierre Moreau
Why remove the const modifier on `nb`?

On 2021-05-11 — 16:28, Zhen Lei wrote:
> When the 'nb' value allocated from 'bl_ida' is greater than or equal to
> 100, it will not be released. In fact, we can simplify operations by
> limiting the range of idas that can be applied for.
> 
> By the way, delete the const modifier of the local variable 'nb'.
> 
> Fixes: db1a0ae21461 ("drm/nouveau/bl: Assign different names to interfaces")
> Signed-off-by: Zhen Lei 
> ---
>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index 72f35a2babcb20e..d1c998e645fb4b6 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -51,8 +51,9 @@ static bool
>  nouveau_get_backlight_name(char backlight_name[BL_NAME_SIZE],
>  struct nouveau_backlight *bl)
>  {
> - const int nb = ida_simple_get(_ida, 0, 0, GFP_KERNEL);
> - if (nb < 0 || nb >= 100)
> + int nb = ida_simple_get(_ida, 0, 100, GFP_KERNEL);
> +
> + if (nb < 0)
>   return false;
>   if (nb > 0)
>   snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
> -- 
> 2.26.0.106.g9fadedd
> 
> 
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v2 2/2] drm/nouveau: Fix error return code in nouveau_backlight_init()

2021-05-12 Thread Pierre Moreau
Reviewed-by: Pierre Moreau 

On 2021-05-11 — 16:28, Zhen Lei wrote:
> Fix to return a negative error code from the error handling case instead
> of 0, as done elsewhere in this function.
> 
> Fixes: db1a0ae21461 ("drm/nouveau/bl: Assign different names to interfaces")
> Suggested-by: Pierre Moreau 
> Signed-off-by: Zhen Lei 
> ---
>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index d1c998e645fb4b6..f0856adbe775624 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -47,20 +47,20 @@ struct nouveau_backlight {
>   int id;
>  };
>  
> -static bool
> +static int
>  nouveau_get_backlight_name(char backlight_name[BL_NAME_SIZE],
>  struct nouveau_backlight *bl)
>  {
>   int nb = ida_simple_get(_ida, 0, 100, GFP_KERNEL);
>  
>   if (nb < 0)
> - return false;
> + return nb;
>   if (nb > 0)
>   snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
>   else
>   snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight");
>   bl->id = nb;
> - return true;
> + return 0;
>  }
>  
>  static int
> @@ -273,7 +273,8 @@ nouveau_backlight_init(struct drm_connector *connector)
>   if (!bl)
>   return -ENOMEM;
>  
> - if (!nouveau_get_backlight_name(backlight_name, bl)) {
> + ret = nouveau_get_backlight_name(backlight_name, bl);
> + if (ret) {
>   NV_ERROR(drm, "Failed to retrieve a unique name for the 
> backlight interface\n");
>   goto fail_alloc;
>   }
> -- 
> 2.26.0.106.g9fadedd
> 
> 
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [PATCH 1/1] drm/nouveau: fix error return code in nouveau_backlight_init()

2021-05-08 Thread Pierre Moreau
Hello Zhen,

There was a similar patch sent in last month, though which does not seem to
have been merged yet; see
https://lists.freedesktop.org/archives/nouveau/2021-April/038451.html.

Whether `ret` should be `-ENOSPC` or `-ENOMEM` is hard to say as
`nouveau_get_backlight_name()` could fail due to either.

I will propose an alternative fix which modifies `nouveau_get_backlight_name()`
to return an int so the actual error code can be propagated back instead of
guessed, as well as fix an ida ID leak which I just spotted.

Best,
Pierre

On 2021-05-08 — 11:48, Zhen Lei wrote:
> Fix to return a negative error code from the error handling case instead
> of 0, as done elsewhere in this function.
> 
> Fixes: db1a0ae21461 ("drm/nouveau/bl: Assign different names to interfaces")
> Reported-by: Hulk Robot 
> Signed-off-by: Zhen Lei 
> ---
>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index 72f35a2babcb..097ca344a086 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -273,6 +273,7 @@ nouveau_backlight_init(struct drm_connector *connector)
>   return -ENOMEM;
>  
>   if (!nouveau_get_backlight_name(backlight_name, bl)) {
> + ret = -ENOSPC;
>   NV_ERROR(drm, "Failed to retrieve a unique name for the 
> backlight interface\n");
>   goto fail_alloc;
>   }
> -- 
> 2.25.1
> 
> 


Re: [Nouveau] [PATCH] drm/nouveau: fix an error code in nouveau_backlight_init()

2021-04-19 Thread Pierre Moreau
I can not remember why the original code did return 0 rather than an error, but
-ENOMEM seems indeed way more fitting.

Reviewed-by: Pierre Moreau 

On 2021-04-14 — 08:58, Dan Carpenter wrote:
> If nouveau_get_backlight_name() fails then this should return -ENOMEM
> but currently it returns success.
> 
> Fixes: db1a0ae21461 ("drm/nouveau/bl: Assign different names to interfaces")
> Signed-off-by: Dan Carpenter 
> ---
> This is from static analysis.  In the original commit db1a0ae21461
> ("drm/nouveau/bl: Assign different names to interfaces") then returning
> zero seemed to be a very deliberate choice.  I do think it was wrong
> though and -ENOMEM is the currect return.
> 
>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
> b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index 72f35a2babcb..3786b1c85182 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -274,6 +274,7 @@ nouveau_backlight_init(struct drm_connector *connector)
>  
>   if (!nouveau_get_backlight_name(backlight_name, bl)) {
>   NV_ERROR(drm, "Failed to retrieve a unique name for the 
> backlight interface\n");
> + ret = -ENOMEM;
>   goto fail_alloc;
>   }
>  
> -- 
> 2.30.2
> 
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2] drm/nouveau/mmu: Remove unneeded semicolon

2019-12-18 Thread Pierre Moreau
Reviewed-by: Pierre Moreau 

On 2019-12-18 — 09:28, zhengbin wrote:
> Fixes coccicheck warning:
> 
> drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c:583:2-3: Unneeded semicolon
> drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h:307:2-3: Unneeded semicolon
> 
> Reported-by: Hulk Robot 
> Signed-off-by: zhengbin 
> ---
> v1->v2: add missing one space after the closing curly bracket
>  drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c | 2 +-
>  drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c 
> b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c
> index 41640e0..199f94e 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c
> @@ -580,7 +580,7 @@ nvkm_vmm_iter(struct nvkm_vmm *vmm, const struct 
> nvkm_vmm_page *page,
>   it.pte[it.lvl]++;
>   }
>   }
> - };
> + }
> 
>   nvkm_vmm_flush();
>   return ~0ULL;
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h 
> b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h
> index 5e55ecb..321bfca 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h
> @@ -304,7 +304,7 @@ int tu102_vmm_new(struct nvkm_mmu *, bool, u64, u64, void 
> *, u32,
>   FILL(VMM, PT, PTEI, _ptes, MAP, _addr);\
>   PTEI += _ptes; \
>   PTEN -= _ptes; \
> - }; \
> + }  \
>   nvkm_done((PT)->memory);   \
>  } while(0)
> 
> --
> 2.7.4
> 


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/nouveau/mmu: Remove unneeded semicolon

2019-12-18 Thread Pierre Moreau
With the comment further below addressed, this patch is
Reviewed-by: Pierre Moreau 

On 2019-12-16 — 11:48, zhengbin wrote:
> Fixes coccicheck warning:
> 
> drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c:583:2-3: Unneeded semicolon
> drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h:307:2-3: Unneeded semicolon
> 
> Reported-by: Hulk Robot 
> Signed-off-by: zhengbin 
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c | 2 +-
>  drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c 
> b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c
> index 41640e0..199f94e 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c
> @@ -580,7 +580,7 @@ nvkm_vmm_iter(struct nvkm_vmm *vmm, const struct 
> nvkm_vmm_page *page,
>   it.pte[it.lvl]++;
>   }
>   }
> - };
> + }
> 
>   nvkm_vmm_flush();
>   return ~0ULL;
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h 
> b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h
> index 5e55ecb..321bfca 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h
> @@ -304,7 +304,7 @@ int tu102_vmm_new(struct nvkm_mmu *, bool, u64, u64, void 
> *, u32,
>   FILL(VMM, PT, PTEI, _ptes, MAP, _addr);\
>   PTEI += _ptes; \
>   PTEN -= _ptes; \
> - }; \
> + } \

You are missing one space after the closing curly bracket, to align the
backslash at the end of the line.

Thanks,
Pierre Moreau

>   nvkm_done((PT)->memory);   \
>  } while(0)
> 
> --
> 2.7.4
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] [PATCH -next] drm/nouveau: fix copy-paste error in nouveau_fence_wait_uevent_handler

2019-02-01 Thread Pierre Moreau
Friendly ping, Ben.
I see that in `nouveau_fence_done()` there is a check on `chan` not being NULL
prior to passing it to `nouveau_fence_update()`. Would something similar be
needed here?

Pierre

On 2018-11-15 — 12:14, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/gpu/drm/nouveau/nouveau_fence.c: In function 
> 'nouveau_fence_wait_uevent_handler':
> drivers/gpu/drm/nouveau/nouveau_fence.c:156:27: warning:
>  variable 'chan' set but not used [-Wunused-but-set-variable]
> 
> nouveau_fence_update should use rcu protected 'chan'
> rather than 'fence->channel'
> 
> Fixes: 0ec5f02f0e2c ("drm/nouveau: prevent stale fence->channel pointers, and 
> protect with rcu")
> Signed-off-by: YueHaibing 
> ---
>  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c 
> b/drivers/gpu/drm/nouveau/nouveau_fence.c
> index d4964f3..91286d0 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> @@ -157,7 +157,7 @@
>  
>   fence = list_entry(fctx->pending.next, typeof(*fence), head);
>   chan = rcu_dereference_protected(fence->channel, 
> lockdep_is_held(>lock));
> - if (nouveau_fence_update(fence->channel, fctx))
> + if (nouveau_fence_update(chan, fctx))
>   ret = NVIF_NOTIFY_DROP;
>   }
>   spin_unlock_irqrestore(>lock, flags);
> 
> 
> 
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] 4.16-rc1: UBSAN warning in nouveau/nvkm/subdev/therm/base.c + oops in nvkm_therm_clkgate_fini

2018-02-15 Thread Pierre Moreau
On 2018-02-14 — 09:36, Ilia Mirkin wrote:
> On Wed, Feb 14, 2018 at 9:35 AM, Ilia Mirkin  wrote:
> > On Wed, Feb 14, 2018 at 9:29 AM, Meelis Roos  wrote:
> >>> This is 4.16-rc1+todays git on a lowly P4 with NV5, worked fine in 4.15:
> >>
> >> NV5 in another PC (secondary card in x86-64) made the systrem crash on
> >> boot, in nvkm_therm_clkgate_fini.
> >
> > Mind booting with nouveau.debug=trace? That should hopefully tell us
> > more exactly which thing is dying. If you have a cross-compile/distcc
> > setup handy, a bisect may be even more useful.
> 
> Erm, sorry, nevermind. You even said it -- nvkm_therm_clkgate_fini is
> somehow mis-hooked up for NV5 now. A bisect result would still make
> the culprit a lot more obvious.

CC’ing Lyude Paul as she hooked up the clockgating support.

Looking at the code, only NV40+ do have a therm engine. Therefore, shouldn’t
nvkm_therm_clkgate_enable(), nvkm_therm_clkgate_fini() and
nvkm_therm_clkgate_oneinit() all check for therm being not NULL, on top of
their check for the clkgate_* hooks being there? Or instead, maybe have the
check in nvkm_device_init() nvkm_device_init()?

Pierre


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs

2017-12-29 Thread Pierre Moreau
Ping: this can still be an issue today as kstrtol() & co haven’t changed their
possible return value since the patch was written.

On 2017-07-22 — 14:13, Pierre Moreau wrote:
> Reviewed-by: Pierre Moreau <pierre.mor...@free.fr>
> 
> On 2017-07-17 — 11:17, Dan Carpenter wrote:
> > kstrtol() and friends can return -EINVAL or -ERANGE.  We have to test
> > for both, otherwise the value is possibly uninitialized.  Also in some
> > of these files we accidentally return "count" on error instead of a
> > negative error code.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c 
> > b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > index 7c965648df80..5e75af91c446 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> > @@ -68,9 +68,11 @@ nouveau_hwmon_set_temp1_auto_point1_temp(struct device 
> > *d,
> > struct nouveau_drm *drm = nouveau_drm(dev);
> > struct nvkm_therm *therm = nvxx_therm(>client.device);
> > long value;
> > +   int ret;
> >  
> > -   if (kstrtol(buf, 10, ) == -EINVAL)
> > -   return count;
> > +   ret = kstrtol(buf, 10, );
> > +   if (ret)
> > +   return ret;
> >  
> > therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST,
> > value / 1000);
> > @@ -101,9 +103,11 @@ nouveau_hwmon_set_temp1_auto_point1_temp_hyst(struct 
> > device *d,
> > struct nouveau_drm *drm = nouveau_drm(dev);
> > struct nvkm_therm *therm = nvxx_therm(>client.device);
> > long value;
> > +   int ret;
> >  
> > -   if (kstrtol(buf, 10, ) == -EINVAL)
> > -   return count;
> > +   ret = kstrtol(buf, 10, );
> > +   if (ret)
> > +   return ret;
> >  
> > therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST_HYST,
> > value / 1000);
> > @@ -156,8 +160,9 @@ nouveau_hwmon_set_pwm1_min(struct device *d, struct 
> > device_attribute *a,
> > long value;
> > int ret;
> >  
> > -   if (kstrtol(buf, 10, ) == -EINVAL)
> > -   return -EINVAL;
> > +   ret = kstrtol(buf, 10, );
> > +   if (ret)
> > +   return ret;
> >  
> > ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MIN_DUTY, value);
> > if (ret < 0)
> > @@ -179,8 +184,9 @@ nouveau_hwmon_set_pwm1_max(struct device *d, struct 
> > device_attribute *a,
> > long value;
> > int ret;
> >  
> > -   if (kstrtol(buf, 10, ) == -EINVAL)
> > -   return -EINVAL;
> > +   ret = kstrtol(buf, 10, );
> > +   if (ret)
> > +   return ret;
> >  
> > ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MAX_DUTY, value);
> > if (ret < 0)
> > ___
> > Nouveau mailing list
> > nouv...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau



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



signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] drm/nouveau: Remove no longer needed local ACPI_VIDEO_NOTIFY_PROBE define

2017-12-29 Thread Pierre Moreau
Ping

On 2017-07-22 — 13:54, Pierre Moreau wrote:
> Reviewed-by: Pierre Moreau <pierre.mor...@free.fr>
> 
> On 2017-06-21 — 15:19, Hans de Goede wrote:
> > Hi,
> > 
> > On 26-05-17 09:35, Hans De Goede wrote:
> > > The local #define of ACPI_VIDEO_NOTIFY_PROBE was only added temporarily
> > > to avoid a dependency between the acpi and nouveau trees while merging.
> > > 
> > > This is now properly defined in acpi/video.h and the local #define can
> > > be dropped.
> > > 
> > > Signed-off-by: Hans de Goede <hdego...@redhat.com>
> > 
> > ping, what is the status of this patch ?
> > 
> > Regards,
> > 
> > Hans
> > 
> > 
> > > ---
> > >   drivers/gpu/drm/nouveau/nouveau_display.c | 9 -
> > >   1 file changed, 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
> > > b/drivers/gpu/drm/nouveau/nouveau_display.c
> > > index 549763f5e17d..70de2079b2cc 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> > > @@ -369,15 +369,6 @@ nouveau_display_hpd_work(struct work_struct *work)
> > >   #ifdef CONFIG_ACPI
> > > -/*
> > > - * Hans de Goede: This define belongs in acpi/video.h, I've submitted a 
> > > patch
> > > - * to the acpi subsys to move it there from drivers/acpi/acpi_video.c .
> > > - * This should be dropped once that is merged.
> > > - */
> > > -#ifndef ACPI_VIDEO_NOTIFY_PROBE
> > > -#define ACPI_VIDEO_NOTIFY_PROBE  0x81
> > > -#endif
> > > -
> > >   static int
> > >   nouveau_display_acpi_ntfy(struct notifier_block *nb, unsigned long val,
> > > void *data)
> > > 
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



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



signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] [PATCH] drm/nouveau/imem/nv50: fix incorrect use of refcount API

2017-12-18 Thread Pierre Moreau
Hey Ard,

It seems that Ben already committed a similar patch to his tree (see [0]). I do
not know whether he is planning to have it part of a pull request of fixes for
4.15.

Best regards,
Pierre

[0]: 
https://github.com/skeggsb/nouveau/commit/9068f1df2394f0e4ab2b2a28cac06b462fe0a0aa

On 2017-12-18 — 09:27, Ard Biesheuvel wrote:
> On 8 December 2017 at 19:30, Ard Biesheuvel  wrote:
> > Commit be55287aa5b ("drm/nouveau/imem/nv50: embed nvkm_instobj directly
> > into nv04_instobj") introduced some new calls to the refcount api to
> > the nv50 mapping code. In one particular instance, it does the
> > following:
> >
> > if (!refcount_inc_not_zero(>maps)) {
> > ...
> > refcount_inc(>maps);
> > }
> >
> > i.e., it calls refcount_inc() to increment the refcount when it is known
> > to be zero, which is explicitly forbidden by the API. Instead, use
> > refcount_set() to set it to 1.
> >
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >
> > Apologies if this was already found and fixed. I don't usually follow
> > the DRM or nouveau mailing lists.
> >
> >  drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c 
> > b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c
> > index 1ba7289684aa..db48a1daca0c 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/nv50.c
> > @@ -249,7 +249,7 @@ nv50_instobj_acquire(struct nvkm_memory *memory)
> > iobj->base.memory.ptrs = _instobj_fast;
> > else
> > iobj->base.memory.ptrs = _instobj_slow;
> > -   refcount_inc(>maps);
> > +   refcount_set(>maps, 1);
> > }
> >
> > mutex_unlock(>subdev.mutex);
> > --
> > 2.11.0
> >
> 
> Ping?
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] [PATCH] drm/nouveau: Document nouveau support for Tegra in DRIVER_DESC

2017-10-01 Thread Pierre Moreau
You are right, Tegra should probably be part of the description, even if we do
not support pre-K1 Tegras.

Acked-by: Pierre Moreau <pierre.mor...@free.fr>

On 2017-08-11 — 10:56, Rhys Kidd wrote:
> nouveau supports the Tegra K1 and higher after the SoC-based GPUs converged
> with the main GeForce GPU families.
> 
> Signed-off-by: Rhys Kidd <rhysk...@gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_drv.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h 
> b/drivers/gpu/drm/nouveau/nouveau_drv.h
> index 822fe1d4d35e..fbe42ec0a5f1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drv.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
> @@ -5,7 +5,7 @@
>  #define DRIVER_EMAIL "nouv...@lists.freedesktop.org"
>  
>  #define DRIVER_NAME  "nouveau"
> -#define DRIVER_DESC  "nVidia Riva/TNT/GeForce/Quadro/Tesla"
> +#define DRIVER_DESC  "nVidia Riva/TNT/GeForce/Quadro/Tesla/Tegra"
>  #define DRIVER_DATE  "20120801"
>  
>  #define DRIVER_MAJOR 1
> -- 
> 2.11.0
> 
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] [PATCH] drm/nouveau/hwmon: Uninitialized variables in sysfs

2017-07-22 Thread Pierre Moreau
Reviewed-by: Pierre Moreau <pierre.mor...@free.fr>

On 2017-07-17 — 11:17, Dan Carpenter wrote:
> kstrtol() and friends can return -EINVAL or -ERANGE.  We have to test
> for both, otherwise the value is possibly uninitialized.  Also in some
> of these files we accidentally return "count" on error instead of a
> negative error code.
> 
> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c 
> b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> index 7c965648df80..5e75af91c446 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> @@ -68,9 +68,11 @@ nouveau_hwmon_set_temp1_auto_point1_temp(struct device *d,
>   struct nouveau_drm *drm = nouveau_drm(dev);
>   struct nvkm_therm *therm = nvxx_therm(>client.device);
>   long value;
> + int ret;
>  
> - if (kstrtol(buf, 10, ) == -EINVAL)
> - return count;
> + ret = kstrtol(buf, 10, );
> + if (ret)
> + return ret;
>  
>   therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST,
>   value / 1000);
> @@ -101,9 +103,11 @@ nouveau_hwmon_set_temp1_auto_point1_temp_hyst(struct 
> device *d,
>   struct nouveau_drm *drm = nouveau_drm(dev);
>   struct nvkm_therm *therm = nvxx_therm(>client.device);
>   long value;
> + int ret;
>  
> - if (kstrtol(buf, 10, ) == -EINVAL)
> - return count;
> + ret = kstrtol(buf, 10, );
> + if (ret)
> + return ret;
>  
>   therm->attr_set(therm, NVKM_THERM_ATTR_THRS_FAN_BOOST_HYST,
>   value / 1000);
> @@ -156,8 +160,9 @@ nouveau_hwmon_set_pwm1_min(struct device *d, struct 
> device_attribute *a,
>   long value;
>   int ret;
>  
> - if (kstrtol(buf, 10, ) == -EINVAL)
> - return -EINVAL;
> + ret = kstrtol(buf, 10, );
> + if (ret)
> + return ret;
>  
>   ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MIN_DUTY, value);
>   if (ret < 0)
> @@ -179,8 +184,9 @@ nouveau_hwmon_set_pwm1_max(struct device *d, struct 
> device_attribute *a,
>   long value;
>   int ret;
>  
> - if (kstrtol(buf, 10, ) == -EINVAL)
> - return -EINVAL;
> + ret = kstrtol(buf, 10, );
> + if (ret)
> + return ret;
>  
>   ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MAX_DUTY, value);
>   if (ret < 0)
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: drm/nouveau: Remove no longer needed local ACPI_VIDEO_NOTIFY_PROBE define

2017-07-22 Thread Pierre Moreau
Reviewed-by: Pierre Moreau <pierre.mor...@free.fr>

On 2017-06-21 — 15:19, Hans de Goede wrote:
> Hi,
> 
> On 26-05-17 09:35, Hans De Goede wrote:
> > The local #define of ACPI_VIDEO_NOTIFY_PROBE was only added temporarily
> > to avoid a dependency between the acpi and nouveau trees while merging.
> > 
> > This is now properly defined in acpi/video.h and the local #define can
> > be dropped.
> > 
> > Signed-off-by: Hans de Goede <hdego...@redhat.com>
> 
> ping, what is the status of this patch ?
> 
> Regards,
> 
> Hans
> 
> 
> > ---
> >   drivers/gpu/drm/nouveau/nouveau_display.c | 9 -
> >   1 file changed, 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
> > b/drivers/gpu/drm/nouveau/nouveau_display.c
> > index 549763f5e17d..70de2079b2cc 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> > @@ -369,15 +369,6 @@ nouveau_display_hpd_work(struct work_struct *work)
> >   #ifdef CONFIG_ACPI
> > -/*
> > - * Hans de Goede: This define belongs in acpi/video.h, I've submitted a 
> > patch
> > - * to the acpi subsys to move it there from drivers/acpi/acpi_video.c .
> > - * This should be dropped once that is merged.
> > - */
> > -#ifndef ACPI_VIDEO_NOTIFY_PROBE
> > -#define ACPI_VIDEO_NOTIFY_PROBE0x81
> > -#endif
> > -
> >   static int
> >   nouveau_display_acpi_ntfy(struct notifier_block *nb, unsigned long val,
> >   void *data)
> > 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 2/2] Do not register interface if Apple GMUX detected

2016-12-08 Thread Pierre Moreau
The Apple GMUX is the one managing the backlight, so there is no need for
Nouveau to register its own backlight interface.

v2: Do not split information message on two lines as it prevents from grepping
it, as pointed out by Lukas Wunner

v3: Add a missing end-of-line character to the printed message

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_backlight.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index a34cd35..8b1ca4a 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -30,6 +30,7 @@
  * Register locations derived from NVClock by Roderick Colenbrander
  */

+#include 
 #include 
 #include 

@@ -267,6 +268,11 @@ nouveau_backlight_init(struct drm_device *dev)
struct nvif_device *device = >device;
struct drm_connector *connector;

+   if (apple_gmux_present()) {
+   NV_INFO(drm, "Apple GMUX detected: not registering Nouveau 
backlight interface\n");
+   return 0;
+   }
+
INIT_LIST_HEAD(>bl_connectors);

list_for_each_entry(connector, >mode_config.connector_list, head) {
-- 
2.10.2



[PATCH v4 1/2] nouveau/bl: Assign different names to interfaces

2016-12-08 Thread Pierre Moreau
Currently, every backlight interface created by Nouveau uses the same name,
nv_backlight. This leads to a sysfs warning as it tries to create an already
existing folder. This patch adds a incremented number to the name, but keeps
the initial name as nv_backlight, to avoid possibly breaking userspace; the
second interface will be named nv_backlight1, and so on.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86539

v2:
* Switch to using ida for generating unique IDs, as suggested by Ilia Mirkin;
* Allocate backlight name on the stack, as suggested by Ilia Mirkin;
* Move `nouveau_get_backlight_name()` to avoid forward declaration, as
  suggested by Ilia Mirkin;
* Fix reference to bug report formatting, as reported by Nick Tenney.

v3:
* Define a macro for the size of the backlight name, to avoid defining
  it multiple times;
* Use snprintf in place of sprintf.

v4:
* Do not create similarly named interfaces when reaching the maximum
  amount of unique names, but fail instead, as pointed out by Lukas Wunner

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_backlight.c | 74 ++---
 drm/nouveau/nouveau_display.h   | 10 ++
 drm/nouveau/nouveau_drm.c   |  2 ++
 drm/nouveau/nouveau_drv.h   |  1 +
 4 files changed, 83 insertions(+), 4 deletions(-)

diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index 5e2c568..a34cd35 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -31,11 +31,35 @@
  */

 #include 
+#include 

 #include "nouveau_drv.h"
 #include "nouveau_reg.h"
 #include "nouveau_encoder.h"

+static struct ida bl_ida;
+#define BL_NAME_SIZE 15 // 12 for name + 2 for digits + 1 for '\0'
+
+struct backlight_connector {
+   struct list_head head;
+   int id;
+};
+
+static bool
+nouveau_get_backlight_name(char backlight_name[BL_NAME_SIZE], struct 
backlight_connector
+   *connector)
+{
+   const int nb = ida_simple_get(_ida, 0, 0, GFP_KERNEL);
+   if (nb < 0 || nb >= 100)
+   return false;
+   if (nb > 0)
+   snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
+   else
+   snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight");
+   connector->id = nb;
+   return true;
+}
+
 static int
 nv40_get_intensity(struct backlight_device *bd)
 {
@@ -74,6 +98,8 @@ nv40_backlight_init(struct drm_connector *connector)
struct nvif_object *device = >device.object;
struct backlight_properties props;
struct backlight_device *bd;
+   struct backlight_connector bl_connector;
+   char backlight_name[BL_NAME_SIZE];

if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK))
return 0;
@@ -81,10 +107,19 @@ nv40_backlight_init(struct drm_connector *connector)
memset(, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 31;
-   bd = backlight_device_register("nv_backlight", connector->kdev, drm,
+   if (!nouveau_get_backlight_name(backlight_name, _connector)) {
+   NV_ERROR(drm, "Failed to retrieve a unique name for the 
backlight interface\n");
+   return 0;
+   }
+   bd = backlight_device_register(backlight_name , connector->kdev, drm,
   _bl_ops, );
-   if (IS_ERR(bd))
+
+   if (IS_ERR(bd)) {
+   if (bl_connector.id > 0)
+   ida_simple_remove(_ida, bl_connector.id);
return PTR_ERR(bd);
+   }
+   list_add(_connector.head, >bl_connectors);
drm->backlight = bd;
bd->props.brightness = nv40_get_intensity(bd);
backlight_update_status(bd);
@@ -182,6 +217,8 @@ nv50_backlight_init(struct drm_connector *connector)
struct backlight_properties props;
struct backlight_device *bd;
const struct backlight_ops *ops;
+   struct backlight_connector bl_connector;
+   char backlight_name[BL_NAME_SIZE];

nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
if (!nv_encoder) {
@@ -203,11 +240,20 @@ nv50_backlight_init(struct drm_connector *connector)
memset(, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 100;
-   bd = backlight_device_register("nv_backlight", connector->kdev,
+   if (!nouveau_get_backlight_name(backlight_name, _connector)) {
+   NV_ERROR(drm, "Failed to retrieve a unique name for the 
backlight interface\n");
+   return 0;
+   }
+   bd = backlight_device_register(backlight_name , connector->kdev,
   nv_encoder, ops, );
-   if (IS_ERR(bd))
+
+   if (IS_ERR(bd)) {
+   if (bl_connector.id &

[Nouveau] [PATCH v3 1/2] nouveau/bl: Assign different names to interfaces

2016-11-14 Thread Pierre Moreau
On 11:17 am - Nov 14 2016, Lukas Wunner wrote:
> On Sun, Nov 13, 2016 at 08:57:06PM +0100, Pierre Moreau wrote:
> > From: Pierre Moreau 
> > 
> > Currently, every backlight interface created by Nouveau uses the same name,
> > nv_backlight. This leads to a sysfs warning as it tries to create an already
> > existing folder. This patch adds a incremented number to the name, but keeps
> > the initial name as nv_backlight, to avoid possibly breaking userspace; the
> > second interface will be named nv_backlight1, and so on.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86539
> > 
> > v2:
> > * Switch to using ida for generating unique IDs, as suggested by Ilia 
> > Mirkin;
> > * Allocate backlight name on the stack, as suggested by Ilia Mirkin;
> > * Move `nouveau_get_backlight_name()` to avoid forward declaration, as
> >   suggested by Ilia Mirkin;
> > * Fix reference to bug report formatting, as reported by Nick Tenney.
> > 
> > v3:
> > * Define a macro for the size of the backlight name, to avoid defining
> >   it multiple times;
> > * Use snprintf in place of sprintf.
> > 
> > Signed-off-by: Pierre Moreau 
> > ---
> >  drivers/gpu/drm/nouveau/nouveau_backlight.c | 65 
> > +++--
> >  drivers/gpu/drm/nouveau/nouveau_display.h   | 10 +
> >  drivers/gpu/drm/nouveau/nouveau_drm.c   |  2 +
> >  drivers/gpu/drm/nouveau/nouveau_drv.h   |  1 +
> >  4 files changed, 74 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
> > b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > index 5e2c568..0e69612 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > @@ -31,11 +31,32 @@
> >   */
> >  
> >  #include 
> > +#include 
> >  
> >  #include "nouveau_drv.h"
> >  #include "nouveau_reg.h"
> >  #include "nouveau_encoder.h"
> >  
> > +static struct ida bl_ida;
> > +#define BL_NAME_SIZE 15 // 12 for name + 2 for digits + 1 for '\0'
> > +
> > +struct backlight_connector {
> > +   struct list_head head;
> > +   int id;
> > +};
> > +
> > +static void
> > +nouveau_get_backlight_name(char backlight_name[BL_NAME_SIZE], struct
> > +   backlight_connector *connector)
> > +{
> > +   const int nb = ida_simple_get(_ida, 0, 0, GFP_KERNEL);
> > +   if (nb > 0 && nb < 100)
> > +   snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
> > +   else
> > +   snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight");
> > +   connector->id = nb;
> > +}
> > +
> 
> Just a minor issue, in the >= 100 case, backlight creation should probably
> fail rather than reverting to "nv_backlight".  Other than that LGTM.

I don’t see how you could get to more than 100 backlight interfaces, but you
are absolutely right, it should fail rather than reverting to "nv_backlight".
I’ll add a return code to `nouveau_get_backlight_name()` and check for that in
the `nvX0_backlight_init()` functions. If it is non-zero, I’m not sure whether
I should return an error code (and if so, which one) or just a plain 0.

Thanks,
Pierre

PS: I have no idea what went wrong with the e-mail address, and why it decided
to send it from my other e-mail address rather than the usual one… I’ll 
have to
fix that before sending the updated version.

> 
> Thanks,
> 
> Lukas
> 
> >  static int
> >  nv40_get_intensity(struct backlight_device *bd)
> >  {
> > @@ -74,6 +95,8 @@ nv40_backlight_init(struct drm_connector *connector)
> > struct nvif_object *device = >device.object;
> > struct backlight_properties props;
> > struct backlight_device *bd;
> > +   struct backlight_connector bl_connector;
> > +   char backlight_name[BL_NAME_SIZE];
> >  
> > if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK))
> > return 0;
> > @@ -81,10 +104,16 @@ nv40_backlight_init(struct drm_connector *connector)
> > memset(, 0, sizeof(struct backlight_properties));
> > props.type = BACKLIGHT_RAW;
> > props.max_brightness = 31;
> > -   bd = backlight_device_register("nv_backlight", connector->kdev, drm,
> > +   nouveau_get_backlight_name(backlight_name, _connector);
> > +   bd = backlight_device_register(backlight_name , connector->kdev, drm,
> >_bl_ops, );
> &

[PATCH REBASED 2/2] Do not register interface if Apple GMUX detected

2016-11-13 Thread Pierre Moreau
From: Pierre Moreau <pierre.mor...@free.fr>

The Apple GMUX is the one managing the backlight, so there is no need for
Nouveau to register its own backlight interface.

v2: Do not split information message on two lines as it prevents from grepping
it, as pointed out by Lukas Wunner

Signed-off-by: Pierre Moreau 
---
 drivers/gpu/drm/nouveau/nouveau_backlight.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 0e69612..3c91c24 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -30,6 +30,7 @@
  * Register locations derived from NVClock by Roderick Colenbrander
  */

+#include 
 #include 
 #include 

@@ -258,6 +259,11 @@ nouveau_backlight_init(struct drm_device *dev)
struct nvif_device *device = >device;
struct drm_connector *connector;

+   if (apple_gmux_present()) {
+   NV_INFO(drm, "Apple GMUX detected: not registering Nouveau 
backlight interface");
+   return 0;
+   }
+
INIT_LIST_HEAD(>bl_connectors);

list_for_each_entry(connector, >mode_config.connector_list, head) {
-- 
2.10.2



[PATCH v3 1/2] nouveau/bl: Assign different names to interfaces

2016-11-13 Thread Pierre Moreau
From: Pierre Moreau <pierre.mor...@free.fr>

Currently, every backlight interface created by Nouveau uses the same name,
nv_backlight. This leads to a sysfs warning as it tries to create an already
existing folder. This patch adds a incremented number to the name, but keeps
the initial name as nv_backlight, to avoid possibly breaking userspace; the
second interface will be named nv_backlight1, and so on.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86539

v2:
* Switch to using ida for generating unique IDs, as suggested by Ilia Mirkin;
* Allocate backlight name on the stack, as suggested by Ilia Mirkin;
* Move `nouveau_get_backlight_name()` to avoid forward declaration, as
  suggested by Ilia Mirkin;
* Fix reference to bug report formatting, as reported by Nick Tenney.

v3:
* Define a macro for the size of the backlight name, to avoid defining
  it multiple times;
* Use snprintf in place of sprintf.

Signed-off-by: Pierre Moreau 
---
 drivers/gpu/drm/nouveau/nouveau_backlight.c | 65 +++--
 drivers/gpu/drm/nouveau/nouveau_display.h   | 10 +
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  2 +
 drivers/gpu/drm/nouveau/nouveau_drv.h   |  1 +
 4 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 5e2c568..0e69612 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -31,11 +31,32 @@
  */

 #include 
+#include 

 #include "nouveau_drv.h"
 #include "nouveau_reg.h"
 #include "nouveau_encoder.h"

+static struct ida bl_ida;
+#define BL_NAME_SIZE 15 // 12 for name + 2 for digits + 1 for '\0'
+
+struct backlight_connector {
+   struct list_head head;
+   int id;
+};
+
+static void
+nouveau_get_backlight_name(char backlight_name[BL_NAME_SIZE], struct
+   backlight_connector *connector)
+{
+   const int nb = ida_simple_get(_ida, 0, 0, GFP_KERNEL);
+   if (nb > 0 && nb < 100)
+   snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
+   else
+   snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight");
+   connector->id = nb;
+}
+
 static int
 nv40_get_intensity(struct backlight_device *bd)
 {
@@ -74,6 +95,8 @@ nv40_backlight_init(struct drm_connector *connector)
struct nvif_object *device = >device.object;
struct backlight_properties props;
struct backlight_device *bd;
+   struct backlight_connector bl_connector;
+   char backlight_name[BL_NAME_SIZE];

if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK))
return 0;
@@ -81,10 +104,16 @@ nv40_backlight_init(struct drm_connector *connector)
memset(, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 31;
-   bd = backlight_device_register("nv_backlight", connector->kdev, drm,
+   nouveau_get_backlight_name(backlight_name, _connector);
+   bd = backlight_device_register(backlight_name , connector->kdev, drm,
   _bl_ops, );
-   if (IS_ERR(bd))
+
+   if (IS_ERR(bd)) {
+   if (bl_connector.id > 0)
+   ida_simple_remove(_ida, bl_connector.id);
return PTR_ERR(bd);
+   }
+   list_add(_connector.head, >bl_connectors);
drm->backlight = bd;
bd->props.brightness = nv40_get_intensity(bd);
backlight_update_status(bd);
@@ -182,6 +211,8 @@ nv50_backlight_init(struct drm_connector *connector)
struct backlight_properties props;
struct backlight_device *bd;
const struct backlight_ops *ops;
+   struct backlight_connector bl_connector;
+   char backlight_name[BL_NAME_SIZE];

nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
if (!nv_encoder) {
@@ -203,11 +234,17 @@ nv50_backlight_init(struct drm_connector *connector)
memset(, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 100;
-   bd = backlight_device_register("nv_backlight", connector->kdev,
+   nouveau_get_backlight_name(backlight_name, _connector);
+   bd = backlight_device_register(backlight_name , connector->kdev,
   nv_encoder, ops, );
-   if (IS_ERR(bd))
+
+   if (IS_ERR(bd)) {
+   if (bl_connector.id > 0)
+   ida_simple_remove(_ida, bl_connector.id);
return PTR_ERR(bd);
+   }

+   list_add(_connector.head, >bl_connectors);
drm->backlight = bd;
bd->props.brightness = bd->ops->get_brightness(bd);
backlight_update_status(bd);
@@ -221,6 +258,8 @@ nouveau_backlight_init(struct drm_device *dev)
struct 

[PATCH] gpu/nouveau/nouveau_acpi.c: Fix Type Mismatch ACPI warning

2016-05-19 Thread Pierre Moreau
Hello Marcos,

I sent a serie a year ago to fix some of the ACPI handling in Nouveau and add
runtime pm support for laptops with an Apple GMUX (see [0], and especially [1]
and [2]). I was told that a more generic work for the runtime pm was in the
work, so I let the whole serie slip away. I was thinking of resubmitting the
serie without the runtime pm additions, to at least fix those warning/error
messages. I thought I had a patch to fix this issue in my serie, but going
through it again, it looks like I did not.
Note that even, though the spec says the 4th argument should be a package, some
old BIOSes expect a different type, like a buffer (see the comment above [5]; I
think there was also a comment about it in the spec, but I can’t find it
anymore).

I added some comments below. I’ll test this patch tonight, to see how it works
on my laptop as I think it expects a buffer as 4th argument.

Thank you!

Regards,
Pierre


[0]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083444.html
[1]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083448.html
[2]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083449.html

On 09:42 PM - May 18 2016, Marcos Paulo de Souza wrote:
> nouveau_optimus_dsm is using ACPI_TYPE_BUFFER, and this triggers warnings on 
> ACPI:
> [7.730564] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [7.730570] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (201509
> 30/nsarguments-95)
> 
> To fix it, change ACPI_TYPE_BUFFER to ACPI_TYPE_PACKAGE, as the warning tells 
> to do.
> 
> Signed-off-by: Marcos Paulo de Souza 
> ---
> 
> After booting my Asus Laptop, and after a suspend/resume too, dmesg shows 
> warnings:
> [1.633361] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [1.633434] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [7.730176] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [7.730564] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [7.730570] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   49.732059] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   49.732424] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [   49.732430] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   74.366300] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   74.366657] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [   74.33] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [  140.357789] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [  140.358532] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [  140.358547] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)

Out of curiosity, which GPU(s) do you have in your laptop? I guess there is at
least an NVIDIA one, but is it an Optimus configuration? And if so, does the
unused card go automatically to sleep after a few seconds?

> 
> I don't know if this is the right thing to do, I just looked at intel_acpi.c 
> to check how to use/check for ACPI Package.
> The patch below silenced the "type mismatch" warnings, and some of the 
> "evaluated _DSM" ones.
> 
> If this is not the right approach, please let me know how to fix it, I don't 
> have knowledge in ACPI, but I really want to help.
> 
>  drivers/gpu/drm/nouveau/nouveau_acpi.c | 14 +-
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c 
> b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> index cdf5227..f04aef3 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> @@ -73,22 +73,10 @@ static const char nouveau_op_dsm_muid[] = {
>  
>  static int nouveau_optimus_dsm(acpi_handle handle, int func, int arg, 
> uint32_t *result)
>  {
> - int i;
>   union acpi_object *obj;
> - char args_buff[4];
> - union acpi_object argv4 = {
> - .buffer.type = ACPI_TYPE_BUFFER,
> - .buffer.length = 4,
> - .buffer.pointer = args_buff
> - };
> -
> - /* ACPI is little endian, AABBCCDD becomes 

[Nouveau] [PATCH v2 2/2] nouveau/bl: Do not register interface if Apple GMUX detected

2016-05-01 Thread Pierre Moreau
On 02:32 PM - May 01 2016, Pierre Moreau wrote:
> The Apple GMUX is the one managing the backlight, so there is no need for
> Nouveau to register its own backlight interface.
> 
> v2: Do not split information message on two lines as it prevents from grepping
> it, as pointed out by Hans de Goede

It should be "it, as pointed out by Lukas Wunner", sorry for the mistake… :-/

Pierre

> 
> Signed-off-by: Pierre Moreau 
> ---
>  drm/nouveau/nouveau_backlight.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
> index 41330e4..8429ceb 100644
> --- a/drm/nouveau/nouveau_backlight.c
> +++ b/drm/nouveau/nouveau_backlight.c
> @@ -30,6 +30,7 @@
>   * Register locations derived from NVClock by Roderick Colenbrander
>   */
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -257,6 +258,11 @@ nouveau_backlight_init(struct drm_device *dev)
>   struct nvif_device *device = >device;
>   struct drm_connector *connector;
>  
> + if (apple_gmux_present()) {
> + NV_INFO(drm, "Apple GMUX detected: not registering Nouveau 
> backlight interface");
> + return 0;
> + }
> +
>   INIT_LIST_HEAD(>bl_connectors);
>  
>   list_for_each_entry(connector, >mode_config.connector_list, head) {
> -- 
> 2.8.2
> 
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160501/69b33442/attachment.sig>


[PATCH v2 2/2] nouveau/bl: Do not register interface if Apple GMUX detected

2016-05-01 Thread Pierre Moreau
The Apple GMUX is the one managing the backlight, so there is no need for
Nouveau to register its own backlight interface.

v2: Do not split information message on two lines as it prevents from grepping
it, as pointed out by Hans de Goede

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_backlight.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index 41330e4..8429ceb 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -30,6 +30,7 @@
  * Register locations derived from NVClock by Roderick Colenbrander
  */

+#include 
 #include 
 #include 

@@ -257,6 +258,11 @@ nouveau_backlight_init(struct drm_device *dev)
struct nvif_device *device = >device;
struct drm_connector *connector;

+   if (apple_gmux_present()) {
+   NV_INFO(drm, "Apple GMUX detected: not registering Nouveau 
backlight interface");
+   return 0;
+   }
+
INIT_LIST_HEAD(>bl_connectors);

list_for_each_entry(connector, >mode_config.connector_list, head) {
-- 
2.8.2



[PATCH REBASED 2/2] nouveau/bl: Do not register interface if Apple GMUX detected

2016-04-17 Thread Pierre Moreau
The Apple GMUX is the one managing the backlight, so there is no need for
Nouveau to register its own backlight interface.

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_backlight.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index 41330e4..94ac3cb 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -30,6 +30,7 @@
  * Register locations derived from NVClock by Roderick Colenbrander
  */

+#include 
 #include 
 #include 

@@ -257,6 +258,12 @@ nouveau_backlight_init(struct drm_device *dev)
struct nvif_device *device = >device;
struct drm_connector *connector;

+   if (apple_gmux_present()) {
+   NV_INFO(drm, "Apple GMUX detected: not registering Nouveau"
+   " backlight interface");
+   return 0;
+   }
+
INIT_LIST_HEAD(>bl_connectors);

list_for_each_entry(connector, >mode_config.connector_list, head) {
-- 
2.8.0



[PATCH v2 1/2] nouveau/bl: Assign different names to interfaces

2016-04-17 Thread Pierre Moreau
Currently, every backlight interface created by Nouveau uses the same name,
nv_backlight. This leads to a sysfs warning as it tries to create an already
existing folder. This patch adds a incremented number to the name, but keeps
the initial name as nv_backlight, to avoid possibly breaking userspace; the
second interface will be named nv_backlight1, and so on.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86539

v2:
* Switch to using ida for generating unique IDs, as suggested by Ilia Mirkin;
* Allocate backlight name on the stack, as suggested by Ilia Mirkin;
* Move `nouveau_get_backlight_name()` to avoid forward declaration, as
  suggested by Ilia Mirkin;
* Fix reference to bug report formatting, as reported by Nick Tenney.

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_backlight.c | 64 ++---
 drm/nouveau/nouveau_display.h   | 10 +++
 drm/nouveau/nouveau_drm.c   |  2 ++
 drm/nouveau/nouveau_drm.h   |  1 +
 4 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index 89eb460..41330e4 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -31,11 +31,31 @@
  */

 #include 
+#include 

 #include "nouveau_drm.h"
 #include "nouveau_reg.h"
 #include "nouveau_encoder.h"

+static struct ida bl_ida;
+
+struct backlight_connector {
+   struct list_head head;
+   int id;
+};
+
+static void
+nouveau_get_backlight_name(char backlight_name[15], struct backlight_connector
+   *connector)
+{
+   const int nb = ida_simple_get(_ida, 0, 0, GFP_KERNEL);
+   if (nb > 0 && nb < 100)
+   sprintf(backlight_name, "nv_backlight%d", nb);
+   else
+   sprintf(backlight_name, "nv_backlight");
+   connector->id = nb;
+}
+
 static int
 nv40_get_intensity(struct backlight_device *bd)
 {
@@ -74,6 +94,8 @@ nv40_backlight_init(struct drm_connector *connector)
struct nvif_object *device = >device.object;
struct backlight_properties props;
struct backlight_device *bd;
+   struct backlight_connector bl_connector;
+   char backlight_name[15]; // 12 for name + 2 for digits + 1 for '\0'

if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK))
return 0;
@@ -81,10 +103,16 @@ nv40_backlight_init(struct drm_connector *connector)
memset(, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 31;
-   bd = backlight_device_register("nv_backlight", connector->kdev, drm,
+   nouveau_get_backlight_name(backlight_name, _connector);
+   bd = backlight_device_register(backlight_name , connector->kdev, drm,
   _bl_ops, );
-   if (IS_ERR(bd))
+
+   if (IS_ERR(bd)) {
+   if (bl_connector.id > 0)
+   ida_simple_remove(_ida, bl_connector.id);
return PTR_ERR(bd);
+   }
+   list_add(_connector.head, >bl_connectors);
drm->backlight = bd;
bd->props.brightness = nv40_get_intensity(bd);
backlight_update_status(bd);
@@ -182,6 +210,8 @@ nv50_backlight_init(struct drm_connector *connector)
struct backlight_properties props;
struct backlight_device *bd;
const struct backlight_ops *ops;
+   struct backlight_connector bl_connector;
+   char backlight_name[15]; // 12 for name + 2 for digits + 1 for '\0'

nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
if (!nv_encoder) {
@@ -203,11 +233,17 @@ nv50_backlight_init(struct drm_connector *connector)
memset(, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 100;
-   bd = backlight_device_register("nv_backlight", connector->kdev,
+   nouveau_get_backlight_name(backlight_name, _connector);
+   bd = backlight_device_register(backlight_name , connector->kdev,
   nv_encoder, ops, );
-   if (IS_ERR(bd))
+
+   if (IS_ERR(bd)) {
+   if (bl_connector.id > 0)
+   ida_simple_remove(_ida, bl_connector.id);
return PTR_ERR(bd);
+   }

+   list_add(_connector.head, >bl_connectors);
drm->backlight = bd;
bd->props.brightness = bd->ops->get_brightness(bd);
backlight_update_status(bd);
@@ -221,6 +257,8 @@ nouveau_backlight_init(struct drm_device *dev)
struct nvif_device *device = >device;
struct drm_connector *connector;

+   INIT_LIST_HEAD(>bl_connectors);
+
list_for_each_entry(connector, >mode_config.connector_list, head) {
if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&

[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces

2016-04-17 Thread Pierre Moreau
[…]

> 
> > +   const int nb = atomic_inc_return(_interfaces_nb) - 1;
> 
> This kinda sucks if you reload nouveau a bunch. How about using an
> "ida". Have a look in drivers/gpu/drm/drm_crtc.c for how I use that
> one.

I had a quick look at drm_crtc.c. This seems to be exactly what I need, except
that I do not see how I can enforce to have one of the active ones being named
`nv_backlight`, to not break (possibly) existing userspace applications. (I’m
guessing that I would be doing the same as in drm_crtc.c: looping over all
connectors and assigning them an ida, regardless of them being active or not,
as they might become active later on.)

There is another issue that I failed to address with this patch, and just
realised about it. Each time `nvXX_backlight_init(connector)` is called on a
connector, a new backlight device is created, and is stored in
`drm->backlight`, which is unique at a device level, not connector level,
meaning the old one just gets overriden (and leaked). Having a list
`drm->backlights` should solve this issue. Would that be fine with you Ben?

Pierre

> 
> > +   if (nb > 0 && nb < 100)
> > +   sprintf(backlight_name, "nv_backlight%d", nb);
> > +   else
> > +   sprintf(backlight_name, "nv_backlight");
> > +   return backlight_name;
> > +}
> > --
> > 2.8.0
> >
> > ___
> > Nouveau mailing list
> > Nouveau at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 



[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces

2016-04-16 Thread Pierre Moreau
On 02:40 PM - Apr 15 2016, Nick Tenney wrote:
> On Fri, Apr 15, 2016 at 11:25 AM, Ilia Mirkin  wrote:
> 
> > On Fri, Apr 15, 2016 at 11:22 AM, Pierre Moreau 
> > wrote:
> > > On 11:06 AM - Apr 15 2016, Ilia Mirkin wrote:
> > >> On Fri, Apr 15, 2016 at 10:57 AM, Pierre Moreau  > >> free.fr>
> > wrote:
> > >> > Currently, every backlight interface created by Nouveau uses the same
> > name,
> > >> > nv_backlight. This leads to a sysfs warning as it tries to create an
> > already
> > >> > existing folder. This patch adds a incremented number to the name,
> > but keeps
> > >> > the initial name as nv_backlight, to avoid possibly breaking
> > userspace; the
> > >> > second interface will be named nv_backlight1, and so on.
> > >> >
> > >> > Fixes: fdo#86539
> >
>  I believe Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86539 is
> the preferred format. I think this is picked up by the mesa release scripts
> or some such.

Ack’ed. I’ll fix that in the v2.

Thanks!
Pierre

> 
> > >> > Signed-off-by: Pierre Moreau 
> > >> > ---
> > >> >  drm/nouveau/nouveau_backlight.c | 35
> > +--
> > >> >  1 file changed, 33 insertions(+), 2 deletions(-)
> > >> >
> > >> > diff --git a/drm/nouveau/nouveau_backlight.c
> > b/drm/nouveau/nouveau_backlight.c
> > >> > index 89eb460..914e2cb 100644
> > >> > --- a/drm/nouveau/nouveau_backlight.c
> > >> > +++ b/drm/nouveau/nouveau_backlight.c
> > >> > @@ -36,6 +36,10 @@
> > >> >  #include "nouveau_reg.h"
> > >> >  #include "nouveau_encoder.h"
> > >> >
> > >> > +static atomic_t bl_interfaces_nb = { 0 };
> > >>
> > >> static data is initialized to 0, this should be unnecessary.
> > >
> > > I didn’t know that. But on the other hand, I like having it explicit,
> > and it
> > > should not add any overhead.
> >
> > It increases the size of the object file. I believe it's kernel policy
> > to avoid static initializations to 0. (Note that this doesn't hold in
> > regular user applications, just the kernel.)
> >
> >   -ilia
> > ___
> > Nouveau mailing list
> > Nouveau at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
> >

> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160416/ca9a9d2f/attachment.sig>


[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces

2016-04-15 Thread Pierre Moreau
On 11:06 AM - Apr 15 2016, Ilia Mirkin wrote:
> On Fri, Apr 15, 2016 at 10:57 AM, Pierre Moreau  
> wrote:
> > Currently, every backlight interface created by Nouveau uses the same name,
> > nv_backlight. This leads to a sysfs warning as it tries to create an already
> > existing folder. This patch adds a incremented number to the name, but keeps
> > the initial name as nv_backlight, to avoid possibly breaking userspace; the
> > second interface will be named nv_backlight1, and so on.
> >
> > Fixes: fdo#86539
> > Signed-off-by: Pierre Moreau 
> > ---
> >  drm/nouveau/nouveau_backlight.c | 35 +--
> >  1 file changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/drm/nouveau/nouveau_backlight.c 
> > b/drm/nouveau/nouveau_backlight.c
> > index 89eb460..914e2cb 100644
> > --- a/drm/nouveau/nouveau_backlight.c
> > +++ b/drm/nouveau/nouveau_backlight.c
> > @@ -36,6 +36,10 @@
> >  #include "nouveau_reg.h"
> >  #include "nouveau_encoder.h"
> >
> > +static atomic_t bl_interfaces_nb = { 0 };
> 
> static data is initialized to 0, this should be unnecessary.

I didn’t know that. But on the other hand, I like having it explicit, and it
should not add any overhead.

> 
> I'd also call it "backlights" or something like that. No need for
> multiple words...

I prefer to use a plural noun when talking about a container of those nouns,
rathter than a counter of those nouns. But I’ll change it.

> 
> > +
> > +static char* nouveau_get_backlight_name(void);
> 
> Please organize the code to avoid forward declarations.
> 
> > +
> >  static int
> >  nv40_get_intensity(struct backlight_device *bd)
> >  {
> > @@ -74,6 +78,7 @@ nv40_backlight_init(struct drm_connector *connector)
> > struct nvif_object *device = >device.object;
> > struct backlight_properties props;
> > struct backlight_device *bd;
> > +   char* backlight_name = NULL;
> >
> > if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & 
> > NV40_PMC_BACKLIGHT_MASK))
> > return 0;
> > @@ -81,8 +86,14 @@ nv40_backlight_init(struct drm_connector *connector)
> > memset(, 0, sizeof(struct backlight_properties));
> > props.type = BACKLIGHT_RAW;
> > props.max_brightness = 31;
> > -   bd = backlight_device_register("nv_backlight", connector->kdev, drm,
> > +   backlight_name = nouveau_get_backlight_name();
> > +   bd = backlight_device_register(backlight_name , connector->kdev, 
> > drm,
> >_bl_ops, );
> > +
> > +   // backlight_device_register() makes a copy
> > +   kfree(backlight_name);
> > +   backlight_name = NULL;
> > +
> > if (IS_ERR(bd))
> > return PTR_ERR(bd);
> > drm->backlight = bd;
> > @@ -182,6 +193,7 @@ nv50_backlight_init(struct drm_connector *connector)
> > struct backlight_properties props;
> > struct backlight_device *bd;
> > const struct backlight_ops *ops;
> > +   char* backlight_name = NULL;
> >
> > nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
> > if (!nv_encoder) {
> > @@ -203,8 +215,14 @@ nv50_backlight_init(struct drm_connector *connector)
> > memset(, 0, sizeof(struct backlight_properties));
> > props.type = BACKLIGHT_RAW;
> > props.max_brightness = 100;
> > -   bd = backlight_device_register("nv_backlight", connector->kdev,
> > +   backlight_name = nouveau_get_backlight_name();
> > +   bd = backlight_device_register(backlight_name, connector->kdev,
> >nv_encoder, ops, );
> > +
> > +   // backlight_device_register() makes a copy
> > +   kfree(backlight_name);
> > +   backlight_name = NULL;
> > +
> > if (IS_ERR(bd))
> > return PTR_ERR(bd);
> >
> > @@ -252,3 +270,16 @@ nouveau_backlight_exit(struct drm_device *dev)
> > drm->backlight = NULL;
> > }
> >  }
> > +
> > +static char*
> > +nouveau_get_backlight_name(void)
> > +{
> > +   // 12 chars for "nv_backlight" + 2 for two digits number + 1 for 
> > '\0'
> > +   char* backlight_name = (char*)kmalloc(sizeof(char[15]), GFP_KERNEL);
> 
> Making this stack-allocated in the caller would be so much simpler...

--" I was planning to use a dynamic 

[PATCH 2/2] nouveau/bl: Do not register interface if Apple GMUX detected

2016-04-15 Thread Pierre Moreau
The Apple GMUX is the one managing the backlight, so there is no need for
Nouveau to register its own backlight interface.

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_backlight.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index 914e2cb..37a1577 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -30,6 +30,7 @@
  * Register locations derived from NVClock by Roderick Colenbrander
  */

+#include 
 #include 

 #include "nouveau_drm.h"
@@ -239,6 +240,12 @@ nouveau_backlight_init(struct drm_device *dev)
struct nvif_device *device = >device;
struct drm_connector *connector;

+   if (apple_gmux_present()) {
+   NV_INFO(drm, "Apple GMUX detected: not registering Nouveau"
+   " backlight interface");
+   return 0;
+   }
+
list_for_each_entry(connector, >mode_config.connector_list, head) {
if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
connector->connector_type != DRM_MODE_CONNECTOR_eDP)
-- 
2.8.0



[PATCH 1/2] nouveau/bl: Assign different names to interfaces

2016-04-15 Thread Pierre Moreau
Currently, every backlight interface created by Nouveau uses the same name,
nv_backlight. This leads to a sysfs warning as it tries to create an already
existing folder. This patch adds a incremented number to the name, but keeps
the initial name as nv_backlight, to avoid possibly breaking userspace; the
second interface will be named nv_backlight1, and so on.

Fixes: fdo#86539
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_backlight.c | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index 89eb460..914e2cb 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -36,6 +36,10 @@
 #include "nouveau_reg.h"
 #include "nouveau_encoder.h"

+static atomic_t bl_interfaces_nb = { 0 };
+
+static char* nouveau_get_backlight_name(void);
+
 static int
 nv40_get_intensity(struct backlight_device *bd)
 {
@@ -74,6 +78,7 @@ nv40_backlight_init(struct drm_connector *connector)
struct nvif_object *device = >device.object;
struct backlight_properties props;
struct backlight_device *bd;
+   char* backlight_name = NULL;

if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK))
return 0;
@@ -81,8 +86,14 @@ nv40_backlight_init(struct drm_connector *connector)
memset(, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 31;
-   bd = backlight_device_register("nv_backlight", connector->kdev, drm,
+   backlight_name = nouveau_get_backlight_name();
+   bd = backlight_device_register(backlight_name , connector->kdev, drm,
   _bl_ops, );
+
+   // backlight_device_register() makes a copy
+   kfree(backlight_name);
+   backlight_name = NULL;
+
if (IS_ERR(bd))
return PTR_ERR(bd);
drm->backlight = bd;
@@ -182,6 +193,7 @@ nv50_backlight_init(struct drm_connector *connector)
struct backlight_properties props;
struct backlight_device *bd;
const struct backlight_ops *ops;
+   char* backlight_name = NULL;

nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
if (!nv_encoder) {
@@ -203,8 +215,14 @@ nv50_backlight_init(struct drm_connector *connector)
memset(, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 100;
-   bd = backlight_device_register("nv_backlight", connector->kdev,
+   backlight_name = nouveau_get_backlight_name();
+   bd = backlight_device_register(backlight_name, connector->kdev,
   nv_encoder, ops, );
+
+   // backlight_device_register() makes a copy
+   kfree(backlight_name);
+   backlight_name = NULL;
+
if (IS_ERR(bd))
return PTR_ERR(bd);

@@ -252,3 +270,16 @@ nouveau_backlight_exit(struct drm_device *dev)
drm->backlight = NULL;
}
 }
+
+static char*
+nouveau_get_backlight_name(void)
+{
+   // 12 chars for "nv_backlight" + 2 for two digits number + 1 for '\0'
+   char* backlight_name = (char*)kmalloc(sizeof(char[15]), GFP_KERNEL);
+   const int nb = atomic_inc_return(_interfaces_nb) - 1;
+   if (nb > 0 && nb < 100)
+   sprintf(backlight_name, "nv_backlight%d", nb);
+   else
+   sprintf(backlight_name, "nv_backlight");
+   return backlight_name;
+}
-- 
2.8.0



Summer of Code Ideas

2016-03-18 Thread Pierre Moreau
Hello Abheek,

Sorry for the long delay.

So, things have changed a bit since last year idea. You should now be able to
start kernels on most generations, atomic operations are supported and image is
in the work. Regarding the compiler, Hans de Goede started working on compiling
LLVM IR to TGSI, which would make it possible to feed OpenCL kernels in a TGSI
form to Nouveau, and I have been working on compiling SPIR-V to Nouveau's IR
(NV50 IR).

On the SPIR-V side, there are some things that shouldn't be too difficult and
some which might take some time, but I have no idea what would fit a GSoC
project (and I am not really planning to be a mentor).

I don't know if Hans has any plans regarding GSoC.
The SPIR-V that I'm using, is generated by LLVM. But that compilation pass,
from LLVM IR to SPIR-V, is a work in progress. Since LLVM is also part of the
GSoC program, you could ask them; that work would benefit to Nouveau, and
possibly other drivers as well. However that task, I would guess, is a quite
difficult one.

Regards,
Pierre Moreau


On 08:22 AM - Feb 24 2016, Abheek Ghosh wrote:
> Thanks for reply. I would like to work on *Compilers*. I have just some
> basic knowledge on compilers. I liked the 2015 ideas related to compilers
> and Nouveau like "OpenCL support" (but requires Tesla, not available). The
> projects were mostly moderate/difficult level. Although, I am at a learning
> stage and may not have good knowledge on compilers, I am willing to work
> very hard and learn(why I am here).
> 
> Regards
> Abheek
> 
> On 24 February 2016 at 02:20, Emil Velikov  
> wrote:
> 
> > Hello Abheek,
> >
> > On 23 February 2016 at 18:08, Abheek Ghosh 
> > wrote:
> > > Hi,
> > > I am Abheek Ghosh, sophomore from Indian Institute of Technology,
> > Guwahati.
> > > I read the Summer of Code and DRM pages, as well as Mehul's discussion
> > but
> > > didn't get much help. I have strong C/C++ skills and familiar with
> > OpenCL. I
> > > have Intel CPU and Nvidia GPU of Fermi architecture. I have made small
> > > contributions to open source projects. I don't have experience in kernel
> > > level development, but very interested to get my hands dirty.
> > >
> > > It would be helpful if any developer can help me get started with some
> > > suitable projects related to available hardware or any hardware
> > independent
> > > projects in OpenCL, OpenGL or Mesa.
> > >
> > > Awaiting a positive response.
> > >
> > As you've noticed in the thread started by Mehul, people here are keen
> > on getting to know what your interest and strengths are. Thus listing
> > those would be beneficial. This way people can come up with more
> > 'personalised' ideas, should the ones on the wikis (short of links
> > atm, sorry) do not suit you.
> >
> > Regards,
> > Emil
> >

> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160317/d63ca8dd/attachment.sig>


[PATCH v5 00/12] Enable GPU switching on pre-retina MacBook Pro

2016-02-02 Thread Pierre Moreau
Hello all,

> On 01 Feb 2016, at 23:49, Lukas Wunner  wrote:
> 
> Hi,
> 
>> On Mon, Jan 11, 2016 at 08:09:20PM +0100, Lukas Wunner wrote:
>> Enable GPU switching on the pre-retina MacBook Pro (2008 - 2013), v5.
> 
> This series hasn't seen any reviews or acks unfortunately.
> Any takers?
> 
> Merging this would allow fdo #61115 to be closed
> (currently assigned to intel-gfx).
> 
> FWIW this series has in the meantime been tested by more folks:
> 
> Tested-by: Pierre Moreau 
>[MBP  5,3 2009  nvidia MCP79 + G96pre-retina  15"]
> Tested-by: Paul Hordiienko 
>[MBP  6,2 2010  intel ILK + nvidia GT216  pre-retina  15"]
> Tested-by: William Brown 
>[MBP  8,2 2011  intel SNB + amd turks pre-retina  15"]
> Tested-by: Lukas Wunner 
>[MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]
> 
> On the latter three models it worked fine. On Pierre Moreau's machine
> the discrete nvidia G96 locks up when woken. This happened in the past
> as well but not on the first wake but only on the 10th or so. Since it
> works fine on the GT216 and GK107, I'm guessing we've got a regression
> in the wakeup code for the G96 which is somehow triggered by this patch
> set (more specifically: triggered by being able to retrieve the proper
> panel resolution and configure a crtc). It needs to be fixed but isn't
> a showstopper for this series IMHO. (Arguably being able to retrieve
> EDID but then locking up on switching isn't really worse than not being
> able to retrieve EDID in the first place.)

I would say it is slightly worse, since the only "downside" of not retrieving 
the EDID means  TTY is set to a default resolution rather than the screen 
resolution, but this is fixed when starting X.
On the other hand, since DRI_PRIME works fine on the laptop, there isn't much 
reason to switch between cards. 

I'll have a look at the resume this week, now that FOSDEM is off my todo list. 

Regards,
Pierre


> 
> Thanks,
> 
> Lukas
> 
>> 
>> The main obstacle on these machines is that the panel mode in VBIOS
>> is bogus. Fortunately gmux can switch DDC independently from the
>> display, thereby allowing the inactive GPU to probe the panel's EDID.
>> 
>> In short, vga_switcheroo and apple-gmux are amended with hooks to
>> switch DDC, DRM core is amended with a drm_get_edid_switcheroo() helper,
>> and relevant drivers are amended to call that for LVDS outputs.
>> 
>> The retina MacBook Pro (2012 - present) uses eDP and cannot switch
>> AUX independently from the main link. The main obstacle there is link
>> training, I'm currently working on this, it will be addressed in a
>> future patch set.
>> 
>> This series is also reviewable on GitHub:
>> https://github.com/l1k/linux/commits/mbp_switcheroo_v5
>> 
>> Changes:
>> 
>> * New patch [01/12]: vga_switcheroo handler flags
>>  Alex Deucher asked if this series might regress on non-Apple laptops.
>>  To address this concern, I let handlers declare their capabilities in
>>  a bitmask. DRM drivers call drm_get_edid_switcheroo() only if the
>>  handler has set the VGA_SWITCHEROO_CAN_SWITCH_DDC flag.
>>  Currently just one other flag is defined which is used on retinas.
>> 
>> * Changed patch [02/12]: vga_switcheroo DDC locking
>>  Rename ddc_lock to mux_hw_lock, suggested by Daniel Vetter.
>> 
>> * New patch [03/12]: track switch state of apple-gmux
>>  Fixes a bug in previous versions of this series which occurred if
>>  the system was suspended while DDC was temporarily switched:
>>  On resume DDC was switched to the wrong GPU.
>> 
>> * New patches [09/12 - 12/12]: deferred probing
>>  Previously I used connector reprobing if the inactive GPU's driver
>>  loaded before gmux. I've ditched that in favor of deferred driver
>>  probing, which is much simpler. Thanks to Daniel Vetter for the
>>  suggestion.
>> 
>> Caution: Patch [09/12] depends on a new acpi_dev_present() API which
>> will land in 4.5 via Rafael J. Wysocki's tree.
>> 
>> I would particularly be interested in feedback on the handler flags
>> patch [01/12]. I'm not 100% happy with the number of characters
>> required to query the flags (e.g.: if (vga_switcheroo_handler_flags() &
>> VGA_SWITCHEROO_CAN_SWITCH_DDC)), but failed to come up with something
>> shorter. Thierry Reding used a struct of bools instead of a bitmask
>> for his recent drm_dp_link_caps patches. Maybe use that instead?
>> http://lists.freedesktop.org/archives/dri-devel/2015-December/097025.html
>> 
>> Thanks,
>> 
>> Lukas
>> 
>> 
>> Lukas Wunne

[PATCH 1/5] [RFC] drm: Documentation style guide

2015-12-08 Thread Pierre Moreau
Hello Daniel,

Just some typo comments below.

On 09:49 AM - Dec 08 2015, Daniel Vetter wrote:
> Every time I type or review docs this seems a bit different. Try to
> document the common style so we can try to unify at least new docs.
> 
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/DocBook/gpu.tmpl | 37 +
>  1 file changed, 37 insertions(+)
> 
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index 86e5b12a49ba..5698c93dae8b 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -124,6 +124,43 @@
>  
>[Insert diagram of typical DRM stack here]
>  
> +  
> +Style Guidelines
> +
> +  For consistency these documentations use American English. 
> Abbreviations
> +  are written as all-uppercase, for example: DRM, KMS, IOCTL, CRTC, and 
> so
> +  on. To aid in reading documentations make full use of the markup
> +  characters kerneldoc provides: @parameter for function paramters, 
> @member

paramters -> parameters

> +  for structure members, structure to refernce structures and

refernce -> reference

> +  function() for functions. These all get automatically hyperlinked if
> +  kerneldoc for the referencec objects exists When referencing entries in

referencec -> referenced, missing '.' after exists

> +  function vtables please use -vfunc(). Note that with kerneldoc does

Isn't "with" too much here? "Note that kerneldoc does not […]"?

> +  not support referncing struct members directly, so please add a 
> reference

referncing -> referencing

> +  to the vtable struct somewhere in the same paragraph or at least 
> section.
> +
> +
> +  Except in special situations (to separate locked from unlocked 
> variants)
> +  locking requirements for functions aren't documented in the kerneldoc.
> +  Instead locking should be check at runtime using e.g.
> +  WARN_ON(!mutex_is_locked(...));. Since it's much easier to
> +  ignore documentation than runtime noise this provides more value. And 
> on
> +  top of that runtime checks do need to be updated when the locking rules
> +  change, increasing the changes that they're correct. Within the
> +  documentation the locking rules should be explained in the relevant
> +  structures: Either in the comment for the lock explaining what it
> +  protects, or data fields need a note about which lock protects them, or
> +  both.
> +
> +
> +  Functions which have a non-void return value should have a
> +  section called "Returns" explaining the expected return values in
> +  different cases an their meanings. Currently there's no consensus 
> whether
> +  that section name should be all upper-case or not, and whether it 
> should
> +  end in a colon or not. Go with the file-local style. Other common 
> section
> +  names are "Notes" with information for dangerous or tricky corner 
> cases,
> +  and "FIXME" where the interface could be cleaned up.

Why not define (and use) a single style for naming all sections? Old
documentation might not use it, but it should be doable to upgrade those old
documents.

Pierre


> +
> +  
>
>  
>
> -- 
> 2.5.1
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Nouveau] [PATCH v3 0/6] Enable gpu switching on the MacBook Pro

2015-10-05 Thread Pierre Moreau
The repo is a fork of Linus' tree, with the patches on top of it. So you just 
need to build that modified version of the kernel and boot it. :-)

Pierre

> On 05 Oct 2015, at 16:15, Evan Foss  wrote:
> 
>> On Mon, Oct 5, 2015 at 9:23 AM, Lukas Wunner  wrote:
>> Hi,
>> 
>> I've also pushed this series to GitHub now to ease reviewing:
>> https://github.com/l1k/linux/commits/mbp_switcheroo_v3
> 
> So to test this all someone has to do is pull this and try it? No
> patching required?
> 
>> Thanks,
>> 
>> Lukas
>> ___
>> Nouveau mailing list
>> Nouveau at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
> 
> 
> 
> -- 
> Home
> http://evanfoss.googlepages.com/
> Work
> http://forge.abcd.harvard.edu/gf/project/epl_engineering/wiki/
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau


[PATCH v3] pci: Handle 5-bit and 8-bit tag field

2015-09-26 Thread Pierre Moreau


- Mail original -
> If the hardware supports extended tag field (8-bit ones), then
> enabled it. This
> is usually done by the VBIOS, but not on some MBPs (see fdo#86537).
> In case extended tag field is not supported, 5-bit tag field is used
> which
> limits the possible values to 32. Apparently bits 7:0 of 0x8841c
> stores some
> number of outstanding requests, so cap it to 32 if extended tag is
> unsupported.
> 
> Fixes: fdo#86537
> 
> v2: Restrict changes to chipsets >= 0x84
> v3:
>   * Add nvkm_pci_mask to pci.h
>   * Mask bit 8 before setting it
> 
> Signed-off-by: Pierre Moreau 

Tested-by: Karol Herbst 

> ---
>  drm/nouveau/include/nvkm/subdev/pci.h |  1 +
>  drm/nouveau/nvkm/subdev/pci/base.c| 25 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/pci.h
> b/drm/nouveau/include/nvkm/subdev/pci.h
> index 5b3c054..774ca66 100644
> --- a/drm/nouveau/include/nvkm/subdev/pci.h
> +++ b/drm/nouveau/include/nvkm/subdev/pci.h
> @@ -24,6 +24,7 @@ struct nvkm_pci {
>  u32 nvkm_pci_rd32(struct nvkm_pci *, u16 addr);
>  void nvkm_pci_wr08(struct nvkm_pci *, u16 addr, u8 data);
>  void nvkm_pci_wr32(struct nvkm_pci *, u16 addr, u32 data);
> +u32 nvkm_pci_mask(struct nvkm_pci *, u16 addr, u32 mask, u32 add);
>  void nvkm_pci_rom_shadow(struct nvkm_pci *, bool shadow);
>  
>  int nv04_pci_new(struct nvkm_device *, int, struct nvkm_pci **);
> diff --git a/drm/nouveau/nvkm/subdev/pci/base.c
> b/drm/nouveau/nvkm/subdev/pci/base.c
> index d1c148e..cb2835b 100644
> --- a/drm/nouveau/nvkm/subdev/pci/base.c
> +++ b/drm/nouveau/nvkm/subdev/pci/base.c
> @@ -46,6 +46,14 @@ nvkm_pci_wr32(struct nvkm_pci *pci, u16 addr, u32
> data)
>   pci->func->wr32(pci, addr, data);
>  }
>  
> +u32
> +nvkm_pci_mask(struct nvkm_pci *pci, u16 addr, u32 mask, u32 add)
> +{
> + u32 data = pci->func->rd32(pci, addr);
> + pci->func->wr32(pci, addr, (data & ~mask) | add);
> + return data;
> +}
> +
>  void
>  nvkm_pci_rom_shadow(struct nvkm_pci *pci, bool shadow)
>  {
> @@ -115,6 +123,23 @@ nvkm_pci_init(struct nvkm_subdev *subdev)
>   if (ret)
>   return ret;
>  
> + if (pci_is_pcie(pdev) && subdev->device->chipset >= 0x84) {
> + /* Tag field is 8-bit long, regardless of EXT_TAG.
> +  * However, if EXT_TAG is disabled, only the lower 5 bits of the
> tag
> +  * field should be used, limiting the number of request to 32.
> +  *
> +  * Apparently, 0x041c stores some limit on the number of 
> requests
> +  * possible, so if EXT_TAG is disabled, limit that requests 
> number
> to
> +  * 32
> +  *
> +  * Fixes fdo#86537
> +  */
> + if (nvkm_pci_rd32(pci, 0x007c) & 0x0020)
> + nvkm_pci_mask(pci, 0x0080, 0x0100, 0x0100);
> + else
> + nvkm_pci_mask(pci, 0x041c, 0x0060, 0x);
> + }
> +
>   pci->irq = pdev->irq;
>   return ret;
>  }
> --
> 2.5.2
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 


[PATCH v3] pci: Handle 5-bit and 8-bit tag field

2015-09-16 Thread Pierre Moreau
If the hardware supports extended tag field (8-bit ones), then enabled it. This
is usually done by the VBIOS, but not on some MBPs (see fdo#86537).
In case extended tag field is not supported, 5-bit tag field is used which
limits the possible values to 32. Apparently bits 7:0 of 0x8841c stores some
number of outstanding requests, so cap it to 32 if extended tag is unsupported.

Fixes: fdo#86537

v2: Restrict changes to chipsets >= 0x84
v3:
  * Add nvkm_pci_mask to pci.h
  * Mask bit 8 before setting it

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/include/nvkm/subdev/pci.h |  1 +
 drm/nouveau/nvkm/subdev/pci/base.c| 25 +
 2 files changed, 26 insertions(+)

diff --git a/drm/nouveau/include/nvkm/subdev/pci.h 
b/drm/nouveau/include/nvkm/subdev/pci.h
index 5b3c054..774ca66 100644
--- a/drm/nouveau/include/nvkm/subdev/pci.h
+++ b/drm/nouveau/include/nvkm/subdev/pci.h
@@ -24,6 +24,7 @@ struct nvkm_pci {
 u32 nvkm_pci_rd32(struct nvkm_pci *, u16 addr);
 void nvkm_pci_wr08(struct nvkm_pci *, u16 addr, u8 data);
 void nvkm_pci_wr32(struct nvkm_pci *, u16 addr, u32 data);
+u32 nvkm_pci_mask(struct nvkm_pci *, u16 addr, u32 mask, u32 add);
 void nvkm_pci_rom_shadow(struct nvkm_pci *, bool shadow);

 int nv04_pci_new(struct nvkm_device *, int, struct nvkm_pci **);
diff --git a/drm/nouveau/nvkm/subdev/pci/base.c 
b/drm/nouveau/nvkm/subdev/pci/base.c
index d1c148e..cb2835b 100644
--- a/drm/nouveau/nvkm/subdev/pci/base.c
+++ b/drm/nouveau/nvkm/subdev/pci/base.c
@@ -46,6 +46,14 @@ nvkm_pci_wr32(struct nvkm_pci *pci, u16 addr, u32 data)
pci->func->wr32(pci, addr, data);
 }

+u32
+nvkm_pci_mask(struct nvkm_pci *pci, u16 addr, u32 mask, u32 add)
+{
+   u32 data = pci->func->rd32(pci, addr);
+   pci->func->wr32(pci, addr, (data & ~mask) | add);
+   return data;
+}
+
 void
 nvkm_pci_rom_shadow(struct nvkm_pci *pci, bool shadow)
 {
@@ -115,6 +123,23 @@ nvkm_pci_init(struct nvkm_subdev *subdev)
if (ret)
return ret;

+   if (pci_is_pcie(pdev) && subdev->device->chipset >= 0x84) {
+   /* Tag field is 8-bit long, regardless of EXT_TAG.
+* However, if EXT_TAG is disabled, only the lower 5 bits of 
the tag
+* field should be used, limiting the number of request to 32.
+*
+* Apparently, 0x041c stores some limit on the number of 
requests
+* possible, so if EXT_TAG is disabled, limit that requests 
number to
+* 32
+*
+* Fixes fdo#86537
+*/
+   if (nvkm_pci_rd32(pci, 0x007c) & 0x0020)
+   nvkm_pci_mask(pci, 0x0080, 0x0100, 0x0100);
+   else
+   nvkm_pci_mask(pci, 0x041c, 0x0060, 0x);
+   }
+
pci->irq = pdev->irq;
return ret;
 }
-- 
2.5.2



[PATCH v2] pci: Handle 5-bit and 8-bit tag field

2015-09-16 Thread Pierre Moreau
If the hardware supports extended tag field (8-bit ones), then enabled it. This
is usually done by the VBIOS, but not on some MBPs (see fdo#86537).
In case extended tag field is not supported, 5-bit tag field is used which
limits the possible values to 32. Apparently bits 7:0 of 0x8841c stores some
number of outstanding requests, so cap it to 32 if extended tag is unsupported.

Fixes: fdo#86537

v2: Restrict changes to chipsets >= 0x84

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nvkm/subdev/pci/base.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/pci/base.c 
b/drm/nouveau/nvkm/subdev/pci/base.c
index d1c148e..bbad29d 100644
--- a/drm/nouveau/nvkm/subdev/pci/base.c
+++ b/drm/nouveau/nvkm/subdev/pci/base.c
@@ -46,6 +46,14 @@ nvkm_pci_wr32(struct nvkm_pci *pci, u16 addr, u32 data)
pci->func->wr32(pci, addr, data);
 }

+u32
+nvkm_pci_mask(struct nvkm_pci *pci, u16 addr, u32 mask, u32 add)
+{
+   u32 data = pci->func->rd32(pci, addr);
+   pci->func->wr32(pci, addr, (data & ~mask) | add);
+   return data;
+}
+
 void
 nvkm_pci_rom_shadow(struct nvkm_pci *pci, bool shadow)
 {
@@ -115,6 +123,23 @@ nvkm_pci_init(struct nvkm_subdev *subdev)
if (ret)
return ret;

+   if (pci_is_pcie(pdev) && subdev->device->chipset >= 0x84) {
+   /* Tag field is 8-bit long, regardless of EXT_TAG.
+* However, if EXT_TAG is disabled, only the lower 5 bits of 
the tag
+* field should be used, limiting the number of request to 32.
+*
+* Apparently, 0x041c stores some limit on the number of 
requests
+* possible, so if EXT_TAG is disabled, limit that requests 
number to
+* 32
+*
+* Fixes fdo#86537
+*/
+   if (nvkm_pci_rd32(pci, 0x007c) & 0x0020)
+   nvkm_pci_mask(pci, 0x0080, 0x, 0x0100);
+   else
+   nvkm_pci_mask(pci, 0x041c, 0x0060, 0x);
+   }
+
pci->irq = pdev->irq;
return ret;
 }
-- 
2.5.2



[PATCH] pci: Handle 5-bit and 8-bit tag field

2015-09-06 Thread Pierre Moreau
If the hardware supports extended tag field (8-bit ones), then enabled it. This
is usually done by the VBIOS, but not on some MBPs (see fdo#86537).
In case extended tag field is not supported, 5-bit tag field is used which
limits the possible values to 32. Apparently bits 7:0 of 0x8841c stores some
number of outstanding requests, so cap it to 32 if extended tag is unsupported.

Fixes: fdo#86537
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nvkm/subdev/pci/base.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/pci/base.c 
b/drm/nouveau/nvkm/subdev/pci/base.c
index d1c148e..70631b3 100644
--- a/drm/nouveau/nvkm/subdev/pci/base.c
+++ b/drm/nouveau/nvkm/subdev/pci/base.c
@@ -46,6 +46,14 @@ nvkm_pci_wr32(struct nvkm_pci *pci, u16 addr, u32 data)
pci->func->wr32(pci, addr, data);
 }

+u32
+nvkm_pci_mask(struct nvkm_pci *pci, u16 addr, u32 mask, u32 add)
+{
+   u32 data = pci->func->rd32(pci, addr);
+   pci->func->wr32(pci, addr, (data & ~mask) | add);
+   return data;
+}
+
 void
 nvkm_pci_rom_shadow(struct nvkm_pci *pci, bool shadow)
 {
@@ -115,6 +123,23 @@ nvkm_pci_init(struct nvkm_subdev *subdev)
if (ret)
return ret;

+   if (pci_is_pcie(pdev)) {
+   /* Tag field is 8-bit long, regardless of EXT_TAG.
+* However, if EXT_TAG is disabled, only the lower 5 bits of 
the tag
+* field should be used, limiting the number of request to 32.
+*
+* Apparently, 0x041c stores some limit on the number of 
requests
+* possible, so if EXT_TAG is disabled, limit that requests 
number to
+* 32
+*
+* Fixes fdo#86537
+*/
+   if (nvkm_pci_rd32(pci, 0x007c) & 0x0020)
+   nvkm_pci_mask(pci, 0x0080, 0x, 0x0100);
+   else
+   nvkm_pci_mask(pci, 0x041c, 0x0060, 0x);
+   }
+
pci->irq = pdev->irq;
return ret;
 }
-- 
2.5.1



[PATCH] pci: Handle 5-bit and 8-bit tag field

2015-09-06 Thread Pierre Moreau
If the hardware supports extended tag field (8-bit ones), then enabled it. This
is usually done by the VBIOS, but not on some MBPs (see fdo#86537).
In case extended tag field is not supported, 5-bit tag field is used which
limits the possible values to 32. Apparently bits 7:0 of 0x8841c stores some
number of outstanding requests, so cap it to 32 if extended tag is unsupported.

Fixes: fdo#86537
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nvkm/subdev/pci/base.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/pci/base.c 
b/drm/nouveau/nvkm/subdev/pci/base.c
index d1c148e..70631b3 100644
--- a/drm/nouveau/nvkm/subdev/pci/base.c
+++ b/drm/nouveau/nvkm/subdev/pci/base.c
@@ -46,6 +46,14 @@ nvkm_pci_wr32(struct nvkm_pci *pci, u16 addr, u32 data)
pci->func->wr32(pci, addr, data);
 }

+u32
+nvkm_pci_mask(struct nvkm_pci *pci, u16 addr, u32 mask, u32 add)
+{
+   u32 data = pci->func->rd32(pci, addr);
+   pci->func->wr32(pci, addr, (data & ~mask) | add);
+   return data;
+}
+
 void
 nvkm_pci_rom_shadow(struct nvkm_pci *pci, bool shadow)
 {
@@ -115,6 +123,23 @@ nvkm_pci_init(struct nvkm_subdev *subdev)
if (ret)
return ret;

+   if (pci_is_pcie(pdev)) {
+   /* Tag field is 8-bit long, regardless of EXT_TAG.
+* However, if EXT_TAG is disabled, only the lower 5 bits of 
the tag
+* field should be used, limiting the number of request to 32.
+*
+* Apparently, 0x041c stores some limit on the number of 
requests
+* possible, so if EXT_TAG is disabled, limit that requests 
number to
+* 32
+*
+* Fixes fdo#86537
+*/
+   if (nvkm_pci_rd32(pci, 0x007c) & 0x0020)
+   nvkm_pci_mask(pci, 0x0080, 0x, 0x0100);
+   else
+   nvkm_pci_mask(pci, 0x041c, 0x0060, 0x);
+   }
+
pci->irq = pdev->irq;
return ret;
 }
-- 
2.5.1



[Nouveau] [PATCH v2 8/9] acpi: Add support for Apple Gmux _DMS

2015-05-31 Thread Pierre Moreau
Hi Lukas

- Mail original -
> Hi Pierre,
> 
> On Thu, May 28, 2015 at 10:52:56AM +0200, Pierre Moreau wrote:
> > How is it supposed to work, by simply echo'ing ON or OFF to
> > vga_switcheroo/switch? Then I probably forgot some stuff as
> > it doesn't want to work on my laptop.
> 
> What exactly doesn't work and which version of the MacBook Pro did
> you try this on? The expected behaviour when switching GPUs on the
> MBP is that the GPU that was inactive on bootup will only show a
> black screen because it was unable to read the EDID (and DPCD on
> retina MBPs) on bootup. In other words, it's broken.

I'm trying on a mid-2009 MBP, which has a double NVidia setup with a 9400M as 
IGD and 9600M GT as DIS. Switching between both works (as long as do a 
PCI-reset of the G96, but that's another story and I should have a patch soon, 
hopefully).
It was also tested by another user on a retina MBP and we ran into the black 
screen issue. I pointed him to your patches, but I don't know yet if it helped 
or not.

> 
> There have been attempts to solve this by multiple folks, the last
> one was submitted to dri-devel in April by yours truly:
> http://lists.freedesktop.org/archives/dri-devel/2015-April/081515.html
> 
> That initial version of my patch set turned out to only work on
> pre-retina MBPs, and only if the active GPU on bootup is the
> discrete one. I'm currently working on a v2 to solve all that.

Cool! Looking forward to it!

> 
> We need to get switching working before we can enable runtime PM on
> MBPs, otherwise the user will be greeted by a black screen when the
> Nvidia GPU goes to sleep and hands over to the integrated GPU.
> So your patch is a bit premature I'm afraid. :-(

It's power management++: we even save energy on the screen by "powering it off" 
for better battery life! ;)

> 
> 
> As to the patch itself, you're checking for existence of the gmux
> but we can't make use of it unless its driver is loaded. Hence it
> may be easier to simply query for the existence of the driver,
> using something along the lines of find_module("apple-gmux").
> I imagine this is much less code than checking for the DMI IDs.

I didn't know there was such a function! Seems nice!

> 
> Also, Dave Airlie and Matthew Garrett already wrote a patch set to
> enable runtime PM for nouveau on MBPs which is more generic: It works
> with any vga_switcheroo handler, not just gmux:
> 
> http://lists.freedesktop.org/archives/dri-devel/2014-June/060927.html
> http://lists.freedesktop.org/archives/dri-devel/2014-June/060928.html
> http://lists.freedesktop.org/archives/dri-devel/2014-June/060942.html
> http://www.codon.org.uk/~mjg59/tmp/retina_patches/0027-nouveau-enable-runtime-pm-on-apple-gmux-machines.patch
> http://www.codon.org.uk/~mjg59/tmp/retina_patches/0028-nouveau-Enable-switcheroo-dynamic-PM-at-switcheroo-e.patch
> 
> The fourth one in that list is by Dave Airlie and all the others are
> by Matthew Garrett. But, as I said, we can't apply these either until
> switching works. I'm working on it. ;-)

I had no idea these patches existed: I should have looked around before.
I'll drop the patches, apart probably from the *cleaning* ones, but I'll wait 
for the previously mentioned ones to first land.

> 
> 
> As to your questions:
> 
> > How is it supposed to work, by simply echo'ing ON or OFF to
> > vga_switcheroo/switch?
> 
> Echoing ON/OFF to the "switch" file will power the inactive device
> up or down. If the currently active gpu is the discrete one (which
> is the default when booting a MacBook Pro)

Not on the mid-2009 MBP! ;) Which is great!

> , this will power up/down
> the integrated gpu. You can use the tool "gpu-switch" to force the
> MBP to the integrated GPU on next bootup, then you can echo ON/OFFI'ddd
> to change the power state of the Nvidia GPU:
> https://github.com/0xbb/gpu-switch
> 
> > If I understand it correctly, if the laptop is an Optimus one or
> > has a mux, we are not "allowed" to opt-out of DynPwr/DynOff by
> > powering down the card?
> 
> If we power the GPU down manually by echoing OFF to the "switch"
> file, the runtime PM code would probably get confused, I imagine
> that's the reason why ON/OFF is ignored if
> client->driver_power_control
> is set to true.

I would have guessed that runtime pm would then get disabled.

> 
> Speaking of which, the external ports of the MBP are soldered to
> the discrete GPU and can't be switched. Only the panel can be
> switched. So if there are DP or HDMI connectors with status
> connected, we can't suspend. That's one of the MBP's numerous
> peculiarities that I haven't verified yet they're handled properly
> by the code.

I could check that on my MBP if you want.



Best regards,

Pierre


> 
> Best regards,
> 
> Lukas
> 


[Nouveau] [PATCH v2 8/9] acpi: Add support for Apple Gmux _DMS

2015-05-28 Thread Pierre Moreau
Hi Dave,


- Mail original -
> Changes since v1:
[...]
> diff --git a/drm/nouveau/nouveau_vga.c b/drm/nouveau/nouveau_vga.c
> index 9a6328f..7b13804 100644
> --- a/drm/nouveau/nouveau_vga.c
> +++ b/drm/nouveau/nouveau_vga.c
> @@ -36,7 +36,7 @@ nouveau_switcheroo_set_state(struct pci_dev *pdev,
>  {
>   struct drm_device *dev = pci_get_drvdata(pdev);
>  
> - if ((nouveau_is_optimus() || nouveau_has_mux()) && state ==
> VGA_SWITCHEROO_OFF)

If I understand it correctly, if the laptop is an Optimus one or has a mux, we 
are not "allowed" to opt-out of DynPwr/DynOff by powering down the card?
In the same commit adding this conditional 
(5addcf0a5f0fadceba6bd562d0616a1c5d4c1a4d), you added the possibility to 
enable/disable dynpm. How is it supposed to work, by simply echo'ing ON or OFF 
to vga_switcheroo/switch? Then I probably forgot some stuff as it doesn't want 
to work on my laptop.

Pierre

> + if (nouveau_has_dsm() && state == VGA_SWITCHEROO_OFF)
>   return;
>  
>   if (state == VGA_SWITCHEROO_ON) {
> @@ -96,11 +96,11 @@ nouveau_vga_init(struct nouveau_drm *drm)
>  
>   if (nouveau_runtime_pm == 1)
>   runtime = true;
> - if ((nouveau_runtime_pm == -1) && (nouveau_is_optimus() ||
> nouveau_has_mux()))
> + if ((nouveau_runtime_pm == -1) && nouveau_has_dsm())
>   runtime = true;
>   vga_switcheroo_register_client(dev->pdev, _switcheroo_ops,
>   runtime);
>  
> - if (runtime && nouveau_has_mux() && !nouveau_is_optimus())
> + if (runtime && (nouveau_has_mux() || nouveau_has_gmux()) &&
> !nouveau_is_optimus())
>   vga_switcheroo_init_domain_pm_ops(drm->dev->dev,
>   >vga_pm_domain);
>  }
>  
> @@ -112,11 +112,11 @@ nouveau_vga_fini(struct nouveau_drm *drm)
>  
>   if (nouveau_runtime_pm == 1)
>   runtime = true;
> - if ((nouveau_runtime_pm == -1) && (nouveau_is_optimus() ||
> nouveau_has_mux()))
> + if ((nouveau_runtime_pm == -1) && nouveau_has_dsm())
>   runtime = true;
>  
>   vga_switcheroo_unregister_client(dev->pdev);
> - if (runtime && nouveau_has_mux() && !nouveau_is_optimus())
> + if (runtime && (nouveau_has_mux() || nouveau_has_gmux()) &&
> !nouveau_is_optimus())
>   vga_switcheroo_fini_domain_pm_ops(drm->dev->dev);
>   vga_client_register(dev->pdev, NULL, NULL, NULL);
>  }
> --
> 2.4.2
> 
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
> 


[Nouveau] [PATCH NEW 5/9] acpi: Use a single type of errors in evaluate functions

2015-05-28 Thread Pierre Moreau
I wasn't sure how to insert this one into the flow, if I should make it a patch 
4.5 or not. I went with patch 5, so all the following patches are incremented 
by 1 compared to the v1, sorry about that.
What should be the way to go?

Pierre



> On 28 May 2015, at 08:40, Pierre Moreau  wrote:
> 
> Signed-off-by: Pierre Moreau 
> ---
> drm/nouveau/nouveau_acpi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
> index 01dbf1d..b837ab1 100644
> --- a/drm/nouveau/nouveau_acpi.c
> +++ b/drm/nouveau/nouveau_acpi.c
> @@ -92,7 +92,7 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
> int func, int arg, u
>  func, , ACPI_TYPE_BUFFER);
>if (!obj) {
>acpi_handle_info(handle, "failed to evaluate _DSM\n");
> -return AE_ERROR;
> +return -EINVAL;
>} else {
>if (result && obj->buffer.length == 4) {
>*result  = obj->buffer.pointer[0];
> @@ -142,7 +142,7 @@ static int nouveau_evaluate_mux_dsm(acpi_handle handle, 
> int func, int arg)
>  func, , ACPI_TYPE_INTEGER);
>if (!obj) {
>acpi_handle_info(handle, "failed to evaluate _DSM\n");
> -return AE_ERROR;
> +return -EINVAL;
>} else {
>if (obj->integer.value == 0x8002)
>ret = -ENODEV;
> -- 
> 2.4.2
> 
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau


[PATCH v2 9/9] acpi: Move detecting available _DSM outside of while loops

2015-05-28 Thread Pierre Moreau
Changes since v1:
* Re-write the whole patch to detect available _DSMs when we enable them rather
  than on each loop run

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 27 ++-
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index ee5fcfa..a970e41 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -301,12 +301,9 @@ static bool nouveau_dsm_detect(void)
char acpi_method_name[255] = { 0 };
struct acpi_buffer buffer = {sizeof(acpi_method_name), 
acpi_method_name};
struct pci_dev *pdev = NULL;
-   int has_mux = 0;
-   int has_optimus = 0;
-   bool has_gmux = false;
int vga_count = 0;
bool guid_valid;
-   int retval;
+   int retval = 0;
bool ret = false;

/* lookup the MXM GUID */
@@ -319,43 +316,31 @@ static bool nouveau_dsm_detect(void)
while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != 
NULL) {
vga_count++;

-   retval = nouveau_dsm_pci_probe(pdev);
-   if (retval & NOUVEAU_DSM_HAS_MUX)
-   has_mux |= 1;
-   if (retval & NOUVEAU_DSM_HAS_OPT)
-   has_optimus = 1;
-   if (retval & NOUVEAU_DSM_HAS_GMUX)
-   has_gmux = true;
+   retval |= nouveau_dsm_pci_probe(pdev);
}

while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL) 
{
vga_count++;

-   retval = nouveau_dsm_pci_probe(pdev);
-   if (retval & NOUVEAU_DSM_HAS_MUX)
-   has_mux |= 1;
-   if (retval & NOUVEAU_DSM_HAS_OPT)
-   has_optimus = 1;
-   if (retval & NOUVEAU_DSM_HAS_GMUX)
-   has_gmux = true;
+   retval |= nouveau_dsm_pci_probe(pdev);
}

/* find the optimus DSM, the mux DSM or the gmux DSM */
-   if (has_optimus == 1) {
+   if (retval & NOUVEAU_DSM_HAS_OPT) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
);
printk(KERN_INFO "VGA switcheroo: detected Optimus DSM method 
%s handle\n",
acpi_method_name);
nouveau_dsm_priv.optimus_detected = true;
ret = true;
-   } else if (vga_count == 2 && has_mux && guid_valid) {
+   } else if (vga_count == 2 && (retval & NOUVEAU_DSM_HAS_MUX) && 
guid_valid) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
);
printk(KERN_INFO "VGA switcheroo: detected mux DSM switching 
method %s handle\n",
acpi_method_name);
nouveau_dsm_priv.mux_detected = true;
ret = true;
-   } else if (has_gmux) {
+   } else if (retval & NOUVEAU_DSM_HAS_GMUX) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
);
printk(KERN_INFO "VGA switcheroo: detected gmux DSM switching 
method %s handle\n",
-- 
2.4.2



[PATCH v2 8/9] acpi: Add support for Apple Gmux _DMS

2015-05-28 Thread Pierre Moreau
Changes since v1:
* Factorise testing of the 3 different _DSMs presence with a single function
* Check for gmux in nouveau_switcheroo_set_state
* Change some comments and messages that were referring explicitly to Optimus
  whereas they were also testing the mux and gmux _DSMs

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 57 --
 drm/nouveau/nouveau_acpi.h |  4 
 drm/nouveau/nouveau_drm.c  | 12 +-
 drm/nouveau/nouveau_vga.c  | 10 
 4 files changed, 70 insertions(+), 13 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index edc4b94..ee5fcfa 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -29,6 +29,10 @@

 #define NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN (NOUVEAU_DSM_OPTIMUS_POWERDOWN_PS3 | 
NOUVEAU_DSM_OPTIMUS_FLAGS_CHANGED)

+#define NOUVEAU_DSM_GMUX_V1_CAPS (3 << 6)
+
+#define NOUVEAU_DSM_GMUX_V2_CAPS (1 << 1)
+
 /* result of the optimus caps function */
 #define OPTIMUS_ENABLED (1 << 0)
 #define OPTIMUS_STATUS_MASK (3 << 3)
@@ -45,6 +49,7 @@
 static struct nouveau_dsm_priv {
bool mux_detected;
bool optimus_detected;
+   bool gmux_detected;
acpi_handle dhandle;
acpi_handle rom_handle;
 } nouveau_dsm_priv;
@@ -57,8 +62,17 @@ bool nouveau_has_mux(void) {
return nouveau_dsm_priv.mux_detected;
 }

+bool nouveau_has_gmux(void) {
+   return nouveau_dsm_priv.gmux_detected;
+}
+
+bool nouveau_has_dsm(void) {
+   return nouveau_is_optimus() || nouveau_has_mux() || nouveau_has_gmux();
+}
+
 #define NOUVEAU_DSM_HAS_MUX 0x1
 #define NOUVEAU_DSM_HAS_OPT 0x2
+#define NOUVEAU_DSM_HAS_GMUX 0x4

 #ifdef CONFIG_VGA_SWITCHEROO
 static const uint32_t nouveau_mux_dsm_rid = 0x0102;
@@ -73,6 +87,18 @@ static const char nouveau_op_dsm_muid[] = {
0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0,
 };

+static const uint32_t nouveau_gmux_v1_dsm_rid = 0x101;
+static const char nouveau_gmux_v1_dsm_muid[] = {
+   0xA6, 0x69, 0x86, 0x99, 0xE9, 0x8B, 0xFB, 0x49,
+   0xBD, 0xDB, 0x51, 0xA1, 0xEF, 0xE1, 0x9C, 0x3D,
+};
+
+static const uint32_t nouveau_gmux_v2_dsm_rid = 0x001;
+static const char nouveau_gmux_v2_dsm_muid[] = {
+   0xC6, 0xB7, 0xB5, 0xA0, 0x18, 0x13, 0x1C, 0x44,
+   0xB0, 0xC9, 0xFE, 0x69, 0x5E, 0xAF, 0x94, 0x9B,
+};
+
 static int nouveau_evaluate_optimus_dsm(acpi_handle handle, int func, int arg, 
uint32_t *result)
 {
int i;
@@ -161,6 +187,17 @@ static int nouveau_evaluate_mux_dsm(acpi_handle handle, 
int func, int arg)
return ret;
 }

+static int nouveau_check_gmux_dsm(acpi_handle handle)
+{
+   bool has_v1 = acpi_check_dsm(handle, nouveau_gmux_v1_dsm_muid,
+nouveau_gmux_v1_dsm_rid,
+NOUVEAU_DSM_GMUX_V1_CAPS);
+   bool has_v2 = acpi_check_dsm(handle, nouveau_gmux_v2_dsm_muid,
+nouveau_gmux_v2_dsm_rid,
+NOUVEAU_DSM_GMUX_V2_CAPS);
+   return has_v1 || has_v2;
+}
+
 static int nouveau_dsm_switch_mux(acpi_handle handle, int mux_id)
 {
mxm_wmi_call_mxmx(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
@@ -238,6 +275,9 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
   1 << NOUVEAU_DSM_MUX_POWER))
retval |= NOUVEAU_DSM_HAS_MUX;

+   if (nouveau_check_gmux_dsm(dhandle))
+   retval |= NOUVEAU_DSM_HAS_GMUX;
+
if (nouveau_check_optimus_dsm(dhandle))
retval |= NOUVEAU_DSM_HAS_OPT;

@@ -263,6 +303,7 @@ static bool nouveau_dsm_detect(void)
struct pci_dev *pdev = NULL;
int has_mux = 0;
int has_optimus = 0;
+   bool has_gmux = false;
int vga_count = 0;
bool guid_valid;
int retval;
@@ -283,6 +324,8 @@ static bool nouveau_dsm_detect(void)
has_mux |= 1;
if (retval & NOUVEAU_DSM_HAS_OPT)
has_optimus = 1;
+   if (retval & NOUVEAU_DSM_HAS_GMUX)
+   has_gmux = true;
}

while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL) 
{
@@ -293,9 +336,11 @@ static bool nouveau_dsm_detect(void)
has_mux |= 1;
if (retval & NOUVEAU_DSM_HAS_OPT)
has_optimus = 1;
+   if (retval & NOUVEAU_DSM_HAS_GMUX)
+   has_gmux = true;
}

-   /* find the optimus DSM or the mux DSM */
+   /* find the optimus DSM, the mux DSM or the gmux DSM */
if (has_optimus == 1) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
);
@@ -310,6 +355,13 @@ static bool nouveau_dsm_detect(void)
acpi_method_name);
nouveau_dsm_

[PATCH 7/9] acpi: Improve some messages level and content

2015-05-28 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 8 
 drm/nouveau/nouveau_vga.c  | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 44a7328..edc4b94 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -91,7 +91,7 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
int func, int arg, u
obj = acpi_evaluate_dsm(handle, nouveau_op_dsm_muid, nouveau_op_dsm_rid,
func, );
if (!obj) {
-   acpi_handle_info(handle, "failed to evaluate _DSM\n");
+   acpi_handle_err(handle, "failed to evaluate Optimus _DSM\n");
return -EINVAL;
} else if (obj->type == ACPI_TYPE_BUFFER) {
if (result && obj->buffer.length == 4) {
@@ -150,7 +150,7 @@ static int nouveau_evaluate_mux_dsm(acpi_handle handle, int 
func, int arg)
obj = acpi_evaluate_dsm_typed(handle, nouveau_mux_dsm_muid, 
nouveau_mux_dsm_rid,
  func, , ACPI_TYPE_INTEGER);
if (!obj) {
-   acpi_handle_info(handle, "failed to evaluate _DSM\n");
+   acpi_handle_err(handle, "failed to evaluate mux _DSM\n");
return -EINVAL;
} else {
if (obj->integer.value == 0x8002)
@@ -272,7 +272,7 @@ static bool nouveau_dsm_detect(void)
guid_valid = mxm_wmi_supported();

if (guid_valid)
-   printk("MXM: GUID detected in BIOS\n");
+   printk(KERN_INFO "MXM: GUID detected in BIOS\n");

/* now do DSM detection */
while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != 
NULL) {
@@ -375,7 +375,7 @@ static int nouveau_rom_call(acpi_handle rom_handle, uint8_t 
*bios,

status = acpi_evaluate_object(rom_handle, NULL, _arg, );
if (ACPI_FAILURE(status)) {
-   printk(KERN_INFO "failed to evaluate ROM got %s\n", 
acpi_format_exception(status));
+   printk(KERN_ERR "ACPI: failed to evaluate ROM got %s\n", 
acpi_format_exception(status));
return -ENODEV;
}
obj = (union acpi_object *)buffer.pointer;
diff --git a/drm/nouveau/nouveau_vga.c b/drm/nouveau/nouveau_vga.c
index 6297566..9a6328f 100644
--- a/drm/nouveau/nouveau_vga.c
+++ b/drm/nouveau/nouveau_vga.c
@@ -40,13 +40,13 @@ nouveau_switcheroo_set_state(struct pci_dev *pdev,
return;

if (state == VGA_SWITCHEROO_ON) {
-   printk(KERN_ERR "VGA switcheroo: switched nouveau on\n");
+   printk(KERN_INFO "VGA switcheroo: switched nouveau on\n");
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
nouveau_pmops_resume(>dev);
drm_kms_helper_poll_enable(dev);
dev->switch_power_state = DRM_SWITCH_POWER_ON;
} else {
-   printk(KERN_ERR "VGA switcheroo: switched nouveau off\n");
+   printk(KERN_INFO "VGA switcheroo: switched nouveau off\n");
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
drm_kms_helper_poll_disable(dev);
nouveau_switcheroo_optimus_dsm();
-- 
2.4.2



[PATCH v2 6/9] acpi: Check returned object type by Optimus _DSM locally

2015-05-28 Thread Pierre Moreau
Most _DSM will return an integer value of 0x8002 when given an unknown
UUID, revision ID or function ID. Checking locally allows us to differentiate
that case from other ACPI errors, and to not report a "failed to evaluate _DSM"
if 0x8002 is returned which was confusing.

Changes since v1:
* Replace ACPI errors with standard kernel ones

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index b837ab1..44a7328 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -88,12 +88,12 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
int func, int arg, u
for (i = 0; i < 4; i++)
args_buff[i] = (arg >> i * 8) & 0xFF;

-   obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
nouveau_op_dsm_rid,
- func, , ACPI_TYPE_BUFFER);
+   obj = acpi_evaluate_dsm(handle, nouveau_op_dsm_muid, nouveau_op_dsm_rid,
+   func, );
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
return -EINVAL;
-   } else {
+   } else if (obj->type == ACPI_TYPE_BUFFER) {
if (result && obj->buffer.length == 4) {
*result  = obj->buffer.pointer[0];
*result |= (obj->buffer.pointer[1] << 8);
@@ -101,6 +101,15 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle 
handle, int func, int arg, u
*result |= (obj->buffer.pointer[3] << 24);
}
ACPI_FREE(obj);
+   } else if (obj->type == ACPI_TYPE_INTEGER &&
+  obj->integer.value == 0x8002) {
+   acpi_handle_debug(handle, "failed to query Optimus _DSM\n");
+   ACPI_FREE(obj);
+   return -ENODEV;
+   } else {
+   acpi_handle_err(handle, "unexpected returned value by Optimus 
_DSM\n");
+   ACPI_FREE(obj);
+   return -EINVAL;
}

return 0;
-- 
2.4.2



[PATCH NEW 5/9] acpi: Use a single type of errors in evaluate functions

2015-05-28 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 01dbf1d..b837ab1 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -92,7 +92,7 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
int func, int arg, u
  func, , ACPI_TYPE_BUFFER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
-   return AE_ERROR;
+   return -EINVAL;
} else {
if (result && obj->buffer.length == 4) {
*result  = obj->buffer.pointer[0];
@@ -142,7 +142,7 @@ static int nouveau_evaluate_mux_dsm(acpi_handle handle, int 
func, int arg)
  func, , ACPI_TYPE_INTEGER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
-   return AE_ERROR;
+   return -EINVAL;
} else {
if (obj->integer.value == 0x8002)
ret = -ENODEV;
-- 
2.4.2



[PATCH v2 4/9] acpi: Allow evaluate_optimus_dsm to take NULL results

2015-05-28 Thread Pierre Moreau
Changes since v1:
* Fix check for result being not NULL in nouveau_evaluate_optimus_dsm

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 3f01067..01dbf1d 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -88,15 +88,14 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
int func, int arg, u
for (i = 0; i < 4; i++)
args_buff[i] = (arg >> i * 8) & 0xFF;

-   *result = 0;
obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
nouveau_op_dsm_rid,
  func, , ACPI_TYPE_BUFFER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
return AE_ERROR;
} else {
-   if (obj->buffer.length == 4) {
-   *result |= obj->buffer.pointer[0];
+   if (result && obj->buffer.length == 4) {
+   *result  = obj->buffer.pointer[0];
*result |= (obj->buffer.pointer[1] << 8);
*result |= (obj->buffer.pointer[2] << 16);
*result |= (obj->buffer.pointer[3] << 24);
@@ -322,18 +321,17 @@ void nouveau_register_dsm_handler(void)
 /* Must be called for Optimus models before the card can be turned off */
 void nouveau_switcheroo_optimus_dsm(void)
 {
-   u32 result = 0;
if (!nouveau_dsm_priv.optimus_detected)
return;

nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
 NOUVEAU_DSM_OPTIMUS_FLAGS,
-0x3, );
+0x3, NULL);

nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
 NOUVEAU_DSM_OPTIMUS_CAPS,
 NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN,
-);
+NULL);

 }

-- 
2.4.2



[PATCH 3/9] acpi: Define static variables for revision ids

2015-05-28 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index e18fbeb..3f01067 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -61,11 +61,13 @@ bool nouveau_has_mux(void) {
 #define NOUVEAU_DSM_HAS_OPT 0x2

 #ifdef CONFIG_VGA_SWITCHEROO
+static const uint32_t nouveau_mux_dsm_rid = 0x0102;
 static const char nouveau_mux_dsm_muid[] = {
0xA0, 0xA0, 0x95, 0x9D, 0x60, 0x00, 0x48, 0x4D,
0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12, 0x9F, 0xD4,
 };

+static const uint32_t nouveau_op_dsm_rid = 0x0100;
 static const char nouveau_op_dsm_muid[] = {
0xF8, 0xD8, 0x86, 0xA4, 0xDA, 0x0B, 0x1B, 0x47,
0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0,
@@ -87,7 +89,7 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
int func, int arg, u
args_buff[i] = (arg >> i * 8) & 0xFF;

*result = 0;
-   obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 0x0100,
+   obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
nouveau_op_dsm_rid,
  func, , ACPI_TYPE_BUFFER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
@@ -137,7 +139,7 @@ static int nouveau_evaluate_mux_dsm(acpi_handle handle, int 
func, int arg)
.integer.value = arg,
};

-   obj = acpi_evaluate_dsm_typed(handle, nouveau_mux_dsm_muid, 0x0102,
+   obj = acpi_evaluate_dsm_typed(handle, nouveau_mux_dsm_muid, 
nouveau_mux_dsm_rid,
  func, , ACPI_TYPE_INTEGER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
@@ -224,7 +226,7 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
if (!acpi_has_method(dhandle, "_DSM"))
return false;

-   if (acpi_check_dsm(dhandle, nouveau_mux_dsm_muid, 0x0102,
+   if (acpi_check_dsm(dhandle, nouveau_mux_dsm_muid, nouveau_mux_dsm_rid,
   1 << NOUVEAU_DSM_MUX_POWER))
retval |= NOUVEAU_DSM_HAS_MUX;

-- 
2.4.2



[PATCH v2 2/9] acpi: Add evaluate to name of functions evaluating _DSM

2015-05-28 Thread Pierre Moreau
This makes it clearer when reading the function name, as well as following the
names of related ACPI functions.

Changes since v1:
* Fix typo in commit message
* Slightly reformulate the commit message to be clearer

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index c90037f..e18fbeb 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -71,7 +71,7 @@ static const char nouveau_op_dsm_muid[] = {
0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0,
 };

-static int nouveau_optimus_dsm(acpi_handle handle, int func, int arg, uint32_t 
*result)
+static int nouveau_evaluate_optimus_dsm(acpi_handle handle, int func, int arg, 
uint32_t *result)
 {
int i;
union acpi_object *obj;
@@ -118,7 +118,7 @@ static int nouveau_check_optimus_dsm(acpi_handle handle)
 * Function 0 returns a Buffer containing available functions.
 * The args parameter is ignored for function 0, so just put 0 in it
 */
-   if (nouveau_optimus_dsm(handle, 0, 0, ))
+   if (nouveau_evaluate_optimus_dsm(handle, 0, 0, ))
return 0;

/*
@@ -128,7 +128,7 @@ static int nouveau_check_optimus_dsm(acpi_handle handle)
return result & 1 && result & (1 << NOUVEAU_DSM_OPTIMUS_CAPS);
 }

-static int nouveau_mux_dsm(acpi_handle handle, int func, int arg)
+static int nouveau_evaluate_mux_dsm(acpi_handle handle, int func, int arg)
 {
int ret = 0;
union acpi_object *obj;
@@ -155,7 +155,7 @@ static int nouveau_dsm_switch_mux(acpi_handle handle, int 
mux_id)
 {
mxm_wmi_call_mxmx(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
mxm_wmi_call_mxds(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
-   return nouveau_mux_dsm(handle, NOUVEAU_DSM_MUX_LED, mux_id);
+   return nouveau_evaluate_mux_dsm(handle, NOUVEAU_DSM_MUX_LED, mux_id);
 }

 static int nouveau_dsm_set_discrete_state(acpi_handle handle, enum 
vga_switcheroo_state state)
@@ -165,7 +165,7 @@ static int nouveau_dsm_set_discrete_state(acpi_handle 
handle, enum vga_switchero
arg = NOUVEAU_DSM_MUX_POWER_SPEED;
else
arg = NOUVEAU_DSM_MUX_POWER_STAMINA;
-   nouveau_mux_dsm(handle, NOUVEAU_DSM_MUX_POWER, arg);
+   nouveau_evaluate_mux_dsm(handle, NOUVEAU_DSM_MUX_POWER, arg);
return 0;
 }

@@ -233,8 +233,8 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)

if (retval & NOUVEAU_DSM_HAS_OPT) {
uint32_t result;
-   nouveau_optimus_dsm(dhandle, NOUVEAU_DSM_OPTIMUS_CAPS, 0,
-   );
+   nouveau_evaluate_optimus_dsm(dhandle, NOUVEAU_DSM_OPTIMUS_CAPS,
+0, );
dev_info(>dev, "optimus capabilities: %s, status %s%s\n",
 (result & OPTIMUS_ENABLED) ? "enabled" : "disabled",
 (result & OPTIMUS_DYNAMIC_PWR_CAP) ? "dynamic power, " 
: "",
@@ -324,11 +324,14 @@ void nouveau_switcheroo_optimus_dsm(void)
if (!nouveau_dsm_priv.optimus_detected)
return;

-   nouveau_optimus_dsm(nouveau_dsm_priv.dhandle, NOUVEAU_DSM_OPTIMUS_FLAGS,
-   0x3, );
+   nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
+NOUVEAU_DSM_OPTIMUS_FLAGS,
+0x3, );

-   nouveau_optimus_dsm(nouveau_dsm_priv.dhandle, NOUVEAU_DSM_OPTIMUS_CAPS,
-   NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN, );
+   nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
+NOUVEAU_DSM_OPTIMUS_CAPS,
+NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN,
+);

 }

-- 
2.4.2



[PATCH v2 1/9] acpi: Rename v1 DSM to mux to avoid ambiguity

2015-05-28 Thread Pierre Moreau
This is especially true when variables or functions are just called dsm without
specifying the v1.

Changes since v1:
* Fix typo in commit explanation
* Change has_dsm to has_mux in nouveau_dsm_detect

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 72 +++---
 drm/nouveau/nouveau_acpi.h |  4 +--
 drm/nouveau/nouveau_drm.c  |  4 +--
 drm/nouveau/nouveau_vga.c  | 10 +++
 4 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 6224246..c90037f 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -9,16 +9,16 @@
 #include "nouveau_drm.h"
 #include "nouveau_acpi.h"

-#define NOUVEAU_DSM_LED 0x02
-#define NOUVEAU_DSM_LED_STATE 0x00
-#define NOUVEAU_DSM_LED_OFF 0x10
-#define NOUVEAU_DSM_LED_STAMINA 0x11
-#define NOUVEAU_DSM_LED_SPEED 0x12
+#define NOUVEAU_DSM_MUX_LED 0x02
+#define NOUVEAU_DSM_MUX_LED_STATE 0x00
+#define NOUVEAU_DSM_MUX_LED_OFF 0x10
+#define NOUVEAU_DSM_MUX_LED_STAMINA 0x11
+#define NOUVEAU_DSM_MUX_LED_SPEED 0x12

-#define NOUVEAU_DSM_POWER 0x03
-#define NOUVEAU_DSM_POWER_STATE 0x00
-#define NOUVEAU_DSM_POWER_SPEED 0x01
-#define NOUVEAU_DSM_POWER_STAMINA 0x02
+#define NOUVEAU_DSM_MUX_POWER 0x03
+#define NOUVEAU_DSM_MUX_POWER_STATE 0x00
+#define NOUVEAU_DSM_MUX_POWER_SPEED 0x01
+#define NOUVEAU_DSM_MUX_POWER_STAMINA 0x02

 #define NOUVEAU_DSM_OPTIMUS_CAPS 0x1A
 #define NOUVEAU_DSM_OPTIMUS_FLAGS 0x1B
@@ -43,7 +43,7 @@
 #define OPTIMUS_HDA_CODEC_MASK (2 << 27) /* hda bios control */

 static struct nouveau_dsm_priv {
-   bool dsm_detected;
+   bool mux_detected;
bool optimus_detected;
acpi_handle dhandle;
acpi_handle rom_handle;
@@ -53,15 +53,15 @@ bool nouveau_is_optimus(void) {
return nouveau_dsm_priv.optimus_detected;
 }

-bool nouveau_is_v1_dsm(void) {
-   return nouveau_dsm_priv.dsm_detected;
+bool nouveau_has_mux(void) {
+   return nouveau_dsm_priv.mux_detected;
 }

 #define NOUVEAU_DSM_HAS_MUX 0x1
 #define NOUVEAU_DSM_HAS_OPT 0x2

 #ifdef CONFIG_VGA_SWITCHEROO
-static const char nouveau_dsm_muid[] = {
+static const char nouveau_mux_dsm_muid[] = {
0xA0, 0xA0, 0x95, 0x9D, 0x60, 0x00, 0x48, 0x4D,
0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12, 0x9F, 0xD4,
 };
@@ -128,7 +128,7 @@ static int nouveau_check_optimus_dsm(acpi_handle handle)
return result & 1 && result & (1 << NOUVEAU_DSM_OPTIMUS_CAPS);
 }

-static int nouveau_dsm(acpi_handle handle, int func, int arg)
+static int nouveau_mux_dsm(acpi_handle handle, int func, int arg)
 {
int ret = 0;
union acpi_object *obj;
@@ -137,7 +137,7 @@ static int nouveau_dsm(acpi_handle handle, int func, int 
arg)
.integer.value = arg,
};

-   obj = acpi_evaluate_dsm_typed(handle, nouveau_dsm_muid, 0x0102,
+   obj = acpi_evaluate_dsm_typed(handle, nouveau_mux_dsm_muid, 0x0102,
  func, , ACPI_TYPE_INTEGER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
@@ -153,30 +153,30 @@ static int nouveau_dsm(acpi_handle handle, int func, int 
arg)

 static int nouveau_dsm_switch_mux(acpi_handle handle, int mux_id)
 {
-   mxm_wmi_call_mxmx(mux_id == NOUVEAU_DSM_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
-   mxm_wmi_call_mxds(mux_id == NOUVEAU_DSM_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
-   return nouveau_dsm(handle, NOUVEAU_DSM_LED, mux_id);
+   mxm_wmi_call_mxmx(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
+   mxm_wmi_call_mxds(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
+   return nouveau_mux_dsm(handle, NOUVEAU_DSM_MUX_LED, mux_id);
 }

 static int nouveau_dsm_set_discrete_state(acpi_handle handle, enum 
vga_switcheroo_state state)
 {
int arg;
if (state == VGA_SWITCHEROO_ON)
-   arg = NOUVEAU_DSM_POWER_SPEED;
+   arg = NOUVEAU_DSM_MUX_POWER_SPEED;
else
-   arg = NOUVEAU_DSM_POWER_STAMINA;
-   nouveau_dsm(handle, NOUVEAU_DSM_POWER, arg);
+   arg = NOUVEAU_DSM_MUX_POWER_STAMINA;
+   nouveau_mux_dsm(handle, NOUVEAU_DSM_MUX_POWER, arg);
return 0;
 }

 static int nouveau_dsm_switchto(enum vga_switcheroo_client_id id)
 {
-   if (!nouveau_dsm_priv.dsm_detected)
+   if (!nouveau_dsm_priv.mux_detected)
return 0;
if (id == VGA_SWITCHEROO_IGD)
-   return nouveau_dsm_switch_mux(nouveau_dsm_priv.dhandle, 
NOUVEAU_DSM_LED_STAMINA);
+   return nouveau_dsm_switch_mux(nouveau_dsm_priv.dhandle, 
NOUVEAU_DSM_MUX_LED_STAMINA);
else
-   return nouveau_dsm_switch_mux(nouveau_dsm_priv.dhandle, 
NOUVEAU_DSM_LED_SPEED);
+   return nouveau_dsm_switch_mux(nouveau_dsm_

[PATCH 8/8] acpi: Use booleans when probing different _DSM types

2015-05-26 Thread Pierre Moreau
I'm thinking of re-writing this patch to just OR the different returned retval 
and test for individual bits directly in the final conditionals.
So this would give something like:


int retval = 0;

while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL) {
vga_count++;
retval |= nouveau_dsm_pci_probe(pdev);
}
while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL) {
vga_count++;
retval |= nouveau_dsm_pci_probe(pdev);
}

if (retval & NOUVEAU_DSM_HAS_OPT) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
);
printk(KERN_INFO "VGA switcheroo: detected Optimus DSM method %s 
handle\n",
acpi_method_name);
nouveau_dsm_priv.optimus_detected = true;
ret = true;
} else if (vga_count == 2 && (retval & NOUVEAU_DSM_HAS_MUX) && guid_valid) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
);
printk(KERN_INFO "VGA switcheroo: detected mux DSM switching method %s 
handle\n",
acpi_method_name);
nouveau_dsm_priv.mux_detected = true;
ret = true;
} else if (retval & NOUVEAU_DSM_HAS_GMUX) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
);
printk(KERN_INFO "VGA switcheroo: detected gmux DSM switching method %s 
handle\n",
acpi_method_name);
nouveau_dsm_priv.gmux_detected = true;
ret = true;
}


Any opinions on this?

Pierre

- Mail original -
> Signed-off-by: Pierre Moreau 
> ---
>  drm/nouveau/nouveau_acpi.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
> index 3d6a1ea..5d63621 100644
> --- a/drm/nouveau/nouveau_acpi.c
> +++ b/drm/nouveau/nouveau_acpi.c
> @@ -297,8 +297,8 @@ static bool nouveau_dsm_detect(void)
>   char acpi_method_name[255] = { 0 };
>   struct acpi_buffer buffer = {sizeof(acpi_method_name),
>   acpi_method_name};
>   struct pci_dev *pdev = NULL;
> - int has_dsm = 0;
> - int has_optimus = 0;
> + bool has_mux = false;
> + bool has_optimus = false;
>   bool has_gmux = false;
>   int vga_count = 0;
>   bool guid_valid;
> @@ -317,9 +317,9 @@ static bool nouveau_dsm_detect(void)
>  
>   retval = nouveau_dsm_pci_probe(pdev);
>   if (retval & NOUVEAU_DSM_HAS_MUX)
> - has_dsm |= 1;
> + has_mux = true;
>   if (retval & NOUVEAU_DSM_HAS_OPT)
> - has_optimus = 1;
> + has_optimus = true;
>   if (retval & NOUVEAU_DSM_HAS_GMUX)
>   has_gmux = true;
>   }
> @@ -329,9 +329,9 @@ static bool nouveau_dsm_detect(void)
>  
>   retval = nouveau_dsm_pci_probe(pdev);
>   if (retval & NOUVEAU_DSM_HAS_MUX)
> - has_dsm |= 1;
> + has_mux = true;
>   if (retval & NOUVEAU_DSM_HAS_OPT)
> - has_optimus = 1;
> + has_optimus = true;
>   if (retval & NOUVEAU_DSM_HAS_GMUX)
>   has_gmux = true;
>   }
> @@ -344,7 +344,7 @@ static bool nouveau_dsm_detect(void)
>   acpi_method_name);
>   nouveau_dsm_priv.optimus_detected = true;
>   ret = true;
> - } else if (vga_count == 2 && has_dsm && guid_valid) {
> + } else if (vga_count == 2 && has_mux && guid_valid) {
>   acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
>   );
>   printk(KERN_INFO "VGA switcheroo: detected mux DSM switching
>   method %s handle\n",
> --
> 2.4.1
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 


[Nouveau] [PATCH 7/8] acpi: Add support for Apple Gmux _DMS

2015-05-26 Thread Pierre Moreau

> On 26 May 2015, at 10:13, Samuel Pitoiset  
> wrote:
> 
> 
> 
> On 05/26/2015 12:22 AM, Pierre Moreau wrote:
>> Signed-off-by: Pierre Moreau 
>> ---
>>  drm/nouveau/nouveau_acpi.c | 53 
>> --
>>  drm/nouveau/nouveau_acpi.h |  2 ++
>>  drm/nouveau/nouveau_drm.c  |  6 --
>>  drm/nouveau/nouveau_vga.c  | 10 +
>>  4 files changed, 63 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
>> index 104d291..3d6a1ea 100644
>> --- a/drm/nouveau/nouveau_acpi.c
>> +++ b/drm/nouveau/nouveau_acpi.c
>> @@ -29,6 +29,10 @@
>>#define NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN 
>> (NOUVEAU_DSM_OPTIMUS_POWERDOWN_PS3 | NOUVEAU_DSM_OPTIMUS_FLAGS_CHANGED)
>>  +#define NOUVEAU_DSM_GMUX_V1_CAPS (3 << 6)
>> +
>> +#define NOUVEAU_DSM_GMUX_V2_CAPS (1 << 1)
>> +
>>  /* result of the optimus caps function */
>>  #define OPTIMUS_ENABLED (1 << 0)
>>  #define OPTIMUS_STATUS_MASK (3 << 3)
>> @@ -45,6 +49,7 @@
>>  static struct nouveau_dsm_priv {
>>  bool mux_detected;
>>  bool optimus_detected;
>> +bool gmux_detected;
>>  acpi_handle dhandle;
>>  acpi_handle rom_handle;
>>  } nouveau_dsm_priv;
>> @@ -57,8 +62,13 @@ bool nouveau_has_mux(void) {
>>  return nouveau_dsm_priv.mux_detected;
>>  }
>>  +bool nouveau_has_gmux(void) {
>> +return nouveau_dsm_priv.gmux_detected;
>> +}
>> +
>>  #define NOUVEAU_DSM_HAS_MUX 0x1
>>  #define NOUVEAU_DSM_HAS_OPT 0x2
>> +#define NOUVEAU_DSM_HAS_GMUX 0x4
>>#ifdef CONFIG_VGA_SWITCHEROO
>>  static const uint32_t nouveau_mux_dsm_rid = 0x0102;
>> @@ -73,6 +83,18 @@ static const char nouveau_op_dsm_muid[] = {
>>  0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0,
>>  };
>>  +static const uint32_t nouveau_gmux_v1_dsm_rid = 0x101;
>> +static const char nouveau_gmux_v1_dsm_muid[] = {
>> +0xA6, 0x69, 0x86, 0x99, 0xE9, 0x8B, 0xFB, 0x49,
>> +0xBD, 0xDB, 0x51, 0xA1, 0xEF, 0xE1, 0x9C, 0x3D,
>> +};
>> +
>> +static const uint32_t nouveau_gmux_v2_dsm_rid = 0x001;
>> +static const char nouveau_gmux_v2_dsm_muid[] = {
>> +0xC6, 0xB7, 0xB5, 0xA0, 0x18, 0x13, 0x1C, 0x44,
>> +0xB0, 0xC9, 0xFE, 0x69, 0x5E, 0xAF, 0x94, 0x9B,
>> +};
>> +
>>  static int nouveau_evaluate_optimus_dsm(acpi_handle handle, int func, int 
>> arg, uint32_t *result)
>>  {
>>  int i;
>> @@ -161,6 +183,17 @@ static int nouveau_evaluate_mux_dsm(acpi_handle handle, 
>> int func, int arg)
>>  return ret;
>>  }
>>  +static int nouveau_check_gmux_dsm(acpi_handle handle)
>> +{
>> +bool has_v1 = acpi_check_dsm(handle, nouveau_gmux_v1_dsm_muid,
>> + nouveau_gmux_v1_dsm_rid,
>> + NOUVEAU_DSM_GMUX_V1_CAPS);
>> +bool has_v2 = acpi_check_dsm(handle, nouveau_gmux_v2_dsm_muid,
>> + nouveau_gmux_v2_dsm_rid,
>> + NOUVEAU_DSM_GMUX_V2_CAPS);
>> +return has_v1 || has_v2;
>> +}
>> +
>>  static int nouveau_dsm_switch_mux(acpi_handle handle, int mux_id)
>>  {
>>  mxm_wmi_call_mxmx(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
>> MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
>> @@ -238,6 +271,9 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
>> 1 << NOUVEAU_DSM_MUX_POWER))
>>  retval |= NOUVEAU_DSM_HAS_MUX;
>>  +   if (nouveau_check_gmux_dsm(dhandle))
>> +retval |= NOUVEAU_DSM_HAS_GMUX;
>> +
>>  if (nouveau_check_optimus_dsm(dhandle))
>>  retval |= NOUVEAU_DSM_HAS_OPT;
>>  @@ -263,6 +299,7 @@ static bool nouveau_dsm_detect(void)
>>  struct pci_dev *pdev = NULL;
>>  int has_dsm = 0;
>>  int has_optimus = 0;
>> +bool has_gmux = false;
>>  int vga_count = 0;
>>  bool guid_valid;
>>  int retval;
>> @@ -283,6 +320,8 @@ static bool nouveau_dsm_detect(void)
>>  has_dsm |= 1;
>>  if (retval & NOUVEAU_DSM_HAS_OPT)
>>  has_optimus = 1;
>> +if (retval & NOUVEAU_DSM_HAS_GMUX)
>> +has_gmux = true;
>>  }
>>  while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL) 
>> {
>> @@ -293,9 +332,11 @@ static bool nouveau_dsm_detect(void)
>>  has_dsm |= 1;
>

[Nouveau] [PATCH 4/8] acpi: Allow evaluate_optimus_dsm to take NULL results

2015-05-26 Thread Pierre Moreau

> On 26 May 2015, at 10:02, Samuel Pitoiset  
> wrote:
> 
> 
> 
> On 05/26/2015 12:22 AM, Pierre Moreau wrote:
>> Signed-off-by: Pierre Moreau 
>> ---
>>  drm/nouveau/nouveau_acpi.c | 10 --
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
>> index 36f4a40..073f7d7 100644
>> --- a/drm/nouveau/nouveau_acpi.c
>> +++ b/drm/nouveau/nouveau_acpi.c
>> @@ -88,15 +88,14 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle 
>> handle, int func, int arg, u
>>  for (i = 0; i < 4; i++)
>>  args_buff[i] = (arg >> i * 8) & 0xFF;
>>  -   *result = 0;
>>  obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
>> nouveau_op_dsm_rid,
>>func, , ACPI_TYPE_BUFFER);
>>  if (!obj) {
>>  acpi_handle_info(handle, "failed to evaluate _DSM\n");
>>  return AE_ERROR;
>>  } else {
>> -if (obj->buffer.length == 4) {
>> -*result |= obj->buffer.pointer[0];
>> +if (!result && obj->buffer.length == 4) {
>> +*result  = obj->buffer.pointer[0];
> 
> segfault here if result is NULL! ;)

Good catch!

> 
>>  *result |= (obj->buffer.pointer[1] << 8);
>>  *result |= (obj->buffer.pointer[2] << 16);
>>  *result |= (obj->buffer.pointer[3] << 24);
>> @@ -322,18 +321,17 @@ void nouveau_register_dsm_handler(void)
>>  /* Must be called for Optimus models before the card can be turned off */
>>  void nouveau_switcheroo_optimus_dsm(void)
>>  {
>> -u32 result = 0;
>>  if (!nouveau_dsm_priv.optimus_detected)
>>  return;
>>  nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
>>   NOUVEAU_DSM_OPTIMUS_FLAGS,
>> - 0x3, );
>> + 0x3, NULL);
>>  nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
>>   NOUVEAU_DSM_OPTIMUS_CAPS,
>>   NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN,
>> - );
>> + NULL);
>>}

-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150526/e86b377b/attachment-0001.html>


[Nouveau] [PATCH 5/8] acpi: Check returned object type by Optimus _DSM locally

2015-05-26 Thread Pierre Moreau

> On 26 May 2015, at 07:17, Ilia Mirkin  wrote:
> 
> On Tue, May 26, 2015 at 1:10 AM, Pierre Moreau  
> wrote:
>>> On 26 May 2015, at 00:39, Ilia Mirkin  wrote:
>>> 
>>>> On Mon, May 25, 2015 at 6:22 PM, Pierre Moreau  
>>>> wrote:
>>>> Most _DSM will return an integer value of 0x8002 when given an unknown
>>>> UUID, revision ID or function ID. Checking locally allows us to 
>>>> differentiate
>>>> that case from other ACPI errors, and to not report a "failed to evaluate 
>>>> _DSM"
>>>> if 0x8002 is returned which was confusing.
>>>> 
>>>> Signed-off-by: Pierre Moreau 
>>>> ---
>>>> drm/nouveau/nouveau_acpi.c | 15 ---
>>>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>>> 
>>>> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
>>>> index 073f7d7..7aeaf7d 100644
>>>> --- a/drm/nouveau/nouveau_acpi.c
>>>> +++ b/drm/nouveau/nouveau_acpi.c
>>>> @@ -88,12 +88,12 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle 
>>>> handle, int func, int arg, u
>>>>   for (i = 0; i < 4; i++)
>>>>   args_buff[i] = (arg >> i * 8) & 0xFF;
>>>> 
>>>> -   obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
>>>> nouveau_op_dsm_rid,
>>>> - func, , ACPI_TYPE_BUFFER);
>>>> +   obj = acpi_evaluate_dsm(handle, nouveau_op_dsm_muid, 
>>>> nouveau_op_dsm_rid,
>>>> +   func, );
>>>>   if (!obj) {
>>>>   acpi_handle_info(handle, "failed to evaluate _DSM\n");
>>>>   return AE_ERROR;
>>>> -   } else {
>>>> +   } else if (obj->type == ACPI_TYPE_BUFFER) {
>>>>   if (!result && obj->buffer.length == 4) {
>>>>   *result  = obj->buffer.pointer[0];
>>>>   *result |= (obj->buffer.pointer[1] << 8);
>>>> @@ -101,6 +101,15 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle 
>>>> handle, int func, int arg, u
>>>>   *result |= (obj->buffer.pointer[3] << 24);
>>>>   }
>>>>   ACPI_FREE(obj);
>>>> +   } else if (obj->type == ACPI_TYPE_INTEGER &&
>>>> +  obj->integer.value == 0x8002) {
>>>> +   acpi_handle_debug(handle, "failed to query Optimus 
>>>> _DSM\n");
>>>> +   ACPI_FREE(obj);
>>>> +   return -ENODEV;
>>> 
>>> should this be AE_ERROR?
>> 
>> I would say no, because ACPI was parsed correctly, just that we didn't it 
>> give the correct arguments, or rather, the _DSM we tested isn't an Optimus 
>> one, but it could a mux or gmux. And I used ENODEV as it is the value 
>> returned by nouveau_evaluate_mux_dsm in the same context.
> 
> Hm ok. It just seemed odd to be returning AE_* in one context, and
> -ENODEV in another context -- they're different types of errors.
> However if the caller handles it, I guess it's OK... I haven't looked
> at the API in depth.

The caller doesn’t care about the returned error and just check wether
it’s non-zero (and sometimes it doesn’t even check).

> 
>> 
>>> 
>>>> +   } else {
>>>> +   acpi_handle_err(handle, "unexpected returned value by 
>>>> Optimus _DSM\n");
>>>> +   ACPI_FREE(obj);
>>>> +   return AE_ERROR;
>>>>   }
>>>> 
>>>>   return 0;
>>>> --
>>>> 2.4.1
>>>> 
>>>> ___
>>>> Nouveau mailing list
>>>> Nouveau at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/nouveau



[Nouveau] [PATCH 5/8] acpi: Check returned object type by Optimus _DSM locally

2015-05-26 Thread Pierre Moreau




> On 26 May 2015, at 00:39, Ilia Mirkin  wrote:
> 
>> On Mon, May 25, 2015 at 6:22 PM, Pierre Moreau  
>> wrote:
>> Most _DSM will return an integer value of 0x8002 when given an unknown
>> UUID, revision ID or function ID. Checking locally allows us to differentiate
>> that case from other ACPI errors, and to not report a "failed to evaluate 
>> _DSM"
>> if 0x8002 is returned which was confusing.
>> 
>> Signed-off-by: Pierre Moreau 
>> ---
>> drm/nouveau/nouveau_acpi.c | 15 ---
>> 1 file changed, 12 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
>> index 073f7d7..7aeaf7d 100644
>> --- a/drm/nouveau/nouveau_acpi.c
>> +++ b/drm/nouveau/nouveau_acpi.c
>> @@ -88,12 +88,12 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle 
>> handle, int func, int arg, u
>>for (i = 0; i < 4; i++)
>>args_buff[i] = (arg >> i * 8) & 0xFF;
>> 
>> -   obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
>> nouveau_op_dsm_rid,
>> - func, , ACPI_TYPE_BUFFER);
>> +   obj = acpi_evaluate_dsm(handle, nouveau_op_dsm_muid, 
>> nouveau_op_dsm_rid,
>> +   func, );
>>if (!obj) {
>>acpi_handle_info(handle, "failed to evaluate _DSM\n");
>>return AE_ERROR;
>> -   } else {
>> +   } else if (obj->type == ACPI_TYPE_BUFFER) {
>>if (!result && obj->buffer.length == 4) {
>>*result  = obj->buffer.pointer[0];
>>*result |= (obj->buffer.pointer[1] << 8);
>> @@ -101,6 +101,15 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle 
>> handle, int func, int arg, u
>>*result |= (obj->buffer.pointer[3] << 24);
>>}
>>ACPI_FREE(obj);
>> +   } else if (obj->type == ACPI_TYPE_INTEGER &&
>> +  obj->integer.value == 0x8002) {
>> +   acpi_handle_debug(handle, "failed to query Optimus _DSM\n");
>> +   ACPI_FREE(obj);
>> +   return -ENODEV;
> 
> should this be AE_ERROR?

I would say no, because ACPI was parsed correctly, just that we didn't it give 
the correct arguments, or rather, the _DSM we tested isn't an Optimus one, but 
it could a mux or gmux. And I used ENODEV as it is the value returned by 
nouveau_evaluate_mux_dsm in the same context. 

> 
>> +   } else {
>> +   acpi_handle_err(handle, "unexpected returned value by 
>> Optimus _DSM\n");
>> +   ACPI_FREE(obj);
>> +   return AE_ERROR;
>>}
>> 
>>return 0;
>> --
>> 2.4.1
>> 
>> ___
>> Nouveau mailing list
>> Nouveau at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau


[PATCH 8/8] acpi: Use booleans when probing different _DSM types

2015-05-26 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 3d6a1ea..5d63621 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -297,8 +297,8 @@ static bool nouveau_dsm_detect(void)
char acpi_method_name[255] = { 0 };
struct acpi_buffer buffer = {sizeof(acpi_method_name), 
acpi_method_name};
struct pci_dev *pdev = NULL;
-   int has_dsm = 0;
-   int has_optimus = 0;
+   bool has_mux = false;
+   bool has_optimus = false;
bool has_gmux = false;
int vga_count = 0;
bool guid_valid;
@@ -317,9 +317,9 @@ static bool nouveau_dsm_detect(void)

retval = nouveau_dsm_pci_probe(pdev);
if (retval & NOUVEAU_DSM_HAS_MUX)
-   has_dsm |= 1;
+   has_mux = true;
if (retval & NOUVEAU_DSM_HAS_OPT)
-   has_optimus = 1;
+   has_optimus = true;
if (retval & NOUVEAU_DSM_HAS_GMUX)
has_gmux = true;
}
@@ -329,9 +329,9 @@ static bool nouveau_dsm_detect(void)

retval = nouveau_dsm_pci_probe(pdev);
if (retval & NOUVEAU_DSM_HAS_MUX)
-   has_dsm |= 1;
+   has_mux = true;
if (retval & NOUVEAU_DSM_HAS_OPT)
-   has_optimus = 1;
+   has_optimus = true;
if (retval & NOUVEAU_DSM_HAS_GMUX)
has_gmux = true;
}
@@ -344,7 +344,7 @@ static bool nouveau_dsm_detect(void)
acpi_method_name);
nouveau_dsm_priv.optimus_detected = true;
ret = true;
-   } else if (vga_count == 2 && has_dsm && guid_valid) {
+   } else if (vga_count == 2 && has_mux && guid_valid) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
);
printk(KERN_INFO "VGA switcheroo: detected mux DSM switching 
method %s handle\n",
-- 
2.4.1



[PATCH 7/8] acpi: Add support for Apple Gmux _DMS

2015-05-26 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 53 --
 drm/nouveau/nouveau_acpi.h |  2 ++
 drm/nouveau/nouveau_drm.c  |  6 --
 drm/nouveau/nouveau_vga.c  | 10 +
 4 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 104d291..3d6a1ea 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -29,6 +29,10 @@

 #define NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN (NOUVEAU_DSM_OPTIMUS_POWERDOWN_PS3 | 
NOUVEAU_DSM_OPTIMUS_FLAGS_CHANGED)

+#define NOUVEAU_DSM_GMUX_V1_CAPS (3 << 6)
+
+#define NOUVEAU_DSM_GMUX_V2_CAPS (1 << 1)
+
 /* result of the optimus caps function */
 #define OPTIMUS_ENABLED (1 << 0)
 #define OPTIMUS_STATUS_MASK (3 << 3)
@@ -45,6 +49,7 @@
 static struct nouveau_dsm_priv {
bool mux_detected;
bool optimus_detected;
+   bool gmux_detected;
acpi_handle dhandle;
acpi_handle rom_handle;
 } nouveau_dsm_priv;
@@ -57,8 +62,13 @@ bool nouveau_has_mux(void) {
return nouveau_dsm_priv.mux_detected;
 }

+bool nouveau_has_gmux(void) {
+   return nouveau_dsm_priv.gmux_detected;
+}
+
 #define NOUVEAU_DSM_HAS_MUX 0x1
 #define NOUVEAU_DSM_HAS_OPT 0x2
+#define NOUVEAU_DSM_HAS_GMUX 0x4

 #ifdef CONFIG_VGA_SWITCHEROO
 static const uint32_t nouveau_mux_dsm_rid = 0x0102;
@@ -73,6 +83,18 @@ static const char nouveau_op_dsm_muid[] = {
0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0,
 };

+static const uint32_t nouveau_gmux_v1_dsm_rid = 0x101;
+static const char nouveau_gmux_v1_dsm_muid[] = {
+   0xA6, 0x69, 0x86, 0x99, 0xE9, 0x8B, 0xFB, 0x49,
+   0xBD, 0xDB, 0x51, 0xA1, 0xEF, 0xE1, 0x9C, 0x3D,
+};
+
+static const uint32_t nouveau_gmux_v2_dsm_rid = 0x001;
+static const char nouveau_gmux_v2_dsm_muid[] = {
+   0xC6, 0xB7, 0xB5, 0xA0, 0x18, 0x13, 0x1C, 0x44,
+   0xB0, 0xC9, 0xFE, 0x69, 0x5E, 0xAF, 0x94, 0x9B,
+};
+
 static int nouveau_evaluate_optimus_dsm(acpi_handle handle, int func, int arg, 
uint32_t *result)
 {
int i;
@@ -161,6 +183,17 @@ static int nouveau_evaluate_mux_dsm(acpi_handle handle, 
int func, int arg)
return ret;
 }

+static int nouveau_check_gmux_dsm(acpi_handle handle)
+{
+   bool has_v1 = acpi_check_dsm(handle, nouveau_gmux_v1_dsm_muid,
+nouveau_gmux_v1_dsm_rid,
+NOUVEAU_DSM_GMUX_V1_CAPS);
+   bool has_v2 = acpi_check_dsm(handle, nouveau_gmux_v2_dsm_muid,
+nouveau_gmux_v2_dsm_rid,
+NOUVEAU_DSM_GMUX_V2_CAPS);
+   return has_v1 || has_v2;
+}
+
 static int nouveau_dsm_switch_mux(acpi_handle handle, int mux_id)
 {
mxm_wmi_call_mxmx(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
@@ -238,6 +271,9 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
   1 << NOUVEAU_DSM_MUX_POWER))
retval |= NOUVEAU_DSM_HAS_MUX;

+   if (nouveau_check_gmux_dsm(dhandle))
+   retval |= NOUVEAU_DSM_HAS_GMUX;
+
if (nouveau_check_optimus_dsm(dhandle))
retval |= NOUVEAU_DSM_HAS_OPT;

@@ -263,6 +299,7 @@ static bool nouveau_dsm_detect(void)
struct pci_dev *pdev = NULL;
int has_dsm = 0;
int has_optimus = 0;
+   bool has_gmux = false;
int vga_count = 0;
bool guid_valid;
int retval;
@@ -283,6 +320,8 @@ static bool nouveau_dsm_detect(void)
has_dsm |= 1;
if (retval & NOUVEAU_DSM_HAS_OPT)
has_optimus = 1;
+   if (retval & NOUVEAU_DSM_HAS_GMUX)
+   has_gmux = true;
}

while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL) 
{
@@ -293,9 +332,11 @@ static bool nouveau_dsm_detect(void)
has_dsm |= 1;
if (retval & NOUVEAU_DSM_HAS_OPT)
has_optimus = 1;
+   if (retval & NOUVEAU_DSM_HAS_GMUX)
+   has_gmux = true;
}

-   /* find the optimus DSM or the mux DSM */
+   /* find the optimus DSM, the mux DSM or the gmux DSM */
if (has_optimus == 1) {
acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
);
@@ -310,6 +351,13 @@ static bool nouveau_dsm_detect(void)
acpi_method_name);
nouveau_dsm_priv.mux_detected = true;
ret = true;
+   } else if (has_gmux) {
+   acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
+   );
+   printk(KERN_INFO "VGA switcheroo: detected gmux DSM switching 
method %s handle\n",
+   acpi_method_name);
+   nouveau_dsm_priv.gmux_detected = true;
+ 

[PATCH 6/8] acpi: Improve some messages level and content

2015-05-26 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 8 
 drm/nouveau/nouveau_vga.c  | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 7aeaf7d..104d291 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -91,7 +91,7 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
int func, int arg, u
obj = acpi_evaluate_dsm(handle, nouveau_op_dsm_muid, nouveau_op_dsm_rid,
func, );
if (!obj) {
-   acpi_handle_info(handle, "failed to evaluate _DSM\n");
+   acpi_handle_err(handle, "failed to evaluate Optimus _DSM\n");
return AE_ERROR;
} else if (obj->type == ACPI_TYPE_BUFFER) {
if (!result && obj->buffer.length == 4) {
@@ -150,7 +150,7 @@ static int nouveau_evaluate_mux_dsm(acpi_handle handle, int 
func, int arg)
obj = acpi_evaluate_dsm_typed(handle, nouveau_mux_dsm_muid, 
nouveau_mux_dsm_rid,
  func, , ACPI_TYPE_INTEGER);
if (!obj) {
-   acpi_handle_info(handle, "failed to evaluate _DSM\n");
+   acpi_handle_err(handle, "failed to evaluate mux _DSM\n");
return AE_ERROR;
} else {
if (obj->integer.value == 0x8002)
@@ -272,7 +272,7 @@ static bool nouveau_dsm_detect(void)
guid_valid = mxm_wmi_supported();

if (guid_valid)
-   printk("MXM: GUID detected in BIOS\n");
+   printk(KERN_INFO "MXM: GUID detected in BIOS\n");

/* now do DSM detection */
while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != 
NULL) {
@@ -375,7 +375,7 @@ static int nouveau_rom_call(acpi_handle rom_handle, uint8_t 
*bios,

status = acpi_evaluate_object(rom_handle, NULL, _arg, );
if (ACPI_FAILURE(status)) {
-   printk(KERN_INFO "failed to evaluate ROM got %s\n", 
acpi_format_exception(status));
+   printk(KERN_ERR "ACPI: failed to evaluate ROM got %s\n", 
acpi_format_exception(status));
return -ENODEV;
}
obj = (union acpi_object *)buffer.pointer;
diff --git a/drm/nouveau/nouveau_vga.c b/drm/nouveau/nouveau_vga.c
index 6297566..9a6328f 100644
--- a/drm/nouveau/nouveau_vga.c
+++ b/drm/nouveau/nouveau_vga.c
@@ -40,13 +40,13 @@ nouveau_switcheroo_set_state(struct pci_dev *pdev,
return;

if (state == VGA_SWITCHEROO_ON) {
-   printk(KERN_ERR "VGA switcheroo: switched nouveau on\n");
+   printk(KERN_INFO "VGA switcheroo: switched nouveau on\n");
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
nouveau_pmops_resume(>dev);
drm_kms_helper_poll_enable(dev);
dev->switch_power_state = DRM_SWITCH_POWER_ON;
} else {
-   printk(KERN_ERR "VGA switcheroo: switched nouveau off\n");
+   printk(KERN_INFO "VGA switcheroo: switched nouveau off\n");
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
drm_kms_helper_poll_disable(dev);
nouveau_switcheroo_optimus_dsm();
-- 
2.4.1



[PATCH 5/8] acpi: Check returned object type by Optimus _DSM locally

2015-05-26 Thread Pierre Moreau
Most _DSM will return an integer value of 0x8002 when given an unknown
UUID, revision ID or function ID. Checking locally allows us to differentiate
that case from other ACPI errors, and to not report a "failed to evaluate _DSM"
if 0x8002 is returned which was confusing.

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 073f7d7..7aeaf7d 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -88,12 +88,12 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
int func, int arg, u
for (i = 0; i < 4; i++)
args_buff[i] = (arg >> i * 8) & 0xFF;

-   obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
nouveau_op_dsm_rid,
- func, , ACPI_TYPE_BUFFER);
+   obj = acpi_evaluate_dsm(handle, nouveau_op_dsm_muid, nouveau_op_dsm_rid,
+   func, );
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
return AE_ERROR;
-   } else {
+   } else if (obj->type == ACPI_TYPE_BUFFER) {
if (!result && obj->buffer.length == 4) {
*result  = obj->buffer.pointer[0];
*result |= (obj->buffer.pointer[1] << 8);
@@ -101,6 +101,15 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle 
handle, int func, int arg, u
*result |= (obj->buffer.pointer[3] << 24);
}
ACPI_FREE(obj);
+   } else if (obj->type == ACPI_TYPE_INTEGER &&
+  obj->integer.value == 0x8002) {
+   acpi_handle_debug(handle, "failed to query Optimus _DSM\n");
+   ACPI_FREE(obj);
+   return -ENODEV;
+   } else {
+   acpi_handle_err(handle, "unexpected returned value by Optimus 
_DSM\n");
+   ACPI_FREE(obj);
+   return AE_ERROR;
}

return 0;
-- 
2.4.1



[PATCH 4/8] acpi: Allow evaluate_optimus_dsm to take NULL results

2015-05-26 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 36f4a40..073f7d7 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -88,15 +88,14 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
int func, int arg, u
for (i = 0; i < 4; i++)
args_buff[i] = (arg >> i * 8) & 0xFF;

-   *result = 0;
obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
nouveau_op_dsm_rid,
  func, , ACPI_TYPE_BUFFER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
return AE_ERROR;
} else {
-   if (obj->buffer.length == 4) {
-   *result |= obj->buffer.pointer[0];
+   if (!result && obj->buffer.length == 4) {
+   *result  = obj->buffer.pointer[0];
*result |= (obj->buffer.pointer[1] << 8);
*result |= (obj->buffer.pointer[2] << 16);
*result |= (obj->buffer.pointer[3] << 24);
@@ -322,18 +321,17 @@ void nouveau_register_dsm_handler(void)
 /* Must be called for Optimus models before the card can be turned off */
 void nouveau_switcheroo_optimus_dsm(void)
 {
-   u32 result = 0;
if (!nouveau_dsm_priv.optimus_detected)
return;

nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
 NOUVEAU_DSM_OPTIMUS_FLAGS,
-0x3, );
+0x3, NULL);

nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
 NOUVEAU_DSM_OPTIMUS_CAPS,
 NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN,
-);
+NULL);

 }

-- 
2.4.1



[PATCH 3/8] acpi: Define static variables for revision ids

2015-05-26 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 1f18018..36f4a40 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -61,11 +61,13 @@ bool nouveau_has_mux(void) {
 #define NOUVEAU_DSM_HAS_OPT 0x2

 #ifdef CONFIG_VGA_SWITCHEROO
+static const uint32_t nouveau_mux_dsm_rid = 0x0102;
 static const char nouveau_mux_dsm_muid[] = {
0xA0, 0xA0, 0x95, 0x9D, 0x60, 0x00, 0x48, 0x4D,
0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12, 0x9F, 0xD4,
 };

+static const uint32_t nouveau_op_dsm_rid = 0x0100;
 static const char nouveau_op_dsm_muid[] = {
0xF8, 0xD8, 0x86, 0xA4, 0xDA, 0x0B, 0x1B, 0x47,
0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0,
@@ -87,7 +89,7 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, 
int func, int arg, u
args_buff[i] = (arg >> i * 8) & 0xFF;

*result = 0;
-   obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 0x0100,
+   obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 
nouveau_op_dsm_rid,
  func, , ACPI_TYPE_BUFFER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
@@ -137,7 +139,7 @@ static int nouveau_evaluate_mux_dsm(acpi_handle handle, int 
func, int arg)
.integer.value = arg,
};

-   obj = acpi_evaluate_dsm_typed(handle, nouveau_mux_dsm_muid, 0x0102,
+   obj = acpi_evaluate_dsm_typed(handle, nouveau_mux_dsm_muid, 
nouveau_mux_dsm_rid,
  func, , ACPI_TYPE_INTEGER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
@@ -224,7 +226,7 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
if (!acpi_has_method(dhandle, "_DSM"))
return false;

-   if (acpi_check_dsm(dhandle, nouveau_mux_dsm_muid, 0x0102,
+   if (acpi_check_dsm(dhandle, nouveau_mux_dsm_muid, nouveau_mux_dsm_rid,
   1 << NOUVEAU_DSM_MUX_POWER))
retval |= NOUVEAU_DSM_HAS_MUX;

-- 
2.4.1



[PATCH 2/8] acpi: Add evaluate to functons evaluating _DSM

2015-05-26 Thread Pierre Moreau
This makes it clearer when reading the function name, as well as following the
names of the related ACPI function.

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 4a48e21..1f18018 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -71,7 +71,7 @@ static const char nouveau_op_dsm_muid[] = {
0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0,
 };

-static int nouveau_optimus_dsm(acpi_handle handle, int func, int arg, uint32_t 
*result)
+static int nouveau_evaluate_optimus_dsm(acpi_handle handle, int func, int arg, 
uint32_t *result)
 {
int i;
union acpi_object *obj;
@@ -118,7 +118,7 @@ static int nouveau_check_optimus_dsm(acpi_handle handle)
 * Function 0 returns a Buffer containing available functions.
 * The args parameter is ignored for function 0, so just put 0 in it
 */
-   if (nouveau_optimus_dsm(handle, 0, 0, ))
+   if (nouveau_evaluate_optimus_dsm(handle, 0, 0, ))
return 0;

/*
@@ -128,7 +128,7 @@ static int nouveau_check_optimus_dsm(acpi_handle handle)
return result & 1 && result & (1 << NOUVEAU_DSM_OPTIMUS_CAPS);
 }

-static int nouveau_mux_dsm(acpi_handle handle, int func, int arg)
+static int nouveau_evaluate_mux_dsm(acpi_handle handle, int func, int arg)
 {
int ret = 0;
union acpi_object *obj;
@@ -155,7 +155,7 @@ static int nouveau_dsm_switch_mux(acpi_handle handle, int 
mux_id)
 {
mxm_wmi_call_mxmx(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
mxm_wmi_call_mxds(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
-   return nouveau_mux_dsm(handle, NOUVEAU_DSM_MUX_LED, mux_id);
+   return nouveau_evaluate_mux_dsm(handle, NOUVEAU_DSM_MUX_LED, mux_id);
 }

 static int nouveau_dsm_set_discrete_state(acpi_handle handle, enum 
vga_switcheroo_state state)
@@ -165,7 +165,7 @@ static int nouveau_dsm_set_discrete_state(acpi_handle 
handle, enum vga_switchero
arg = NOUVEAU_DSM_MUX_POWER_SPEED;
else
arg = NOUVEAU_DSM_MUX_POWER_STAMINA;
-   nouveau_mux_dsm(handle, NOUVEAU_DSM_MUX_POWER, arg);
+   nouveau_evaluate_mux_dsm(handle, NOUVEAU_DSM_MUX_POWER, arg);
return 0;
 }

@@ -233,8 +233,8 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)

if (retval & NOUVEAU_DSM_HAS_OPT) {
uint32_t result;
-   nouveau_optimus_dsm(dhandle, NOUVEAU_DSM_OPTIMUS_CAPS, 0,
-   );
+   nouveau_evaluate_optimus_dsm(dhandle, NOUVEAU_DSM_OPTIMUS_CAPS,
+0, );
dev_info(>dev, "optimus capabilities: %s, status %s%s\n",
 (result & OPTIMUS_ENABLED) ? "enabled" : "disabled",
 (result & OPTIMUS_DYNAMIC_PWR_CAP) ? "dynamic power, " 
: "",
@@ -324,11 +324,14 @@ void nouveau_switcheroo_optimus_dsm(void)
if (!nouveau_dsm_priv.optimus_detected)
return;

-   nouveau_optimus_dsm(nouveau_dsm_priv.dhandle, NOUVEAU_DSM_OPTIMUS_FLAGS,
-   0x3, );
+   nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
+NOUVEAU_DSM_OPTIMUS_FLAGS,
+0x3, );

-   nouveau_optimus_dsm(nouveau_dsm_priv.dhandle, NOUVEAU_DSM_OPTIMUS_CAPS,
-   NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN, );
+   nouveau_evaluate_optimus_dsm(nouveau_dsm_priv.dhandle,
+NOUVEAU_DSM_OPTIMUS_CAPS,
+NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN,
+);

 }

-- 
2.4.1



[PATCH 1/8] acpi: Rename v1 DSM to mux to avoid ambiguity

2015-05-26 Thread Pierre Moreau
This is especially true when variables or functions are just called dsm without
precising the v1.

Signed-off-by: Pierre Moreau 
---
 drm/nouveau/nouveau_acpi.c | 64 +++---
 drm/nouveau/nouveau_acpi.h |  4 +--
 drm/nouveau/nouveau_drm.c  |  4 +--
 drm/nouveau/nouveau_vga.c  | 10 
 4 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c
index 6224246..4a48e21 100644
--- a/drm/nouveau/nouveau_acpi.c
+++ b/drm/nouveau/nouveau_acpi.c
@@ -9,16 +9,16 @@
 #include "nouveau_drm.h"
 #include "nouveau_acpi.h"

-#define NOUVEAU_DSM_LED 0x02
-#define NOUVEAU_DSM_LED_STATE 0x00
-#define NOUVEAU_DSM_LED_OFF 0x10
-#define NOUVEAU_DSM_LED_STAMINA 0x11
-#define NOUVEAU_DSM_LED_SPEED 0x12
+#define NOUVEAU_DSM_MUX_LED 0x02
+#define NOUVEAU_DSM_MUX_LED_STATE 0x00
+#define NOUVEAU_DSM_MUX_LED_OFF 0x10
+#define NOUVEAU_DSM_MUX_LED_STAMINA 0x11
+#define NOUVEAU_DSM_MUX_LED_SPEED 0x12

-#define NOUVEAU_DSM_POWER 0x03
-#define NOUVEAU_DSM_POWER_STATE 0x00
-#define NOUVEAU_DSM_POWER_SPEED 0x01
-#define NOUVEAU_DSM_POWER_STAMINA 0x02
+#define NOUVEAU_DSM_MUX_POWER 0x03
+#define NOUVEAU_DSM_MUX_POWER_STATE 0x00
+#define NOUVEAU_DSM_MUX_POWER_SPEED 0x01
+#define NOUVEAU_DSM_MUX_POWER_STAMINA 0x02

 #define NOUVEAU_DSM_OPTIMUS_CAPS 0x1A
 #define NOUVEAU_DSM_OPTIMUS_FLAGS 0x1B
@@ -43,7 +43,7 @@
 #define OPTIMUS_HDA_CODEC_MASK (2 << 27) /* hda bios control */

 static struct nouveau_dsm_priv {
-   bool dsm_detected;
+   bool mux_detected;
bool optimus_detected;
acpi_handle dhandle;
acpi_handle rom_handle;
@@ -53,15 +53,15 @@ bool nouveau_is_optimus(void) {
return nouveau_dsm_priv.optimus_detected;
 }

-bool nouveau_is_v1_dsm(void) {
-   return nouveau_dsm_priv.dsm_detected;
+bool nouveau_has_mux(void) {
+   return nouveau_dsm_priv.mux_detected;
 }

 #define NOUVEAU_DSM_HAS_MUX 0x1
 #define NOUVEAU_DSM_HAS_OPT 0x2

 #ifdef CONFIG_VGA_SWITCHEROO
-static const char nouveau_dsm_muid[] = {
+static const char nouveau_mux_dsm_muid[] = {
0xA0, 0xA0, 0x95, 0x9D, 0x60, 0x00, 0x48, 0x4D,
0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12, 0x9F, 0xD4,
 };
@@ -128,7 +128,7 @@ static int nouveau_check_optimus_dsm(acpi_handle handle)
return result & 1 && result & (1 << NOUVEAU_DSM_OPTIMUS_CAPS);
 }

-static int nouveau_dsm(acpi_handle handle, int func, int arg)
+static int nouveau_mux_dsm(acpi_handle handle, int func, int arg)
 {
int ret = 0;
union acpi_object *obj;
@@ -137,7 +137,7 @@ static int nouveau_dsm(acpi_handle handle, int func, int 
arg)
.integer.value = arg,
};

-   obj = acpi_evaluate_dsm_typed(handle, nouveau_dsm_muid, 0x0102,
+   obj = acpi_evaluate_dsm_typed(handle, nouveau_mux_dsm_muid, 0x0102,
  func, , ACPI_TYPE_INTEGER);
if (!obj) {
acpi_handle_info(handle, "failed to evaluate _DSM\n");
@@ -153,30 +153,30 @@ static int nouveau_dsm(acpi_handle handle, int func, int 
arg)

 static int nouveau_dsm_switch_mux(acpi_handle handle, int mux_id)
 {
-   mxm_wmi_call_mxmx(mux_id == NOUVEAU_DSM_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
-   mxm_wmi_call_mxds(mux_id == NOUVEAU_DSM_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
-   return nouveau_dsm(handle, NOUVEAU_DSM_LED, mux_id);
+   mxm_wmi_call_mxmx(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
+   mxm_wmi_call_mxds(mux_id == NOUVEAU_DSM_MUX_LED_STAMINA ? 
MXM_MXDS_ADAPTER_IGD : MXM_MXDS_ADAPTER_0);
+   return nouveau_mux_dsm(handle, NOUVEAU_DSM_MUX_LED, mux_id);
 }

 static int nouveau_dsm_set_discrete_state(acpi_handle handle, enum 
vga_switcheroo_state state)
 {
int arg;
if (state == VGA_SWITCHEROO_ON)
-   arg = NOUVEAU_DSM_POWER_SPEED;
+   arg = NOUVEAU_DSM_MUX_POWER_SPEED;
else
-   arg = NOUVEAU_DSM_POWER_STAMINA;
-   nouveau_dsm(handle, NOUVEAU_DSM_POWER, arg);
+   arg = NOUVEAU_DSM_MUX_POWER_STAMINA;
+   nouveau_mux_dsm(handle, NOUVEAU_DSM_MUX_POWER, arg);
return 0;
 }

 static int nouveau_dsm_switchto(enum vga_switcheroo_client_id id)
 {
-   if (!nouveau_dsm_priv.dsm_detected)
+   if (!nouveau_dsm_priv.mux_detected)
return 0;
if (id == VGA_SWITCHEROO_IGD)
-   return nouveau_dsm_switch_mux(nouveau_dsm_priv.dhandle, 
NOUVEAU_DSM_LED_STAMINA);
+   return nouveau_dsm_switch_mux(nouveau_dsm_priv.dhandle, 
NOUVEAU_DSM_MUX_LED_STAMINA);
else
-   return nouveau_dsm_switch_mux(nouveau_dsm_priv.dhandle, 
NOUVEAU_DSM_LED_SPEED);
+   return nouveau_dsm_switch_mux(nouveau_dsm_priv.dhandle, 
NOUVEAU_DSM_MUX_LED_SPEED);
 }

 static int nouveau_dsm_power_state(enum vga_switcheroo_client_id id,
@

[Nouveau] [PATCH v3 2/2] fb/nvaa: Enable non-isometric poller on NVAA/NVAC

2014-12-11 Thread Pierre Moreau
Hi Ben,

My bad, I will correct that. I'll have a better look at the three patches
you attached tomorrow.

Cheers,

Pierre

On 01:42 PM - Dec 11 2014, Ben Skeggs wrote:
> On Wed, Dec 10, 2014 at 5:53 PM, Pierre Moreau  
> wrote:
> > (This is a v3 of patch "drm/nouveau/fb/nv50: Add PFB writes")
> >
> > This fix a GPU lockup on 9400M (NVAC) when using acceleration, see
> > https://bugs.freedesktop.org/show_bug.cgi?id=27501
> >
> > v2:
> > - Move code to subdev/fb/nv50.c as suggested by Roy Spliet;
> > - Remove arbitrary writes to 100c18/100c24
> > - Replace write to 100c1c of arbitrary value by the address of a scratch 
> > page
> >   as proposed by Ilia Mirkin;
> > - Remove enabling of bits 16 and 0 as they don't yield in any changes.
> >
> > v3:
> > - Move code to subdev/fb/nvaa.c as suggested by Ilia Mirkin.
> > The following changes were made thanks to information provided by Robert 
> > Morell
> > from NVidia:
> > - Allocate a dma page for use by the pollers;
> > - Re-enable pollers at bits 16 and 0;
> > - Set pollers address to a proper value.
> Hey Pierre,
> 
> This patch is incorrect.  As Robert pointed out in an older thread,
> the registers don't take the physical address of a page, but a
> somewhat odd "negative offset from the end of carveout".
> 
> See this example (referring to Robert's last email):
> 
> 220.926392read32 #3 +0x00100e10 -> 0x0007
> 220.926406read32 #3 +0x00100e14 -> 0x0001
> 
> carveout_base = 0x7000
> carveout_size = 0x1000
> 
> -- snip --
> 
> 223.300495read32 #3 +0x00100c14 -> 0x
> 223.300521read32 #3 +0x00100c18 -> 0x
> 223.300547   write32 #3 +0x00100c18 <- 0x27ff
> 
> base = (0x7000 + 0x1000) - ((0x27ff + 1) << 5)
> base = 0x7ffb << in carveout
> 
> 223.300573read32 #3 +0x00100c14 -> 0x
> 223.300599   write32 #3 +0x00100c14 <- 0x0001
> 223.300625read32 #3 +0x00100c1c -> 0x0002
> 223.300651   write32 #3 +0x00100c1c <- 0x27fe
> 
> base = (0x7000 + 0x1000) - ((0x27fe + 1) << 5)
> base = 0x7ffb0020 << in carveout
> 
> 223.300677read32 #3 +0x00100c14 -> 0x0001
> 223.300702   write32 #3 +0x00100c14 <- 0x0003
> 223.300728read32 #3 +0x00100c24 -> 0x0004
> 223.300754   write32 #3 +0x00100c24 <- 0x27fd
> 
> base = (0x7000 + 0x1000) - ((0x27ff + 1) << 5)
> base = 0x7ffb0040 << in carveout
> 
> 223.300780read32 #3 +0x00100c14 -> 0x0003
> 223.300806   write32 #3 +0x00100c14 <- 0x00010003
> 
> It's a bit tricky at that point in the nouveau init process to
> allocate device memory, but, we already reserve 1MiB of memory at the
> end of "VRAM" (carveout / stolen memory) so we can make use of that.
> 
> I've attached 3 patches.  The first two of them, I'll probably merge as-is.
> 
> The third patch is an example of how this should probably be done, in
> case you want to continue tackling this some more :)
> 
> Cheers,
> Ben.
> 
> >
> > Signed-off-by: Pierre Moreau 
> > ---
> >  drm/core/subdev/fb/nvaa.h |  1 +
> >  nvkm/subdev/fb/nv50.h |  4 +++
> >  nvkm/subdev/fb/nvaa.c | 74 
> > ---
> >  nvkm/subdev/fb/nvaa.h | 19 
> >  4 files changed, 94 insertions(+), 4 deletions(-)
> >  create mode 12 drm/core/subdev/fb/nvaa.h
> >  create mode 100644 nvkm/subdev/fb/nvaa.h
> >
> > diff --git a/drm/core/subdev/fb/nvaa.h b/drm/core/subdev/fb/nvaa.h
> > new file mode 12
> > index 000..b450e8c
> > --- /dev/null
> > +++ b/drm/core/subdev/fb/nvaa.h
> > @@ -0,0 +1 @@
> > +../../../../nvkm/subdev/fb/nvaa.h
> > \ No newline at end of file
> > diff --git a/nvkm/subdev/fb/nv50.h b/nvkm/subdev/fb/nv50.h
> > index c5e5a88..0b20975 100644
> > --- a/nvkm/subdev/fb/nv50.h
> > +++ b/nvkm/subdev/fb/nv50.h
> > @@ -9,6 +9,10 @@ struct nv50_fb_priv {
> > dma_addr_t r100c08;
> >  };
> >
> > +#define nv50_fb_create(p,e,c,d,o)  
> > \
> > +   nv50_fb_ctor((p), (e), (c), (d), sizeof(**o),   
> >\
> > +   (struct nouveau_object **)o)
> > +
> >  int  nv50_fb_ctor(struct nouveau_object *, struct nouveau_object *,
> >   struct nouveau_oclass *, void *, u32,
> >   struct nouveau_object **);
> > diff --git a/nvkm/subdev/fb/nvaa.c b/nvkm/subde

[PATCH v3 2/2] fb/nvaa: Enable non-isometric poller on NVAA/NVAC

2014-12-10 Thread Pierre Moreau
(This is a v3 of patch "drm/nouveau/fb/nv50: Add PFB writes")

This fix a GPU lockup on 9400M (NVAC) when using acceleration, see
https://bugs.freedesktop.org/show_bug.cgi?id=27501

v2:
- Move code to subdev/fb/nv50.c as suggested by Roy Spliet;
- Remove arbitrary writes to 100c18/100c24
- Replace write to 100c1c of arbitrary value by the address of a scratch page
  as proposed by Ilia Mirkin;
- Remove enabling of bits 16 and 0 as they don't yield in any changes.

v3:
- Move code to subdev/fb/nvaa.c as suggested by Ilia Mirkin.
The following changes were made thanks to information provided by Robert Morell
from NVidia:
- Allocate a dma page for use by the pollers;
- Re-enable pollers at bits 16 and 0;
- Set pollers address to a proper value.

Signed-off-by: Pierre Moreau 
---
 drm/core/subdev/fb/nvaa.h |  1 +
 nvkm/subdev/fb/nv50.h |  4 +++
 nvkm/subdev/fb/nvaa.c | 74 ---
 nvkm/subdev/fb/nvaa.h | 19 
 4 files changed, 94 insertions(+), 4 deletions(-)
 create mode 12 drm/core/subdev/fb/nvaa.h
 create mode 100644 nvkm/subdev/fb/nvaa.h

diff --git a/drm/core/subdev/fb/nvaa.h b/drm/core/subdev/fb/nvaa.h
new file mode 12
index 000..b450e8c
--- /dev/null
+++ b/drm/core/subdev/fb/nvaa.h
@@ -0,0 +1 @@
+../../../../nvkm/subdev/fb/nvaa.h
\ No newline at end of file
diff --git a/nvkm/subdev/fb/nv50.h b/nvkm/subdev/fb/nv50.h
index c5e5a88..0b20975 100644
--- a/nvkm/subdev/fb/nv50.h
+++ b/nvkm/subdev/fb/nv50.h
@@ -9,6 +9,10 @@ struct nv50_fb_priv {
dma_addr_t r100c08;
 };

+#define nv50_fb_create(p,e,c,d,o)  
\
+   nv50_fb_ctor((p), (e), (c), (d), sizeof(**o),  \
+   (struct nouveau_object **)o)
+
 int  nv50_fb_ctor(struct nouveau_object *, struct nouveau_object *,
  struct nouveau_oclass *, void *, u32,
  struct nouveau_object **);
diff --git a/nvkm/subdev/fb/nvaa.c b/nvkm/subdev/fb/nvaa.c
index cba8e68..b70ab2f 100644
--- a/nvkm/subdev/fb/nvaa.c
+++ b/nvkm/subdev/fb/nvaa.c
@@ -22,15 +22,81 @@
  * Authors: Ben Skeggs
  */

-#include "nv50.h"
+#include "nvaa.h"
+
+int
+nvaa_fb_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
+struct nouveau_oclass *oclass, void *data, u32 size,
+struct nouveau_object **pobject)
+{
+   struct nouveau_device *device = nv_device(parent);
+   struct nvaa_fb_priv *priv;
+   int ret;
+
+   ret = nv50_fb_create(parent, engine, oclass, data, );
+   *pobject = nv_object(priv);
+   if (ret)
+   return ret;
+
+   priv = (struct nvaa_fb_priv *)(*pobject);
+
+   priv->r100c18_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+   if (priv->r100c18_page) {
+   priv->r100c18 = dma_map_page(nv_device_base(device),
+priv->r100c18_page, 0, PAGE_SIZE,
+DMA_BIDIRECTIONAL);
+   if (dma_mapping_error(nv_device_base(device), priv->r100c18))
+   return -EFAULT;
+   } else {
+   nv_warn(priv, "failed 0x100c18 page alloc\n");
+   }
+   return 0;
+}
+
+void
+nvaa_fb_dtor(struct nouveau_object *object)
+{
+   struct nouveau_device *device = nv_device(object);
+   struct nvaa_fb_priv *priv = (void *)object;
+
+   if (priv->r100c18_page) {
+   dma_unmap_page(nv_device_base(device), priv->r100c18, PAGE_SIZE,
+  DMA_BIDIRECTIONAL);
+   __free_page(priv->r100c18_page);
+   }
+
+   nv50_fb_dtor(object);
+}
+
+int
+nvaa_fb_init(struct nouveau_object *object)
+{
+   struct nvaa_fb_priv *priv = (void *)object;
+   int ret;
+
+   ret = nv50_fb_init(object);
+   if (ret)
+   return ret;
+
+   /* Enable NISO poller for various clients and set their associated
+* read address, only for MCP77/78 and MCP79/7A. (fd#25701)
+*/
+   nv_wr32(priv, 0x100c18, priv->r100c18 >> 8);
+   nv_mask(priv, 0x100c14, 0x, 0x0001);
+   nv_wr32(priv, 0x100c1c, (priv->r100c18 >> 8) + 1);
+   nv_mask(priv, 0x100c14, 0x, 0x0002);
+   nv_wr32(priv, 0x100c24, (priv->r100c18 >> 8) + 2);
+   nv_mask(priv, 0x100c14, 0x, 0x0001);
+   return 0;
+}

 struct nouveau_oclass *
 nvaa_fb_oclass = &(struct nv50_fb_impl) {
.base.base.handle = NV_SUBDEV(FB, 0xaa),
.base.base.ofuncs = &(struct nouveau_ofuncs) {
-   .ctor = nv50_fb_ctor,
-   .dtor = nv50_fb_dtor,
-   .init = nv50_fb_init,
+   .ctor = nvaa_fb_ctor,
+   .dtor = nvaa_fb_dtor,
+   .init = nvaa_fb_init,
.fini = _nouveau_fb_fini,
},
.base.memtype = nv50_fb_memtype_v

[PATCH RESEND 1/2] Allow noaccel to be a pci address

2014-12-10 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/nouveau_drm.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drm/nouveau_drm.c b/drm/nouveau_drm.c
index afb93bb..ffa1e4f 100644
--- a/drm/nouveau_drm.c
+++ b/drm/nouveau_drm.c
@@ -61,9 +61,10 @@ MODULE_PARM_DESC(debug, "debug string to pass to driver 
core");
 static char *nouveau_debug;
 module_param_named(debug, nouveau_debug, charp, 0400);

-MODULE_PARM_DESC(noaccel, "disable kernel/abi16 acceleration");
-static int nouveau_noaccel = 0;
-module_param_named(noaccel, nouveau_noaccel, int, 0400);
+MODULE_PARM_DESC(noaccel, "disable kernel/abi16 acceleration for all cards,"
+ "or only for the card given its pci bus name");
+static char *nouveau_noaccel;
+module_param_named(noaccel, nouveau_noaccel, charp, 0400);

 MODULE_PARM_DESC(modeset, "enable driver (default: auto, "
  "0 = disabled, 1 = enabled, 2 = headless)");
@@ -149,8 +150,13 @@ nouveau_accel_init(struct nouveau_drm *drm)
u32 sclass[16];
int ret, i;

-   if (nouveau_noaccel)
+   if (nouveau_noaccel != NULL &&
+   (!strcmp(nouveau_noaccel, "1") ||
+!strcmp(nouveau_noaccel, nvkm_device(device)->name)))
+   {
+   NV_WARN(drm, "Acceleration disabled for card on bus %s\n", 
nvkm_device(device)->name);
return;
+   }

/* initialise synchronisation routines */
/*XXX: this is crap, but the fence/channel stuff is a little
@@ -1021,7 +1027,7 @@ static void nouveau_display_options(void)
DRM_DEBUG_DRIVER("... nofbaccel: %d\n", nouveau_nofbaccel);
DRM_DEBUG_DRIVER("... config   : %s\n", nouveau_config);
DRM_DEBUG_DRIVER("... debug: %s\n", nouveau_debug);
-   DRM_DEBUG_DRIVER("... noaccel  : %d\n", nouveau_noaccel);
+   DRM_DEBUG_DRIVER("... noaccel  : %s\n", nouveau_noaccel);
DRM_DEBUG_DRIVER("... modeset  : %d\n", nouveau_modeset);
DRM_DEBUG_DRIVER("... runpm: %d\n", nouveau_runtime_pm);
DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
-- 
2.1.3



[PATCH v2 1/2] drm/nouveau/fb/nv50: Add PFB writes

2014-10-03 Thread Pierre Moreau
(This is a v2 of patch "drm/nouveau/disp/nv50: Add PFB writes")

This fix a GPU lockup on 9400M (NVAC) when using acceleration, see
https://bugs.freedesktop.org/show_bug.cgi?id=27501

v2:
- Move code to subdev/fb/nv50.c as suggested by Roy Spliet;
- Remove arbitrary writes to 100c18/100c24 as suggested by Roy Spliet;
- Replace write to 100c1c of arbitrary value by the address of a scratch page
  as proposed by Ilia Mirkin;
- Remove enabling of bits 16 and 0 as they don't yield in any changes.

Signed-off-by: Pierre Moreau 
---
 drivers/gpu/drm/nouveau/core/subdev/fb/nv50.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/core/subdev/fb/nv50.c 
b/drivers/gpu/drm/nouveau/core/subdev/fb/nv50.c
index 4150b0d..5c84d13 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/fb/nv50.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/fb/nv50.c
@@ -289,6 +289,17 @@ nv50_fb_init(struct nouveau_object *object)
if (ret)
return ret;

+   /* Not a clue what this is exactly. Without enabling bit 1 of
+* 100c14, system will lockup while initialising the card
+* (#27501)
+*/
+   if (nv_device(priv)->chipset == 0xac) {
+   if ((nv_rd32(priv, 0x100c14) & 0x0002) == 0x) {
+   nv_wr32(priv, 0x100c1c, priv->r100c08 >> 8);
+   nv_mask(priv, 0x100c14, 0x, 0x0002);
+   }
+   }
+
/* Not a clue what this is exactly.  Without pointing it at a
 * scratch page, VRAM->GART blits with M2MF (as in DDX DFS)
 * cause IOMMU "read from address 0" errors (rh#561267)
-- 
2.1.2



[PATCH 2/2] drm/nouveau: Allow noaccel to be a pci address

2014-09-25 Thread Pierre Moreau
noaccel option now defaults to null which has no effect, it can still equal 1,
which disables acceleration for all cards, or be a pci address and disable
acceleration for that card only

Signed-off-by: Pierre Moreau 
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 244d78f..04bd8f1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -61,9 +61,10 @@ MODULE_PARM_DESC(debug, "debug string to pass to driver 
core");
 static char *nouveau_debug;
 module_param_named(debug, nouveau_debug, charp, 0400);

-MODULE_PARM_DESC(noaccel, "disable kernel/abi16 acceleration");
-static int nouveau_noaccel = 0;
-module_param_named(noaccel, nouveau_noaccel, int, 0400);
+MODULE_PARM_DESC(noaccel, "disable kernel/abi16 acceleration for all cards,"
+ "or only for the card given its pci bus name");
+static char *nouveau_noaccel;
+module_param_named(noaccel, nouveau_noaccel, charp, 0400);

 MODULE_PARM_DESC(modeset, "enable driver (default: auto, "
  "0 = disabled, 1 = enabled, 2 = headless)");
@@ -149,8 +150,13 @@ nouveau_accel_init(struct nouveau_drm *drm)
u32 sclass[16];
int ret, i;

-   if (nouveau_noaccel)
+   if (nouveau_noaccel != NULL &&
+   (!strcmp(nouveau_noaccel, "1") ||
+!strcmp(nouveau_noaccel, nvkm_device(device)->name)))
+   {
+   NV_WARN(drm, "Acceleration disabled for card on bus %s\n", 
nvkm_device(device)->name);
return;
+   }

/* initialise synchronisation routines */
/*XXX: this is crap, but the fence/channel stuff is a little
@@ -1039,7 +1045,7 @@ static void nouveau_display_options(void)
DRM_DEBUG_DRIVER("... nofbaccel: %d\n", nouveau_nofbaccel);
DRM_DEBUG_DRIVER("... config   : %s\n", nouveau_config);
DRM_DEBUG_DRIVER("... debug: %s\n", nouveau_debug);
-   DRM_DEBUG_DRIVER("... noaccel  : %d\n", nouveau_noaccel);
+   DRM_DEBUG_DRIVER("... noaccel  : %s\n", nouveau_noaccel);
DRM_DEBUG_DRIVER("... modeset  : %d\n", nouveau_modeset);
DRM_DEBUG_DRIVER("... runpm: %d\n", nouveau_runtime_pm);
DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
-- 
2.1.0



[PATCH 1/2] drm/nouveau/disp/nv50: Add PFB writes

2014-09-25 Thread Pierre Moreau
This fix a GPU lockup on 9400M (NVAC) when using acceleration, see #27501.

Signed-off-by: Pierre Moreau 
---
 drivers/gpu/drm/nouveau/core/engine/disp/nv50.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c 
b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
index a7efbff..e425604 100644
--- a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
+++ b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
@@ -1137,6 +1137,15 @@ nv50_disp_base_init(struct nouveau_object *object)
if (ret)
return ret;

+   if (nv_device(priv)->chipset == 0xac) {
+   nv_mask(priv, 0x100c18, 0x, 0x27ff);
+   nv_mask(priv, 0x100c14, 0x, 0x0001);
+   nv_mask(priv, 0x100c1c, 0x, 0x27fc);
+   nv_mask(priv, 0x100c14, 0x, 0x0002);
+   nv_mask(priv, 0x100c24, 0x0002, 0x27fd);
+   nv_mask(priv, 0x100c14, 0x, 0x0001);
+   }
+
/* The below segments of code copying values from one register to
 * another appear to inform EVO of the display capabilities or
 * something similar.  NFI what the 0x614004 caps are for..
-- 
2.1.0



[PATCH] drm: Display Nouveau boot options at launch

2014-08-18 Thread Pierre Moreau
It can help to remove any ambiguity about which options were passed to Nouveau,
especially in case the user had some options set in /etc/modprobe.d/*.conf that
he forgot about, as they won't appear in a dmesg.

Signed-off-by: Pierre Moreau 
---
 drm/nouveau_chan.c  |  2 +-
 drm/nouveau_chan.h  |  2 ++
 drm/nouveau_connector.c |  6 +++---
 drm/nouveau_connector.h |  4 
 drm/nouveau_drm.c   | 20 
 drm/nouveau_fbcon.c |  2 +-
 drm/nouveau_fbcon.h |  3 +++
 drm/nouveau_sysfs.c |  2 +-
 drm/nouveau_sysfs.h |  2 ++
 9 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/drm/nouveau_chan.c b/drm/nouveau_chan.c
index 99cd9e4..9a362dd 100644
--- a/drm/nouveau_chan.c
+++ b/drm/nouveau_chan.c
@@ -36,7 +36,7 @@
 #include "nouveau_abi16.h"

 MODULE_PARM_DESC(vram_pushbuf, "Create DMA push buffers in VRAM");
-static int nouveau_vram_pushbuf;
+int nouveau_vram_pushbuf;
 module_param_named(vram_pushbuf, nouveau_vram_pushbuf, int, 0400);

 int
diff --git a/drm/nouveau_chan.h b/drm/nouveau_chan.h
index 2016370..8309c24 100644
--- a/drm/nouveau_chan.h
+++ b/drm/nouveau_chan.h
@@ -47,4 +47,6 @@ int  nouveau_channel_new(struct nouveau_drm *, struct 
nvif_device *,
 void nouveau_channel_del(struct nouveau_channel **);
 int  nouveau_channel_idle(struct nouveau_channel *);

+extern int nouveau_vram_pushbuf;
+
 #endif
diff --git a/drm/nouveau_connector.c b/drm/nouveau_connector.c
index 1ec44c8..c8ac948 100644
--- a/drm/nouveau_connector.c
+++ b/drm/nouveau_connector.c
@@ -45,15 +45,15 @@
 #include 

 MODULE_PARM_DESC(tv_disable, "Disable TV-out detection");
-static int nouveau_tv_disable = 0;
+int nouveau_tv_disable = 0;
 module_param_named(tv_disable, nouveau_tv_disable, int, 0400);

 MODULE_PARM_DESC(ignorelid, "Ignore ACPI lid status");
-static int nouveau_ignorelid = 0;
+int nouveau_ignorelid = 0;
 module_param_named(ignorelid, nouveau_ignorelid, int, 0400);

 MODULE_PARM_DESC(duallink, "Allow dual-link TMDS (default: enabled)");
-static int nouveau_duallink = 1;
+int nouveau_duallink = 1;
 module_param_named(duallink, nouveau_duallink, int, 0400);

 struct nouveau_encoder *
diff --git a/drm/nouveau_connector.h b/drm/nouveau_connector.h
index 68029d0..629a380 100644
--- a/drm/nouveau_connector.h
+++ b/drm/nouveau_connector.h
@@ -105,4 +105,8 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)
 struct drm_connector *
 nouveau_connector_create(struct drm_device *, int index);

+extern int nouveau_tv_disable;
+extern int nouveau_ignorelid;
+extern int nouveau_duallink;
+
 #endif /* __NOUVEAU_CONNECTOR_H__ */
diff --git a/drm/nouveau_drm.c b/drm/nouveau_drm.c
index 606cc6b..aaccb25 100644
--- a/drm/nouveau_drm.c
+++ b/drm/nouveau_drm.c
@@ -51,6 +51,7 @@
 #include "nouveau_fence.h"
 #include "nouveau_debugfs.h"
 #include "nouveau_usif.h"
+#include "nouveau_connector.h"

 MODULE_PARM_DESC(config, "option string to pass to driver core");
 static char *nouveau_config;
@@ -1025,6 +1026,23 @@ static int nouveau_pmops_runtime_idle(struct device *dev)
return 1;
 }

+static void nouveau_display_options(void)
+{
+   DRM_DEBUG_DRIVER("Loading Nouveau with parameters:\n");
+
+   DRM_DEBUG_DRIVER("... tv_disable   : %d\n", nouveau_tv_disable);
+   DRM_DEBUG_DRIVER("... ignorelid: %d\n", nouveau_ignorelid);
+   DRM_DEBUG_DRIVER("... duallink : %d\n", nouveau_duallink);
+   DRM_DEBUG_DRIVER("... nofbaccel: %d\n", nouveau_nofbaccel);
+   DRM_DEBUG_DRIVER("... config   : %s\n", nouveau_config);
+   DRM_DEBUG_DRIVER("... debug: %s\n", nouveau_debug);
+   DRM_DEBUG_DRIVER("... noaccel  : %d\n", nouveau_noaccel);
+   DRM_DEBUG_DRIVER("... modeset  : %d\n", nouveau_modeset);
+   DRM_DEBUG_DRIVER("... runpm: %d\n", nouveau_runtime_pm);
+   DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
+   DRM_DEBUG_DRIVER("... pstate   : %d\n", nouveau_pstate);
+}
+
 static const struct dev_pm_ops nouveau_pm_ops = {
.suspend = nouveau_pmops_suspend,
.resume = nouveau_pmops_resume,
@@ -1085,6 +1103,8 @@ EXPORT_SYMBOL(nouveau_platform_device_create_);
 static int __init
 nouveau_drm_init(void)
 {
+   nouveau_display_options();
+
if (nouveau_modeset == -1) {
 #ifdef CONFIG_VGA_CONSOLE
if (vgacon_text_force())
diff --git a/drm/nouveau_fbcon.c b/drm/nouveau_fbcon.c
index 8bdd270..f0ae10c 100644
--- a/drm/nouveau_fbcon.c
+++ b/drm/nouveau_fbcon.c
@@ -52,7 +52,7 @@
 #include "nouveau_crtc.h"

 MODULE_PARM_DESC(nofbaccel, "Disable fbcon acceleration");
-static int nouveau_nofbaccel = 0;
+int nouveau_nofbaccel = 0;
 module_param_named(nofbaccel, nouveau_nofbaccel, int, 0400);

 static void
diff --git a/drm/n

[PATCH] drm: Fix duplicate definition of NV04_PFB_BOOT_0_*

2014-08-18 Thread Pierre Moreau
Signed-off-by: Pierre Moreau 
---
 drm/core/include/subdev/fb/regsnv04.h |  1 +
 nvkm/include/subdev/fb/regsnv04.h | 21 +
 nvkm/subdev/devinit/fbmem.h   | 18 ++
 nvkm/subdev/fb/ramnv04.c  | 17 +
 4 files changed, 25 insertions(+), 32 deletions(-)
 create mode 12 drm/core/include/subdev/fb/regsnv04.h
 create mode 100644 nvkm/include/subdev/fb/regsnv04.h

diff --git a/drm/core/include/subdev/fb/regsnv04.h 
b/drm/core/include/subdev/fb/regsnv04.h
new file mode 12
index 000..cc2ea25
--- /dev/null
+++ b/drm/core/include/subdev/fb/regsnv04.h
@@ -0,0 +1 @@
+../../../../../nvkm/include/subdev/fb/regsnv04.h
\ No newline at end of file
diff --git a/nvkm/include/subdev/fb/regsnv04.h 
b/nvkm/include/subdev/fb/regsnv04.h
new file mode 100644
index 000..0f7fc0c
--- /dev/null
+++ b/nvkm/include/subdev/fb/regsnv04.h
@@ -0,0 +1,21 @@
+#ifndef __NOUVEAU_FB_REGS_04_H__
+#define __NOUVEAU_FB_REGS_04_H__
+
+#define NV04_PFB_BOOT_0
0x0010
+#  define NV04_PFB_BOOT_0_RAM_AMOUNT   0x0003
+#  define NV04_PFB_BOOT_0_RAM_AMOUNT_32MB  0x
+#  define NV04_PFB_BOOT_0_RAM_AMOUNT_4MB   0x0001
+#  define NV04_PFB_BOOT_0_RAM_AMOUNT_8MB   0x0002
+#  define NV04_PFB_BOOT_0_RAM_AMOUNT_16MB  0x0003
+#  define NV04_PFB_BOOT_0_RAM_WIDTH_1280x0004
+#  define NV04_PFB_BOOT_0_RAM_TYPE 0x0028
+#  define NV04_PFB_BOOT_0_RAM_TYPE_SGRAM_8MBIT 0x
+#  define NV04_PFB_BOOT_0_RAM_TYPE_SGRAM_16MBIT0x0008
+#  define NV04_PFB_BOOT_0_RAM_TYPE_SGRAM_16MBIT_4BANK  0x0010
+#  define NV04_PFB_BOOT_0_RAM_TYPE_SDRAM_16MBIT0x0018
+#  define NV04_PFB_BOOT_0_RAM_TYPE_SDRAM_64MBIT0x0020
+#  define NV04_PFB_BOOT_0_RAM_TYPE_SDRAM_64MBITX16 0x0028
+#  define NV04_PFB_BOOT_0_UMA_ENABLE   0x0100
+#  define NV04_PFB_BOOT_0_UMA_SIZE 0xf000
+
+#endif
diff --git a/nvkm/subdev/devinit/fbmem.h b/nvkm/subdev/devinit/fbmem.h
index 4fe49cf..6103484 100644
--- a/nvkm/subdev/devinit/fbmem.h
+++ b/nvkm/subdev/devinit/fbmem.h
@@ -26,22 +26,8 @@

 #include 

-#define NV04_PFB_BOOT_0
0x0010
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT   0x0003
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT_32MB  0x
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT_4MB   0x0001
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT_8MB   0x0002
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT_16MB  0x0003
-#  define NV04_PFB_BOOT_0_RAM_WIDTH_1280x0004
-#  define NV04_PFB_BOOT_0_RAM_TYPE 0x0028
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SGRAM_8MBIT 0x
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SGRAM_16MBIT0x0008
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SGRAM_16MBIT_4BANK  0x0010
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SDRAM_16MBIT0x0018
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SDRAM_64MBIT0x0020
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SDRAM_64MBITX16 0x0028
-#  define NV04_PFB_BOOT_0_UMA_ENABLE   0x0100
-#  define NV04_PFB_BOOT_0_UMA_SIZE 0xf000
+#include 
+
 #define NV04_PFB_DEBUG_0   0x00100080
 #  define NV04_PFB_DEBUG_0_PAGE_MODE   0x0001
 #  define NV04_PFB_DEBUG_0_REFRESH_OFF 0x0010
diff --git a/nvkm/subdev/fb/ramnv04.c b/nvkm/subdev/fb/ramnv04.c
index e781080..1972268 100644
--- a/nvkm/subdev/fb/ramnv04.c
+++ b/nvkm/subdev/fb/ramnv04.c
@@ -22,22 +22,7 @@
  * Authors: Ben Skeggs
  */

-#define NV04_PFB_BOOT_0
0x0010
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT   0x0003
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT_32MB  0x
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT_4MB   0x0001
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT_8MB   0x0002
-#  define NV04_PFB_BOOT_0_RAM_AMOUNT_16MB  0x0003
-#  define NV04_PFB_BOOT_0_RAM_WIDTH_1280x0004
-#  define NV04_PFB_BOOT_0_RAM_TYPE 0x0028
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SGRAM_8MBIT 0x
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SGRAM_16MBIT0x0008
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SGRAM_16MBIT_4BANK  0x0010
-#  define NV04_PFB_BOOT_0_RAM_TYPE_SDRAM_16MBIT0x0018
-#  define

[PATCH 2/2] drm/nouveau: Remove an unneeded write while initialising PGRAPH

2014-06-13 Thread Pierre Moreau
The blob does not seem to write at that place for my NVAC, though it does for
my NV96, agreeing with what is done in the if/else structure below. I guess
someone forgot to remove the line when the if/else was put in place.

Signed-off-by: Pierre Moreau 
---
 drivers/gpu/drm/nouveau/core/engine/graph/nv50.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/core/engine/graph/nv50.c 
b/drivers/gpu/drm/nouveau/core/engine/graph/nv50.c
index a36adcd..ca1b886 100644
--- a/drivers/gpu/drm/nouveau/core/engine/graph/nv50.c
+++ b/drivers/gpu/drm/nouveau/core/engine/graph/nv50.c
@@ -976,7 +976,6 @@ nv50_graph_init(struct nouveau_object *object)
break;
case 0xa0:
default:
-   nv_wr32(priv, 0x402cc0, 0x);
if (nv_device(priv)->chipset == 0xa0 ||
nv_device(priv)->chipset == 0xaa ||
nv_device(priv)->chipset == 0xac) {
-- 
2.0.0



[PATCH 1/2] drm/nouveau: Fix overlap while zeroing zcull regions

2014-06-13 Thread Pierre Moreau
The specified stride was not correct, resulting in erases overlapping and part
of the zcull regions being not erased at all.

Signed-off-by: Pierre Moreau 
---
 drivers/gpu/drm/nouveau/core/engine/graph/nv50.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/core/engine/graph/nv50.c 
b/drivers/gpu/drm/nouveau/core/engine/graph/nv50.c
index 2c7809e..a36adcd 100644
--- a/drivers/gpu/drm/nouveau/core/engine/graph/nv50.c
+++ b/drivers/gpu/drm/nouveau/core/engine/graph/nv50.c
@@ -991,10 +991,10 @@ nv50_graph_init(struct nouveau_object *object)

/* zero out zcull regions */
for (i = 0; i < 8; i++) {
-   nv_wr32(priv, 0x402c20 + (i * 8), 0x);
-   nv_wr32(priv, 0x402c24 + (i * 8), 0x);
-   nv_wr32(priv, 0x402c28 + (i * 8), 0x);
-   nv_wr32(priv, 0x402c2c + (i * 8), 0x);
+   nv_wr32(priv, 0x402c20 + (i * 0x10), 0x);
+   nv_wr32(priv, 0x402c24 + (i * 0x10), 0x);
+   nv_wr32(priv, 0x402c28 + (i * 0x10), 0x);
+   nv_wr32(priv, 0x402c2c + (i * 0x10), 0x);
}
return 0;
 }
-- 
2.0.0