Hey
Op 21-02-2019 om 12:14 schreef james qian wang (Arm Technology China):
> The writeback job will not be added to writeback queue if the state is
> check only or check failed, to avoid leak, need to cleanup writeback job
> in connector_destroy_state if the job existed.
>
> Signed-off-by: James Qian Wang (Arm Technology China)
> <james.qian.w...@arm.com>
Is writeback_job in the conn_state set to null somewhere? I'm worried we might
end up freeing writeback_job twice on success.
~Maarten
> drivers/gpu/drm/drm_atomic_state_helper.c | 4 ++++
> drivers/gpu/drm/drm_writeback.c | 21 ++++++++++++++++++---
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 4985384e51f6..6a8e414233de 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -28,6 +28,7 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_plane.h>
> #include <drm/drm_connector.h>
> +#include <drm/drm_writeback.h>
> #include <drm/drm_atomic.h>
> #include <drm/drm_device.h>
>
> @@ -407,6 +408,9 @@
> EXPORT_SYMBOL(drm_atomic_helper_connector_duplicate_state);
> void
> __drm_atomic_helper_connector_destroy_state(struct drm_connector_state
> *state)
> {
> + if (state->writeback_job)
> + drm_writeback_cleanup_job(state->writeback_job);
> +
> if (state->crtc)
> drm_connector_put(state->connector);
>
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index c20e6fe00cb3..486121150eaa 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -268,6 +268,22 @@ void drm_writeback_queue_job(struct
> drm_writeback_connector *wb_connector,
> }
> EXPORT_SYMBOL(drm_writeback_queue_job);
>
> +/**
> + * drm_writeback_cleanup_job - cleanup a writeback job
> + * @job: The job to cleanup
> + */
> +void drm_writeback_cleanup_job(struct drm_writeback_job *job)
> +{
> + if (job->fb)
> + drm_framebuffer_put(job->fb);
> +
> + if (job->out_fence)
> + dma_fence_put(job->out_fence);
> +
> + kfree(job);
> +}
> +EXPORT_SYMBOL(drm_writeback_cleanup_job);
> +
> /*
> * @cleanup_work: deferred cleanup of a writeback job
> *
> @@ -280,11 +296,9 @@ static void cleanup_work(struct work_struct *work)
> struct drm_writeback_job *job = container_of(work,
> struct drm_writeback_job,
> cleanup_work);
> - drm_framebuffer_put(job->fb);
> - kfree(job);
> + drm_writeback_cleanup_job(job);
> }
>
> -
> /**
> * drm_writeback_signal_completion - Signal the completion of a writeback job
> * @wb_connector: The writeback connector whose job is complete
> @@ -319,6 +333,7 @@ drm_writeback_signal_completion(struct
> drm_writeback_connector *wb_connector,
> dma_fence_set_error(job->out_fence, status);
> dma_fence_signal(job->out_fence);
> dma_fence_put(job->out_fence);
> + job->out_fence = NULL;
> }
> }
> spin_unlock_irqrestore(&wb_connector->job_lock, flags);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
