| Message ID | 20190705171727.27501-1-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/8] drm/etnaviv: simplify unbind checks | expand |
On Fri, 2019-07-05 at 19:17 +0200, Lucas Stach wrote: > Remember if the GPU has been sucessfully initialized. Only in that case > do we need to clean up various structures in the unbind path. If the > GPU hasn't been sucessfully initialized all the cleanups should happen > in the failure paths of the init function. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 20 +++++++------------- > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 1 + > 2 files changed, 8 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index 4822549500ee..e84a0ed904aa 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -799,17 +799,16 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) > pm_runtime_mark_last_busy(gpu->dev); > pm_runtime_put_autosuspend(gpu->dev); > > + gpu->initialized = true; > + > return 0; > > free_buffer: > etnaviv_cmdbuf_free(&gpu->buffer); > - gpu->buffer.suballoc = NULL; > destroy_suballoc: > etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc); > - gpu->cmdbuf_suballoc = NULL; > destroy_iommu: > etnaviv_iommu_destroy(gpu->mmu); > - gpu->mmu = NULL; > fail: > pm_runtime_mark_last_busy(gpu->dev); > pm_runtime_put_autosuspend(gpu->dev); > @@ -1521,7 +1520,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms) > > static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu) > { > - if (gpu->buffer.suballoc) { > + if (gpu->initialized) { > /* Replace the last WAIT with END */ > mutex_lock(&gpu->lock); > etnaviv_buffer_end(gpu); > @@ -1680,19 +1679,14 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master, > etnaviv_gpu_hw_suspend(gpu); > #endif > > - if (gpu->buffer.suballoc) > + if (gpu->initialized) { > etnaviv_cmdbuf_free(&gpu->buffer); > - > - if (gpu->cmdbuf_suballoc) { > etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc); > - gpu->cmdbuf_suballoc = NULL; > - } > - > - if (gpu->mmu) { > etnaviv_iommu_destroy(gpu->mmu); > - gpu->mmu = NULL; > + gpu->initialized = false; > } > > + Superfluous whitespace, apart from that Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp
Hi, On Fri, Jul 05, 2019 at 07:17:20PM +0200, Lucas Stach wrote: > Remember if the GPU has been sucessfully initialized. Only in that case > do we need to clean up various structures in the unbind path. If the > GPU hasn't been sucessfully initialized all the cleanups should happen > in the failure paths of the init function. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 20 +++++++------------- > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 1 + > 2 files changed, 8 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index 4822549500ee..e84a0ed904aa 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -799,17 +799,16 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) > pm_runtime_mark_last_busy(gpu->dev); > pm_runtime_put_autosuspend(gpu->dev); > > + gpu->initialized = true; > + > return 0; > > free_buffer: > etnaviv_cmdbuf_free(&gpu->buffer); > - gpu->buffer.suballoc = NULL; > destroy_suballoc: > etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc); > - gpu->cmdbuf_suballoc = NULL; > destroy_iommu: > etnaviv_iommu_destroy(gpu->mmu); > - gpu->mmu = NULL; > fail: > pm_runtime_mark_last_busy(gpu->dev); > pm_runtime_put_autosuspend(gpu->dev); > @@ -1521,7 +1520,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms) > > static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu) > { > - if (gpu->buffer.suballoc) { > + if (gpu->initialized) { > /* Replace the last WAIT with END */ > mutex_lock(&gpu->lock); > etnaviv_buffer_end(gpu); > @@ -1680,19 +1679,14 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master, > etnaviv_gpu_hw_suspend(gpu); > #endif > > - if (gpu->buffer.suballoc) > + if (gpu->initialized) { > etnaviv_cmdbuf_free(&gpu->buffer); > - > - if (gpu->cmdbuf_suballoc) { > etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc); > - gpu->cmdbuf_suballoc = NULL; > - } > - > - if (gpu->mmu) { > etnaviv_iommu_destroy(gpu->mmu); > - gpu->mmu = NULL; > + gpu->initialized = false; > } > > + Maybe drop this line, otherwise: Reviewed-by: Guido Günther <agx@sigxcpu.org> > gpu->drm = NULL; > idr_destroy(&gpu->fence_idr); > > @@ -1827,7 +1821,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev) > return ret; > > /* Re-initialise the basic hardware state */ > - if (gpu->drm && gpu->buffer.suballoc) { > + if (gpu->drm && gpu->initialized) { > ret = etnaviv_gpu_hw_resume(gpu); > if (ret) { > etnaviv_gpu_clk_disable(gpu); > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h > index 9bcf151f706b..b06c7c98d522 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h > @@ -99,6 +99,7 @@ struct etnaviv_gpu { > enum etnaviv_sec_mode sec_mode; > struct workqueue_struct *wq; > struct drm_gpu_scheduler sched; > + bool initialized; > > /* 'ring'-buffer: */ > struct etnaviv_cmdbuf buffer; > -- > 2.20.1 > > _______________________________________________ > etnaviv mailing list > etnaviv@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/etnaviv
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 4822549500ee..e84a0ed904aa 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -799,17 +799,16 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) pm_runtime_mark_last_busy(gpu->dev); pm_runtime_put_autosuspend(gpu->dev); + gpu->initialized = true; + return 0; free_buffer: etnaviv_cmdbuf_free(&gpu->buffer); - gpu->buffer.suballoc = NULL; destroy_suballoc: etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc); - gpu->cmdbuf_suballoc = NULL; destroy_iommu: etnaviv_iommu_destroy(gpu->mmu); - gpu->mmu = NULL; fail: pm_runtime_mark_last_busy(gpu->dev); pm_runtime_put_autosuspend(gpu->dev); @@ -1521,7 +1520,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms) static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu) { - if (gpu->buffer.suballoc) { + if (gpu->initialized) { /* Replace the last WAIT with END */ mutex_lock(&gpu->lock); etnaviv_buffer_end(gpu); @@ -1680,19 +1679,14 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master, etnaviv_gpu_hw_suspend(gpu); #endif - if (gpu->buffer.suballoc) + if (gpu->initialized) { etnaviv_cmdbuf_free(&gpu->buffer); - - if (gpu->cmdbuf_suballoc) { etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc); - gpu->cmdbuf_suballoc = NULL; - } - - if (gpu->mmu) { etnaviv_iommu_destroy(gpu->mmu); - gpu->mmu = NULL; + gpu->initialized = false; } + gpu->drm = NULL; idr_destroy(&gpu->fence_idr); @@ -1827,7 +1821,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev) return ret; /* Re-initialise the basic hardware state */ - if (gpu->drm && gpu->buffer.suballoc) { + if (gpu->drm && gpu->initialized) { ret = etnaviv_gpu_hw_resume(gpu); if (ret) { etnaviv_gpu_clk_disable(gpu); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h index 9bcf151f706b..b06c7c98d522 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h @@ -99,6 +99,7 @@ struct etnaviv_gpu { enum etnaviv_sec_mode sec_mode; struct workqueue_struct *wq; struct drm_gpu_scheduler sched; + bool initialized; /* 'ring'-buffer: */ struct etnaviv_cmdbuf buffer;
Remember if the GPU has been sucessfully initialized. Only in that case do we need to clean up various structures in the unbind path. If the GPU hasn't been sucessfully initialized all the cleanups should happen in the failure paths of the init function. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 20 +++++++------------- drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 1 + 2 files changed, 8 insertions(+), 13 deletions(-)