[PATCH] [v2] nouveau: fix function cast warning

[Arnd Bergmann]

From: Arnd Bergmann 

Calling a function through an incompatible pointer type causes breaks
kcfi, so clang warns about the assignment:

drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c:73:10: error: cast from 
'void (*)(const void *)' to 'void (*)(void *)' converts to incompatible 
function type [-Werror,-Wcast-function-type-strict]
   73 | .fini = (void(*)(void *))kfree,

Avoid this with a trivial wrapper.

Fixes: c39f472e9f14 ("drm/nouveau: remove symlinks, move core/ to nvkm/ (no 
code changes)")
Signed-off-by: Arnd Bergmann 
---
v2: avoid 'return kfree()' expression returning a void
---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
index 4bf486b57101..cb05f7f48a98 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
@@ -66,11 +66,16 @@ of_init(struct nvkm_bios *bios, const char *name)
return ERR_PTR(-EINVAL);
 }
 
+static void of_fini(void *p)
+{
+   kfree(p);
+}
+
 const struct nvbios_source
 nvbios_of = {
.name = "OpenFirmware",
.init = of_init,
-   .fini = (void(*)(void *))kfree,
+   .fini = of_fini,
.read = of_read,
.size = of_size,
.rw = false,
-- 
2.39.2

Re: [PATCH 06/12] nouveau: fix function cast warning

[Arnd Bergmann]

On Tue, Mar 26, 2024, at 16:20, Timur Tabi wrote:
> On Tue, 2024-03-26 at 15:51 +0100, Arnd Bergmann wrote:
>> Calling a function through an incompatible pointer type causes breaks
>> kcfi, so clang warns about the assignment:
>> 
>
> ...
>
>> +static void of_fini(void *p)
>> +{
>> +   return kfree(p);
>> +}
>> +
>
> I don't know anything about kfci, but shouldn't this just be "kfree(p)",
> without the "return"?

You are right, fixed this up locally now, waiting for more
comments before I resend patches from my series.

I think it works because of a gcc extension, but isn't
valid C otherwise, and I did not mean to rely on this.

 Arnd

[PATCH 06/12] nouveau: fix function cast warning

[Arnd Bergmann]

From: Arnd Bergmann 

Calling a function through an incompatible pointer type causes breaks
kcfi, so clang warns about the assignment:

drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c:73:10: error: cast from 
'void (*)(const void *)' to 'void (*)(void *)' converts to incompatible 
function type [-Werror,-Wcast-function-type-strict]
   73 | .fini = (void(*)(void *))kfree,

Avoid this with a trivial wrapper.

Fixes: c39f472e9f14 ("drm/nouveau: remove symlinks, move core/ to nvkm/ (no 
code changes)")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
index 4bf486b57101..0dbcc23305f3 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
@@ -66,11 +66,16 @@ of_init(struct nvkm_bios *bios, const char *name)
return ERR_PTR(-EINVAL);
 }
 
+static void of_fini(void *p)
+{
+   return kfree(p);
+}
+
 const struct nvbios_source
 nvbios_of = {
.name = "OpenFirmware",
.init = of_init,
-   .fini = (void(*)(void *))kfree,
+   .fini = of_fini,
.read = of_read,
.size = of_size,
.rw = false,
-- 
2.39.2

[PATCH 00/12] kbuild: enable some -Wextra warnings by default

[Arnd Bergmann]

From: Arnd Bergmann 

This is a follow-up on a couple of patch series I sent in the past,
enabling -Wextra (aside from stuff that is explicitly disabled),
-Wcast-function-pointer-strict and -Wrestrict.

I have tested these on 'defconfig' and 'allmodconfig' builds across
all architectures, as well as many 'randconfig' builds on x86, arm and
arm64. It would be nice to have all the Makefile.extrawarn changes in
v6.10, hopefully with the driver fixes going in before that through
the respective subsystem trees.

 Arnd

Arnd Bergmann (12):
  kbuild: make -Woverride-init warnings more consistent
  [v3] parport: mfc3: avoid empty-body warning
  kbuild: turn on -Wextra by default
  kbuild: remove redundant extra warning flags
  firmware: dmi-id: add a release callback function
  nouveau: fix function cast warning
  cxlflash: fix function pointer cast warnings
  x86: math-emu: fix function cast warnings
  kbuild: enable -Wcast-function-type-strict unconditionally
  sata: sx4: fix pdc20621_get_from_dimm() on 64-bit
  [v4] kallsyms: rework symbol lookup return codes
  kbuild: turn on -Wrestrict by default

 arch/x86/math-emu/fpu_etc.c   |  9 +++--
 arch/x86/math-emu/fpu_trig.c  |  6 ++--
 arch/x86/math-emu/reg_constant.c  |  7 +++-
 drivers/ata/sata_sx4.c|  6 ++--
 drivers/firmware/dmi-id.c |  7 +++-
 .../gpu/drm/amd/display/dc/dce110/Makefile|  2 +-
 .../gpu/drm/amd/display/dc/dce112/Makefile|  2 +-
 .../gpu/drm/amd/display/dc/dce120/Makefile|  2 +-
 drivers/gpu/drm/amd/display/dc/dce60/Makefile |  2 +-
 drivers/gpu/drm/amd/display/dc/dce80/Makefile |  2 +-
 drivers/gpu/drm/i915/Makefile |  6 ++--
 .../drm/nouveau/nvkm/subdev/bios/shadowof.c   |  7 +++-
 drivers/gpu/drm/xe/Makefile   |  4 +--
 drivers/net/ethernet/renesas/sh_eth.c |  2 +-
 drivers/parport/parport_mfc3.c|  3 +-
 drivers/pinctrl/aspeed/Makefile   |  2 +-
 drivers/scsi/cxlflash/lunmgt.c|  4 +--
 drivers/scsi/cxlflash/main.c  | 14 
 drivers/scsi/cxlflash/superpipe.c | 34 +--
 drivers/scsi/cxlflash/superpipe.h | 11 +++---
 drivers/scsi/cxlflash/vlun.c  |  7 ++--
 fs/proc/Makefile  |  2 +-
 include/linux/filter.h| 14 
 include/linux/ftrace.h|  6 ++--
 include/linux/module.h| 14 
 kernel/bpf/Makefile   |  2 +-
 kernel/bpf/core.c |  7 ++--
 kernel/kallsyms.c | 23 +++--
 kernel/module/kallsyms.c  | 26 +++---
 kernel/trace/ftrace.c | 13 +++
 mm/Makefile   |  3 +-
 scripts/Makefile.extrawarn| 33 --
 32 files changed, 134 insertions(+), 148 deletions(-)

-- 
2.39.2

Cc: Bill Metzenthen 
Cc: Thomas Gleixner 
Cc: x...@kernel.org
Cc: Damien Le Moal 
Cc: Jean Delvare 
Cc: Harry Wentland 
Cc: Jani Nikula 
Cc: Sergey Shtylyov 
Cc: Jakub Kicinski 
Cc: Sudip Mukherjee 
Cc: Andrew Jeffery 
Cc: "Manoj N. Kumar" 
Cc: "Martin K. Petersen" 
Cc: Alexei Starovoitov 
Cc: Steven Rostedt 
Cc: Luis Chamberlain 
Cc: Andrew Morton 
Cc: Masahiro Yamada 
Cc: Nathan Chancellor 
Cc: Nicolas Schier 
Cc: Greg Kroah-Hartman 
Cc: linux-ker...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: amd-...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: intel...@lists.freedesktop.org
Cc: net...@vger.kernel.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-s...@vger.kernel.org
Cc: b...@vger.kernel.org
Cc: linux-trace-ker...@vger.kernel.org
Cc: linux-modu...@vger.kernel.org
Cc: linux...@kvack.org
Cc: linux-kbu...@vger.kernel.org
Cc: l...@lists.linux.dev

[PATCH] nouveau: fix function cast warnings

[Arnd Bergmann]

From: Arnd Bergmann 

clang-16 warns about casting between incompatible function types:

drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c:161:10: error: cast from 
'void (*)(const struct firmware *)' to 'void (*)(void *)' converts to 
incompatible function type [-Werror,-Wcast-function-type-strict]
  161 | .fini = (void(*)(void *))release_firmware,

This one was done to use the generic shadow_fw_release() function as a
callback for struct nvbios_source. Change it to use the same prototype
as the other five instances, with a trivial helper function that actually
calls release_firmware.

Fixes: 70c0f263cc2e ("drm/nouveau/bios: pull in basic vbios subdev, more to 
come later")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
index 19188683c8fc..8c2bf1c16f2a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
@@ -154,11 +154,17 @@ shadow_fw_init(struct nvkm_bios *bios, const char *name)
return (void *)fw;
 }
 
+static void
+shadow_fw_release(void *fw)
+{
+   release_firmware(fw);
+}
+
 static const struct nvbios_source
 shadow_fw = {
.name = "firmware",
.init = shadow_fw_init,
-   .fini = (void(*)(void *))release_firmware,
+   .fini = shadow_fw_release,
.read = shadow_fw_read,
.rw = false,
 };
-- 
2.39.2

[PATCH] nouveau/svm: fix kvcalloc() argument order

[Arnd Bergmann]

From: Arnd Bergmann 

The conversion to kvcalloc() mixed up the object size and count
arguments, causing a warning:

drivers/gpu/drm/nouveau/nouveau_svm.c: In function 
'nouveau_svm_fault_buffer_ctor':
drivers/gpu/drm/nouveau/nouveau_svm.c:1010:40: error: 'kvcalloc' sizes 
specified with 'sizeof' in the earlier argument and not in the later argument 
[-Werror=calloc-transposed-args]
 1010 | buffer->fault = kvcalloc(sizeof(*buffer->fault), 
buffer->entries, GFP_KERNEL);
  |^
drivers/gpu/drm/nouveau/nouveau_svm.c:1010:40: note: earlier argument should 
specify number of elements, later size of each element

The behavior is still correct aside from the warning, but fixing it avoids
the warnings and can help the compiler track the individual objects better.

Fixes: 71e4bbca070e ("nouveau/svm: Use kvcalloc() instead of kvzalloc()")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c 
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 4d1008915499..b4da82ddbb6b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -1007,7 +1007,7 @@ nouveau_svm_fault_buffer_ctor(struct nouveau_svm *svm, 
s32 oclass, int id)
if (ret)
return ret;
 
-   buffer->fault = kvcalloc(sizeof(*buffer->fault), buffer->entries, 
GFP_KERNEL);
+   buffer->fault = kvcalloc(buffer->entries, sizeof(*buffer->fault), 
GFP_KERNEL);
if (!buffer->fault)
return -ENOMEM;
 
-- 
2.39.2

[Nouveau] [PATCH] drm/nouveau/kms/nv50: hide unused variables

[Arnd Bergmann]

From: Arnd Bergmann 

After a recent change, two variables are only used in an #ifdef:

drivers/gpu/drm/nouveau/dispnv50/disp.c: In function 'nv50_sor_atomic_disable':
drivers/gpu/drm/nouveau/dispnv50/disp.c:1569:13: error: unused variable 'ret' 
[-Werror=unused-variable]
 1569 | int ret;
  | ^~~
drivers/gpu/drm/nouveau/dispnv50/disp.c:1568:28: error: unused variable 'aux' 
[-Werror=unused-variable]
 1568 | struct drm_dp_aux *aux = _connector->aux;
  |^~~

Move them into the same conditional block, along with the nv_connector variable
that becomes unused during that fix.

Fixes: 757033808c95b ("drm/nouveau/kms/nv50-: fixup sink D3 before tearing down 
link")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 52f1569ee37c1..a0ac8c258d9ff 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1560,15 +1560,13 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
 {
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nv50_head *head = nv50_head(nv_encoder->crtc);
-   struct nouveau_connector *nv_connector = 
nv50_outp_get_old_connector(state, nv_encoder);
 #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
+   struct nouveau_connector *nv_connector = 
nv50_outp_get_old_connector(state, nv_encoder);
struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
struct nouveau_backlight *backlight = nv_connector->backlight;
-#endif
struct drm_dp_aux *aux = _connector->aux;
int ret;
 
-#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
if (backlight && backlight->uses_dpcd) {
ret = drm_edp_backlight_disable(aux, >edp_info);
if (ret < 0)
-- 
2.39.2

[Nouveau] [PATCH] [RESEND] drm/nouveau: remove unused tu102_gr_load() function

[Arnd Bergmann]

From: Arnd Bergmann 

tu102_gr_load() is completely unused and can be removed to address
this warning:

drivers/gpu/drm/nouveau/dispnv50/disp.c:2517:1: error: no previous prototype 
for 'nv50_display_create'

Another patch was sent in the meantime to mark the function static but
that would just cause a different warning about an unused function.

Fixes: 1cd97b5490c8 ("drm/nouveau/gr/tu102-: use sw_veid_bundle_init from 
firmware")
Link: 
https://lore.kernel.org/all/caco55tuanoyphhyb9+ygi9anxvuf49etsw7x2x5k5ietfna...@mail.gmail.com/
Link: https://lore.kernel.org/all/20230417210310.2443152-1-a...@kernel.org/
Signed-off-by: Arnd Bergmann 
---
I send this one back in April, and Lee Jones sent a similar patch in May.
Please apply so we can enable the warning by default.
---
 drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c
index 3b6c8100a242..a7775aa18541 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c
@@ -206,19 +206,6 @@ tu102_gr_av_to_init_veid(struct nvkm_blob *blob, struct 
gf100_gr_pack **ppack)
return gk20a_gr_av_to_init_(blob, 64, 0x0010, ppack);
 }
 
-int
-tu102_gr_load(struct gf100_gr *gr, int ver, const struct gf100_gr_fwif *fwif)
-{
-   int ret;
-
-   ret = gm200_gr_load(gr, ver, fwif);
-   if (ret)
-   return ret;
-
-   return gk20a_gr_load_net(gr, "gr/", "sw_veid_bundle_init", ver, 
tu102_gr_av_to_init_veid,
->bundle_veid);
-}
-
 static const struct gf100_gr_fwif
 tu102_gr_fwif[] = {
{  0, gm200_gr_load, _gr, _gr_fecs_acr, _gr_gpccs_acr 
},
-- 
2.39.2

[Nouveau] [PATCH] drm/nouveau: dispnv50: fix missing-prototypes warning

[Arnd Bergmann]

From: Arnd Bergmann 

nv50_display_create() is declared in another header, along with
a couple of declarations that are now outdated:

drivers/gpu/drm/nouveau/dispnv50/disp.c:2517:1: error: no previous prototype 
for 'nv50_display_create'

Fixes: ba801ef068c1 ("drm/nouveau/kms: display destroy/init/fini hooks can be 
static")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 1 +
 drivers/gpu/drm/nouveau/nv50_display.h  | 4 +---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 5bb777ff1313..9b6824f6b9e4 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -64,6 +64,7 @@
 #include "nouveau_connector.h"
 #include "nouveau_encoder.h"
 #include "nouveau_fence.h"
+#include "nv50_display.h"
 
 #include 
 
diff --git a/drivers/gpu/drm/nouveau/nv50_display.h 
b/drivers/gpu/drm/nouveau/nv50_display.h
index fbd3b15583bc..60f77766766e 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.h
+++ b/drivers/gpu/drm/nouveau/nv50_display.h
@@ -31,7 +31,5 @@
 #include "nouveau_reg.h"
 
 int  nv50_display_create(struct drm_device *);
-void nv50_display_destroy(struct drm_device *);
-int  nv50_display_init(struct drm_device *);
-void nv50_display_fini(struct drm_device *);
+
 #endif /* __NV50_DISPLAY_H__ */
-- 
2.39.2

[Nouveau] [PATCH] drm/nouveau: remove unused tu102_gr_load() function

[Arnd Bergmann]

From: Arnd Bergmann 

tu102_gr_load() is completely unused and can be removed to address
this warning:

drivers/gpu/drm/nouveau/dispnv50/disp.c:2517:1: error: no previous prototype 
for 'nv50_display_create'

Fixes: 1cd97b5490c8 ("drm/nouveau/gr/tu102-: use sw_veid_bundle_init from 
firmware")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c
index 3b6c8100a242..a7775aa18541 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c
@@ -206,19 +206,6 @@ tu102_gr_av_to_init_veid(struct nvkm_blob *blob, struct 
gf100_gr_pack **ppack)
return gk20a_gr_av_to_init_(blob, 64, 0x0010, ppack);
 }
 
-int
-tu102_gr_load(struct gf100_gr *gr, int ver, const struct gf100_gr_fwif *fwif)
-{
-   int ret;
-
-   ret = gm200_gr_load(gr, ver, fwif);
-   if (ret)
-   return ret;
-
-   return gk20a_gr_load_net(gr, "gr/", "sw_veid_bundle_init", ver, 
tu102_gr_av_to_init_veid,
->bundle_veid);
-}
-
 static const struct gf100_gr_fwif
 tu102_gr_fwif[] = {
{  0, gm200_gr_load, _gr, _gr_fecs_acr, _gr_gpccs_acr 
},
-- 
2.39.2

[Nouveau] [PATCH] nouveau: fix nv50_wndw_new_() prototype

[Arnd Bergmann]

From: Arnd Bergmann 

gcc-13 complains about a mismatched function declaration:

drivers/gpu/drm/nouveau/dispnv50/wndw.c:696:1: error: conflicting types for 
'nv50_wndw_new_' due to enum/integer mismatch; have 'int(const struct 
nv50_wndw_func *, struct drm_device *, enum drm_plane_type,  const char *, int, 
 const u32 *, u32,  enum nv50_disp_interlock_type,  u32,  struct nv50_wndw **)' 
{aka 'int(const struct nv50_wndw_func *, struct drm_device *, enum 
drm_plane_type,  const char *, int,  const unsigned int *, unsigned int,  enum 
nv50_disp_interlock_type,  unsigned int,  struct nv50_wndw **)'} 
[-Werror=enum-int-mismatch]
  696 | nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device 
*dev,
  | ^~
In file included from drivers/gpu/drm/nouveau/dispnv50/wndw.c:22:
drivers/gpu/drm/nouveau/dispnv50/wndw.h:36:5: note: previous declaration of 
'nv50_wndw_new_' with type 'int(const struct nv50_wndw_func *, struct 
drm_device *, enum drm_plane_type,  const char *, int,  const u32 *, enum 
nv50_disp_interlock_type,  u32,  u32,  struct nv50_wndw **)' {aka 'int(const 
struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type,  const char 
*, int,  const unsigned int *, enum nv50_disp_interlock_type,  unsigned int,  
unsigned int,  struct nv50_wndw **)'}
   36 | int nv50_wndw_new_(const struct nv50_wndw_func *, struct drm_device *,
  | ^~

All the callers are ok, just the header file mixes up the order of
the arguments.

Fixes: 53e0a3e70de6 ("drm/nouveau/kms/nv50-: simplify tracking of channel 
interlocks")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/dispnv50/wndw.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h 
b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
index 591c852f326b..76a6ae5d5652 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
@@ -35,8 +35,9 @@ struct nv50_wndw {
 
 int nv50_wndw_new_(const struct nv50_wndw_func *, struct drm_device *,
   enum drm_plane_type, const char *name, int index,
-  const u32 *format, enum nv50_disp_interlock_type,
-  u32 interlock_data, u32 heads, struct nv50_wndw **);
+  const u32 *format, u32 heads,
+  enum nv50_disp_interlock_type, u32 interlock_data,
+  struct nv50_wndw **);
 void nv50_wndw_flush_set(struct nv50_wndw *, u32 *interlock,
 struct nv50_wndw_atom *);
 void nv50_wndw_flush_clr(struct nv50_wndw *, u32 *interlock, bool flush,
-- 
2.39.0

Re: [Nouveau] [PATCH] nouveau: make backlight support non optional

[Arnd Bergmann]

On Mon, Aug 9, 2021 at 3:20 PM Jani Nikula  wrote:
>
> On Sat, 24 Jul 2021, Arnd Bergmann  wrote:
> > On Sat, Jul 24, 2021 at 4:14 PM Karol Herbst  wrote:
> >>
> >> we use the MXM_WMI in code. We also have to keep arm in mind and not
> >> break stuff there. So I will try to play around with your changes and
> >> see how that goes.
> >
> > Ok, should find any randconfig build failures for arm, arm64 or x86 over the
> > weekend. I also this on linux-next today
> >
> > ld: drivers/gpu/drm/i915/display/intel_panel.o: in function
> > `intel_backlight_device_register':
> > intel_panel.c:(.text+0x2804): undefined reference to 
> > `backlight_device_register'
> > ld: intel_panel.c:(.text+0x284e): undefined reference to
> > `backlight_device_register'
> > ld: drivers/gpu/drm/i915/display/intel_panel.o: in function
> > `intel_backlight_device_unregister':
> > intel_panel.c:(.text+0x28b1): undefined reference to
> > `backlight_device_unregister'
> >
> > and I added this same thing there to see how it goes:
>
> Last I checked (and it was a while a go) you really had to make all
> users of BACKLIGHT_CLASS_DEVICE depend not select it, otherwise you end
> up with recursive dependencies.

Yes, that is correct. It turns out that my randconfig tree already had a local
patch to change most of the other users (everything outside of drivers/gpu)
to 'depends on'.

  Arnd

Re: [Nouveau] [PATCH] depend on BACKLIGHT_CLASS_DEVICE for more devices

[Arnd Bergmann]

On Wed, Aug 4, 2021 at 8:59 PM Karol Herbst  wrote:
> On Wed, Aug 4, 2021 at 4:43 PM Karol Herbst  wrote:
> > On Wed, Aug 4, 2021 at 4:19 PM Arnd Bergmann  wrote:
> > > On Wed, Aug 4, 2021 at 4:10 PM Karol Herbst  wrote:
> > > >
> > > > playing around a little bit with this, I think the original "select
> > > > BACKLIGHT_CLASS_DEVICE" is fine. Atm we kind of have this weird mix of
> > > > drivers selecting and others depending on it. We could of course convert
> > > > everything over to depend, and break those cycling dependency issues 
> > > > with
> > > > this.
> > > >
> > > > Anyway this change on top of my initial patch is enough to make Kconfig
> > > > happy and has the advantage of not having to mess with the deps of 
> > > > nouveau
> > > > too much.
> > >
> > > Looks good to me. We'd probably want to make the BACKLIGHT_CLASS_DEVICE
> > > option itself 'default FB || DRM' though, to ensure that defconfigs
> > > keep working.
> > >
> >
> > okay cool. Will send out a proper updated patch series soonish.
> >
>
> mhh, actually that breaks drivers selecting FB_BACKLIGHT as now
> BACKLIGHT_CLASS_DEVICE might be disabled :(

Are you sure? It should already be the case that any driver that selects
FB_BACKLIGHT either 'depends on BACKLIGHT_CLASS_DEVICE'
or 'select BACKLIGHT_CLASS_DEVICE'.

If you change all the 'select BACKLIGHT_CLASS_DEVICE' to 'depends
on', I don't see a problem with doing 'select FB_BACKLIGHT' from
those.

I have applied your patch to my randconfig tree and built a few dozen
kernels, don't see any regressions so far, but will let it run over night.

  Arnd

Re: [Nouveau] [PATCH] depend on BACKLIGHT_CLASS_DEVICE for more devices

[Arnd Bergmann]

On Wed, Aug 4, 2021 at 4:10 PM Karol Herbst  wrote:
>
> playing around a little bit with this, I think the original "select
> BACKLIGHT_CLASS_DEVICE" is fine. Atm we kind of have this weird mix of
> drivers selecting and others depending on it. We could of course convert
> everything over to depend, and break those cycling dependency issues with
> this.
>
> Anyway this change on top of my initial patch is enough to make Kconfig
> happy and has the advantage of not having to mess with the deps of nouveau
> too much.

Looks good to me. We'd probably want to make the BACKLIGHT_CLASS_DEVICE
option itself 'default FB || DRM' though, to ensure that defconfigs
keep working.

  Arnd

Re: [Nouveau] [PATCH] depend on BACKLIGHT_CLASS_DEVICE for more devices

[Arnd Bergmann]

On Thu, Aug 5, 2021 at 12:01 AM Karol Herbst  wrote:
>
> On Wed, Aug 4, 2021 at 11:10 PM Arnd Bergmann  wrote:
> >
> > On Wed, Aug 4, 2021 at 8:59 PM Karol Herbst  wrote:
> > > On Wed, Aug 4, 2021 at 4:43 PM Karol Herbst  wrote:
> > > > On Wed, Aug 4, 2021 at 4:19 PM Arnd Bergmann  wrote:
> > > > > On Wed, Aug 4, 2021 at 4:10 PM Karol Herbst  
> > > > > wrote:
> > > > > >
> > > > > > playing around a little bit with this, I think the original "select
> > > > > > BACKLIGHT_CLASS_DEVICE" is fine. Atm we kind of have this weird mix 
> > > > > > of
> > > > > > drivers selecting and others depending on it. We could of course 
> > > > > > convert
> > > > > > everything over to depend, and break those cycling dependency 
> > > > > > issues with
> > > > > > this.
> > > > > >
> > > > > > Anyway this change on top of my initial patch is enough to make 
> > > > > > Kconfig
> > > > > > happy and has the advantage of not having to mess with the deps of 
> > > > > > nouveau
> > > > > > too much.
> > > > >
> > > > > Looks good to me. We'd probably want to make the 
> > > > > BACKLIGHT_CLASS_DEVICE
> > > > > option itself 'default FB || DRM' though, to ensure that defconfigs
> > > > > keep working.
> > > > >
> > > >
> > > > okay cool. Will send out a proper updated patch series soonish.
> > > >
> > >
> > > mhh, actually that breaks drivers selecting FB_BACKLIGHT as now
> > > BACKLIGHT_CLASS_DEVICE might be disabled :(
> >
> > Are you sure? It should already be the case that any driver that selects
> > FB_BACKLIGHT either 'depends on BACKLIGHT_CLASS_DEVICE'
> > or 'select BACKLIGHT_CLASS_DEVICE'.
> >
> none of the fb drivers seem to do that.

Ah, right, I see now that my randconfig series has a couple of patches
applied that deal with other random failures, including this one:

https://patchwork.kernel.org/project/linux-fbdev/patch/2020041713.675905-8-a...@arndb.de/

Part of the series went in (through different ways) now, but this one
never did.

  Arnd

Re: [Nouveau] [PATCH] nouveau: make backlight support non optional

[Arnd Bergmann]

On Sat, Jul 24, 2021 at 12:47 AM Karol Herbst  wrote:
>
> In the past this only led to compilation issues. Also the small amount of
> extra .text shouldn't really matter compared to the entire nouveau driver
> anyway.
>

> select DRM_TTM_HELPER
> -   select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT
> -   select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && INPUT
> +   select BACKLIGHT_CLASS_DEVICE
> +   select ACPI_VIDEO if ACPI && X86 && INPUT
> select X86_PLATFORM_DEVICES if ACPI && X86
> select ACPI_WMI if ACPI && X86

I think the logic needs to be the reverse: instead of 'select
BACKLIGHT_CLASS_DEVICE',
this should be 'depends on BACKLIGHT_CLASS_DEVICE', and the same for ACPI_VIDEO.

We may want to add 'default DRM || FB' to BACKLIGHT_CLASS_DEVICE in the
process so we don't lose it for users doing 'make oldconfig' or 'make defconfig'

The rest of the patch looks good to me.

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

Re: [Nouveau] [PATCH] nouveau: make backlight support non optional

[Arnd Bergmann]

On Sat, Jul 24, 2021 at 4:14 PM Karol Herbst  wrote:
>
> we use the MXM_WMI in code. We also have to keep arm in mind and not
> break stuff there. So I will try to play around with your changes and
> see how that goes.

Ok, should find any randconfig build failures for arm, arm64 or x86 over the
weekend. I also this on linux-next today

ld: drivers/gpu/drm/i915/display/intel_panel.o: in function
`intel_backlight_device_register':
intel_panel.c:(.text+0x2804): undefined reference to `backlight_device_register'
ld: intel_panel.c:(.text+0x284e): undefined reference to
`backlight_device_register'
ld: drivers/gpu/drm/i915/display/intel_panel.o: in function
`intel_backlight_device_unregister':
intel_panel.c:(.text+0x28b1): undefined reference to
`backlight_device_unregister'

and I added this same thing there to see how it goes:

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 87825d36335b..69c6b7aec49e 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -3,6 +3,8 @@ config DRM_I915
tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
depends on DRM
depends on X86 && PCI
+   depends on ACPI_VIDEO || !ACPI
+   depends on BACKLIGHT_CLASS_DEVICE
select INTEL_GTT
select INTERVAL_TREE
# we need shmfs for the swappable backing store, and in particular
@@ -16,10 +18,6 @@ config DRM_I915
select IRQ_WORK
# i915 depends on ACPI_VIDEO when ACPI is enabled
# but for select to work, need to select ACPI_VIDEO's dependencies, ick
-   select DRM_I915_BACKLIGHT if ACPI
-   select INPUT if ACPI
-   select ACPI_VIDEO if ACPI
-   select ACPI_BUTTON if ACPI
select SYNC_FILE
select IOSF_MBI
select CRC32
@@ -64,13 +62,7 @@ config DRM_I915_FORCE_PROBE
  Use "*" to force probe the driver for all known devices.

 config DRM_I915_BACKLIGHT
-   tristate "Control backlight support"
-   depends on DRM_I915
-   default DRM_I915
-   select BACKLIGHT_CLASS_DEVICE
-   help
-  Say Y here if you want to control the backlight of your display
-  (e.g. a laptop panel).
+   def_tristate DRM_I915

 config DRM_I915_CAPTURE_ERROR
bool "Enable capturing GPU state following a hang"
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] nouveau: make backlight support non optional

[Arnd Bergmann]

On Sat, Jul 24, 2021 at 2:52 PM Karol Herbst  wrote:
>
> On Sat, Jul 24, 2021 at 2:10 PM Karol Herbst  wrote:
> >
> > On Sat, Jul 24, 2021 at 1:56 PM Arnd Bergmann  wrote:
> > >
> > > On Sat, Jul 24, 2021 at 11:55 AM Karol Herbst  wrote:
> > >
> > > - You run into dependency loops that prevent a successful build when some
> > >other driver has a 'depends on'. Preventing these loops was the main
> > >reason I said we should do this change.
> > >
> > > In theory we could change the other 85 drivers that use 'depends on' 
> > > today,
> > > and make BACKLIGHT_CLASS_DEVICE a hidden symbol that only ever
> > > selected by the drivers that need it. This would avoid the third problem 
> > > but
> > > not the other one.
> > >
> > I see. Yeah, I guess we can do it this way then. I just wasn't aware
> > of the bigger picture here. Thanks for explaining.
>
> yeah... that doesn't work. So the issue is, that X86_PLATFORM_DEVICES
> is a little bit in the way. If I remove the select
> X86_PLATFORM_DEVICES then I guess problems once ACPI is enabled, but
> if I keep it, I get cyclic dep errors :/

Right, this is the exact problem I explained: since all other drivers use
'depends on X86_PLATFORM_DEVICES' instead of 'select', you get a
loop again. Prior to changing the BACKLIGHT_CLASS_DEVICE dependency,
nouveau was pretty much on top of everything else in the hierarchy,
changing part of it can result in a loop.

I see that there are about ten more 'select' statements that look like
they should not be there, and almost all of them were added in order
to be able to 'select MXM_WMI'.

I think we can go as far as the patch below, which I've put in my
randconfig build machine, on top of your patch. I'm not entirely
sure how strong the dependency on MXM_WMI is: does it cause
a build failure when that is not enabled, or was this select just
added for convenience so users don't get surprised when it's missing?

   Arnd

diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 9c2108b48524..f2585416507e 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -3,21 +3,14 @@ config DRM_NOUVEAU
tristate "Nouveau (NVIDIA) cards"
depends on DRM && PCI && MMU
depends on AGP || !AGP
+   depends on ACPI_VIDEO || !ACPI
+   depends on BACKLIGHT_CLASS_DEVICE
+   depends on MXM_WMI || !X86 || !ACPI
select IOMMU_API
select FW_LOADER
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
-   select BACKLIGHT_CLASS_DEVICE
-   select ACPI_VIDEO if ACPI && X86 && INPUT
-   select X86_PLATFORM_DEVICES if ACPI && X86
-   select ACPI_WMI if ACPI && X86
-   select MXM_WMI if ACPI && X86
-   select POWER_SUPPLY
-   # Similar to i915, we need to select ACPI_VIDEO and it's dependencies
-   select INPUT if ACPI && X86
-   select THERMAL if ACPI && X86
-   select ACPI_VIDEO if ACPI && X86
select SND_HDA_COMPONENT if SND_HDA_CORE
help
  Choose this option for open-source NVIDIA support.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] nouveau: make backlight support non optional

[Arnd Bergmann]

On Sat, Jul 24, 2021 at 11:55 AM Karol Herbst  wrote:
>
> On Sat, Jul 24, 2021 at 8:55 AM Arnd Bergmann  wrote:
> >
> > On Sat, Jul 24, 2021 at 12:47 AM Karol Herbst  wrote:
> > >
> > > In the past this only led to compilation issues. Also the small amount of
> > > extra .text shouldn't really matter compared to the entire nouveau driver
> > > anyway.
> > >
> >
> > > select DRM_TTM_HELPER
> > > -   select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT
> > > -   select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && 
> > > INPUT
> > > +   select BACKLIGHT_CLASS_DEVICE
> > > +   select ACPI_VIDEO if ACPI && X86 && INPUT
> > > select X86_PLATFORM_DEVICES if ACPI && X86
> > > select ACPI_WMI if ACPI && X86
> >
> > I think the logic needs to be the reverse: instead of 'select
> > BACKLIGHT_CLASS_DEVICE',
> > this should be 'depends on BACKLIGHT_CLASS_DEVICE', and the same for 
> > ACPI_VIDEO.
> >
> > We may want to add 'default DRM || FB' to BACKLIGHT_CLASS_DEVICE in the
> > process so we don't lose it for users doing 'make oldconfig' or 'make 
> > defconfig'
> >
>
> I think the problem with
> "depends" is that the user needs to enable backlight support first
> before even seeing nouveau and I don't know if that makes sense. But
> maybe "default" is indeed helping here in this case.

In general, no driver should ever 'select' a subsystem. Otherwise you end up
with two problems:

- enabling this one driver suddenly makes all other drivers that have
a dependency
  on this visible, and some of those might have a 'default y', so you
end up with
  a ton of stuff in the kernel that would otherwise not be there.

- It becomes impossible to turn it off as long as some driver has that 'select'.
  This is the pretty much the same problem as the one you describe, just
   the other side of it.

- You run into dependency loops that prevent a successful build when some
   other driver has a 'depends on'. Preventing these loops was the main
   reason I said we should do this change.

In theory we could change the other 85 drivers that use 'depends on' today,
and make BACKLIGHT_CLASS_DEVICE a hidden symbol that only ever
selected by the drivers that need it. This would avoid the third problem but
not the other one.

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

[Nouveau] [PATCH] drm/nouveau/kms/nv50-: fix build failure with CONFIG_BACKLIGHT=n

[Arnd Bergmann]

From: Arnd Bergmann 

When the backlight support is disabled, the driver fails to build:

drivers/gpu/drm/nouveau/dispnv50/disp.c: In function 'nv50_sor_atomic_disable':
drivers/gpu/drm/nouveau/dispnv50/disp.c:1665:59: error: 'struct 
nouveau_connector' has no member named 'backlight'
 1665 | struct nouveau_backlight *backlight = nv_connector->backlight;
  |   ^~
drivers/gpu/drm/nouveau/dispnv50/disp.c:1670:35: error: invalid use of 
undefined type 'struct nouveau_backlight'
 1670 | if (backlight && backlight->uses_dpcd) {
  |   ^~
drivers/gpu/drm/nouveau/dispnv50/disp.c:1671:64: error: invalid use of 
undefined type 'struct nouveau_backlight'
 1671 | ret = drm_edp_backlight_disable(aux, 
>edp_info);
  |^~

The patch that introduced the problem already contains some #ifdef
checks, so just add another one that makes it build again.

Fixes: 6eca310e8924 ("drm/nouveau/kms/nv50-: Add basic DPCD backlight support 
for nouveau")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 093e1f7163b3..fcf53e24db21 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1659,20 +1659,23 @@ static void
 nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state 
*state)
 {
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
-   struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
struct nouveau_crtc *nv_crtc = nouveau_crtc(nv_encoder->crtc);
struct nouveau_connector *nv_connector = 
nv50_outp_get_old_connector(state, nv_encoder);
-   struct nouveau_backlight *backlight = nv_connector->backlight;
struct drm_dp_aux *aux = _connector->aux;
-   int ret;
u8 pwr;
 
+#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
+   struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
+   struct nouveau_backlight *backlight = nv_connector->backlight;
+
if (backlight && backlight->uses_dpcd) {
-   ret = drm_edp_backlight_disable(aux, >edp_info);
+   int ret = drm_edp_backlight_disable(aux, >edp_info);
+
if (ret < 0)
NV_ERROR(drm, "Failed to disable backlight on 
[CONNECTOR:%d:%s]: %d\n",
 nv_connector->base.base.id, 
nv_connector->base.name, ret);
}
+#endif
 
if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
int ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, );
-- 
2.29.2

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

Re: [Nouveau] [PATCH] drm/nouveau/kms/nv50-: fix build failure with CONFIG_BACKLIGHT=n

[Arnd Bergmann]

On Fri, Jul 23, 2021 at 11:25 AM Daniel Vetter  wrote:
>
> On Fri, Jul 23, 2021 at 11:15 AM Arnd Bergmann  wrote:
> >
> > From: Arnd Bergmann 
> >
> > When the backlight support is disabled, the driver fails to build:
> >
> > drivers/gpu/drm/nouveau/dispnv50/disp.c: In function 
> > 'nv50_sor_atomic_disable':
> > drivers/gpu/drm/nouveau/dispnv50/disp.c:1665:59: error: 'struct 
> > nouveau_connector' has no member named 'backlight'
> >  1665 | struct nouveau_backlight *backlight = 
> > nv_connector->backlight;
> >   |   ^~
> > drivers/gpu/drm/nouveau/dispnv50/disp.c:1670:35: error: invalid use of 
> > undefined type 'struct nouveau_backlight'
> >  1670 | if (backlight && backlight->uses_dpcd) {
> >   |   ^~
> > drivers/gpu/drm/nouveau/dispnv50/disp.c:1671:64: error: invalid use of 
> > undefined type 'struct nouveau_backlight'
> >  1671 | ret = drm_edp_backlight_disable(aux, 
> > >edp_info);
> >   |^~
> >
> > The patch that introduced the problem already contains some #ifdef
> > checks, so just add another one that makes it build again.
> >
> > Fixes: 6eca310e8924 ("drm/nouveau/kms/nv50-: Add basic DPCD backlight 
> > support for nouveau")
> > Signed-off-by: Arnd Bergmann 
>
> Can we just toss the idea that BACKTLIGHT=n is a reasonable config for
> drm drivers using backlights, and add depends BACKLIGHT to all of
> them?
>
> I mean this is a perfect source of continued patch streams to keep us
> all busy, but beyond that I really don't see the point ... I frankly
> have better things to do, and especially with the big drivers we have
> making backlight optional saves comparitively nothing.
> -Daniel

Yes! I'd definitely be in favor of that, I've wasted way too much time trying
to sort through dependency loops and other problems with backlight support.

Maybe we should leave the drivers/video/fbdev/ drivers untouched in this
regard, at least for the moment, but for the drivers/gpu/drm users of
backlight that would be a nice simplification, and even the smallest ones
are unlikely to be used on systems that are too memory constrained to
deal with 4KB extra .text.

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

Re: [Nouveau] [PATCH] drm/nouveau/kms/nv50-: fix build failure with CONFIG_BACKLIGHT=n

[Arnd Bergmann]

On Fri, Jul 23, 2021 at 6:34 PM Karol Herbst  wrote:
> On Fri, Jul 23, 2021 at 6:31 PM Randy Dunlap  wrote:
> > On 7/23/21 8:15 AM, Karol Herbst wrote:
> > > On Fri, Jul 23, 2021 at 5:10 PM Randy Dunlap  
> > > wrote:
> > >
> > > what's actually the use case of compiling with
> > > CONFIG_DRM_NOUVEAU_BACKLIGHT=n anyway?
> >
> > Dunno. In this case it was just a randconfig. Still, it needs to be
> > handled in some way - such as the other suggestion in this thread.
> >
>
> sure, I was just curious if there was a specific use case or just
> something random as you mentioned.

I think this is purely done because of tradition. A long time ago, we had
tiny framebuffer drivers and most PCs did not have backlights, so it
made sense to leave this optional.

This was probably just always carried over.

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

[Nouveau] [PATCH] drm/nouveau/pci: rework AGP dependency

[Arnd Bergmann]

From: Arnd Bergmann 

I noticed a warning from 'nm' when CONFIG_TRIM_UNUSED_KSYMS is set
and IS_REACHABLE(CONFIG_AGP) is false:

drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.o: no symbols

I later found this is completely harmless and we should find a way
to suppress the warning, but at that point I had already done a
cleanup patch to address this instance.

It turns out this code could be improved anyway, as the current version
behaves unexpectedly when AGP is a loadable module but nouveau is built-in
itself, in which case it silently omits agp support.

A better way to handle this is with a Kconfig dependency that requires
AGP either to be disabled, or forces nouveau to be a module for AGP=m.
With this change, the compile-time hack can be removed and lld no
longer warns.

Fixes: 340b0e7c500a ("drm/nouveau/pci: merge agp handling from nouveau drm")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/Kbuild | 1 +
 drivers/gpu/drm/nouveau/Kconfig| 1 +
 drivers/gpu/drm/nouveau/nvkm/subdev/pci/Kbuild | 2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.c  | 2 --
 drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.h  | 9 +
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild
index 60586fb8275e..173b8d9d85e3 100644
--- a/drivers/gpu/drm/nouveau/Kbuild
+++ b/drivers/gpu/drm/nouveau/Kbuild
@@ -15,6 +15,7 @@ nouveau-y := $(nvif-y)
 #- code also used by various userspace tools/tests
 include $(src)/nvkm/Kbuild
 nouveau-y += $(nvkm-y)
+nouveau-m += $(nvkm-m)
 
 # DRM - general
 ifdef CONFIG_X86
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 278e048235a9..90276a557a70 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -2,6 +2,7 @@
 config DRM_NOUVEAU
tristate "Nouveau (NVIDIA) cards"
depends on DRM && PCI && MMU
+   depends on AGP || !AGP
select IOMMU_API
select FW_LOADER
select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/Kbuild 
b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/Kbuild
index 174bdf995271..a400c680cf65 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/Kbuild
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/Kbuild
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: MIT
-nvkm-y += nvkm/subdev/pci/agp.o
+nvkm-$(CONFIG_AGP) += nvkm/subdev/pci/agp.o
 nvkm-y += nvkm/subdev/pci/base.o
 nvkm-y += nvkm/subdev/pci/pcie.o
 nvkm-y += nvkm/subdev/pci/nv04.o
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.c
index 385a90f91ed6..86c9e1d658af 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.c
@@ -20,7 +20,6 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 #include "agp.h"
-#ifdef __NVKM_PCI_AGP_H__
 #include 
 
 struct nvkm_device_agp_quirk {
@@ -172,4 +171,3 @@ nvkm_agp_ctor(struct nvkm_pci *pci)
 
pci->agp.mtrr = arch_phys_wc_add(pci->agp.base, pci->agp.size);
 }
-#endif
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.h 
b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.h
index ad4d3621d02b..041fe1fbf093 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/agp.h
@@ -1,15 +1,14 @@
-/* SPDX-License-Identifier: MIT */
-#include "priv.h"
-#if defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && defined(MODULE))
 #ifndef __NVKM_PCI_AGP_H__
 #define __NVKM_PCI_AGP_H__
 
+/* SPDX-License-Identifier: MIT */
+#include "priv.h"
+#if IS_ENABLED(CONFIG_AGP)
 void nvkm_agp_ctor(struct nvkm_pci *);
 void nvkm_agp_dtor(struct nvkm_pci *);
 void nvkm_agp_preinit(struct nvkm_pci *);
 int nvkm_agp_init(struct nvkm_pci *);
 void nvkm_agp_fini(struct nvkm_pci *);
-#endif
 #else
 static inline void nvkm_agp_ctor(struct nvkm_pci *pci) {}
 static inline void nvkm_agp_dtor(struct nvkm_pci *pci) {}
@@ -17,3 +16,5 @@ static inline void nvkm_agp_preinit(struct nvkm_pci *pci) {}
 static inline int nvkm_agp_init(struct nvkm_pci *pci) { return -ENOSYS; }
 static inline void nvkm_agp_fini(struct nvkm_pci *pci) {}
 #endif
+
+#endif
-- 
2.29.2

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

Re: [Nouveau] [PATCH] nouveau: add fbdev dependency

[Arnd Bergmann]

On Thu, May 28, 2020 at 7:37 AM Dave Airlie  wrote:
>
> On Thu, 28 May 2020 at 00:36, Arnd Bergmann  wrote:
> >
> > On Wed, May 27, 2020 at 4:05 PM Ilia Mirkin  wrote:
> > >
> > > Isn't this already fixed by
> > >
> > > https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403
> >
> > Ok, I see that fixes the link error, but I when I created my fix, that did
> > not seem like the correct solution because it reverts part of the original
> > patch without reverting the rest of it. Unfortunately there was no
> > changelog text in the first patch to explain why this is safe.
>
> No it doesn't, I think you missed the pci in API name.
>
> The initial behaviour doesn't use the pci version of the API, the
> replacement did, and the fix used the drm wrapper around the pci one.
>
> So this patch isn't necessary now that I've fixed it the other way,

Ok, got it, thanks for explaining!

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

Re: [Nouveau] [PATCH] nouveau: add fbdev dependency

[Arnd Bergmann]

On Wed, May 27, 2020 at 4:05 PM Ilia Mirkin  wrote:
>
> Isn't this already fixed by
>
> https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403

Ok, I see that fixes the link error, but I when I created my fix, that did
not seem like the correct solution because it reverts part of the original
patch without reverting the rest of it. Unfortunately there was no
changelog text in the first patch to explain why this is safe.

Could you double-check if the behavior is still correct after the two patches?

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

[Nouveau] [PATCH] nouveau: add fbdev dependency

[Arnd Bergmann]

Calling directly into the fbdev stack only works when the
fbdev layer is built into the kernel as well, or both are
loadable modules:

drivers/gpu/drm/nouveau/nouveau_drm.o: in function `nouveau_drm_probe':
nouveau_drm.c:(.text+0x1f90): undefined reference to 
`remove_conflicting_pci_framebuffers'

The change seems to have been intentional, so add an explicit
dependency here but allow it to still be compiled if FBDEV
is completely disabled.

Fixes: 2dd4d163cd9c ("drm/nouveau: remove open-coded version of 
remove_conflicting_pci_framebuffers()")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/Kconfig   | 1 +
 drivers/gpu/drm/nouveau/nouveau_drm.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 980ed09bd7f6..8c640f003358 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -18,6 +18,7 @@ config DRM_NOUVEAU
select THERMAL if ACPI && X86
select ACPI_VIDEO if ACPI && X86
select SND_HDA_COMPONENT if SND_HDA_CORE
+   depends on FBDEV || !FBDEV
help
  Choose this option for open-source NVIDIA support.
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index eb10c80ed853..e8560444ab57 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -697,7 +697,8 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
nvkm_device_del();
 
/* Remove conflicting drivers (vesafb, efifb etc). */
-   ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb");
+   if (IS_ENABLED(CONFIG_FBDEV))
+   ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb");
if (ret)
return ret;
 
-- 
2.26.2

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

Re: [Nouveau] [PATCH 2/2] nouveau: fix dependencies for DEVICE_PRIVATE

[Arnd Bergmann]

On Fri, May 8, 2020 at 9:04 PM Jason Gunthorpe  wrote:
>
> On Fri, May 08, 2020 at 05:05:03PM +0200, Arnd Bergmann wrote:
> > On Fri, May 8, 2020 at 5:00 PM Jason Gunthorpe  wrote:
> > >
> > > On Fri, May 08, 2020 at 04:40:09PM +0200, Arnd Bergmann wrote:
> > > > CONFIG_DEVICE_PRIVATE cannot be selected in configurations
> > > > without ZONE_DEVICE:
> > >
> > > It is kind of unfortunate to lift dependencies from DEVICE_PRIVATE
> > > into the users, is this really how kconfig is supposed to work or is
> > > something else wrong here?
> >
> > Usually the problem is trying to use 'select' for something that can
> > be done with 'depends on'. I have actually no idea what
> > CONFIG_DEVICE_PRIVATE does, as it lacks a help text
> > and is a rather generic term.
>
> It is a sub mode of ZONE_DEVICE, ie DEVICE_PRIVATE turns on a certain
> kind of ZONE_DEVICE page.
>
> Both ZONE_DEVICE and DEVICE_PRIVATE are APIs families drivers use,
> there is no reason for a user to select either of these directly.

Ok, then how about making ZONE_DEVICE a hidden symbol and adding
something like

config ZONE_DEVICE_POSSIBLE
def_bool y
depends on MEMORY_HOTPLUG
depends on MEMORY_HOTREMOVE
depends on SPARSEMEM_VMEMMAP
depends on ARCH_HAS_PTE_DEVMAP

then drivers that want it can do

config FOO
tristate "user visible option"
depends on ZONE_DEVICE_POSSIBLE
select ZONE_DEVICE

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

Re: [Nouveau] [PATCH 2/2] nouveau: fix dependencies for DEVICE_PRIVATE

[Arnd Bergmann]

On Fri, May 8, 2020 at 5:00 PM Jason Gunthorpe  wrote:
>
> On Fri, May 08, 2020 at 04:40:09PM +0200, Arnd Bergmann wrote:
> > CONFIG_DEVICE_PRIVATE cannot be selected in configurations
> > without ZONE_DEVICE:
>
> It is kind of unfortunate to lift dependencies from DEVICE_PRIVATE
> into the users, is this really how kconfig is supposed to work or is
> something else wrong here?

Usually the problem is trying to use 'select' for something that can
be done with 'depends on'. I have actually no idea what
CONFIG_DEVICE_PRIVATE does, as it lacks a help text
and is a rather generic term.

Would it be possible to decouple DEVICE_PRIVATE from ZONE_DEVICE?
It sounds like the first is related to the device model, while
the second is for memory management, so maybe the dependency
is not necessary.

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

[Nouveau] [PATCH 2/2] nouveau: fix dependencies for DEVICE_PRIVATE

[Arnd Bergmann]

CONFIG_DEVICE_PRIVATE cannot be selected in configurations
without ZONE_DEVICE:

WARNING: unmet direct dependencies detected for DEVICE_PRIVATE
  Depends on [n]: ZONE_DEVICE [=n]
  Selected by [y]:
  - DRM_NOUVEAU_SVM [=y] && HAS_IOMEM [=y] && DRM_NOUVEAU [=y] && MMU [=y] && 
STAGING [=y]
kernel/resource.c:1653:28: error: use of undeclared identifier 
'PA_SECTION_SHIFT'
size = ALIGN(size, 1UL << PA_SECTION_SHIFT);
  ^
kernel/resource.c:1654:48: error: use of undeclared identifier 
'MAX_PHYSMEM_BITS'

Add a dependency for Nouveau to avoid broken randconfig builds.

Fixes: d2c63df2242e ("mm/hmm: make CONFIG_DEVICE_PRIVATE into a select")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/nouveau/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index af5793f3e7c2..996ec5475908 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -87,6 +87,7 @@ config DRM_NOUVEAU_BACKLIGHT
 config DRM_NOUVEAU_SVM
bool "(EXPERIMENTAL) Enable SVM (Shared Virtual Memory) support"
depends on DRM_NOUVEAU
+   depends on ZONE_DEVICE
depends on MMU
depends on STAGING
select DEVICE_PRIVATE
-- 
2.26.0

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

Re: [Nouveau] [RFT 00/13] iomap: Constify ioreadX() iomem argument

[Arnd Bergmann]

On Wed, Jan 8, 2020 at 10:15 AM Krzysztof Kozlowski  wrote:

> > The __force-cast that removes the __iomem here also means that
> > the 'volatile' keyword could be dropped from the argument list,
> > as it has no real effect any more, but then there are a few drivers
> > that mark their iomem pointers as either 'volatile void __iomem*' or
> > (worse) 'volatile void *', so we keep it in the argument list to not
> > add warnings for those drivers.
> >
> > It may be time to change these drivers to not use volatile for __iomem
> > pointers, but that seems out of scope for what Krzysztof is trying
> > to do. Ideally we would be consistent here though, either using volatile
> > all the time or never.
>
> Indeed. I guess there are no objections around const so let me send v2
> for const only.

Ok, sounds good. Maybe mention in the changelog then that the
'volatile' in the interface is intentionally left out, and that only users
of readl/writel still have it to deal with existing drivers.

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

Re: [Nouveau] [RFT 03/13] sh: Constify ioreadX() iomem argument (as in generic implementation)

[Arnd Bergmann]

On Tue, Jan 7, 2020 at 5:54 PM Krzysztof Kozlowski  wrote:
>
> The ioreadX() helpers have inconsistent interface.  On some architectures
> void *__iomem address argument is a pointer to const, on some not.
>
> Implementations of ioreadX() do not modify the memory under the address
> so they can be converted to a "const" version for const-safety and
> consistency among architectures.
>
> Signed-off-by: Krzysztof Kozlowski 

The patch looks good, but I think this has to be done together with the powerpc
version and the header that declares the function, for bisectibility.

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

Re: [Nouveau] [RFT 06/13] arc: Constify ioreadX() iomem argument (as in generic implementation)

[Arnd Bergmann]

On Tue, Jan 7, 2020 at 5:54 PM Krzysztof Kozlowski  wrote:
>
> The ioreadX() helpers have inconsistent interface.  On some architectures
> void *__iomem address argument is a pointer to const, on some not.
>
> Implementations of ioreadX() do not modify the memory under the
> address so they can be converted to a "const" version for const-safety
> and consistency among architectures.
>
> Signed-off-by: Krzysztof Kozlowski 

The patch looks correct, but I would not bother here, as it has no
effect on the compiler or its output.

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

Re: [Nouveau] [RFT 00/13] iomap: Constify ioreadX() iomem argument

[Arnd Bergmann]

On Wed, Jan 8, 2020 at 9:36 AM Christophe Leroy  wrote:
> Le 08/01/2020 à 09:18, Krzysztof Kozlowski a écrit :
> > On Wed, 8 Jan 2020 at 09:13, Geert Uytterhoeven  
> > wrote:
> > I'll add to this one also changes to ioreadX_rep() and add another
> > patch for volatile for reads and writes. I guess your review will be
> > appreciated once more because of ioreadX_rep()
> >
>
> volatile should really only be used where deemed necessary:
>
> https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html
>
> It is said: " ...  accessor functions might use volatile on
> architectures where direct I/O memory access does work. Essentially,
> each accessor call becomes a little critical section on its own and
> ensures that the access happens as expected by the programmer."

The I/O accessors are one of the few places in which 'volatile' generally
makes sense, at least for the implementations that do a plain pointer
dereference (probably none of the ones in question here).

In case of readl/writel, this is what we do in asm-generic:

static inline u32 __raw_readl(const volatile void __iomem *addr)
{
return *(const volatile u32 __force *)addr;
}

The __force-cast that removes the __iomem here also means that
the 'volatile' keyword could be dropped from the argument list,
as it has no real effect any more, but then there are a few drivers
that mark their iomem pointers as either 'volatile void __iomem*' or
(worse) 'volatile void *', so we keep it in the argument list to not
add warnings for those drivers.

It may be time to change these drivers to not use volatile for __iomem
pointers, but that seems out of scope for what Krzysztof is trying
to do. Ideally we would be consistent here though, either using volatile
all the time or never.

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

Re: [Nouveau] [PATCH v2 1/9] iomap: Constify ioreadX() iomem argument (as in generic implementation)

[Arnd Bergmann]

On Wed, Jan 8, 2020 at 9:05 PM Krzysztof Kozlowski  wrote:
>
> The ioreadX() and ioreadX_rep() helpers have inconsistent interface.  On
> some architectures void *__iomem address argument is a pointer to const,
> on some not.
>
> Implementations of ioreadX() do not modify the memory under the address
> so they can be converted to a "const" version for const-safety and
> consistency among architectures.
>
> Suggested-by: Geert Uytterhoeven 
> Signed-off-by: Krzysztof Kozlowski 
> Reviewed-by: Geert Uytterhoeven 

Thanks for getting this done!

Reviewed-by: Arnd Bergmann 

> Changes since v1:
> 1. Constify also ioreadX_rep() and mmio_insX(),
> 2. Squash lib+alpha+powerpc+parisc+sh into one patch for bisectability,

The alpha and parisc versions should be independent, I was thinking
you leave them as separate patches, but this work for me too.

I have no real opinion on the other 8 patches, I would have left
them out completely, but they don't hurt either.

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

[Nouveau] [PATCH] drm: nouveau: use larger buffer in nvif_vmm_map

[Arnd Bergmann]

gcc points out a buffer that is clearly too small to be used
in a meaningful way, as the 'sizeof(*args) + argc > sizeof(stack)'
will always fail:

In function 'memcpy',
inlined from 'nvif_vmm_map' at drivers/gpu/drm/nouveau/nvif/vmm.c:55:2:
include/linux/string.h:353:9: error: '__builtin_memcpy' offset 40 is out of the 
bounds [0, 16] of object 'stack' with type 'u8[16]' {aka 'unsigned char[16]'} 
[-Werror=array-bounds]
  return __builtin_memcpy(p, q, size);
 ^~~~
drivers/gpu/drm/nouveau/nvif/vmm.c: In function 'nvif_vmm_map':
drivers/gpu/drm/nouveau/nvif/vmm.c:40:5: note: 'stack' declared here

This makes the buffer large enough so it should serve the purpose
that the author presumably had in mind. Alternatively we could
just get rid of it completely and simplify the code at the cost
of always doing the kmalloc (as we do in the current version).

Fixes: 920d2b5ef215 ("drm/nouveau/mmu: define user interfaces to mmu vmm 
opertaions")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
Cc: Martin Sebor <mse...@gcc.gnu.org>

Martin: this one is interesting, I think it qualifies as a
false-positive warning that gcc should not print because there
is no overflow, but the code is still wrong because we never
copy into the fixed-size buffer that was intended as a
micro-optimization
---
 drivers/gpu/drm/nouveau/nvif/vmm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvif/vmm.c 
b/drivers/gpu/drm/nouveau/nvif/vmm.c
index 31cdb2d2e1ff..191832be6c65 100644
--- a/drivers/gpu/drm/nouveau/nvif/vmm.c
+++ b/drivers/gpu/drm/nouveau/nvif/vmm.c
@@ -37,7 +37,7 @@ nvif_vmm_map(struct nvif_vmm *vmm, u64 addr, u64 size, void 
*argv, u32 argc,
 struct nvif_mem *mem, u64 offset)
 {
struct nvif_vmm_map_v0 *args;
-   u8 stack[16];
+   u8 stack[48];
int ret;
 
if (sizeof(*args) + argc > sizeof(stack)) {
-- 
2.9.0

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

[Nouveau] [PATCH] [RESEND v2] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning

[Arnd Bergmann]

gcc thinks that interpreting a multiplication result as a bool
is confusing:

drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c: In function 'read_pll':
drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c:133:8: error: '*' in boolean 
context, suggest '&&' instead [-Werror=int-in-bool-context]

Adding a temporary variable to contain the divisor helps make
it clear what is going on and avoids that warning.

Fixes: 7632b30e4b8b ("drm/nouveau/clk: namespace + nvidia gpu names (no binary 
change)")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
Originally submitted on July 14, but no reply. Resent on Sep 6, got
one comment and sent an updated version, which again got no other
replies.

This is the v2 version again.

The warning is currently disabled in mainline, but I think we can turn
it back on in the future, and this change here seems harmless.

v2: use a temporary variable as suggested by Karol Herbst.
---
 drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
index 96e0941c8edd..f0a26881d9b9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
@@ -110,6 +110,7 @@ read_pll(struct gt215_clk *clk, int idx, u32 pll)
struct nvkm_device *device = clk->base.subdev.device;
u32 ctrl = nvkm_rd32(device, pll + 0);
u32 sclk = 0, P = 1, N = 1, M = 1;
+   u32 MP;
 
if (!(ctrl & 0x0008)) {
if (ctrl & 0x0001) {
@@ -130,10 +131,12 @@ read_pll(struct gt215_clk *clk, int idx, u32 pll)
sclk = read_clk(clk, 0x10 + idx, false);
}
 
-   if (M * P)
-   return sclk * N / (M * P);
+   MP = M * P;
 
-   return 0;
+   if (!MP)
+   return 0;
+
+   return sclk * N / MP;
 }
 
 static int
-- 
2.9.0

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

[Nouveau] [PATCH] drm: nouveau: use correct string length

[Arnd Bergmann]

gcc-8 reports

drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c: In function 'nvkm_perfmon_mthd':
include/linux/string.h:265:9: error: '__builtin_strncpy' specified bound 64 
equals destination size [-Werror=stringop-truncation]

We need one less byte or call strlcpy() to make it a
nul-terminated string.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
index dde89a4a0f5b..53859b6254d6 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
@@ -462,7 +462,7 @@ nvkm_perfmon_mthd_query_domain(struct nvkm_perfmon *perfmon,
 
args->v0.id = di;
args->v0.signal_nr  = nvkm_perfdom_count_perfsig(dom);
-   strncpy(args->v0.name, dom->name, sizeof(args->v0.name));
+   strncpy(args->v0.name, dom->name, sizeof(args->v0.name) - 1);
 
/* Currently only global counters (PCOUNTER) are implemented
 * but this will be different for local counters (MP). */
@@ -514,7 +514,7 @@ nvkm_perfmon_mthd_query_signal(struct nvkm_perfmon *perfmon,
 "/%s/%02x", dom->name, si);
} else {
strncpy(args->v0.name, sig->name,
-   sizeof(args->v0.name));
+   sizeof(args->v0.name) - 1);
}
 
args->v0.signal = si;
@@ -572,7 +572,7 @@ nvkm_perfmon_mthd_query_source(struct nvkm_perfmon *perfmon,
 
args->v0.source = sig->source[si];
args->v0.mask   = src->mask;
-   strncpy(args->v0.name, src->name, sizeof(args->v0.name));
+   strncpy(args->v0.name, src->name, sizeof(args->v0.name) - 1);
}
 
if (++si < source_nr) {
-- 
2.9.0

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

[Nouveau] [PATCH] [v2] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning

[Arnd Bergmann]

gcc thinks that interpreting a multiplication result as a bool
is confusing:

drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c: In function 'read_pll':
drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c:133:8: error: '*' in boolean 
context, suggest '&&' instead [-Werror=int-in-bool-context]

Adding a temporary variable to contain the divisor helps make
it clear what is going on and avoids that warning.

Fixes: 7632b30e4b8b ("drm/nouveau/clk: namespace + nvidia gpu names (no binary 
change)")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
Originally submitted on July 14, but no reply. This is the same
patch again. The warning is currently disabled in mainline, but
I think we can turn it back on in the future, and this change here
seems harmless.

v2: use a temporary variable as suggested by Karol Herbst.
---
 drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
index 96e0941c8edd..f0a26881d9b9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
@@ -110,6 +110,7 @@ read_pll(struct gt215_clk *clk, int idx, u32 pll)
struct nvkm_device *device = clk->base.subdev.device;
u32 ctrl = nvkm_rd32(device, pll + 0);
u32 sclk = 0, P = 1, N = 1, M = 1;
+   u32 MP;
 
if (!(ctrl & 0x0008)) {
if (ctrl & 0x0001) {
@@ -130,10 +131,12 @@ read_pll(struct gt215_clk *clk, int idx, u32 pll)
sclk = read_clk(clk, 0x10 + idx, false);
}
 
-   if (M * P)
-   return sclk * N / (M * P);
+   MP = M * P;
 
-   return 0;
+   if (!MP)
+   return 0;
+
+   return sclk * N / MP;
 }
 
 static int
-- 
2.9.0

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

Re: [Nouveau] [PATCH] [RESEND] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning

[Arnd Bergmann]

On Wed, Sep 6, 2017 at 10:15 PM, Karol Herbst <karolher...@gmail.com> wrote:
> On Wed, Sep 6, 2017 at 10:11 PM, Arnd Bergmann <a...@arndb.de> wrote:
>>> but with a better name for "denominator".
>>
>> I don't know what M and P actually are in this function, so I couldn't
>> come up with a much better name either, how about simply 'divisor'?
>>
>
> what about "MP"? M and P are simply dividers for the PLL configuration
> and there are two of them. I think "P" is the post divider.
>

Fine with me. That was actually what I initially did after your reply and
then changed it to divisor. I've changed it back now and sent the new
version.

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

Re: [Nouveau] [PATCH] [RESEND] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning

[Arnd Bergmann]

On Wed, Sep 6, 2017 at 4:20 PM, Karol Herbst  wrote:
>> In this instance, I think using multiplication is more intuitive
>> than '&&', so I'm adding a comparison to zero instead to shut up
>> the warning. To further improve readability, I also make the
>> error case indented and leave the normal case as the final 'return'
>> statement.
>>
>
> I think to make perfectly clear why this check is done, we simply
> should precompute the denominator and do something like this:
>
> int denominator = M * P
> if (denominator == 0)
>return 0;
> return sclk * N / denominator;
>
> but with a better name for "denominator".

I don't know what M and P actually are in this function, so I couldn't
come up with a much better name either, how about simply 'divisor'?

> I even imagine, that there
> are some macros in the kernel for dealing with something like this
> already, so that we could do instead:
>
> return AWESOME_DIV_MACRO(sclk * N, M * P)

I don't think that exists, the behavior for divide-by-zero really
depends a lot on the context, and returning zero is probably
often not a good solution.

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

[Nouveau] [PATCH] [RESEND] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning

[Arnd Bergmann]

gcc thinks that interpreting a multiplication result as a bool
is confusing:

drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c: In function 'read_pll':
drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c:133:8: error: '*' in boolean 
context, suggest '&&' instead [-Werror=int-in-bool-context]

In this instance, I think using multiplication is more intuitive
than '&&', so I'm adding a comparison to zero instead to shut up
the warning. To further improve readability, I also make the
error case indented and leave the normal case as the final 'return'
statement.

Fixes: 7632b30e4b8b ("drm/nouveau/clk: namespace + nvidia gpu names (no binary 
change)")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
Originally submitted on July 14, but no reply. This is the same
patch again. The warning is currently disabled in mainline, but
I think we can turn it back on in the future, and this change here
seems harmless.
---
 drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
index 96e0941c8edd..04b4f4ccf186 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
@@ -130,10 +130,10 @@ read_pll(struct gt215_clk *clk, int idx, u32 pll)
sclk = read_clk(clk, 0x10 + idx, false);
}
 
-   if (M * P)
-   return sclk * N / (M * P);
+   if (M * P == 0)
+   return 0;
 
-   return 0;
+   return sclk * N / (M * P);
 }
 
 static int
-- 
2.9.0

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

[Nouveau] [PATCH 1/3] backlight: always select BACKLIGHT_LCD_SUPPORT for BACKLIGHT_CLASS_DEVICE

[Arnd Bergmann]

randconfig builds occasionally produce this Kconfig warning:

warning: (DRM_RADEON && DRM_AMDGPU && DRM_NOUVEAU && DRM_I915 && DRM_GMA500 && 
DRM_SHMOBILE && DRM_TILCDC && DRM_FSL_DCU && DRM_TINYDRM && DRM_PARADE_PS8622 
&& FB_BACKLIGHT && FB_ARMCLCD && FB_MX3 && USB_APPLEDISPLAY && FB_OLPC_DCON && 
ACPI_CMPC && SAMSUNG_Q10) selects BACKLIGHT_CLASS_DEVICE which has unmet direct 
dependencies (HAS_IOMEM && BACKLIGHT_LCD_SUPPORT)

It turns out that amost all users of BACKLIGHT_CLASS_DEVICE also select
BACKLIGHT_LCD_SUPPORT, but not all of them do. This makes the remaining
ones behave like the others.

It would probably be best to rework the way those two options related
entirely, but for now this takes the simpler and safer approach to
fix the warnings without introducing regressions.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/gpu/drm/gma500/Kconfig| 1 +
 drivers/platform/x86/Kconfig  | 2 ++
 drivers/staging/olpc_dcon/Kconfig | 1 +
 3 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
index df11582f1efc..52d855cb0569 100644
--- a/drivers/gpu/drm/gma500/Kconfig
+++ b/drivers/gpu/drm/gma500/Kconfig
@@ -6,6 +6,7 @@ config DRM_GMA500
# GMA500 depends on ACPI_VIDEO when ACPI is enabled, just like i915
select ACPI_VIDEO if ACPI
select BACKLIGHT_CLASS_DEVICE if ACPI
+   select BACKLIGHT_LCD_SUPPORT if ACPI
select INPUT if ACPI
help
  Say yes for an experimental 2D KMS framebuffer driver for the
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 80b87954f6dd..e0ca673bf564 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -785,6 +785,7 @@ config ACPI_CMPC
depends on RFKILL || RFKILL=n
select INPUT
select BACKLIGHT_CLASS_DEVICE
+   select BACKLIGHT_LCD_SUPPORT
default n
help
  Support for Intel Classmate PC ACPI devices, including some
@@ -1000,6 +1001,7 @@ config SAMSUNG_Q10
tristate "Samsung Q10 Extras"
depends on ACPI
select BACKLIGHT_CLASS_DEVICE
+   select BACKLIGHT_LCD_SUPPORT
---help---
  This driver provides support for backlight control on Samsung Q10
  and related laptops, including Dell Latitude X200.
diff --git a/drivers/staging/olpc_dcon/Kconfig 
b/drivers/staging/olpc_dcon/Kconfig
index d277f048789e..845f756886f0 100644
--- a/drivers/staging/olpc_dcon/Kconfig
+++ b/drivers/staging/olpc_dcon/Kconfig
@@ -4,6 +4,7 @@ config FB_OLPC_DCON
depends on I2C
depends on (GPIO_CS5535 || GPIO_CS5535=n)
select BACKLIGHT_CLASS_DEVICE
+   select BACKLIGHT_LCD_SUPPORT
---help---
  In order to support very low power operation, the XO laptop uses a
  secondary Display CONtroller, or DCON.  This secondary controller
-- 
2.9.0

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

[Nouveau] [PATCH 0/3] Kconfig dependencies: acpi-video, backlight and thermal

[Arnd Bergmann]

Hi everyone,

It took me a while to figure this out properly, as I kept getting
circular or missing dependencies with video drivers.

This set of three patches should simplify the situation a bit,
mostly by cleaning up the dependencies around CONFIG_ACPI_VIDEO.
With all three patches applied, I no longer run into those related
warnings. If everyone agrees on the general direction, I hope
we can merge all three through the DRM tree.

I originally had another larger patch in the series to replace all
of the 'select BACKLIGHT_LCD_SUPPORT; select BACKLIGHT_CLASS_DEVICE'
statements with 'depends on LCD_CLASS_DEVICE', that would clean
it up some more, but it is also a more invasive change that we
can do separately at some point.

   Arnd

Arnd Bergmann (3):
  backlight: always select BACKLIGHT_LCD_SUPPORT for
BACKLIGHT_CLASS_DEVICE
  ACPI/DRM: rework ACPI_VIDEO Kconfig dependencies
  drm/etnaviv: add thermal dependency

 drivers/acpi/Kconfig  |  7 +--
 drivers/gpu/drm/etnaviv/Kconfig   |  1 +
 drivers/gpu/drm/gma500/Kconfig|  5 +
 drivers/gpu/drm/i915/Kconfig  |  7 +--
 drivers/gpu/drm/nouveau/Kconfig   | 10 ++
 drivers/platform/x86/Kconfig  |  9 -
 drivers/staging/olpc_dcon/Kconfig |  1 +
 7 files changed, 15 insertions(+), 25 deletions(-)

To: dri-de...@lists.freedesktop.org
Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>
Cc: Len Brown <l...@kernel.org>
Cc: Lucas Stach <l.st...@pengutronix.de>
Cc: Russell King <linux+etna...@armlinux.org.uk>
Cc: Christian Gmeiner <christian.gmei...@gmail.com>
Cc: David Airlie <airl...@linux.ie>
Cc: Patrik Jakobsson <patrik.r.jakobs...@gmail.com>
Cc: Daniel Vetter <daniel.vet...@intel.com>
Cc: Jani Nikula <jani.nik...@linux.intel.com>
Cc: Ben Skeggs <bske...@redhat.com>
Cc: Darren Hart <dvh...@infradead.org>
Cc: Andy Shevchenko <a...@infradead.org>
Cc: Jens Frederich <jfreder...@gmail.com>
Cc: Daniel Drake <d...@laptop.org>
Cc: Jon Nettleton <jon.nettle...@gmail.com>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: linux-a...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: etna...@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: platform-driver-...@vger.kernel.org
Cc: de...@driverdev.osuosl.org

-- 
2.9.0

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

[Nouveau] [PATCH 3/3] drm/etnaviv: add thermal dependency

[Arnd Bergmann]

When CONFIG_THERMAL is enabled as a loadable module, and etnaviv is
built-in, we get a link error:

drivers/gpu/drm/etnaviv/etnaviv_gpu.o: In function `etnaviv_gpu_bind':
etnaviv_gpu.c:(.text.etnaviv_gpu_bind+0x34): undefined reference to 
`thermal_of_cooling_device_register'
etnaviv_gpu.c:(.text.etnaviv_gpu_bind+0xe0): undefined reference to 
`thermal_cooling_device_unregister'
drivers/gpu/drm/etnaviv/etnaviv_gpu.o: In function `etnaviv_gpu_unbind':
etnaviv_gpu.c:(.text.etnaviv_gpu_unbind+0xf0): undefined reference to 
`thermal_cooling_device_unregister'

This adds a Kconfig dependency to prevent etnaviv from being built-in
with CONFIG_THERMAL=m, while still allowing the valid configurations.
Unfortunately, simply adding the dependency here breaks Kconfig through
a dependency loop involving lots of symbols all the way until ACPI_VIDEO,
which is the only video driver that explicitly selects 'THERMAL'. Turning
this into a 'depends on' addresses the problem.
For completeness, I'm also removing the redundant 'select THERMAL'
from INTEL_MENLOW, so no other driver uses that statement.

Fixes: bcdfb5e56dc5 ("drm/etnaviv: add etnaviv cooling device")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
v2: spend more thought on ACPI_VIDEO dependencies, as we need another
patch before this.
---
 drivers/acpi/Kconfig| 2 +-
 drivers/gpu/drm/etnaviv/Kconfig | 1 +
 drivers/platform/x86/Kconfig| 1 -
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index a8f5a40e2914..1282b2936164 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -184,7 +184,7 @@ config ACPI_VIDEO
tristate "Video"
depends on X86
depends on INPUT
-   select THERMAL
+   depends on THERMAL
select BACKLIGHT_CLASS_DEVICE
select BACKLIGHT_LCD_SUPPORT
default y
diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
index 71cee4e9fefb..1d6c744e7924 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -3,6 +3,7 @@ config DRM_ETNAVIV
tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
depends on DRM
depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST)
+   depends on THERMAL || THERMAL=n
depends on MMU
select SHMEM
select SYNC_FILE
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 2d9fb46a8d11..d9238e9ff54a 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -532,7 +532,6 @@ config SENSORS_HDAPS
 config INTEL_MENLOW
tristate "Thermal Management driver for Intel menlow platform"
depends on ACPI_THERMAL
-   select THERMAL
---help---
  ACPI thermal management enhancement driver on
  Intel Menlow platform.
-- 
2.9.0

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

[Nouveau] [PATCH 12/14] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning

[Arnd Bergmann]

gcc thinks that interpreting a multiplication result as a bool
is confusing:

drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c: In function 'read_pll':
drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c:133:8: error: '*' in boolean 
context, suggest '&&' instead [-Werror=int-in-bool-context]

In this instance, I think using multiplication is more intuitive
than '&&', so I'm adding a comparison to zero instead to shut up
the warning. To further improve readability, I also make the
error case indented and leave the normal case as the final 'return'
statement.

Fixes: 7632b30e4b8b ("drm/nouveau/clk: namespace + nvidia gpu names (no binary 
change)")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
index 96e0941c8edd..04b4f4ccf186 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
@@ -130,10 +130,10 @@ read_pll(struct gt215_clk *clk, int idx, u32 pll)
sclk = read_clk(clk, 0x10 + idx, false);
}
 
-   if (M * P)
-   return sclk * N / (M * P);
+   if (M * P == 0)
+   return 0;
 
-   return 0;
+   return sclk * N / (M * P);
 }
 
 static int
-- 
2.9.0

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

[Nouveau] [PATCH] drm/nouveau: use proper prototype in nouveau_pmops_runtime() definition

[Arnd Bergmann]

There is a prototype for this function in the header, but the function
itself lacks a 'void' in the argument list, causing a harmless warning
when building with 'make W=1':

drivers/gpu/drm/nouveau/nouveau_drm.c: In function 'nouveau_pmops_runtime':
drivers/gpu/drm/nouveau/nouveau_drm.c:730:1: error: old-style function 
definition [-Werror=old-style-definition]

Fixes: 321f5c5f2c49 ("drm/nouveau: replace multiple open-coded runpm support 
checks with function")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 7acd1cf8c5a6..90757af9bc73 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -727,7 +727,7 @@ nouveau_pmops_thaw(struct device *dev)
 }
 
 bool
-nouveau_pmops_runtime()
+nouveau_pmops_runtime(void)
 {
if (nouveau_runtime_pm == -1)
return nouveau_is_optimus() || nouveau_is_v1_dsm();
-- 
2.9.0

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

Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration

[Arnd Bergmann]

On Tuesday, November 8, 2016 5:07:01 PM CET Lukas Wunner wrote:
> On Tue, Nov 08, 2016 at 04:52:49PM +0100, Arnd Bergmann wrote:
> > The underlying problem is that we already have a number of other
> > symbols that either have "depends on LEDS_CLASS" or
> > "select LEDS_CLASS". To clean that up properly, we should either
> > make the symbol itself hidden and only select it from other drivers,
> > or use "depends on LEDS_CLASS" everywhere.
> > 
> > Another option is to use the IS_REACHABLE() macro instead of IS_ENABLED()
> > in the header file, to stub out the calls into the new file, but
> > that can be a bit confusing.
> 
> Why don't you just add empty inline stubs for nouveau_led_init / _fini /
> _suspend / _resume?
> 

That's what I was suggesting:

diff --git a/drivers/gpu/drm/nouveau/nouveau_led.h 
b/drivers/gpu/drm/nouveau/nouveau_led.h
index 9c9bb6ac938e..bc5f47cb516b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_led.h
+++ b/drivers/gpu/drm/nouveau/nouveau_led.h
@@ -35,21 +35,21 @@ struct nouveau_led {
struct led_classdev led;
 };
 
 static inline struct nouveau_led *
 nouveau_led(struct drm_device *dev)
 {
return nouveau_drm(dev)->led;
 }
 
 /* nouveau_led.c */
-#if IS_ENABLED(CONFIG_LEDS_CLASS)
+#if IS_REACHABLE(CONFIG_LEDS_CLASS)
 int  nouveau_led_init(struct drm_device *dev);
 void nouveau_led_suspend(struct drm_device *dev);
 void nouveau_led_resume(struct drm_device *dev);
 void nouveau_led_fini(struct drm_device *dev);
 #else
 static inline int  nouveau_led_init(struct drm_device *dev) { return 0; };
 static inline void nouveau_led_suspend(struct drm_device *dev) { };
 static inline void nouveau_led_resume(struct drm_device *dev) { };
 static inline void nouveau_led_fini(struct drm_device *dev) { };
 #endif

The downside is that now the nouveau_led_init() just won't be called
if CONFIG_LEDS_CLASS=m and CONFIG_DRM_NOUVEAU=y, which can be
surprising to users.

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

Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration

[Arnd Bergmann]

On Tuesday, November 8, 2016 10:46:07 AM CET Ilia Mirkin wrote:
> > diff --git a/drivers/gpu/drm/nouveau/Kconfig 
> > b/drivers/gpu/drm/nouveau/Kconfig
> > index 78631fb61adf..715cd6f4dc31 100644
> > --- a/drivers/gpu/drm/nouveau/Kconfig
> > +++ b/drivers/gpu/drm/nouveau/Kconfig
> > @@ -46,6 +46,14 @@ config NOUVEAU_DEBUG
> >   The paranoia and spam levels will add a lot of extra checks which
> >   may potentially slow down driver operation.
> >
> > +config DRM_NOUVEAU_LED
> > +   bool "Support for logo LED"
> > +   depends on DRM_NOUVEAU && LEDS_CLASS
> > +   depends on !(DRM_NOUVEAU=y && LEDS_CLASS=m)
> > +   help
> > + Say Y here to enabling controlling the brightness of the
> > + LED behind NVIDIA logo on certain Titan cards.
> 
> This is a very odd restriction... could this be written as
> 
> depends on DRM_NOUVEAU
> select LEDS_CLASS
> 
> Or will that not flip the LEDS_CLASS from m to y in case
> DRM_NOUVEAU=y? If not, is there a way to cause that to happen?

Using 'select' on user-selectable symbols risks introducing
circular dependencies.

> Separately, perhaps we should just drop this LEDS_CLASS select into
> DRM_NOUVEAU? We've tended to avoid adding tons of options.

My first attempt was to add "depends on LEDS_CLASS || !LEDS_CLASS"
to DRM_NOUVEAU, which led to one long circle:

drivers/usb/Kconfig:39:error: recursive dependency detected!
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/usb/Kconfig:39: symbol USB is selected by MOUSE_APPLETOUCH
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/mouse/Kconfig:187:symbol MOUSE_APPLETOUCH depends on INPUT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/Kconfig:8:symbol INPUT is selected by VT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/tty/Kconfig:12: symbol VT is selected by FB_STI
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/video/fbdev/Kconfig:669:symbol FB_STI depends on FB
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/video/fbdev/Kconfig:5:  symbol FB is selected by DRM_KMS_FB_HELPER
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/Kconfig:42: symbol DRM_KMS_FB_HELPER depends on 
DRM_KMS_HELPER
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/Kconfig:36: symbol DRM_KMS_HELPER is selected by DRM_NOUVEAU
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/nouveau/Kconfig:1:  symbol DRM_NOUVEAU depends on LEDS_CLASS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/leds/Kconfig:16:symbol LEDS_CLASS is selected by OMAP_DEBUG_LEDS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
arch/arm/plat-omap/Kconfig:19:  symbol OMAP_DEBUG_LEDS depends on NEW_LEDS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by ATH9K_HTC
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/net/wireless/ath/ath9k/Kconfig:158: symbol ATH9K_HTC depends on USB
Tue, 08 Nov 2016 11:49:44 +0100 build/0x3053A542_defconfig warnings
drivers/usb/Kconfig:39:error: recursive dependency detected!
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/usb/Kconfig:39: symbol USB is selected by MOUSE_APPLETOUCH
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/mouse/Kconfig:187:symbol MOUSE_APPLETOUCH depends on INPUT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/Kconfig:8:symbol INPUT is selected by VT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/tty/Kconfig:12: symbol VT is selected by FB_STI
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"

[Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration

[Arnd Bergmann]

The newly introduced LED handling for nouveau fails to link when the
driver is built-in but the LED subsystem is a loadable module:

drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_suspend':
tvnv17.c:(.text.nouveau_do_suspend+0x10): undefined reference to 
`nouveau_led_suspend'
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_resume':
tvnv17.c:(.text.nouveau_do_resume+0xf0): undefined reference to 
`nouveau_led_resume'
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_unload':
tvnv17.c:(.text.nouveau_drm_unload+0x34): undefined reference to 
`nouveau_led_fini'
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_load':
tvnv17.c:(.text.nouveau_drm_load+0x7d0): undefined reference to 
`nouveau_led_init'

This adds a separate Kconfig symbol for the LED support that
correctly tracks the dependencies.

Fixes: 8d021d71b324 ("drm/nouveau/drm/nouveau: add a LED driver for the NVIDIA 
logo")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/gpu/drm/nouveau/Kbuild| 2 +-
 drivers/gpu/drm/nouveau/Kconfig   | 8 
 drivers/gpu/drm/nouveau/nouveau_led.h | 2 +-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild
index fde6e3656636..5e00e911daa6 100644
--- a/drivers/gpu/drm/nouveau/Kbuild
+++ b/drivers/gpu/drm/nouveau/Kbuild
@@ -22,7 +22,7 @@ nouveau-$(CONFIG_DEBUG_FS) += nouveau_debugfs.o
 nouveau-y += nouveau_drm.o
 nouveau-y += nouveau_hwmon.o
 nouveau-$(CONFIG_COMPAT) += nouveau_ioc32.o
-nouveau-$(CONFIG_LEDS_CLASS) += nouveau_led.o
+nouveau-$(CONFIG_DRM_NOUVEAU_LED) += nouveau_led.o
 nouveau-y += nouveau_nvif.o
 nouveau-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o
 nouveau-y += nouveau_usif.o # userspace <-> nvif
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 78631fb61adf..715cd6f4dc31 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -46,6 +46,14 @@ config NOUVEAU_DEBUG
  The paranoia and spam levels will add a lot of extra checks which
  may potentially slow down driver operation.
 
+config DRM_NOUVEAU_LED
+   bool "Support for logo LED"
+   depends on DRM_NOUVEAU && LEDS_CLASS
+   depends on !(DRM_NOUVEAU=y && LEDS_CLASS=m)
+   help
+ Say Y here to enabling controlling the brightness of the
+ LED behind NVIDIA logo on certain Titan cards.
+
 config NOUVEAU_DEBUG_DEFAULT
int "Default debug level"
depends on DRM_NOUVEAU
diff --git a/drivers/gpu/drm/nouveau/nouveau_led.h 
b/drivers/gpu/drm/nouveau/nouveau_led.h
index 187ecdb82002..9c9bb6ac938e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_led.h
+++ b/drivers/gpu/drm/nouveau/nouveau_led.h
@@ -42,7 +42,7 @@ nouveau_led(struct drm_device *dev)
 }
 
 /* nouveau_led.c */
-#if IS_ENABLED(CONFIG_LEDS_CLASS)
+#if IS_ENABLED(CONFIG_DRM_NOUVEAU_LED)
 int  nouveau_led_init(struct drm_device *dev);
 void nouveau_led_suspend(struct drm_device *dev);
 void nouveau_led_resume(struct drm_device *dev);
-- 
2.9.0

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

[Nouveau] [PATCH] nouveau: hide gcc-4.9 -Wmaybe-uninitialized

[Arnd Bergmann]

gcc-4.9 notices that the validate_init() function returns unintialized
data when called with a zero 'nr_buffers' argument, when called with the
-Wmaybe-uninitialized flag:

drivers/gpu/drm/nouveau/nouveau_gem.c: In function ‘validate_init.isra.6’:
drivers/gpu/drm/nouveau/nouveau_gem.c:457:5: error: ‘ret’ may be used 
uninitialized in this function [-Werror=maybe-uninitialized]

However, the only caller of this function always passes a nonzero
argument, and gcc-6 is clever enough to take this into account and
not warn about it any more.

Adding an explicit initialization to -EINVAL here is correct even if
the caller changed, and it avoids the warning on gcc-4.9 as well.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 0bd7164bc817..4f3c5f3eaa16 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -369,7 +369,7 @@ validate_init(struct nouveau_channel *chan, struct drm_file 
*file_priv,
 {
struct nouveau_cli *cli = nouveau_cli(file_priv);
int trycnt = 0;
-   int ret, i;
+   int ret = -EINVAL, i;
struct nouveau_bo *res_bo = NULL;
LIST_HEAD(gart_list);
LIST_HEAD(vram_list);
-- 
2.9.0

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

Re: [Nouveau] [PATCH] fix:overlay: add missing header dependencies

[Arnd Bergmann]

On Sunday, August 28, 2016 1:02:52 PM CEST Baoyou Xie wrote:
> We get 1 warning when build kernel with W=1:
> drivers/gpu/drm/nouveau/dispnv04/overlay.c:496:1: warning: no previous 
> prototype for 'nouveau_overlay_init' [-Wmissing-prototypes]
> 
> In fact, this function is declared in disp.h, so this patch
> add missing header dependencies
> 
> Signed-off-by: Baoyou Xie <baoyou@linaro.org>
> 

Acked-by: Arnd Bergmann <a...@arndb.de>

a few general notes:

- please use my a...@arndb.de address on patch submissions, not the linaro
  address.

- for the email subject lines, have a look at what the typical style
  is for the file. "overlay:" is not a good name here because it's not
  unique to the driver, so better use "drm/nouveau", which will also
  get the attention of the right people.

- please drop the "fix:" prefix in the subject.

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

Re: [Nouveau] [PATCH] drm/nouveau: silence warnings reported during builds with W=1

[Arnd Bergmann]

On Tuesday 30 August 2016, Baoyou Xie wrote:
> We get some warnings when building kernel with W=1:
> drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf117.c:222:1: warning: no previous 
> prototype for 'gf117_grctx_generate_main' [-Wmissing-prototypes]
> drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxnv50.c:255:1: warning: no previous 
> prototype for 'nv50_grctx_fill' [-Wmissing-prototypes]
> drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxnv50.c:265:1: warning: no previous 
> prototype for 'nv50_grctx_init' [-Wmissing-prototypes]
> 
> 
> In fact, some functions are only used in the file in which they
> are declared and don't need a declaration, but can be made static.
> and others are declared in the header files, but need to add
> missing header dependencies.
> 
> Signed-off-by: Baoyou Xie <baoyou@linaro.org>

The patch looks very good, and this is better than one patch per file. I would 
personally
do this as two patches, as you are touching a number of files and are doing two 
separate
things here. As a rule of thumb, when the changelog text mentions two or more 
things
that the patch does, the patch should be split up so that each part does one 
thing.
After you do that, it also becomes easier to write a good subject line like 
"mark
function as static" and "add missing includes", which tells you more about the 
patch
than "silence warnings".

It's quite likely that the nouveau maintainers are less picky than I am though, 
so
if they want to apply the patch as-is

Acked-by: Arnd Bergmann <a...@arndb.de>

Otherwise please split and resend in a few days, in case there are other 
comments
that need to be addressed.

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

Re: [Nouveau] [PATCH v2] drm/nouveau/gr: mark gm107_grctx_generate_tpcid() static

[Arnd Bergmann]

On Tuesday 30 August 2016, Baoyou Xie wrote:
> On 30 August 2016 at 00:01, Sean Paul  wrote:
> 
> > On Mon, Aug 29, 2016 at 9:02 AM, Baoyou Xie  wrote:
> > > We get 1 warning when build kernel with W=1:
> > > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgm107.c:937:1: warning: no
> > previous prototype for 'gm107_grctx_generate_tpcid' [-Wmissing-prototypes]
> > >
> > > In fact, this function is only used in the file in which it is
> > > declared and don't need a declaration, but can be made static.
> > > so this patch marks this function with 'static'.
> > >
> >
> > It would make things a lot easier if you either consolidated all of
> > these static changes into one patch, or released them as a series.
> >
> >
> Do you mean that I need to consolidate all of them into one patch this
> time? or next time?

I'd suggest consolidating the 'nouveau' changes into a single patch,
as this is one (very big) driver and resend that one.

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

[Nouveau] [PATCH 4/5] drm: exynos: mark pm functions as __maybe_unused

[Arnd Bergmann]

The rework of the exynos DRM clock handling introduced
warnings for configurations that have CONFIG_PM disabled:

drivers/gpu/drm/exynos/exynos_hdmi.c:736:13: error: 'hdmi_clk_disable_gates' 
defined but not used [-Werror=unused-function]
 static void hdmi_clk_disable_gates(struct hdmi_context *hdata)
 ^~
drivers/gpu/drm/exynos/exynos_hdmi.c:717:12: error: 'hdmi_clk_enable_gates' 
defined but not used [-Werror=unused-function]
 static int hdmi_clk_enable_gates(struct hdmi_context *hdata)

The problem is that the PM functions themselves are inside of
an #ifdef, but some functions they call are not.

This patch removes the #ifdef and instead marks the PM functions
as __maybe_unused, which is a more reliable way to get it right.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: 9be7e9898444 ("drm/exynos/hdmi: clock code re-factoring")
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 58de5a430508..ea4b2b7d7ad7 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1934,8 +1934,7 @@ static int hdmi_remove(struct platform_device *pdev)
return 0;
 }
 
-#ifdef CONFIG_PM
-static int exynos_hdmi_suspend(struct device *dev)
+static int __maybe_unused exynos_hdmi_suspend(struct device *dev)
 {
struct hdmi_context *hdata = dev_get_drvdata(dev);
 
@@ -1944,7 +1943,7 @@ static int exynos_hdmi_suspend(struct device *dev)
return 0;
 }
 
-static int exynos_hdmi_resume(struct device *dev)
+static int __maybe_unused exynos_hdmi_resume(struct device *dev)
 {
struct hdmi_context *hdata = dev_get_drvdata(dev);
int ret;
@@ -1955,7 +1954,6 @@ static int exynos_hdmi_resume(struct device *dev)
 
return 0;
 }
-#endif
 
 static const struct dev_pm_ops exynos_hdmi_pm_ops = {
SET_RUNTIME_PM_OPS(exynos_hdmi_suspend, exynos_hdmi_resume, NULL)
-- 
2.7.0

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

[Nouveau] [PATCH 2/5] drm: mediatek: add CONFIG_OF dependency

[Arnd Bergmann]

The mediatek DRM driver can be configured for compile testing with
CONFIG_OF disabled, but then fails to link:

drivers/gpu/built-in.o: In function `mtk_drm_bind':
analogix_dp_reg.c:(.text+0x52888): undefined reference to 
`of_find_device_by_node'
analogix_dp_reg.c:(.text+0x52930): undefined reference to 
`of_find_device_by_node'

This adds an explicit Kconfig dependency.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/gpu/drm/mediatek/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index 545973f6b743..67183e26971d 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -3,6 +3,7 @@ config DRM_MEDIATEK
depends on DRM
depends on ARCH_MEDIATEK || (ARM && COMPILE_TEST)
depends on COMMON_CLK
+   depends on OF
select DRM_GEM_CMA_HELPER
select DRM_KMS_HELPER
select DRM_MIPI_DSI
-- 
2.7.0

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

[Nouveau] [PATCH 5/5] drm: remove unused dev variables

[Arnd Bergmann]

After drm_gem_object_lookup() was changed along with all its callers,
we have several drivers that have unused variables:

drm/armada/armada_crtc.c: In function 'armada_drm_crtc_cursor_set':
drm/armada/armada_crtc.c:900:21: error: unused variable 'dev' 
[-Werror=unused-variable]
drm/nouveau/nouveau_gem.c: In function 'validate_init':
drm/nouveau/nouveau_gem.c:371:21: error: unused variable 'dev' 
[-Werror=unused-variable]
drm/nouveau/nv50_display.c: In function 'nv50_crtc_cursor_set':
drm/nouveau/nv50_display.c:1308:21: error: unused variable 'dev' 
[-Werror=unused-variable]
drm/radeon/radeon_cs.c: In function 'radeon_cs_parser_relocs':
drm/radeon/radeon_cs.c:77:21: error: unused variable 'ddev' 
[-Werror=unused-variable]

This fixes all the instances I found with ARM randconfig builds so far.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: a8ad0bd84f98 ("drm: Remove unused drm_device from 
drm_gem_object_lookup()")
---
 drivers/gpu/drm/armada/armada_crtc.c   | 1 -
 drivers/gpu/drm/nouveau/nouveau_gem.c  | 1 -
 drivers/gpu/drm/nouveau/nv50_display.c | 1 -
 drivers/gpu/drm/radeon/radeon_cs.c | 1 -
 4 files changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_crtc.c 
b/drivers/gpu/drm/armada/armada_crtc.c
index a9b7e0f36513..3130aa8bcdd0 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -897,7 +897,6 @@ static void cursor_update(void *data)
 static int armada_drm_crtc_cursor_set(struct drm_crtc *crtc,
struct drm_file *file, uint32_t handle, uint32_t w, uint32_t h)
 {
-   struct drm_device *dev = crtc->dev;
struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc);
struct armada_gem_object *obj = NULL;
int ret;
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 445a9e2fb91a..a030e218adf7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -368,7 +368,6 @@ validate_init(struct nouveau_channel *chan, struct drm_file 
*file_priv,
  int nr_buffers, struct validate_op *op)
 {
struct nouveau_cli *cli = nouveau_cli(file_priv);
-   struct drm_device *dev = chan->drm->dev;
int trycnt = 0;
int ret, i;
struct nouveau_bo *res_bo = NULL;
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c 
b/drivers/gpu/drm/nouveau/nv50_display.c
index 47761a92926e..ec2e67eb8980 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1305,7 +1305,6 @@ nv50_crtc_cursor_set(struct drm_crtc *crtc, struct 
drm_file *file_priv,
 uint32_t handle, uint32_t width, uint32_t height)
 {
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-   struct drm_device *dev = crtc->dev;
struct drm_gem_object *gem = NULL;
struct nouveau_bo *nvbo = NULL;
int ret = 0;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
b/drivers/gpu/drm/radeon/radeon_cs.c
index 271652963fa1..510ea371dacc 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -74,7 +74,6 @@ static void radeon_cs_buckets_get_list(struct 
radeon_cs_buckets *b,
 
 static int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
 {
-   struct drm_device *ddev = p->rdev->ddev;
struct radeon_cs_chunk *chunk;
struct radeon_cs_buckets buckets;
unsigned i;
-- 
2.7.0

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

[Nouveau] [PATCH 0/5] drm: fixes for merge-window regressions

[Arnd Bergmann]

A couple of new warnings and build errors appeared through the DRM
tree in linux-next after the merge window opened. The first patch
here is for scripts/headers_check.pl, but as the bug is only
present in drm-next at the moment, it would be good to add all
five patches to that tree before they make it into mainline.

It's likely that some of these have already been fixed since
this morning's linux-next, so just ignore the ones are no
longer needed.

Arnd Bergmann (5):
  headers_check: don't warn about c++ guards
  drm: mediatek: add CONFIG_OF dependency
  drm: mediatek: fixup drm_gem_object_lookup API change
  drm: exynos: mark pm functions as __maybe_unused
  drm: remove unused dev variables

 drivers/gpu/drm/armada/armada_crtc.c   | 1 -
 drivers/gpu/drm/exynos/exynos_hdmi.c   | 6 ++
 drivers/gpu/drm/mediatek/Kconfig   | 1 +
 drivers/gpu/drm/mediatek/mtk_drm_fb.c  | 2 +-
 drivers/gpu/drm/mediatek/mtk_drm_gem.c | 2 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c  | 1 -
 drivers/gpu/drm/nouveau/nv50_display.c | 1 -
 drivers/gpu/drm/radeon/radeon_cs.c | 1 -
 scripts/headers_check.pl   | 4 
 9 files changed, 9 insertions(+), 10 deletions(-)


Cc: Russell King <rmk+ker...@armlinux.org.uk>
Cc: Inki Dae <inki@samsung.com>
Cc: Joonyoung Shim <jy0922.s...@samsung.com>
Cc: Seung-Woo Kim <sw0312@samsung.com>
Cc: Kyungmin Park <kyungmin.p...@samsung.com>
Cc: Kukjin Kim <kg...@kernel.org>
Cc: Krzysztof Kozlowski <k.kozlow...@samsung.com>
Cc: Matthias Brugger <matthias@gmail.com>
Cc: Ben Skeggs <bske...@redhat.com>
Cc: Alex Deucher <alexander.deuc...@amd.com>
Cc: "Christian König" <christian.koe...@amd.com>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: dri-de...@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-media...@lists.infradead.org
Cc: nouveau@lists.freedesktop.org

-- 
2.7.0

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

[Nouveau] [PATCH 1/5] headers_check: don't warn about c++ guards

[Arnd Bergmann]

A recent addition to the DRM tree for 4.7 added 'extern "C"' guards
for c++ to all the DRM headers, and that now causes warnings
in 'make headers_check':

usr/include/drm/amdgpu_drm.h:38: userspace cannot reference function or 
variable defined in the kernel
usr/include/drm/drm.h:63: userspace cannot reference function or variable 
defined in the kernel
usr/include/drm/drm.h:699: userspace cannot reference function or variable 
defined in the kernel
usr/include/drm/drm_fourcc.h:30: userspace cannot reference function or 
variable defined in the kernel
usr/include/drm/drm_mode.h:33: userspace cannot reference function or variable 
defined in the kernel
usr/include/drm/drm_sarea.h:38: userspace cannot reference function or variable 
defined in the kernel
usr/include/drm/exynos_drm.h:21: userspace cannot reference function or 
variable defined in the kernel
usr/include/drm/i810_drm.h:7: userspace cannot reference function or variable 
defined in the kernel

This changes the headers_check.pl script to not warn about this.
I'm listing the merge commit as introducing the problem, because
there are several patches in this branch that each do this for
one file.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: 7c10ddf87472 ("Merge branch 'drm-uapi-extern-c-fixes' of 
https://github.com/evelikov/linux into drm-next")
---
 scripts/headers_check.pl | 4 
 1 file changed, 4 insertions(+)

diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl
index 62320f93e903..8b2da054cdc3 100755
--- a/scripts/headers_check.pl
+++ b/scripts/headers_check.pl
@@ -69,6 +69,10 @@ sub check_declarations
if ($line =~ m/^void seqbuf_dump\(void\);/) {
return;
}
+   # drm headers are being C++ friendly
+   if ($line =~ m/^extern "C"/) {
+   return;
+   }
if ($line =~ m/^(\s*extern|unsigned|char|short|int|long|void)\b/) {
printf STDERR "$filename:$lineno: " .
  "userspace cannot reference function or " .
-- 
2.7.0

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

[Nouveau] [PATCH 0/5] drm: fixes for merge-window regressions

[Arnd Bergmann]

A couple of new warnings and build errors appeared through the DRM
tree in linux-next after the merge window opened. The first patch
here is for scripts/headers_check.pl, but as the bug is only
present in drm-next at the moment, it would be good to add all
five patches to that tree before they make it into mainline.

It's likely that some of these have already been fixed since
this morning's linux-next, so just ignore the ones are no
longer needed.

[resending because of missing Cc list for the actual patches,
 sorry about that]

Arnd Bergmann (5):
  headers_check: don't warn about c++ guards
  drm: mediatek: add CONFIG_OF dependency
  drm: mediatek: fixup drm_gem_object_lookup API change
  drm: exynos: mark pm functions as __maybe_unused
  drm: remove unused dev variables

 drivers/gpu/drm/armada/armada_crtc.c   | 1 -
 drivers/gpu/drm/exynos/exynos_hdmi.c   | 6 ++
 drivers/gpu/drm/mediatek/Kconfig   | 1 +
 drivers/gpu/drm/mediatek/mtk_drm_fb.c  | 2 +-
 drivers/gpu/drm/mediatek/mtk_drm_gem.c | 2 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c  | 1 -
 drivers/gpu/drm/nouveau/nv50_display.c | 1 -
 drivers/gpu/drm/radeon/radeon_cs.c | 1 -
 scripts/headers_check.pl   | 4 
 9 files changed, 9 insertions(+), 10 deletions(-)


Cc: Russell King <rmk+ker...@armlinux.org.uk>
Cc: Inki Dae <inki@samsung.com>
Cc: Joonyoung Shim <jy0922.s...@samsung.com>
Cc: Seung-Woo Kim <sw0312@samsung.com>
Cc: Kyungmin Park <kyungmin.p...@samsung.com>
Cc: Kukjin Kim <kg...@kernel.org>
Cc: Krzysztof Kozlowski <k.kozlow...@samsung.com>
Cc: Matthias Brugger <matthias@gmail.com>
Cc: Ben Skeggs <bske...@redhat.com>
Cc: Alex Deucher <alexander.deuc...@amd.com>
Cc: "Christian König" <christian.koe...@amd.com>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: dri-de...@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-media...@lists.infradead.org
Cc: nouveau@lists.freedesktop.org

-- 
2.7.0

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

[Nouveau] [PATCH 3/5] drm: mediatek: fixup drm_gem_object_lookup API change

[Arnd Bergmann]

The drm_gem_object_lookup() function prototype changed while this
driver was added, so it fails to build now:

drivers/gpu/drm/mediatek/mtk_drm_gem.c: In function 
'mtk_drm_gem_dumb_map_offset':
drivers/gpu/drm/mediatek/mtk_drm_gem.c:142:30: error: passing argument 1 of 
'drm_gem_object_lookup' from incompatible pointer type 
[-Werror=incompatible-pointer-types]
  obj = drm_gem_object_lookup(dev, file_priv, handle);

This fixes the new caller as well.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: a8ad0bd84f98 ("drm: Remove unused drm_device from 
drm_gem_object_lookup()")
---
 drivers/gpu/drm/mediatek/mtk_drm_fb.c  | 2 +-
 drivers/gpu/drm/mediatek/mtk_drm_gem.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c 
b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
index 33d30c19f35f..147df85399ab 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
@@ -138,7 +138,7 @@ struct drm_framebuffer *mtk_drm_mode_fb_create(struct 
drm_device *dev,
if (drm_format_num_planes(cmd->pixel_format) != 1)
return ERR_PTR(-EINVAL);
 
-   gem = drm_gem_object_lookup(dev, file, cmd->handles[0]);
+   gem = drm_gem_object_lookup(file, cmd->handles[0]);
if (!gem)
return ERR_PTR(-ENOENT);
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c 
b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
index a773bfaea913..fa2ec0cd00e8 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
@@ -139,7 +139,7 @@ int mtk_drm_gem_dumb_map_offset(struct drm_file *file_priv,
struct drm_gem_object *obj;
int ret;
 
-   obj = drm_gem_object_lookup(dev, file_priv, handle);
+   obj = drm_gem_object_lookup(file_priv, handle);
if (!obj) {
DRM_ERROR("failed to lookup gem object.\n");
return -EINVAL;
-- 
2.7.0

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