[RFC,v2,9/9] drm: mali-dp: Add writeback out-fence support
Commit Message
If userspace has asked for an out-fence for the writeback, we add a
fence to malidp_mw_job, to be signaled when the writeback job has
completed.
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
drivers/gpu/drm/arm/malidp_hw.c | 5 ++++-
drivers/gpu/drm/arm/malidp_mw.c | 18 +++++++++++++++++-
drivers/gpu/drm/arm/malidp_mw.h | 3 +++
3 files changed, 24 insertions(+), 2 deletions(-)
Comments
2016-10-26 Brian Starkey <brian.starkey@arm.com>:
> If userspace has asked for an out-fence for the writeback, we add a
> fence to malidp_mw_job, to be signaled when the writeback job has
> completed.
>
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> ---
> drivers/gpu/drm/arm/malidp_hw.c | 5 ++++-
> drivers/gpu/drm/arm/malidp_mw.c | 18 +++++++++++++++++-
> drivers/gpu/drm/arm/malidp_mw.h | 3 +++
> 3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> index 1689547..3032226 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
> unsigned long irqflags;
> /*
> * We can't unreference the framebuffer here, so we queue it
> - * up on our threaded handler.
> + * up on our threaded handler. However, signal the fence
> + * as soon as possible
> */
> + malidp_mw_job_signal(drm, malidp->current_mw, 0);
Drivers should not deal with fences directly. We need some sort of
drm_writeback_finished() that will do the signalling for you.
Gustavo
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 26, 2016 at 07:43:57PM -0200, Gustavo Padovan wrote:
>2016-10-26 Brian Starkey <brian.starkey@arm.com>:
>
>> If userspace has asked for an out-fence for the writeback, we add a
>> fence to malidp_mw_job, to be signaled when the writeback job has
>> completed.
>>
>> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
>> ---
>> drivers/gpu/drm/arm/malidp_hw.c | 5 ++++-
>> drivers/gpu/drm/arm/malidp_mw.c | 18 +++++++++++++++++-
>> drivers/gpu/drm/arm/malidp_mw.h | 3 +++
>> 3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
>> index 1689547..3032226 100644
>> --- a/drivers/gpu/drm/arm/malidp_hw.c
>> +++ b/drivers/gpu/drm/arm/malidp_hw.c
>> @@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
>> unsigned long irqflags;
>> /*
>> * We can't unreference the framebuffer here, so we queue it
>> - * up on our threaded handler.
>> + * up on our threaded handler. However, signal the fence
>> + * as soon as possible
>> */
>> + malidp_mw_job_signal(drm, malidp->current_mw, 0);
>
>Drivers should not deal with fences directly. We need some sort of
>drm_writeback_finished() that will do the signalling for you.
>
With a signature like this?
drm_writeback_finished(struct drm_connector_state *state);
I'll have to think about how to achieve that. The state isn't
refcounted and the driver isn't in charge of it's lifetime. I'm not
sure how/where to ensure the state doesn't get destroyed before its
been signaled.
-Brian
>Gustavo
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
2016-10-27 Brian Starkey <brian.starkey@arm.com>:
> On Wed, Oct 26, 2016 at 07:43:57PM -0200, Gustavo Padovan wrote:
> > 2016-10-26 Brian Starkey <brian.starkey@arm.com>:
> >
> > > If userspace has asked for an out-fence for the writeback, we add a
> > > fence to malidp_mw_job, to be signaled when the writeback job has
> > > completed.
> > >
> > > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > > ---
> > > drivers/gpu/drm/arm/malidp_hw.c | 5 ++++-
> > > drivers/gpu/drm/arm/malidp_mw.c | 18 +++++++++++++++++-
> > > drivers/gpu/drm/arm/malidp_mw.h | 3 +++
> > > 3 files changed, 24 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> > > index 1689547..3032226 100644
> > > --- a/drivers/gpu/drm/arm/malidp_hw.c
> > > +++ b/drivers/gpu/drm/arm/malidp_hw.c
> > > @@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
> > > unsigned long irqflags;
> > > /*
> > > * We can't unreference the framebuffer here, so we queue it
> > > - * up on our threaded handler.
> > > + * up on our threaded handler. However, signal the fence
> > > + * as soon as possible
> > > */
> > > + malidp_mw_job_signal(drm, malidp->current_mw, 0);
> >
> > Drivers should not deal with fences directly. We need some sort of
> > drm_writeback_finished() that will do the signalling for you.
> >
>
> With a signature like this?
> drm_writeback_finished(struct drm_connector_state *state);
>
> I'll have to think about how to achieve that. The state isn't
> refcounted and the driver isn't in charge of it's lifetime. I'm not
> sure how/where to ensure the state doesn't get destroyed before its
> been signaled.
Maybe we should do something similar to the crtc vblank handlers and
even hide the connector_state. Those helpers only take the crtc.
They are able to hold ref to the event as well.
Gustavo
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 27, 2016 at 09:25:19AM -0200, Gustavo Padovan wrote:
> 2016-10-27 Brian Starkey <brian.starkey@arm.com>:
>
> > On Wed, Oct 26, 2016 at 07:43:57PM -0200, Gustavo Padovan wrote:
> > > 2016-10-26 Brian Starkey <brian.starkey@arm.com>:
> > >
> > > > If userspace has asked for an out-fence for the writeback, we add a
> > > > fence to malidp_mw_job, to be signaled when the writeback job has
> > > > completed.
> > > >
> > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > > > ---
> > > > drivers/gpu/drm/arm/malidp_hw.c | 5 ++++-
> > > > drivers/gpu/drm/arm/malidp_mw.c | 18 +++++++++++++++++-
> > > > drivers/gpu/drm/arm/malidp_mw.h | 3 +++
> > > > 3 files changed, 24 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> > > > index 1689547..3032226 100644
> > > > --- a/drivers/gpu/drm/arm/malidp_hw.c
> > > > +++ b/drivers/gpu/drm/arm/malidp_hw.c
> > > > @@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
> > > > unsigned long irqflags;
> > > > /*
> > > > * We can't unreference the framebuffer here, so we queue it
> > > > - * up on our threaded handler.
> > > > + * up on our threaded handler. However, signal the fence
> > > > + * as soon as possible
> > > > */
> > > > + malidp_mw_job_signal(drm, malidp->current_mw, 0);
> > >
> > > Drivers should not deal with fences directly. We need some sort of
> > > drm_writeback_finished() that will do the signalling for you.
> > >
> >
> > With a signature like this?
> > drm_writeback_finished(struct drm_connector_state *state);
> >
> > I'll have to think about how to achieve that. The state isn't
> > refcounted and the driver isn't in charge of it's lifetime. I'm not
> > sure how/where to ensure the state doesn't get destroyed before its
> > been signaled.
>
> Maybe we should do something similar to the crtc vblank handlers and
> even hide the connector_state. Those helpers only take the crtc.
> They are able to hold ref to the event as well.
I guess we could reuse the drm_event stuff, but not sure that's too much
overkill. It would at least be a consistent driver interface, and drivers
could reuse stuff like arm_event and similar functions. Which might be
rather beneficial.
-Daniel
@@ -707,8 +707,11 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
unsigned long irqflags;
/*
* We can't unreference the framebuffer here, so we queue it
- * up on our threaded handler.
+ * up on our threaded handler. However, signal the fence
+ * as soon as possible
*/
+ malidp_mw_job_signal(drm, malidp->current_mw, 0);
+
spin_lock_irqsave(&malidp->mw_lock, irqflags);
list_add_tail(&malidp->current_mw->list,
&malidp->finished_mw_jobs);
@@ -34,11 +34,24 @@ struct malidp_mw_connector_state {
u8 n_planes;
};
+void malidp_mw_job_signal(struct drm_device *drm,
+ struct malidp_mw_job *job, int status)
+{
+ DRM_DEV_DEBUG_DRIVER(drm->dev, "MW job signal %p\n", job);
+
+ if (!job->fence)
+ return;
+
+ job->fence->status = status;
+ fence_signal(job->fence);
+}
+
void malidp_mw_job_cleanup(struct drm_device *drm,
struct malidp_mw_job *job)
{
DRM_DEV_DEBUG_DRIVER(drm->dev, "MW job cleanup %p\n", job);
drm_framebuffer_unreference(job->fb);
+ fence_put(job->fence);
kfree(job);
}
@@ -107,8 +120,10 @@ static void malidp_mw_connector_destroy_state(struct drm_connector *connector,
struct malidp_mw_connector_state *mw_state = to_mw_state(state);
__drm_atomic_helper_connector_destroy_state(&mw_state->base);
- if (mw_state->job)
+ if (mw_state->job) {
+ malidp_mw_job_signal(connector->dev, mw_state->job, -EPIPE);
malidp_mw_job_cleanup(connector->dev, mw_state->job);
+ }
kfree(mw_state);
}
@@ -177,6 +192,7 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
/* We can take ownership of the framebuffer reference in the job. */
mw_state->job->fb = conn_state->fb;
conn_state->fb = NULL;
+ mw_state->job->fence = fence_get(conn_state->out_fence);
return 0;
}
@@ -15,11 +15,14 @@
struct malidp_mw_job {
struct list_head list;
struct drm_framebuffer *fb;
+ struct fence *fence;
};
int malidp_mw_connector_init(struct drm_device *drm);
void malidp_mw_atomic_commit(struct drm_device *drm,
struct drm_atomic_state *old_state);
+void malidp_mw_job_signal(struct drm_device *drm,
+ struct malidp_mw_job *job, int status);
void malidp_mw_job_cleanup(struct drm_device *drm,
struct malidp_mw_job *job);
#endif