All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
@ 2019-02-20 23:24 Daniel Vetter
  2019-02-21  8:19 ` Laurent Pinchart
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2019-02-20 23:24 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Daniel Vetter, Liviu Dudau

No implementation, no callers.

Cc: Brian Starkey <brian.starkey@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 include/drm/drm_writeback.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index 23df9d463003..f34895f7fcb1 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
 void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector,
 			     struct drm_writeback_job *job);
 
-void drm_writeback_cleanup_job(struct drm_writeback_job *job);
-
 void
 drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector,
 				int status);
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
  2019-02-20 23:24 [PATCH] drm/writeback: Delete drm_writeback_cleanup_job Daniel Vetter
@ 2019-02-21  8:19 ` Laurent Pinchart
  2019-02-21  9:30   ` Daniel Vetter
  2019-02-21  9:34   ` Brian Starkey
  0 siblings, 2 replies; 6+ messages in thread
From: Laurent Pinchart @ 2019-02-21  8:19 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Liviu Dudau, DRI Development

Hi Daniel,

Thank you for the patch.

On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote:
> No implementation, no callers.

The issue here isn't that the function is declared, but that it's not
defined. Jobs are leaked when atomic commit fails (or when using test
commits). I'm working on a fix, please don't apply this patch in the
meantime.

> Cc: Brian Starkey <brian.starkey@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  include/drm/drm_writeback.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index 23df9d463003..f34895f7fcb1 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
>  void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector,
>  			     struct drm_writeback_job *job);
>  
> -void drm_writeback_cleanup_job(struct drm_writeback_job *job);
> -
>  void
>  drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector,
>  				int status);

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
  2019-02-21  8:19 ` Laurent Pinchart
@ 2019-02-21  9:30   ` Daniel Vetter
  2019-02-21 10:36     ` Laurent Pinchart
  2019-02-21  9:34   ` Brian Starkey
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2019-02-21  9:30 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Daniel Vetter, Liviu Dudau, DRI Development

On Thu, Feb 21, 2019 at 9:19 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Daniel,
>
> Thank you for the patch.
>
> On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote:
> > No implementation, no callers.
>
> The issue here isn't that the function is declared, but that it's not
> defined. Jobs are leaked when atomic commit fails (or when using test
> commits). I'm working on a fix, please don't apply this patch in the
> meantime.

Hm, can't I merge this one and give you a clean slate? Function in
header but nowhere else kinda upsets my ocd :-)
-Daniel

> > Cc: Brian Starkey <brian.starkey@arm.com>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Eric Anholt <eric@anholt.net>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  include/drm/drm_writeback.h | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> > index 23df9d463003..f34895f7fcb1 100644
> > --- a/include/drm/drm_writeback.h
> > +++ b/include/drm/drm_writeback.h
> > @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
> >  void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector,
> >                            struct drm_writeback_job *job);
> >
> > -void drm_writeback_cleanup_job(struct drm_writeback_job *job);
> > -
> >  void
> >  drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector,
> >                               int status);
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
  2019-02-21  8:19 ` Laurent Pinchart
  2019-02-21  9:30   ` Daniel Vetter
@ 2019-02-21  9:34   ` Brian Starkey
  2019-02-21 10:36     ` Laurent Pinchart
  1 sibling, 1 reply; 6+ messages in thread
From: Brian Starkey @ 2019-02-21  9:34 UTC (permalink / raw)
  To: Laurent Pinchart, james qian wang (Arm Technology China)
  Cc: Daniel Vetter, nd, Liviu Dudau, DRI Development, Daniel Vetter

On Thu, Feb 21, 2019 at 10:19:35AM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote:
> > No implementation, no callers.
> 
> The issue here isn't that the function is declared, but that it's not
> defined. Jobs are leaked when atomic commit fails (or when using test
> commits). I'm working on a fix, please don't apply this patch in the
> meantime.

Yeah, looking at the series somehow the call to cleanup the writeback
job on failure looks like it got lost between v9 and v10. I saw a
patch internally, but looks like James didn't send it to the list yet.

@James, could you send out your patch which fixes the cleanup on
failure?

Thanks,
-Brian

> 
> > Cc: Brian Starkey <brian.starkey@arm.com>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Eric Anholt <eric@anholt.net>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  include/drm/drm_writeback.h | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> > index 23df9d463003..f34895f7fcb1 100644
> > --- a/include/drm/drm_writeback.h
> > +++ b/include/drm/drm_writeback.h
> > @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
> >  void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector,
> >  			     struct drm_writeback_job *job);
> >  
> > -void drm_writeback_cleanup_job(struct drm_writeback_job *job);
> > -
> >  void
> >  drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector,
> >  				int status);
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
  2019-02-21  9:34   ` Brian Starkey
@ 2019-02-21 10:36     ` Laurent Pinchart
  0 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2019-02-21 10:36 UTC (permalink / raw)
  To: Brian Starkey
  Cc: Daniel Vetter, Liviu Dudau, DRI Development,
	james qian wang (Arm Technology China),
	Daniel Vetter, nd

Hi Brian,

On Thu, Feb 21, 2019 at 09:34:10AM +0000, Brian Starkey wrote:
> On Thu, Feb 21, 2019 at 10:19:35AM +0200, Laurent Pinchart wrote:
> > Hi Daniel,
> > 
> > Thank you for the patch.
> > 
> > On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote:
> >> No implementation, no callers.
> > 
> > The issue here isn't that the function is declared, but that it's not
> > defined. Jobs are leaked when atomic commit fails (or when using test
> > commits). I'm working on a fix, please don't apply this patch in the
> > meantime.
> 
> Yeah, looking at the series somehow the call to cleanup the writeback
> job on failure looks like it got lost between v9 and v10. I saw a
> patch internally, but looks like James didn't send it to the list yet.
> 
> @James, could you send out your patch which fixes the cleanup on
> failure?

I've just posted

[PATCH v5 12/19] drm: writeback: Cleanup job ownership handling when queuing job
[PATCH v5 13/19] drm: writeback: Fix leak of writeback job

that should address this issue. The series also includes the RFC-like

[PATCH v5 14/19] drm: writeback: Add job prepare and cleanup operations

that addresses a separate issue with writeback support.

> >> Cc: Brian Starkey <brian.starkey@arm.com>
> >> Cc: Liviu Dudau <liviu.dudau@arm.com>
> >> Cc: Eric Anholt <eric@anholt.net>
> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >> ---
> >>  include/drm/drm_writeback.h | 2 --
> >>  1 file changed, 2 deletions(-)
> >> 
> >> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> >> index 23df9d463003..f34895f7fcb1 100644
> >> --- a/include/drm/drm_writeback.h
> >> +++ b/include/drm/drm_writeback.h
> >> @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
> >>  void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector,
> >>  			     struct drm_writeback_job *job);
> >>  
> >> -void drm_writeback_cleanup_job(struct drm_writeback_job *job);
> >> -
> >>  void
> >>  drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector,
> >>  				int status);

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
  2019-02-21  9:30   ` Daniel Vetter
@ 2019-02-21 10:36     ` Laurent Pinchart
  0 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2019-02-21 10:36 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Liviu Dudau, DRI Development

Hi Daniel,

On Thu, Feb 21, 2019 at 10:30:24AM +0100, Daniel Vetter wrote:
> On Thu, Feb 21, 2019 at 9:19 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > Hi Daniel,
> >
> > Thank you for the patch.
> >
> > On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote:
> >> No implementation, no callers.
> >
> > The issue here isn't that the function is declared, but that it's not
> > defined. Jobs are leaked when atomic commit fails (or when using test
> > commits). I'm working on a fix, please don't apply this patch in the
> > meantime.
> 
> Hm, can't I merge this one and give you a clean slate? Function in
> header but nowhere else kinda upsets my ocd :-)

I'm afraid I've already posted the fix patches that depend on this
prototype :-)

> >> Cc: Brian Starkey <brian.starkey@arm.com>
> >> Cc: Liviu Dudau <liviu.dudau@arm.com>
> >> Cc: Eric Anholt <eric@anholt.net>
> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >> ---
> >>  include/drm/drm_writeback.h | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> >> index 23df9d463003..f34895f7fcb1 100644
> >> --- a/include/drm/drm_writeback.h
> >> +++ b/include/drm/drm_writeback.h
> >> @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
> >>  void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector,
> >>                            struct drm_writeback_job *job);
> >>
> >> -void drm_writeback_cleanup_job(struct drm_writeback_job *job);
> >> -
> >>  void
> >>  drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector,
> >>                               int status);

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-21 10:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-20 23:24 [PATCH] drm/writeback: Delete drm_writeback_cleanup_job Daniel Vetter
2019-02-21  8:19 ` Laurent Pinchart
2019-02-21  9:30   ` Daniel Vetter
2019-02-21 10:36     ` Laurent Pinchart
2019-02-21  9:34   ` Brian Starkey
2019-02-21 10:36     ` Laurent Pinchart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.