| Message ID | 20180716235936.11268-5-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd the Linux glue code to the i2c stuff instead, but this is all done from inside of nvkm. I think we should move it out into drm/nouveau/nouveau_i2c.c and do the handling there. On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul <lyude@redhat.com> wrote: > The i2c bus can be both accessed by DRM itself, along with any of it's > devnodes (/sys/class/i2c). So, we need to make sure that all codepaths > using the i2c bus keep the GPU resumed. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: Karol Herbst <karolherbst@gmail.com> > Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > index 807a2b67bd64..1de48c990b80 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus) > BUS_TRACE(bus, "release"); > nvkm_i2c_pad_release(pad); > mutex_unlock(&bus->mutex); > + pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev); > } > > int > nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus) > { > struct nvkm_i2c_pad *pad = bus->pad; > + struct device *dev = pad->i2c->subdev.device->dev; > int ret; > + > BUS_TRACE(bus, "acquire"); > + > + ret = pm_runtime_get_sync(dev); > + if (ret < 0 && ret != -EACCES) > + return ret; > + > mutex_lock(&bus->mutex); > ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C); > - if (ret) > + if (ret) { > mutex_unlock(&bus->mutex); > + pm_runtime_put_autosuspend(dev); > + } > return ret; > } > > -- > 2.17.1 > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
On Tue, 17 Jul 2018 at 20:18, Karol Herbst <kherbst@redhat.com> wrote: > > mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd > the Linux glue code to the i2c stuff instead, but this is all done > from inside of nvkm. I think we should move it out into > drm/nouveau/nouveau_i2c.c and do the handling there. Huh? No, this is completely fine. > > On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul <lyude@redhat.com> wrote: > > The i2c bus can be both accessed by DRM itself, along with any of it's > > devnodes (/sys/class/i2c). So, we need to make sure that all codepaths > > using the i2c bus keep the GPU resumed. > > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > Cc: Karol Herbst <karolherbst@gmail.com> > > Cc: stable@vger.kernel.org > > --- > > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > > index 807a2b67bd64..1de48c990b80 100644 > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c > > @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus) > > BUS_TRACE(bus, "release"); > > nvkm_i2c_pad_release(pad); > > mutex_unlock(&bus->mutex); > > + pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev); > > } > > > > int > > nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus) > > { > > struct nvkm_i2c_pad *pad = bus->pad; > > + struct device *dev = pad->i2c->subdev.device->dev; > > int ret; > > + > > BUS_TRACE(bus, "acquire"); > > + > > + ret = pm_runtime_get_sync(dev); > > + if (ret < 0 && ret != -EACCES) > > + return ret; > > + > > mutex_lock(&bus->mutex); > > ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C); > > - if (ret) > > + if (ret) { > > mutex_unlock(&bus->mutex); > > + pm_runtime_put_autosuspend(dev); > > + } > > return ret; > > } > > > > -- > > 2.17.1 > > > > _______________________________________________ > > 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
On Tue, Jul 17, 2018 at 1:54 PM, Ben Skeggs <skeggsb@gmail.com> wrote: > On Tue, 17 Jul 2018 at 20:18, Karol Herbst <kherbst@redhat.com> wrote: >> >> mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd >> the Linux glue code to the i2c stuff instead, but this is all done >> from inside of nvkm. I think we should move it out into >> drm/nouveau/nouveau_i2c.c and do the handling there. > Huh? No, this is completely fine. > okay, then the the two patches adding that guard code is reviewed-by me >> >> On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul <lyude@redhat.com> wrote: >> > The i2c bus can be both accessed by DRM itself, along with any of it's >> > devnodes (/sys/class/i2c). So, we need to make sure that all codepaths >> > using the i2c bus keep the GPU resumed. >> > >> > Signed-off-by: Lyude Paul <lyude@redhat.com> >> > Cc: Karol Herbst <karolherbst@gmail.com> >> > Cc: stable@vger.kernel.org >> > --- >> > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++++++++++- >> > 1 file changed, 11 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c >> > index 807a2b67bd64..1de48c990b80 100644 >> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c >> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c >> > @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus) >> > BUS_TRACE(bus, "release"); >> > nvkm_i2c_pad_release(pad); >> > mutex_unlock(&bus->mutex); >> > + pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev); >> > } >> > >> > int >> > nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus) >> > { >> > struct nvkm_i2c_pad *pad = bus->pad; >> > + struct device *dev = pad->i2c->subdev.device->dev; >> > int ret; >> > + >> > BUS_TRACE(bus, "acquire"); >> > + >> > + ret = pm_runtime_get_sync(dev); >> > + if (ret < 0 && ret != -EACCES) >> > + return ret; >> > + >> > mutex_lock(&bus->mutex); >> > ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C); >> > - if (ret) >> > + if (ret) { >> > mutex_unlock(&bus->mutex); >> > + pm_runtime_put_autosuspend(dev); >> > + } >> > return ret; >> > } >> > >> > -- >> > 2.17.1 >> > >> > _______________________________________________ >> > 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
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c index 807a2b67bd64..1de48c990b80 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus) BUS_TRACE(bus, "release"); nvkm_i2c_pad_release(pad); mutex_unlock(&bus->mutex); + pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev); } int nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus) { struct nvkm_i2c_pad *pad = bus->pad; + struct device *dev = pad->i2c->subdev.device->dev; int ret; + BUS_TRACE(bus, "acquire"); + + ret = pm_runtime_get_sync(dev); + if (ret < 0 && ret != -EACCES) + return ret; + mutex_lock(&bus->mutex); ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C); - if (ret) + if (ret) { mutex_unlock(&bus->mutex); + pm_runtime_put_autosuspend(dev); + } return ret; }
The i2c bus can be both accessed by DRM itself, along with any of it's devnodes (/sys/class/i2c). So, we need to make sure that all codepaths using the i2c bus keep the GPU resumed. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Karol Herbst <karolherbst@gmail.com> Cc: stable@vger.kernel.org --- drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)