[Nouveau] [PATCH] drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()
While we currently grab a runtime PM ref in nouveau's normal connector
detection code, we apparently don't do this for MST. This means if we're
in a scenario where the GPU is suspended and userspace attempts to do a
connector probe on an MSTC connector, the probe will fail entirely due
to the DP aux channel and GPU not being woken up:
[ 316.633489] nouveau :01:00.0: i2c: aux 000a: begin idle timeout
[ 316.635713] nouveau :01:00.0: i2c: aux 000a: begin idle timeout
[ 316.637785] nouveau :01:00.0: i2c: aux 000a: begin idle timeout
...
So, grab a runtime PM ref here.
Signed-off-by: Lyude Paul
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index c94b7f42d62d..9da0bdfe1e1c 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -938,9 +938,22 @@ static enum drm_connector_status
nv50_mstc_detect(struct drm_connector *connector, bool force)
{
struct nv50_mstc *mstc = nv50_mstc(connector);
+ enum drm_connector_status conn_status;
+ int ret;
+
if (!mstc->port)
return connector_status_disconnected;
- return drm_dp_mst_detect_port(connector, mstc->port->mgr, mstc->port);
+
+ ret = pm_runtime_get_sync(connector->dev->dev);
+ if (ret < 0 && ret != -EACCES)
+ return connector_status_disconnected;
+
+ conn_status = drm_dp_mst_detect_port(connector, mstc->port->mgr,
+mstc->port);
+
+ pm_runtime_mark_last_busy(connector->dev->dev);
+ pm_runtime_put_autosuspend(connector->dev->dev);
+ return conn_status;
}
static void
--
2.17.1
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] drm: nouveau: use match_string() helper to simplify the code
match_string() returns the index of an array for a matching string,
which can be used intead of open coded implementation.
Signed-off-by: zhong jiang
---
drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
index 6a4ca13..053d794 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
@@ -641,17 +641,13 @@ static int nv17_tv_create_resources(struct drm_encoder
*encoder,
struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
int num_tv_norms = dcb->tvconf.has_component_output ? NUM_TV_NORMS :
NUM_LD_TV_NORMS;
- int i;
+ int index;
if (nouveau_tv_norm) {
- for (i = 0; i < num_tv_norms; i++) {
- if (!strcmp(nv17_tv_norm_names[i], nouveau_tv_norm)) {
- tv_enc->tv_norm = i;
- break;
- }
- }
-
- if (i == num_tv_norms)
+ index = match_string(nv17_tv_norm_names, num_tv_norms,
nouveau_tv_norm);
+ if (index >= 0)
+ tv_enc->tv_norm = index;
+ else
NV_WARN(drm, "Invalid TV norm setting \"%s\"\n",
nouveau_tv_norm);
}
--
1.7.12.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 1/2] drm/nouveau: Disable atomic support on a per-device basis
On Thu, Sep 13, 2018 at 05:02:05PM -0400, Lyude Paul wrote: > Hm, one nitpick here. Since /sys/kernel/debug/dri/*/state creation depends on > the driver supporting atomic, maybe it would be good to make it so that we set > DRIVER_ATOMIC in the driver_stub structure, then disable it per-device > depending > on the nouveau_atomic setting + hw support. That way we can always have the > state debugfs file while maintaining nouveau's current behavior with exposing > atomic ioctls. Assuming that wouldn't have any unintended side-effects of > course I'm not sure why there are three driver structs in nouveau. Maybe someone can just nuke two of them? > > On Thu, 2018-09-13 at 19:31 +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > We now have per-device driver_features, so let's use that > > to disable atomic only for pre-nv50. > > > > Cc: Ben Skeggs > > Cc: Lyude Paul > > Cc: nouveau@lists.freedesktop.org > > Cc: Daniel Vetter > > Reviewed-by: Daniel Vetter > > Suggested-by: Daniel Vetter > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c > > b/drivers/gpu/drm/nouveau/dispnv04/disp.c > > index 70dce544984e..670535a68d3b 100644 > > --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c > > +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c > > @@ -56,7 +56,7 @@ nv04_display_create(struct drm_device *dev) > > nouveau_display(dev)->fini = nv04_display_fini; > > > > /* Pre-nv50 doesn't support atomic, so don't expose the ioctls */ > > - dev->driver->driver_features &= ~DRIVER_ATOMIC; > > + dev->driver_features &= ~DRIVER_ATOMIC; > > > > nouveau_hw_save_vga_fonts(dev, 1); > > -- Ville Syrjälä Intel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] drm/nouveau: Don't disable polling in fallback mode
On 14/09/2018 10:28, Ben Skeggs wrote:
> On Wed, 12 Sep 2018 at 20:59, Takashi Iwai wrote:
>>
>> When a fan is controlled via linear fallback without cstate, we
>> shouldn't stop polling. Otherwise it won't be adjusted again and
>> keeps running at an initial crazy pace.
> Martin,
>
> Any thoughts on this?
>
> Ben.
Wow, blast from the past!
Anyway, the analysis is pretty spot on here. When using the cstate-based
fan speed (change the speed of the fan based on what frequency is used),
then polling is unnecessary and this function should only be called when
changing the pstate.
However, in the absence of ANY information, we fallback to a
temperature-based management which requires constant polling, so the
patch is accurate and poll = false should only be set if we have a cstate.
So, the patch is Reviewed-by: Martin Peres
>
>>
>> Fixes: 800efb4c2857 ("drm/nouveau/drm/therm/fan: add a fallback if no fan
>> control is specified in the vbios")
>> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1103356
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107447
I see that Thomas has been having issues with the noise level anyway. I
suggest he should bump the value of temp1_auto_point1_temp (see
https://www.kernel.org/doc/Documentation/thermal/nouveau_thermal).
The default value is set to 90°C which is quite safe on these old GPUs
(NVIDIA G71 / nv49). I would say that it is safe to go up to 110°C.
Which should reduce the noise level.
Another technique may be to reduce the minimum fan speed to something
lower than 30°C. It should increase the slope but reduce the noise level
at a given temperature.
One reason why these GPUs run so hot on nouveau is the lack of power and
clock gating. I am sorry that I never finished to reverse engineer these...
Anyway, thanks a lot for the patch!
>> Reported-by: Thomas Blume
>> Signed-off-by: Takashi Iwai
>>
>> ---
>> drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c | 7 ---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
>> b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
>> index 3695cde669f8..07914e36939e 100644
>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
>> @@ -132,11 +132,12 @@ nvkm_therm_update(struct nvkm_therm *therm, int mode)
>> duty = nvkm_therm_update_linear(therm);
>> break;
>> case NVBIOS_THERM_FAN_OTHER:
>> - if (therm->cstate)
>> + if (therm->cstate) {
>> duty = therm->cstate;
>> - else
>> + poll = false;
>> + } else {
>> duty =
>> nvkm_therm_update_linear_fallback(therm);
>> - poll = false;
>> + }
>> break;
>> }
>> immd = false;
>> --
>> 2.18.0
>>
>> ___
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] drm/nouveau/secboot/acr: fix memory leak
On Fri, 14 Sep 2018 at 07:35, Kees Cook wrote:
>
> On Fri, Sep 7, 2018 at 8:02 PM, John Hubbard wrote:
> > On 8/2/18 12:51 PM, Gustavo A. R. Silva wrote:
> >> Hi all,
> >>
> >> Friendly ping! Who can take this?
> >>
> >> Thanks
> >> --
> >> Gustavo
> >>
> >> On 07/24/2018 08:27 AM, Gustavo A. R. Silva wrote:
> >>> In case memory resources for *bl_desc* were allocated, release
> >>> them before return.
> >>>
> >>> Addresses-Coverity-ID: 1472021 ("Resource leak")
> >>> Fixes: 0d466901552a ("drm/nouveau/secboot/acr: Remove VLA usage")
> >>> Signed-off-by: Gustavo A. R. Silva
> >>> ---
> >>> drivers/gpu/drm/nouveau/nvkm/subdev/secboot/acr_r352.c | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/acr_r352.c
> >>> b/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/acr_r352.c
> >>> index d02e183..5c14d6a 100644
> >>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/acr_r352.c
> >>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/acr_r352.c
> >>> @@ -801,6 +801,7 @@ acr_r352_load(struct nvkm_acr *_acr, struct
> >>> nvkm_falcon *falcon,
> >>> bl = acr->hsbl_unload_blob;
> >>> } else {
> >>> nvkm_error(_acr->subdev, "invalid secure boot blob!\n");
> >>> +kfree(bl_desc);
> >>> return -EINVAL;
> >>> }
> >>>
> >>>
> >
> > Hi Gustavo,
> >
> > Seeing as how I've been working on this corner of Nouveau lately (don't
> > ask, haha),
> > I reviewed and also tested this. It looks good, you can add:
> >
> > Reviewed-by: John Hubbard
>
> Ben can you take this?
>
> Reviewed-by: Kees Cook
I've got it in my tree. Thanks!
>
> -Kees
>
> --
> Kees Cook
> Pixel Security
> ___
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 1/2] drm/nouveau: Disable atomic support on a per-device basis
Hi Ville,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc3 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-nouveau-Disable-atomic-support-on-a-per-device-basis/20180914-111059
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/gpu/drm/nouveau/dispnv04/disp.c: In function 'nv04_display_create':
>> drivers/gpu/drm/nouveau/dispnv04/disp.c:59:5: error: 'struct drm_device' has
>> no member named 'driver_features'
dev->driver_features &= ~DRIVER_ATOMIC;
^~
vim +59 drivers/gpu/drm/nouveau/dispnv04/disp.c
33
34 int
35 nv04_display_create(struct drm_device *dev)
36 {
37 struct nouveau_drm *drm = nouveau_drm(dev);
38 struct nvkm_i2c *i2c = nvxx_i2c(>client.device);
39 struct dcb_table *dcb = >vbios.dcb;
40 struct drm_connector *connector, *ct;
41 struct drm_encoder *encoder;
42 struct nouveau_encoder *nv_encoder;
43 struct nouveau_crtc *crtc;
44 struct nv04_display *disp;
45 int i, ret;
46
47 disp = kzalloc(sizeof(*disp), GFP_KERNEL);
48 if (!disp)
49 return -ENOMEM;
50
51 nvif_object_map(>client.device.object, NULL, 0);
52
53 nouveau_display(dev)->priv = disp;
54 nouveau_display(dev)->dtor = nv04_display_destroy;
55 nouveau_display(dev)->init = nv04_display_init;
56 nouveau_display(dev)->fini = nv04_display_fini;
57
58 /* Pre-nv50 doesn't support atomic, so don't expose the ioctls
*/
> 59 dev->driver_features &= ~DRIVER_ATOMIC;
60
61 nouveau_hw_save_vga_fonts(dev, 1);
62
63 nv04_crtc_create(dev, 0);
64 if (nv_two_heads(dev))
65 nv04_crtc_create(dev, 1);
66
67 for (i = 0; i < dcb->entries; i++) {
68 struct dcb_output *dcbent = >entry[i];
69
70 connector = nouveau_connector_create(dev,
dcbent->connector);
71 if (IS_ERR(connector))
72 continue;
73
74 switch (dcbent->type) {
75 case DCB_OUTPUT_ANALOG:
76 ret = nv04_dac_create(connector, dcbent);
77 break;
78 case DCB_OUTPUT_LVDS:
79 case DCB_OUTPUT_TMDS:
80 ret = nv04_dfp_create(connector, dcbent);
81 break;
82 case DCB_OUTPUT_TV:
83 if (dcbent->location == DCB_LOC_ON_CHIP)
84 ret = nv17_tv_create(connector, dcbent);
85 else
86 ret = nv04_tv_create(connector, dcbent);
87 break;
88 default:
89 NV_WARN(drm, "DCB type %d not known\n",
dcbent->type);
90 continue;
91 }
92
93 if (ret)
94 continue;
95 }
96
97 list_for_each_entry_safe(connector, ct,
98 >mode_config.connector_list,
head) {
99 if (!connector->encoder_ids[0]) {
100 NV_WARN(drm, "%s has no encoders, removing\n",
101 connector->name);
102 connector->funcs->destroy(connector);
103 }
104 }
105
106 list_for_each_entry(encoder, >mode_config.encoder_list,
head) {
107 struct nouveau_encoder *nv_encoder =
nouveau_encoder(encoder);
108 struct nvkm_i2c_bus *bus =
109 nvkm_i2c_bus_find(i2c,
nv_encoder->dcb->i2c_index);
110 nv_encoder->i2c = bus ? >i2c : NULL;
111 }
112
113 /* Save previous state */
114 list_for_each_entry(crtc, >mode_config.crtc_list,
base.head)
115 crtc->save(>base);
116
117 list_for_each_entry(nv_encoder, >mode_config.encoder_list,
base.base.head)
118 nv_encoder->enc_save(_encoder->base.base);
119
120 nouveau_ove
Re: [Nouveau] [PATCH 1/2] drm/nouveau: Disable atomic support on a per-device basis
On Thu, Sep 13, 2018 at 11:02 PM, Lyude Paul wrote: > Hm, one nitpick here. Since /sys/kernel/debug/dri/*/state creation depends on > the driver supporting atomic, maybe it would be good to make it so that we set > DRIVER_ATOMIC in the driver_stub structure, then disable it per-device > depending > on the nouveau_atomic setting + hw support. That way we can always have the > state debugfs file while maintaining nouveau's current behavior with exposing > atomic ioctls. Assuming that wouldn't have any unintended side-effects of > course dri/*/state only works with atomic drivers. There's no explicit state with legacy drivers at all, it's all just implicit in hw and some random driver structures. We should make sure though that the debugfs stuff looks at drm_drv_uses_atomic_modsetting(), and not DRIVER_ATOMIC. Former is about the internals (i915 is internally atomic everywhere), latter about the uapi (some old platforms aren't properly validated for full atomic features, hence why it's disabled). -Daniel > On Thu, 2018-09-13 at 19:31 +0300, Ville Syrjala wrote: >> From: Ville Syrjälä >> >> We now have per-device driver_features, so let's use that >> to disable atomic only for pre-nv50. >> >> Cc: Ben Skeggs >> Cc: Lyude Paul >> Cc: nouveau@lists.freedesktop.org >> Cc: Daniel Vetter >> Reviewed-by: Daniel Vetter >> Suggested-by: Daniel Vetter >> Signed-off-by: Ville Syrjälä >> --- >> drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c >> b/drivers/gpu/drm/nouveau/dispnv04/disp.c >> index 70dce544984e..670535a68d3b 100644 >> --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c >> +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c >> @@ -56,7 +56,7 @@ nv04_display_create(struct drm_device *dev) >> nouveau_display(dev)->fini = nv04_display_fini; >> >> /* Pre-nv50 doesn't support atomic, so don't expose the ioctls */ >> - dev->driver->driver_features &= ~DRIVER_ATOMIC; >> + dev->driver_features &= ~DRIVER_ATOMIC; >> >> nouveau_hw_save_vga_fonts(dev, 1); >> > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] drm/nouveau: Don't disable polling in fallback mode
On Wed, 12 Sep 2018 at 20:59, Takashi Iwai wrote:
>
> When a fan is controlled via linear fallback without cstate, we
> shouldn't stop polling. Otherwise it won't be adjusted again and
> keeps running at an initial crazy pace.
Martin,
Any thoughts on this?
Ben.
>
> Fixes: 800efb4c2857 ("drm/nouveau/drm/therm/fan: add a fallback if no fan
> control is specified in the vbios")
> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1103356
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107447
> Reported-by: Thomas Blume
> Signed-off-by: Takashi Iwai
>
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
> index 3695cde669f8..07914e36939e 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
> @@ -132,11 +132,12 @@ nvkm_therm_update(struct nvkm_therm *therm, int mode)
> duty = nvkm_therm_update_linear(therm);
> break;
> case NVBIOS_THERM_FAN_OTHER:
> - if (therm->cstate)
> + if (therm->cstate) {
> duty = therm->cstate;
> - else
> + poll = false;
> + } else {
> duty =
> nvkm_therm_update_linear_fallback(therm);
> - poll = false;
> + }
> break;
> }
> immd = false;
> --
> 2.18.0
>
> ___
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
