| Message ID | 20210622165511.3169559-15-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | implicit fencing/dma-resv rules for shared buffers | expand |
Am 22.06.21 um 18:55 schrieb Daniel Vetter: > Spotted while trying to convert panfrost to these. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: "Christian König" <christian.koenig@amd.com> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/drm_gem.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index ba2e64ed8b47..68deb1de8235 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1302,6 +1302,9 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations); > * @fence_array: array of dma_fence * for the job to block on. > * @fence: the dma_fence to add to the list of dependencies. > * > + * This functions consumes the reference for @fence both on success and error > + * cases. > + * Oh, the later is a bit ugly I think. But good to know. Reviewed-by: Christian König <christian.koenig@amd.com> > * Returns: > * 0 on success, or an error on failing to expand the array. > */
On Wed, Jun 23, 2021 at 10:42:50AM +0200, Christian König wrote: > Am 22.06.21 um 18:55 schrieb Daniel Vetter: > > Spotted while trying to convert panfrost to these. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: "Christian König" <christian.koenig@amd.com> > > Cc: Lucas Stach <l.stach@pengutronix.de> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Maxime Ripard <mripard@kernel.org> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: David Airlie <airlied@linux.ie> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > --- > > drivers/gpu/drm/drm_gem.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > index ba2e64ed8b47..68deb1de8235 100644 > > --- a/drivers/gpu/drm/drm_gem.c > > +++ b/drivers/gpu/drm/drm_gem.c > > @@ -1302,6 +1302,9 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations); > > * @fence_array: array of dma_fence * for the job to block on. > > * @fence: the dma_fence to add to the list of dependencies. > > * > > + * This functions consumes the reference for @fence both on success and error > > + * cases. > > + * > > Oh, the later is a bit ugly I think. But good to know. > > Reviewed-by: Christian König <christian.koenig@amd.com> Merged to drm-misc-next, thanks for taking a look. Can you perhaps take a look at the drm/armada patch too, then I think I have reviews/acks for all of them? Thanks, Daniel > > > * Returns: > > * 0 on success, or an error on failing to expand the array. > > */ >
Am 24.06.21 um 14:41 schrieb Daniel Vetter: > On Wed, Jun 23, 2021 at 10:42:50AM +0200, Christian König wrote: >> Am 22.06.21 um 18:55 schrieb Daniel Vetter: >>> Spotted while trying to convert panfrost to these. >>> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >>> Cc: "Christian König" <christian.koenig@amd.com> >>> Cc: Lucas Stach <l.stach@pengutronix.de> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >>> Cc: Maxime Ripard <mripard@kernel.org> >>> Cc: Thomas Zimmermann <tzimmermann@suse.de> >>> Cc: David Airlie <airlied@linux.ie> >>> Cc: Daniel Vetter <daniel@ffwll.ch> >>> --- >>> drivers/gpu/drm/drm_gem.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >>> index ba2e64ed8b47..68deb1de8235 100644 >>> --- a/drivers/gpu/drm/drm_gem.c >>> +++ b/drivers/gpu/drm/drm_gem.c >>> @@ -1302,6 +1302,9 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations); >>> * @fence_array: array of dma_fence * for the job to block on. >>> * @fence: the dma_fence to add to the list of dependencies. >>> * >>> + * This functions consumes the reference for @fence both on success and error >>> + * cases. >>> + * >> Oh, the later is a bit ugly I think. But good to know. >> >> Reviewed-by: Christian König <christian.koenig@amd.com> > Merged to drm-misc-next, thanks for taking a look. Can you perhaps take a > look at the drm/armada patch too, then I think I have reviews/acks for all > of them? What are you talking about? I only see drm/armada patches for the irq stuff Thomas is working on. Christian. > > Thanks, Daniel > >>> * Returns: >>> * 0 on success, or an error on failing to expand the array. >>> */
On Thu, Jun 24, 2021 at 02:48:54PM +0200, Christian König wrote: > > > Am 24.06.21 um 14:41 schrieb Daniel Vetter: > > On Wed, Jun 23, 2021 at 10:42:50AM +0200, Christian König wrote: > > > Am 22.06.21 um 18:55 schrieb Daniel Vetter: > > > > Spotted while trying to convert panfrost to these. > > > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > > Cc: "Christian König" <christian.koenig@amd.com> > > > > Cc: Lucas Stach <l.stach@pengutronix.de> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > Cc: Maxime Ripard <mripard@kernel.org> > > > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > > > Cc: David Airlie <airlied@linux.ie> > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > > --- > > > > drivers/gpu/drm/drm_gem.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > > > index ba2e64ed8b47..68deb1de8235 100644 > > > > --- a/drivers/gpu/drm/drm_gem.c > > > > +++ b/drivers/gpu/drm/drm_gem.c > > > > @@ -1302,6 +1302,9 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations); > > > > * @fence_array: array of dma_fence * for the job to block on. > > > > * @fence: the dma_fence to add to the list of dependencies. > > > > * > > > > + * This functions consumes the reference for @fence both on success and error > > > > + * cases. > > > > + * > > > Oh, the later is a bit ugly I think. But good to know. > > > > > > Reviewed-by: Christian König <christian.koenig@amd.com> > > Merged to drm-misc-next, thanks for taking a look. Can you perhaps take a > > look at the drm/armada patch too, then I think I have reviews/acks for all > > of them? > > What are you talking about? I only see drm/armada patches for the irq stuff > Thomas is working on. There was one in this series, but Maxime was quicker. I'm going to apply all the remaining ones now. After that I'll send out a patch set to add some dependency tracking to drm_sched_job so that there's not so much copypasta going on there. I stumbled over that when reviewing how we handle dependencies. -Daniel
Am 24.06.21 um 15:32 schrieb Daniel Vetter: > On Thu, Jun 24, 2021 at 02:48:54PM +0200, Christian König wrote: >> >> Am 24.06.21 um 14:41 schrieb Daniel Vetter: >>> On Wed, Jun 23, 2021 at 10:42:50AM +0200, Christian König wrote: >>>> Am 22.06.21 um 18:55 schrieb Daniel Vetter: >>>>> Spotted while trying to convert panfrost to these. >>>>> >>>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >>>>> Cc: "Christian König" <christian.koenig@amd.com> >>>>> Cc: Lucas Stach <l.stach@pengutronix.de> >>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >>>>> Cc: Maxime Ripard <mripard@kernel.org> >>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de> >>>>> Cc: David Airlie <airlied@linux.ie> >>>>> Cc: Daniel Vetter <daniel@ffwll.ch> >>>>> --- >>>>> drivers/gpu/drm/drm_gem.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >>>>> index ba2e64ed8b47..68deb1de8235 100644 >>>>> --- a/drivers/gpu/drm/drm_gem.c >>>>> +++ b/drivers/gpu/drm/drm_gem.c >>>>> @@ -1302,6 +1302,9 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations); >>>>> * @fence_array: array of dma_fence * for the job to block on. >>>>> * @fence: the dma_fence to add to the list of dependencies. >>>>> * >>>>> + * This functions consumes the reference for @fence both on success and error >>>>> + * cases. >>>>> + * >>>> Oh, the later is a bit ugly I think. But good to know. >>>> >>>> Reviewed-by: Christian König <christian.koenig@amd.com> >>> Merged to drm-misc-next, thanks for taking a look. Can you perhaps take a >>> look at the drm/armada patch too, then I think I have reviews/acks for all >>> of them? >> What are you talking about? I only see drm/armada patches for the irq stuff >> Thomas is working on. > There was one in this series, but Maxime was quicker. I'm going to apply > all the remaining ones now. After that I'll send out a patch set to add > some dependency tracking to drm_sched_job so that there's not so much > copypasta going on there. I stumbled over that when reviewing how we > handle dependencies. Do you mean a common container for dma_fence objects a drm_sched_job depends on? Thanks, Christian. > -Daniel
On Thu, Jun 24, 2021 at 03:35:19PM +0200, Christian König wrote: > Am 24.06.21 um 15:32 schrieb Daniel Vetter: > > On Thu, Jun 24, 2021 at 02:48:54PM +0200, Christian König wrote: > > > > > > Am 24.06.21 um 14:41 schrieb Daniel Vetter: > > > > On Wed, Jun 23, 2021 at 10:42:50AM +0200, Christian König wrote: > > > > > Am 22.06.21 um 18:55 schrieb Daniel Vetter: > > > > > > Spotted while trying to convert panfrost to these. > > > > > > > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > > > > Cc: "Christian König" <christian.koenig@amd.com> > > > > > > Cc: Lucas Stach <l.stach@pengutronix.de> > > > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > Cc: Maxime Ripard <mripard@kernel.org> > > > > > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > > > > > Cc: David Airlie <airlied@linux.ie> > > > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > > > > --- > > > > > > drivers/gpu/drm/drm_gem.c | 3 +++ > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > > > > > index ba2e64ed8b47..68deb1de8235 100644 > > > > > > --- a/drivers/gpu/drm/drm_gem.c > > > > > > +++ b/drivers/gpu/drm/drm_gem.c > > > > > > @@ -1302,6 +1302,9 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations); > > > > > > * @fence_array: array of dma_fence * for the job to block on. > > > > > > * @fence: the dma_fence to add to the list of dependencies. > > > > > > * > > > > > > + * This functions consumes the reference for @fence both on success and error > > > > > > + * cases. > > > > > > + * > > > > > Oh, the later is a bit ugly I think. But good to know. > > > > > > > > > > Reviewed-by: Christian König <christian.koenig@amd.com> > > > > Merged to drm-misc-next, thanks for taking a look. Can you perhaps take a > > > > look at the drm/armada patch too, then I think I have reviews/acks for all > > > > of them? > > > What are you talking about? I only see drm/armada patches for the irq stuff > > > Thomas is working on. > > There was one in this series, but Maxime was quicker. I'm going to apply > > all the remaining ones now. After that I'll send out a patch set to add > > some dependency tracking to drm_sched_job so that there's not so much > > copypasta going on there. I stumbled over that when reviewing how we > > handle dependencies. > > Do you mean a common container for dma_fence objects a drm_sched_job depends > on? Yup. Well the real usefulness is the interfaces, so that you can just grep for those when trying to figure out htf a driver handles its implicit dependencies. And amdgpu is unfortunately going to be a bit in the cold because it's special (but should be able to benefit too, just more than 1-2 patches to convert it over). Anyway I'm going to type the cover letter rsn. -Daniel
Am 24.06.21 um 15:41 schrieb Daniel Vetter: > On Thu, Jun 24, 2021 at 03:35:19PM +0200, Christian König wrote: >> Am 24.06.21 um 15:32 schrieb Daniel Vetter: >>> On Thu, Jun 24, 2021 at 02:48:54PM +0200, Christian König wrote: >>>> Am 24.06.21 um 14:41 schrieb Daniel Vetter: >>>>> On Wed, Jun 23, 2021 at 10:42:50AM +0200, Christian König wrote: >>>>>> Am 22.06.21 um 18:55 schrieb Daniel Vetter: >>>>>>> Spotted while trying to convert panfrost to these. >>>>>>> >>>>>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >>>>>>> Cc: "Christian König" <christian.koenig@amd.com> >>>>>>> Cc: Lucas Stach <l.stach@pengutronix.de> >>>>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >>>>>>> Cc: Maxime Ripard <mripard@kernel.org> >>>>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de> >>>>>>> Cc: David Airlie <airlied@linux.ie> >>>>>>> Cc: Daniel Vetter <daniel@ffwll.ch> >>>>>>> --- >>>>>>> drivers/gpu/drm/drm_gem.c | 3 +++ >>>>>>> 1 file changed, 3 insertions(+) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >>>>>>> index ba2e64ed8b47..68deb1de8235 100644 >>>>>>> --- a/drivers/gpu/drm/drm_gem.c >>>>>>> +++ b/drivers/gpu/drm/drm_gem.c >>>>>>> @@ -1302,6 +1302,9 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations); >>>>>>> * @fence_array: array of dma_fence * for the job to block on. >>>>>>> * @fence: the dma_fence to add to the list of dependencies. >>>>>>> * >>>>>>> + * This functions consumes the reference for @fence both on success and error >>>>>>> + * cases. >>>>>>> + * >>>>>> Oh, the later is a bit ugly I think. But good to know. >>>>>> >>>>>> Reviewed-by: Christian König <christian.koenig@amd.com> >>>>> Merged to drm-misc-next, thanks for taking a look. Can you perhaps take a >>>>> look at the drm/armada patch too, then I think I have reviews/acks for all >>>>> of them? >>>> What are you talking about? I only see drm/armada patches for the irq stuff >>>> Thomas is working on. >>> There was one in this series, but Maxime was quicker. I'm going to apply >>> all the remaining ones now. After that I'll send out a patch set to add >>> some dependency tracking to drm_sched_job so that there's not so much >>> copypasta going on there. I stumbled over that when reviewing how we >>> handle dependencies. >> Do you mean a common container for dma_fence objects a drm_sched_job depends >> on? > Yup. Well the real usefulness is the interfaces, so that you can just grep > for those when trying to figure out htf a driver handles its implicit > dependencies. And amdgpu is unfortunately going to be a bit in the cold > because it's special (but should be able to benefit too, just more than > 1-2 patches to convert it over). I had that on the TODO list for quite a while as well. Essentially extracting what the dma_resv_list object is doing into a new object (but maybe without RCU). Christian. > > Anyway I'm going to type the cover letter rsn. > -Daniel
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index ba2e64ed8b47..68deb1de8235 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1302,6 +1302,9 @@ EXPORT_SYMBOL(drm_gem_unlock_reservations); * @fence_array: array of dma_fence * for the job to block on. * @fence: the dma_fence to add to the list of dependencies. * + * This functions consumes the reference for @fence both on success and error + * cases. + * * Returns: * 0 on success, or an error on failing to expand the array. */
Spotted while trying to convert panfrost to these. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: "Christian König" <christian.koenig@amd.com> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/drm_gem.c | 3 +++ 1 file changed, 3 insertions(+)