search for: nvkm_i2c_aux_acquir

On Tue, Jul 17, 2018 at 02:24:31PM -0400, Lyude Paul wrote: > On Tue, 2018-07-17 at 20:20 +0200, Lukas Wunner wrote: > > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire() > > wants it in resumed state, so is waiting forever for the device to > > runtime suspend in order to resume it again immediately afterwards. > > > > The deadlock in the stack trace you've posted could be resolved using > > the technique I used in d61a5c106351...

...e reason that patch was added was mainly for the patches later in the > series that add guards around the i2c bus and aux bus, since both of those > require that the device be awake for it to work. Currently, the spot where it > would recurse is: Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire() wants it in resumed state, so is waiting forever for the device to runtime suspend in order to resume it again immediately afterwards. The deadlock in the stack trace you've posted could be resolved using the technique I used in d61a5c106351 by adding the following to include/linux/pm_runti...

On Tue, 2018-07-17 at 20:32 +0200, Lukas Wunner wrote: > On Tue, Jul 17, 2018 at 02:24:31PM -0400, Lyude Paul wrote: > > On Tue, 2018-07-17 at 20:20 +0200, Lukas Wunner wrote: > > > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire() > > > wants it in resumed state, so is waiting forever for the device to > > > runtime suspend in order to resume it again immediately afterwards. > > > > > > The deadlock in the stack trace you've posted could be resolved using > > > the techniq...

...ainly for the patches later in the > > series that add guards around the i2c bus and aux bus, since both of those > > require that the device be awake for it to work. Currently, the spot where > > it > > would recurse is: > > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire() > wants it in resumed state, so is waiting forever for the device to > runtime suspend in order to resume it again immediately afterwards. > > The deadlock in the stack trace you've posted could be resolved using > the technique I used in d61a5c106351 by adding the following...

...as added was mainly for the patches later in the >> series that add guards around the i2c bus and aux bus, since both of those >> require that the device be awake for it to work. Currently, the spot where it >> would recurse is: > > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire() > wants it in resumed state, so is waiting forever for the device to > runtime suspend in order to resume it again immediately afterwards. > > The deadlock in the stack trace you've posted could be resolved using > the technique I used in d61a5c106351 by adding the following t...

On Wed, Jul 18, 2018 at 09:38:41AM +0200, Rafael J. Wysocki wrote: > On Tue, Jul 17, 2018 at 8:20 PM, Lukas Wunner <lukas at wunner.de> wrote: > > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire() > > wants it in resumed state, so is waiting forever for the device to > > runtime suspend in order to resume it again immediately afterwards. > > > > The deadlock in the stack trace you've posted could be resolved using > > the technique I used in d61a5c106351...

[cc += linux-pm] Hi Lyude, First of all, thanks a lot for looking into this. On Mon, Jul 16, 2018 at 07:59:25PM -0400, Lyude Paul wrote: > In order to fix all of the spots that need to have runtime PM get/puts() > added, we need to ensure that it's possible for us to call > pm_runtime_get/put() in any context, regardless of how deep, since > almost all of the spots that are

...enabled; u32 intr; }; diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c index 4c1f547da463..b4e7404fe660 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c @@ -105,9 +105,15 @@ nvkm_i2c_aux_acquire(struct nvkm_i2c_aux *aux) { struct nvkm_i2c_pad *pad = aux->pad; int ret; + AUX_TRACE(aux, "acquire"); mutex_lock(&aux->mutex); - ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_AUX); + + if (aux->enabled) + ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_AUX); + else +...

...2 0x80000000 [ 246.719636] Workqueue: pm pm_runtime_work [ 246.719772] Call Trace: [ 246.719874] __schedule+0x322/0xaf0 [ 246.722800] schedule+0x33/0x90 [ 246.724269] rpm_resume+0x19c/0x850 [ 246.725128] ? finish_wait+0x90/0x90 [ 246.725990] __pm_runtime_resume+0x4e/0x90 [ 246.726876] nvkm_i2c_aux_acquire+0x39/0xc0 [nouveau] [ 246.727713] nouveau_connector_aux_xfer+0x5c/0xd0 [nouveau] [ 246.728546] drm_dp_dpcd_access+0x77/0x110 [drm_kms_helper] [ 246.729349] drm_dp_dpcd_write+0x2b/0xb0 [drm_kms_helper] [ 246.730085] drm_dp_mst_topology_mgr_suspend+0x4e/0x90 [drm_kms_helper] [ 246.730828]...

...pu/drm/nouveau/nvkm/subdev/i2c/aux.c > > b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c > > index 4c1f547da463..b4e7404fe660 100644 > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c > > @@ -105,9 +105,15 @@ nvkm_i2c_aux_acquire(struct nvkm_i2c_aux *aux) > > { > > struct nvkm_i2c_pad *pad = aux->pad; > > int ret; > > + > > AUX_TRACE(aux, "acquire"); > > mutex_lock(&aux->mutex); > > - ret = nvkm_i2c_pad_acquire(pad, NVKM_I2...

...}; > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c > index 4c1f547da463..b4e7404fe660 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c > @@ -105,9 +105,15 @@ nvkm_i2c_aux_acquire(struct nvkm_i2c_aux *aux) > { > struct nvkm_i2c_pad *pad = aux->pad; > int ret; > + > AUX_TRACE(aux, "acquire"); > mutex_lock(&aux->mutex); > - ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_AUX); > + > + if...

This fixes quite a number of runtime PM bugs I found that have been causing some pretty nasty issues such as: - Deadlocking on boot - Connector probing potentially not working while the GPU is in runtime suspend - i2c char dev not working while the GPU is in runtime suspend - aux char dev not working while the GPU is in runtime suspend There's definitely more parts of nouveau that need