Re: [Freedreno] [PATCH v16 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device

2018-10-01 Thread Vivek Gautam
On Tue, Oct 2, 2018 at 9:44 AM Vivek Gautam  wrote:
>
> Hi Will,
>
> On Mon, Oct 1, 2018 at 6:29 PM Will Deacon  wrote:
> >
> > Hi Vivek,
> >
> > On Thu, Aug 30, 2018 at 08:15:38PM +0530, Vivek Gautam wrote:
> > > From: Sricharan R 
> > >
> > > The smmu device probe/remove and add/remove master device callbacks
> > > gets called when the smmu is not linked to its master, that is without
> > > the context of the master device. So calling runtime apis in those places
> > > separately.
> > > Global locks are also initialized before enabling runtime pm as the
> > > runtime_resume() calls device_reset() which does tlb_sync_global()
> > > that ultimately requires locks to be initialized.
> > >
> > > Signed-off-by: Sricharan R 
> > > [vivek: Cleanup pm runtime calls]
> > > Signed-off-by: Vivek Gautam 
> > > Reviewed-by: Tomasz Figa 
> > > Tested-by: Srinivas Kandagatla 
> > > ---
> > >  drivers/iommu/arm-smmu.c | 89 
> > > +++-
> > >  1 file changed, 81 insertions(+), 8 deletions(-)
> >
> > This doesn't apply on my tree[1], possibly because I've got Robin's 
> > non-strict
> > invalidation queued there. However, that got me thinking -- how does this
> > work in conjunction with the timer-based TLB invalidation? Do we need to
> > rpm_{get,put} around flush_iotlb_all()? If so, do we still need the calls
> > in map/unmap when non-strict mode is in use?

For map/unmap(), i think there would be no harm in having additional
power.usage_count even for the non-strict mode.
So, I will just add rpm{get,put} in arm_smmu_flush_iotlb_all(), and
arm_smmu_iotlb_sync().

Regards
Vivek

>
> I haven't tested things with flush queues, but from what it looks like
> both .flush_iotlb_all, and .iotlb_sync callbacks need rpm_get/put().
> I will respin the patches.
>
> Thanks
> Vivek
> >
> > Will
> >
> > [1] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates
>
>
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation



-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v16 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device

2018-10-01 Thread Vivek Gautam
Hi Will,

On Mon, Oct 1, 2018 at 6:29 PM Will Deacon  wrote:
>
> Hi Vivek,
>
> On Thu, Aug 30, 2018 at 08:15:38PM +0530, Vivek Gautam wrote:
> > From: Sricharan R 
> >
> > The smmu device probe/remove and add/remove master device callbacks
> > gets called when the smmu is not linked to its master, that is without
> > the context of the master device. So calling runtime apis in those places
> > separately.
> > Global locks are also initialized before enabling runtime pm as the
> > runtime_resume() calls device_reset() which does tlb_sync_global()
> > that ultimately requires locks to be initialized.
> >
> > Signed-off-by: Sricharan R 
> > [vivek: Cleanup pm runtime calls]
> > Signed-off-by: Vivek Gautam 
> > Reviewed-by: Tomasz Figa 
> > Tested-by: Srinivas Kandagatla 
> > ---
> >  drivers/iommu/arm-smmu.c | 89 
> > +++-
> >  1 file changed, 81 insertions(+), 8 deletions(-)
>
> This doesn't apply on my tree[1], possibly because I've got Robin's non-strict
> invalidation queued there. However, that got me thinking -- how does this
> work in conjunction with the timer-based TLB invalidation? Do we need to
> rpm_{get,put} around flush_iotlb_all()? If so, do we still need the calls
> in map/unmap when non-strict mode is in use?

I haven't tested things with flush queues, but from what it looks like
both .flush_iotlb_all, and .iotlb_sync callbacks need rpm_get/put().
I will respin the patches.

Thanks
Vivek
>
> Will
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates



-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 01/13] drm/msm: Track GPU fences with idr

2018-10-01 Thread Rob Clark
On Mon, Oct 1, 2018 at 3:04 PM Jordan Crouse  wrote:
>
> On Mon, Oct 01, 2018 at 06:01:33PM +0530, Sharat Masetty wrote:
> > Track the GPU fences created at submit time with idr instead of the ring
> > the sequence number. This helps with easily changing the underlying
> > fence to something we don't truly own, like the scheduler fence.
> >
> > Also move the GPU fence allocation to msm_gpu_submit() and have
> > the function return the fence. This suits well when integrating with the
> > GPU scheduler.
> >
> > Additionally remove the non-interruptible wait variant from msm_wait_fence()
> > as it is not used.
>
> So basically this is just propping up the msm_wait_fence ioctl a bit more?
> At what point should we just deprecate that bad boy and move on with our 
> lives?
>

As I mentioned on IRC, wait_fence ioctl is still in use, so
unfortunately I don't think we can just deprecate it.  I guess it is
worth some profiling, and if this shows up as enough overhead to care
about perhaps we can introduce a submit ioctl flag to disable "old"
fences.

Personally, my guestimate about what we want to do for performance
from a kernel standpoint is more towards introducing 64b reloc's
(rather than using 2x 32b relocs), or possibly skip straight ahead to
something like i915's softpin.. there are a couple other things that
I'd started on a few months back to reduce userspace draw-overhead
that I think I need to pick back up, but reloc overhead is something
near the top of the list.  (Reloc and submit overhead is partially
mitigated with freedreno, since the way the driver is structured it
gets pushed off to a background thread, but still I think there is
some room for improvement.)

BR,
-R
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 2/2] drm/msm: dpu: Make legacy cursor updates asynchronous

2018-10-01 Thread Sean Paul
On Wed, Sep 26, 2018 at 11:56:47AM -0700, Jeykumar Sankaran wrote:
> On 2018-09-19 11:56, Sean Paul wrote:
> > From: Sean Paul 
> > 
> > This patch sprinkles a few async/legacy_cursor_update checks
> > through commit to ensure that cursor updates aren't blocked on vsync.
> > There are 2 main components to this, the first is that we don't want to
> > wait_for_commit_done in msm_atomic  before returning from
> > atomic_complete.
> > The second is that in dpu we don't want to wait for frame_done events
> > when
> > updating the cursor.
> > 
> > Signed-off-by: Sean Paul 
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 44 +++--
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h|  3 +-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 21 ++
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  6 ++-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  5 ++-
> >  drivers/gpu/drm/msm/msm_atomic.c|  3 +-
> >  6 files changed, 48 insertions(+), 34 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index a8f2dd7a37c7..b23f00a2554b 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -816,7 +816,7 @@ static int _dpu_crtc_wait_for_frame_done(struct
> > drm_crtc *crtc)
> > return rc;
> >  }
> > 
> > -void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> > +void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async)
> >  {
> > struct drm_encoder *encoder;
> > struct drm_device *dev;
> > @@ -862,27 +862,30 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > *crtc)
> >  * Encoder will flush/start now, unless it has a tx
> > pending.
> >  * If so, it may delay and flush at an irq event (e.g.
> > ppdone)
> >  */
> > -   dpu_encoder_prepare_for_kickoff(encoder, );
> > +   dpu_encoder_prepare_for_kickoff(encoder, , async);
> > }
> > 
> > -   /* wait for frame_event_done completion */
> > -   DPU_ATRACE_BEGIN("wait_for_frame_done_event");
> > -   ret = _dpu_crtc_wait_for_frame_done(crtc);
> > -   DPU_ATRACE_END("wait_for_frame_done_event");
> > -   if (ret) {
> > -   DPU_ERROR("crtc%d wait for frame done
> > failed;frame_pending%d\n",
> > -   crtc->base.id,
> > -   atomic_read(_crtc->frame_pending));
> > -   goto end;
> > -   }
> > 
> > -   if (atomic_inc_return(_crtc->frame_pending) == 1) {
> > -   /* acquire bandwidth and other resources */
> > -   DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
> > -   } else
> > -   DPU_DEBUG("crtc%d commit\n", crtc->base.id);
> > +   if (!async) {
> > +   /* wait for frame_event_done completion */
> > +   DPU_ATRACE_BEGIN("wait_for_frame_done_event");
> > +   ret = _dpu_crtc_wait_for_frame_done(crtc);
> > +   DPU_ATRACE_END("wait_for_frame_done_event");
> > +   if (ret) {
> > +   DPU_ERROR("crtc%d wait for frame done
> > failed;frame_pending%d\n",
> > +   crtc->base.id,
> > +
> > atomic_read(_crtc->frame_pending));
> > +   goto end;
> > +   }
> > +
> > +   if (atomic_inc_return(_crtc->frame_pending) == 1) {
> > +   /* acquire bandwidth and other resources */
> > +   DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
> > +   } else
> > +   DPU_DEBUG("crtc%d commit\n", crtc->base.id);
> > 
> > -   dpu_crtc->play_count++;
> > +   dpu_crtc->play_count++;
> > +   }
> > 
> > dpu_vbif_clear_errors(dpu_kms);
> > 
> > @@ -890,11 +893,12 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > *crtc)
> > if (encoder->crtc != crtc)
> > continue;
> > 
> > -   dpu_encoder_kickoff(encoder);
> > +   dpu_encoder_kickoff(encoder, async);
> > }
> > 
> >  end:
> > -   reinit_completion(_crtc->frame_done_comp);
> > +   if (!async)
> > +   reinit_completion(_crtc->frame_done_comp);
> > DPU_ATRACE_END("crtc_commit");
> >  }
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > index 3723b4830335..67c9f59997cf 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > @@ -285,8 +285,9 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
> >  /**
> >   * dpu_crtc_commit_kickoff - trigger kickoff of the commit for this
> > crtc
> >   * @crtc: Pointer to drm crtc object
> > + * @async: true if the commit is asynchronous, false otherwise
> >   */
> > -void dpu_crtc_commit_kickoff(struct drm_crtc *crtc);
> > +void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async);
> > 
> >  /**
> >   * dpu_crtc_complete_commit - callback signalling completion of current
> > commit
> > diff --git 

Re: [Freedreno] [PATCH 1/2] drm/msm: dpu: Only check flush register against pending flushes

2018-10-01 Thread Sean Paul
On Wed, Sep 26, 2018 at 11:51:35AM -0700, Jeykumar Sankaran wrote:
> On 2018-09-19 11:56, Sean Paul wrote:
> > From: Sean Paul 
> > 
> > There exists a case where a flush of a plane/dma may have been triggered
> > & started from an async commit. If that plane/dma is subsequently
> > disabled
> > by the next commit, the flush register will continue to hold the flush
> > bit for the disabled plane. Since the bit remains active,
> > pending_kickoff_cnt will never decrement and we'll miss frame_done
> > events.
> > 
> I suppose this is the vblank in between the async commit and the next commit
> (one where
> the plane is disabled).
> 
> If this vblank had consumed the flush bits, it means the HW
> has read the configuration and it should have cleared bits.
> 
> If you still see the flush bit active, it means the async commit has missed
> the VBLANK boundary
> and the HW has not yet taken the cursor configuration. So you are not
> supposed to
> get frame_done event.

Right, we're not getting frame_done until the next frame comes in. The issue is
that we get 2 commits in between vblanks, the first commit triggers the cursor
for flush and the second one disables it. Unfortunately the first commit has
already called CTL_START and made it impossible for the second commit to clear
that flush bit (afaict).

The frame_done events seem to flow properly, only being triggered once per
vblank and only when a non-async commit has happened.

So is there a way to clear the CTL_FLUSH register on subsequent commits? I've
poked around some more and can't seem to figure it out.

> 
> Comments outside the scope of this patch: To support async and sync updates
> on the same display commit thread, we should be adding more protection to
> support
> concurrency scenarios to avoid more than one ctl flushes per VBLANK period.

Yeah, certainly easier said than done. I'm not really sure how to implement
that, tbh.

There's no way to know how many commits you'll have, and there's no way to
delay the FLUSH until right before vblank. Do you have any ideas that I might be
missing?

Sean

> 
> Thanks,
> Jeykumar S.
> 
> 
> 
> > This patch limits the check of flush_register to include only those bits
> > which have been updated with the latest commit.
> > 
> > Signed-off-by: Sean Paul 
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > index 84de385a9f62..60f146f02b77 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > @@ -331,7 +331,7 @@ static void dpu_encoder_phys_vid_vblank_irq(void
> > *arg,
> > int irq_idx)
> > if (hw_ctl && hw_ctl->ops.get_flush_register)
> > flush_register = hw_ctl->ops.get_flush_register(hw_ctl);
> > 
> > -   if (flush_register == 0)
> > +   if (!(flush_register & hw_ctl->ops.get_pending_flush(hw_ctl)))
> > new_cnt =
> > atomic_add_unless(_enc->pending_kickoff_cnt,
> > -1, 0);
> > spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 01/13] drm/msm: Track GPU fences with idr

2018-10-01 Thread Jordan Crouse
On Mon, Oct 01, 2018 at 06:01:33PM +0530, Sharat Masetty wrote:
> Track the GPU fences created at submit time with idr instead of the ring
> the sequence number. This helps with easily changing the underlying
> fence to something we don't truly own, like the scheduler fence.
> 
> Also move the GPU fence allocation to msm_gpu_submit() and have
> the function return the fence. This suits well when integrating with the
> GPU scheduler.
> 
> Additionally remove the non-interruptible wait variant from msm_wait_fence()
> as it is not used.

So basically this is just propping up the msm_wait_fence ioctl a bit more?
At what point should we just deprecate that bad boy and move on with our lives?

> Signed-off-by: Sharat Masetty 
> ---
>  drivers/gpu/drm/msm/msm_drv.c|  3 +--
>  drivers/gpu/drm/msm/msm_fence.c  | 30 ++
>  drivers/gpu/drm/msm/msm_fence.h  |  5 +++--
>  drivers/gpu/drm/msm/msm_gem.h|  1 +
>  drivers/gpu/drm/msm/msm_gem_submit.c | 26 ++
>  drivers/gpu/drm/msm/msm_gpu.c| 10 --
>  drivers/gpu/drm/msm/msm_gpu.h|  4 ++--
>  drivers/gpu/drm/msm/msm_ringbuffer.c |  5 +
>  drivers/gpu/drm/msm/msm_ringbuffer.h |  1 +
>  9 files changed, 53 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 021a0b6..8eaa1bd 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -746,8 +746,7 @@ static int msm_ioctl_wait_fence(struct drm_device *dev, 
> void *data,
>   if (!queue)
>   return -ENOENT;
>  
> - ret = msm_wait_fence(gpu->rb[queue->prio]->fctx, args->fence, ,
> - true);
> + ret = msm_wait_fence(gpu->rb[queue->prio], args->fence, );
>  
>   msm_submitqueue_put(queue);
>   return ret;
> diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
> index 349c12f..0e7912b 100644
> --- a/drivers/gpu/drm/msm/msm_fence.c
> +++ b/drivers/gpu/drm/msm/msm_fence.c
> @@ -50,41 +50,39 @@ static inline bool fence_completed(struct 
> msm_fence_context *fctx, uint32_t fenc
>  }
>  
>  /* legacy path for WAIT_FENCE ioctl: */
> -int msm_wait_fence(struct msm_fence_context *fctx, uint32_t fence,
> - ktime_t *timeout, bool interruptible)
> +int msm_wait_fence(struct msm_ringbuffer *ring, uint32_t fence_id,
> + ktime_t *timeout)
>  {
> + struct dma_fence *fence;
>   int ret;
>  
> - if (fence > fctx->last_fence) {
> - DRM_ERROR("%s: waiting on invalid fence: %u (of %u)\n",
> - fctx->name, fence, fctx->last_fence);
> - return -EINVAL;
> + rcu_read_lock();
> + fence = idr_find(>fence_idr, fence_id);
> +
> + if (!fence || !dma_fence_get_rcu(fence)) {
> + rcu_read_unlock();
> + return 0;
>   }
> + rcu_read_unlock();
>  
>   if (!timeout) {
>   /* no-wait: */
> - ret = fence_completed(fctx, fence) ? 0 : -EBUSY;
> + ret = dma_fence_is_signaled(fence) ? 0 : -EBUSY;
>   } else {
>   unsigned long remaining_jiffies = timeout_to_jiffies(timeout);
>  
> - if (interruptible)
> - ret = wait_event_interruptible_timeout(fctx->event,
> - fence_completed(fctx, fence),
> - remaining_jiffies);
> - else
> - ret = wait_event_timeout(fctx->event,
> - fence_completed(fctx, fence),
> - remaining_jiffies);
> + ret = dma_fence_wait_timeout(fence, true, remaining_jiffies);
>  
>   if (ret == 0) {
>   DBG("timeout waiting for fence: %u (completed: %u)",
> - fence, fctx->completed_fence);
> + fence_id, ring->memptrs->fence);
>   ret = -ETIMEDOUT;
>   } else if (ret != -ERESTARTSYS) {
>   ret = 0;
>   }
>   }
>  
> + dma_fence_put(fence);
>   return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h
> index b9fe059..a8133f7 100644
> --- a/drivers/gpu/drm/msm/msm_fence.h
> +++ b/drivers/gpu/drm/msm/msm_fence.h
> @@ -19,6 +19,7 @@
>  #define __MSM_FENCE_H__
>  
>  #include "msm_drv.h"
> +#include "msm_ringbuffer.h"
>  
>  struct msm_fence_context {
>   struct drm_device *dev;
> @@ -35,8 +36,8 @@ struct msm_fence_context * msm_fence_context_alloc(struct 
> drm_device *dev,
>   const char *name);
>  void msm_fence_context_free(struct msm_fence_context *fctx);
>  
> -int msm_wait_fence(struct msm_fence_context *fctx, uint32_t fence,
> - ktime_t *timeout, bool interruptible);
> +int msm_wait_fence(struct msm_ringbuffer *ring, uint32_t fence_id,
> + ktime_t *timeout);
>  void 

Re: [Freedreno] [PATCH 09/13] drm/msm: Use the DRM common Scheduler

2018-10-01 Thread Jordan Crouse
On Mon, Oct 01, 2018 at 06:01:41PM +0530, Sharat Masetty wrote:
> This patch hooks up the DRM gpu scheduler to the msm DRM driver. The
> most noticeable changes to the driver are as follows. The submit path is
> split into two parts, in the user context the submit(job) is created and
> added to one of the entity's scheduler run queue. The scheduler then
> tries to drain the queue by submitting the jobs the hardware to act upon.
> The submit job sits on the scheduler queue until all the dependent
> fences are waited upon successfully.
> 
> We have one scheduler instance per ring. The submitqueues will host a
> scheduler entity object. This entity will be mapped to the scheduler's
> default runqueue. This should be good for now, but in future it is possible
> to extend the API to allow for scheduling amongst the submitqueues on the
> same ring.
> 
> With this patch the role of the struct_mutex lock has been greatly reduced in
> scope in the submit path, evidently when actually putting the job on the
> ringbuffer. This should enable us with increased parallelism in the
> driver which should translate to better performance overall hopefully.
> 
> Signed-off-by: Sharat Masetty 
> ---
>  drivers/gpu/drm/msm/Kconfig   |   1 +
>  drivers/gpu/drm/msm/Makefile  |   3 +-
>  drivers/gpu/drm/msm/msm_drv.h |   3 +-
>  drivers/gpu/drm/msm/msm_gem.c |   8 +-
>  drivers/gpu/drm/msm/msm_gem.h |   6 +
>  drivers/gpu/drm/msm/msm_gem_submit.c  | 138 +--
>  drivers/gpu/drm/msm/msm_gpu.c |  72 ++--
>  drivers/gpu/drm/msm/msm_gpu.h |   2 +
>  drivers/gpu/drm/msm/msm_ringbuffer.c  |   7 +
>  drivers/gpu/drm/msm/msm_ringbuffer.h  |   3 +
>  drivers/gpu/drm/msm/msm_sched.c   | 313 
> ++
>  drivers/gpu/drm/msm/msm_sched.h   |  18 ++
>  drivers/gpu/drm/msm/msm_submitqueue.c |  18 +-
>  13 files changed, 507 insertions(+), 85 deletions(-)
>  create mode 100644 drivers/gpu/drm/msm/msm_sched.c
>  create mode 100644 drivers/gpu/drm/msm/msm_sched.h
> 
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index 38cbde9..0d01810 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -15,6 +15,7 @@ config DRM_MSM
>   select SND_SOC_HDMI_CODEC if SND_SOC
>   select SYNC_FILE
>   select PM_OPP
> + select DRM_SCHED
>   default y
>   help
> DRM/KMS driver for MSM/snapdragon.
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index cd40c05..71ed921 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -60,7 +60,8 @@ msm-y := \
>   msm_perf.o \
>   msm_rd.o \
>   msm_ringbuffer.o \
> - msm_submitqueue.o
> + msm_submitqueue.o \
> + msm_sched.o
>  
>  msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o
>  
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index b2da1fb..e461a9c 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -213,8 +213,7 @@ struct drm_gem_object 
> *msm_gem_prime_import_sg_table(struct drm_device *dev,
>  int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv);
>  int msm_gem_sync_object(struct drm_gem_object *obj,
>   struct msm_fence_context *fctx, bool exclusive);
> -void msm_gem_move_to_active(struct drm_gem_object *obj,
> - struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence);
> +void msm_gem_move_to_active(struct drm_gem_object *obj, struct msm_gpu *gpu);
>  void msm_gem_move_to_inactive(struct drm_gem_object *obj);
>  int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t 
> *timeout);
>  int msm_gem_cpu_fini(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index f583bb4..7a12f30 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -663,16 +663,12 @@ int msm_gem_sync_object(struct drm_gem_object *obj,
>   return 0;
>  }
>  
> -void msm_gem_move_to_active(struct drm_gem_object *obj,
> - struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence)
> +void msm_gem_move_to_active(struct drm_gem_object *obj, struct msm_gpu *gpu)
>  {
>   struct msm_gem_object *msm_obj = to_msm_bo(obj);
>   WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED);
>   msm_obj->gpu = gpu;
> - if (exclusive)
> - reservation_object_add_excl_fence(msm_obj->resv, fence);
> - else
> - reservation_object_add_shared_fence(msm_obj->resv, fence);
> +
>   list_del_init(_obj->mm_list);
>   list_add_tail(_obj->mm_list, >active_list);
>  }
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index cae3aa5..8c6269f 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -20,6 +20,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include "msm_drv.h"
>  
>  /* 

Re: [Freedreno] [PATCH 06/13] drm/msm: Use kzalloc for submit struct allocation

2018-10-01 Thread Jordan Crouse
On Mon, Oct 01, 2018 at 06:01:38PM +0530, Sharat Masetty wrote:
> This patch changes to kzalloc and avoids setting individual submit
> struct fields to zero manually.

I don't think this one is worth it.  There are so many members in submit and so
few that get reset to 0 - I don't think the extra cycles are worth it in this
fast path.

> Signed-off-by: Sharat Masetty 
> ---
>  drivers/gpu/drm/msm/msm_gem_submit.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c 
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index a7c8cbc..7931c2a 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -41,24 +41,19 @@ static struct msm_gem_submit *submit_create(struct 
> drm_device *dev,
>   if (sz > SIZE_MAX)
>   return NULL;
>  
> - submit = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
> + submit = kzalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
>   if (!submit)
>   return NULL;
>  
>   submit->dev = dev;
>   submit->gpu = gpu;
>   submit->ctx = ctx;
> - submit->hw_fence = NULL;
>   submit->out_fence_id = -1;
>   submit->pid = get_pid(task_pid(current));
>   submit->cmd = (void *)>bos[nr_bos];
>   submit->queue = queue;
>   submit->ring = gpu->rb[queue->prio];
>  
> - /* initially, until copy_from_user() and bo lookup succeeds: */
> - submit->nr_bos = 0;
> - submit->nr_cmds = 0;
> -
>   INIT_LIST_HEAD(>node);
>   INIT_LIST_HEAD(>bo_list);
>   ww_acquire_init(>ticket, _ww_class);
> -- 
> 1.9.1
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 03/13] drm/msm: Save the ring name in the ring structure

2018-10-01 Thread Jordan Crouse
On Mon, Oct 01, 2018 at 06:01:35PM +0530, Sharat Masetty wrote:
> The scheduler needs an instance name mostly for debug purposes. Save the
> name in the ringbuffer instead of a stack variable, so that the name
> can be shared with the scheduler.
> 
> Signed-off-by: Sharat Masetty 
> ---
>  drivers/gpu/drm/msm/msm_ringbuffer.c | 5 ++---
>  drivers/gpu/drm/msm/msm_ringbuffer.h | 1 +
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c 
> b/drivers/gpu/drm/msm/msm_ringbuffer.c
> index 734f2b8..0889766 100644
> --- a/drivers/gpu/drm/msm/msm_ringbuffer.c
> +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
> @@ -22,7 +22,6 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu 
> *gpu, int id,
>   void *memptrs, uint64_t memptrs_iova)
>  {
>   struct msm_ringbuffer *ring;
> - char name[32];
>   int ret;
>  
>   /* We assume everwhere that MSM_GPU_RINGBUFFER_SZ is a power of 2 */
> @@ -55,9 +54,9 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu 
> *gpu, int id,
>   INIT_LIST_HEAD(>submits);
>   spin_lock_init(>lock);
>  
> - snprintf(name, sizeof(name), "gpu-ring-%d", ring->id);
> + snprintf(ring->name, sizeof(ring->name), "msm-gpu-ring-%d", ring->id);

Okay I guess, but can't we just generate this on the fly when the
scheduler needs it? Its not like the name is random or anything.

> - ring->fctx = msm_fence_context_alloc(gpu->dev, name);
> + ring->fctx = msm_fence_context_alloc(gpu->dev, ring->name);
>  
>   idr_init(>fence_idr);
>  
> diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h 
> b/drivers/gpu/drm/msm/msm_ringbuffer.h
> index b74a0a9..523373b 100644
> --- a/drivers/gpu/drm/msm/msm_ringbuffer.h
> +++ b/drivers/gpu/drm/msm/msm_ringbuffer.h
> @@ -31,6 +31,7 @@ struct msm_rbmemptrs {
>  struct msm_ringbuffer {
>   struct msm_gpu *gpu;
>   int id;
> + char name[16];
>   struct drm_gem_object *bo;
>   uint32_t *start, *end, *cur, *next;
>   struct list_head submits;
> -- 
> 1.9.1
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 02/13] drm/msm: Change msm_gpu_submit() API

2018-10-01 Thread Jordan Crouse
On Mon, Oct 01, 2018 at 06:01:34PM +0530, Sharat Masetty wrote:
> When the scheduler comes into picture, the msm_gpu_submit() will be
> called from the scheduler worker thread. So save the necessary information
> into msm job structure for use at the actual GPU submission time. This
> also simplifies the msm_gpu_submit() API.

When I read this, I immediately thought you were changing the format of the
submit ioctl struct, but really you are just changing the parameters of the
functions - this isn't an API by definition because it isn't an interface
to anything else.  Words like API get people all worked up - I would recommend
rewording this to be less scary / more accurate.

> Signed-off-by: Sharat Masetty 
> ---
>  drivers/gpu/drm/msm/msm_gem.h| 1 +
>  drivers/gpu/drm/msm/msm_gem_submit.c | 8 +---
>  drivers/gpu/drm/msm/msm_gpu.c| 8 
>  drivers/gpu/drm/msm/msm_gpu.h| 3 +--
>  4 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index 287f974..289abe5 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -137,6 +137,7 @@ enum msm_gem_lock {
>   */
>  struct msm_gem_submit {
>   struct drm_device *dev;
> + struct msm_file_private *ctx;
>   struct msm_gpu *gpu;
>   struct list_head node;   /* node in ring submit list */
>   struct list_head bo_list;
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c 
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 00e6347..14906b9 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -32,7 +32,7 @@
>  
>  static struct msm_gem_submit *submit_create(struct drm_device *dev,
>   struct msm_gpu *gpu, struct msm_gpu_submitqueue *queue,
> - uint32_t nr_bos, uint32_t nr_cmds)
> + uint32_t nr_bos, uint32_t nr_cmds, struct msm_file_private *ctx)

submit_create() is a catch-22.  If I saw all that code inline I would probably
want it to be a sub function too, but on the other hand we're steadily adding
new parameters to it and adding new lines of code that the calling function
doesn't need to translate.

>  {
>   struct msm_gem_submit *submit;
>   uint64_t sz = sizeof(*submit) + ((u64)nr_bos * sizeof(submit->bos[0])) +
> @@ -47,6 +47,7 @@ static struct msm_gem_submit *submit_create(struct 
> drm_device *dev,
>  
>   submit->dev = dev;
>   submit->gpu = gpu;
> + submit->ctx = ctx;
>   submit->fence = NULL;
>   submit->out_fence_id = -1;
>   submit->pid = get_pid(task_pid(current));
> @@ -474,7 +475,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void 
> *data,
>   }
>   }
>  
> - submit = submit_create(dev, gpu, queue, args->nr_bos, args->nr_cmds);
> + submit = submit_create(dev, gpu, queue, args->nr_bos,
> + args->nr_cmds, ctx);

Like maybe we can compromise and pass in args directly to submit_create to help
offset the new parameters.

>   if (!submit) {
>   ret = -ENOMEM;
>   goto out_unlock;
> @@ -560,7 +562,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void 
> *data,
>  
>   submit->nr_cmds = i;
>  
> - msm_gpu_submit(gpu, submit, ctx);
> + msm_gpu_submit(submit);
>   if (IS_ERR(submit->fence)) {
>   ret = PTR_ERR(submit->fence);
>   submit->fence = NULL;
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index eb67172..5f9cd85 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -602,9 +602,9 @@ void msm_gpu_retire(struct msm_gpu *gpu)
>  }
>  
>  /* add bo's to gpu's ring, and kick gpu: */
> -struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu,
> - struct msm_gem_submit *submit, struct msm_file_private *ctx)
> +struct dma_fence *msm_gpu_submit(struct msm_gem_submit *submit)
>  {
> + struct msm_gpu *gpu = submit->gpu;
>   struct drm_device *dev = gpu->dev;
>   struct msm_drm_private *priv = dev->dev_private;
>   struct msm_ringbuffer *ring = submit->ring;
> @@ -648,8 +648,8 @@ struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu,
>   msm_gem_move_to_active(_obj->base, gpu, false, 
> submit->fence);
>   }
>  
> - gpu->funcs->submit(gpu, submit, ctx);
> - priv->lastctx = ctx;
> + gpu->funcs->submit(gpu, submit, submit->ctx);
> + priv->lastctx = submit->ctx;
>  
>   hangcheck_timer_reset(gpu);
>  
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index b562b25..dd55979 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -235,8 +235,7 @@ int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint32_t 
> *activetime,
>   uint32_t *totaltime, uint32_t ncntrs, uint32_t *cntrs);
>  
>  void msm_gpu_retire(struct msm_gpu *gpu);
> -struct dma_fence *msm_gpu_submit(struct msm_gpu 

Re: [Freedreno] [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2

2018-10-01 Thread Will Deacon
On Mon, Oct 01, 2018 at 12:36:09PM -0500, Rob Herring wrote:
> On Mon, Oct 1, 2018 at 7:18 AM Will Deacon  wrote:
> >
> > On Thu, Aug 30, 2018 at 08:15:40PM +0530, Vivek Gautam wrote:
> > > Add bindings doc for Qcom's smmu-v2 implementation.
> > >
> > > Signed-off-by: Vivek Gautam 
> > > Reviewed-by: Tomasz Figa 
> > > Tested-by: Srinivas Kandagatla 
> > > ---
> > >  .../devicetree/bindings/iommu/arm,smmu.txt | 39 
> > > ++
> > >  1 file changed, 39 insertions(+)
> >
> > It would be nice to have an Ack from a DT maintainer on this, since it's
> > adding new compatible strings...
> 
> I did...

Oops, sorry, missed that.

Will
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2

2018-10-01 Thread Rob Herring
On Mon, Oct 1, 2018 at 7:18 AM Will Deacon  wrote:
>
> On Thu, Aug 30, 2018 at 08:15:40PM +0530, Vivek Gautam wrote:
> > Add bindings doc for Qcom's smmu-v2 implementation.
> >
> > Signed-off-by: Vivek Gautam 
> > Reviewed-by: Tomasz Figa 
> > Tested-by: Srinivas Kandagatla 
> > ---
> >  .../devicetree/bindings/iommu/arm,smmu.txt | 39 
> > ++
> >  1 file changed, 39 insertions(+)
>
> It would be nice to have an Ack from a DT maintainer on this, since it's
> adding new compatible strings...

I did...

Rob
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v16 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device

2018-10-01 Thread Will Deacon
Hi Vivek,

On Thu, Aug 30, 2018 at 08:15:38PM +0530, Vivek Gautam wrote:
> From: Sricharan R 
> 
> The smmu device probe/remove and add/remove master device callbacks
> gets called when the smmu is not linked to its master, that is without
> the context of the master device. So calling runtime apis in those places
> separately.
> Global locks are also initialized before enabling runtime pm as the
> runtime_resume() calls device_reset() which does tlb_sync_global()
> that ultimately requires locks to be initialized.
> 
> Signed-off-by: Sricharan R 
> [vivek: Cleanup pm runtime calls]
> Signed-off-by: Vivek Gautam 
> Reviewed-by: Tomasz Figa 
> Tested-by: Srinivas Kandagatla 
> ---
>  drivers/iommu/arm-smmu.c | 89 
> +++-
>  1 file changed, 81 insertions(+), 8 deletions(-)

This doesn't apply on my tree[1], possibly because I've got Robin's non-strict
invalidation queued there. However, that got me thinking -- how does this
work in conjunction with the timer-based TLB invalidation? Do we need to
rpm_{get,put} around flush_iotlb_all()? If so, do we still need the calls
in map/unmap when non-strict mode is in use?

Will

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH 09/13] drm/msm: Use the DRM common Scheduler

2018-10-01 Thread Sharat Masetty
This patch hooks up the DRM gpu scheduler to the msm DRM driver. The
most noticeable changes to the driver are as follows. The submit path is
split into two parts, in the user context the submit(job) is created and
added to one of the entity's scheduler run queue. The scheduler then
tries to drain the queue by submitting the jobs the hardware to act upon.
The submit job sits on the scheduler queue until all the dependent
fences are waited upon successfully.

We have one scheduler instance per ring. The submitqueues will host a
scheduler entity object. This entity will be mapped to the scheduler's
default runqueue. This should be good for now, but in future it is possible
to extend the API to allow for scheduling amongst the submitqueues on the
same ring.

With this patch the role of the struct_mutex lock has been greatly reduced in
scope in the submit path, evidently when actually putting the job on the
ringbuffer. This should enable us with increased parallelism in the
driver which should translate to better performance overall hopefully.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/msm/Kconfig   |   1 +
 drivers/gpu/drm/msm/Makefile  |   3 +-
 drivers/gpu/drm/msm/msm_drv.h |   3 +-
 drivers/gpu/drm/msm/msm_gem.c |   8 +-
 drivers/gpu/drm/msm/msm_gem.h |   6 +
 drivers/gpu/drm/msm/msm_gem_submit.c  | 138 +--
 drivers/gpu/drm/msm/msm_gpu.c |  72 ++--
 drivers/gpu/drm/msm/msm_gpu.h |   2 +
 drivers/gpu/drm/msm/msm_ringbuffer.c  |   7 +
 drivers/gpu/drm/msm/msm_ringbuffer.h  |   3 +
 drivers/gpu/drm/msm/msm_sched.c   | 313 ++
 drivers/gpu/drm/msm/msm_sched.h   |  18 ++
 drivers/gpu/drm/msm/msm_submitqueue.c |  18 +-
 13 files changed, 507 insertions(+), 85 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/msm_sched.c
 create mode 100644 drivers/gpu/drm/msm/msm_sched.h

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 38cbde9..0d01810 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -15,6 +15,7 @@ config DRM_MSM
select SND_SOC_HDMI_CODEC if SND_SOC
select SYNC_FILE
select PM_OPP
+   select DRM_SCHED
default y
help
  DRM/KMS driver for MSM/snapdragon.
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index cd40c05..71ed921 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -60,7 +60,8 @@ msm-y := \
msm_perf.o \
msm_rd.o \
msm_ringbuffer.o \
-   msm_submitqueue.o
+   msm_submitqueue.o \
+   msm_sched.o
 
 msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o
 
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index b2da1fb..e461a9c 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -213,8 +213,7 @@ struct drm_gem_object *msm_gem_prime_import_sg_table(struct 
drm_device *dev,
 int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv);
 int msm_gem_sync_object(struct drm_gem_object *obj,
struct msm_fence_context *fctx, bool exclusive);
-void msm_gem_move_to_active(struct drm_gem_object *obj,
-   struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence);
+void msm_gem_move_to_active(struct drm_gem_object *obj, struct msm_gpu *gpu);
 void msm_gem_move_to_inactive(struct drm_gem_object *obj);
 int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t 
*timeout);
 int msm_gem_cpu_fini(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index f583bb4..7a12f30 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -663,16 +663,12 @@ int msm_gem_sync_object(struct drm_gem_object *obj,
return 0;
 }
 
-void msm_gem_move_to_active(struct drm_gem_object *obj,
-   struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence)
+void msm_gem_move_to_active(struct drm_gem_object *obj, struct msm_gpu *gpu)
 {
struct msm_gem_object *msm_obj = to_msm_bo(obj);
WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED);
msm_obj->gpu = gpu;
-   if (exclusive)
-   reservation_object_add_excl_fence(msm_obj->resv, fence);
-   else
-   reservation_object_add_shared_fence(msm_obj->resv, fence);
+
list_del_init(_obj->mm_list);
list_add_tail(_obj->mm_list, >active_list);
 }
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index cae3aa5..8c6269f 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -20,6 +20,7 @@
 
 #include 
 #include 
+#include 
 #include "msm_drv.h"
 
 /* Additional internal-use only BO flags: */
@@ -136,6 +137,7 @@ enum msm_gem_lock {
  * lasts for the duration of the submit-ioctl.
  */
 struct msm_gem_submit {
+   struct drm_sched_job sched_job;
struct drm_device *dev;
struct 

[Freedreno] [PATCH 13/13] drm/msm: Implement better timeout detection

2018-10-01 Thread Sharat Masetty
The base scheduler patch has barebones timeout implementation, it does
not account for issues like starvation on lower priority rings. This
patch enables more accurate measurement on time spent on each
ringbuffer, thereby helping us with better timeout detection mechanism.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 29 +
 drivers/gpu/drm/msm/adreno/adreno_gpu.c   |  3 +++
 drivers/gpu/drm/msm/msm_ringbuffer.h  |  2 ++
 drivers/gpu/drm/msm/msm_sched.c   | 42 +++
 4 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c 
b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
index 6a3c560..8bf81c1c 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
@@ -165,6 +165,33 @@ void a5xx_preempt_trigger(struct msm_gpu *gpu)
gpu_write(gpu, REG_A5XX_CP_CONTEXT_SWITCH_CNTL, 1);
 }
 
+static void update_ring_timestamps(struct msm_ringbuffer *prev_ring,
+   struct msm_ringbuffer *cur_ring)
+{
+   unsigned long flags;
+
+   /*
+* For the outgoing ring(prev_ring), capture the last sample of time
+* spent on this ring and add it to the ring's total active_time.
+*/
+   spin_lock_irqsave(_ring->lock, flags);
+
+   prev_ring->active_time += jiffies_delta_to_msecs(jiffies -
+   prev_ring->last_ts);
+
+   spin_unlock_irqrestore(_ring->lock, flags);
+
+   /*
+* For the incoming ring(cur_ring), save the new current timestamp to
+* restart active time measurement
+*/
+   spin_lock_irqsave(_ring->lock, flags);
+
+   cur_ring->last_ts = jiffies_to_msecs(jiffies);
+
+   spin_unlock_irqrestore(_ring->lock, flags);
+}
+
 void a5xx_preempt_irq(struct msm_gpu *gpu)
 {
uint32_t status;
@@ -194,6 +221,8 @@ void a5xx_preempt_irq(struct msm_gpu *gpu)
return;
}
 
+   update_ring_timestamps(a5xx_gpu->cur_ring, a5xx_gpu->next_ring);
+
a5xx_gpu->cur_ring = a5xx_gpu->next_ring;
a5xx_gpu->next_ring = NULL;
 
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 17d0506..f8b5f4a 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -212,6 +212,9 @@ int adreno_hw_init(struct msm_gpu *gpu)
/* reset completed fence seqno: */
ring->memptrs->fence = ring->seqno;
ring->memptrs->rptr = 0;
+
+   ring->last_ts = 0;
+   ring->active_time = 0;
}
 
/*
diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h 
b/drivers/gpu/drm/msm/msm_ringbuffer.h
index 10ae4a8..27e0ab2 100644
--- a/drivers/gpu/drm/msm/msm_ringbuffer.h
+++ b/drivers/gpu/drm/msm/msm_ringbuffer.h
@@ -46,6 +46,8 @@ struct msm_ringbuffer {
struct mutex fence_idr_lock;
spinlock_t lock;
struct drm_gpu_scheduler sched;
+   u32 last_ts;
+   u32 active_time;
 };
 
 struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu *gpu, int id,
diff --git a/drivers/gpu/drm/msm/msm_sched.c b/drivers/gpu/drm/msm/msm_sched.c
index 8b805ce..70b7713 100644
--- a/drivers/gpu/drm/msm/msm_sched.c
+++ b/drivers/gpu/drm/msm/msm_sched.c
@@ -191,6 +191,9 @@ static void msm_sched_timedout_job(struct drm_sched_job 
*bad_job)
struct msm_gem_submit *submit = to_msm_gem_submit(bad_job);
struct msm_gpu *gpu = submit->gpu;
struct msm_ringbuffer *ring = submit->ring;
+   struct drm_gpu_scheduler *sched = >sched;
+   unsigned long flags;
+   u32 total_time = 0;
 
/*
 * If this submission completed in the mean time, then the timeout is
@@ -199,6 +202,23 @@ static void msm_sched_timedout_job(struct drm_sched_job 
*bad_job)
if (submit->seqno <= submit->ring->memptrs->fence)
return;
 
+   spin_lock_irqsave(>lock, flags);
+
+   total_time = ring->active_time;
+
+   /* Measure the last sample only if this is the active ring */
+   if (ring == gpu->funcs->active_ring(gpu))
+   total_time += jiffies_delta_to_msecs(jiffies - ring->last_ts);
+
+   spin_unlock_irqrestore(>lock, flags);
+
+   if (total_time < sched->timeout) {
+   schedule_delayed_work(_job->work_tdr,
+   msecs_to_jiffies(sched->timeout - total_time));
+   return;
+   }
+
+   /* Timeout occurred, go for a recovery */
dev_err(>pdev->dev, "%s: hangcheck detected gpu lockup rb %d!\n",
gpu->name, ring->id);
dev_err(>pdev->dev, "%s: completed fence: %u\n",
@@ -231,11 +251,33 @@ static void msm_sched_free_job(struct drm_sched_job 
*sched_job)
msm_gem_submit_free(submit);
 }
 
+static void msm_sched_timeout_start(struct drm_sched_job *sched_job)
+{
+   struct msm_gem_submit *submit = to_msm_gem_submit(sched_job);
+   

[Freedreno] [PATCH 11/13] drm/scheduler: Add a timeout_start_notify function op

2018-10-01 Thread Sharat Masetty
Add an optional backend function op which will let the scheduler clients
know when the timeout work got scheduled for a job. This will help
drivers with multiple schedulers(one per ring) measure time spent on the
ring accurately, eventually helping with better timeout detection.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 16 +---
 include/drm/gpu_scheduler.h   |  6 ++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c 
b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index bf0e0c9..f5534ff 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -69,6 +69,16 @@ static void drm_sched_rq_remove_entity(struct drm_sched_rq 
*rq,
spin_unlock(>lock);
 }
 
+static void drm_sched_queue_delayed_work(struct drm_sched_job *s_job)
+{
+   struct drm_gpu_scheduler *sched = s_job->sched;
+
+   schedule_delayed_work(_job->work_tdr, sched->timeout);
+
+   if (sched->ops->timeout_start_notify)
+   sched->ops->timeout_start_notify(s_job);
+}
+
 /**
  * Select an entity which could provide a job to run
  *
@@ -467,7 +477,7 @@ static void drm_sched_job_finish(struct work_struct *work)
struct drm_sched_job, node);
 
if (next)
-   schedule_delayed_work(>work_tdr, sched->timeout);
+   drm_sched_queue_delayed_work(next);
}
spin_unlock(>job_list_lock);
dma_fence_put(_job->s_fence->finished);
@@ -494,7 +504,7 @@ static void drm_sched_job_begin(struct drm_sched_job *s_job)
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
list_first_entry_or_null(>ring_mirror_list,
 struct drm_sched_job, node) == s_job)
-   schedule_delayed_work(_job->work_tdr, sched->timeout);
+   drm_sched_queue_delayed_work(s_job);
spin_unlock(>job_list_lock);
 }
 
@@ -560,7 +570,7 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
s_job = list_first_entry_or_null(>ring_mirror_list,
 struct drm_sched_job, node);
if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
-   schedule_delayed_work(_job->work_tdr, sched->timeout);
+   drm_sched_queue_delayed_work(s_job);
 
list_for_each_entry_safe(s_job, tmp, >ring_mirror_list, node) {
struct drm_sched_fence *s_fence = s_job->s_fence;
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index dec6558..5c59c38 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -157,6 +157,12 @@ struct drm_sched_backend_ops {
 * it's time to clean it up.
 */
void (*free_job)(struct drm_sched_job *sched_job);
+
+   /*
+* (Optional) Called to let the driver know that a timeout detection
+* timer has been started for this job.
+*/
+   void (*timeout_start_notify)(struct drm_sched_job *sched_job);
 };
 
 /**
-- 
1.9.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH 12/13] jiffies: add utility function to calculate delta in ms

2018-10-01 Thread Sharat Masetty
From: Matteo Croce 

add jiffies_delta_to_msecs() helper func to calculate the delta between
two times and eventually 0 if negative.

Suggested-by: Eric Dumazet 
Signed-off-by: Matteo Croce 
Reviewed-by: Eric Dumazet 
Acked-by: Simon Horman 
Signed-off-by: Pablo Neira Ayuso 
---
 include/linux/jiffies.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index a27cf66..fa92824 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -447,6 +447,11 @@ static inline clock_t jiffies_delta_to_clock_t(long delta)
return jiffies_to_clock_t(max(0L, delta));
 }
 
+static inline unsigned int jiffies_delta_to_msecs(long delta)
+{
+   return jiffies_to_msecs(max(0L, delta));
+}
+
 extern unsigned long clock_t_to_jiffies(unsigned long x);
 extern u64 jiffies_64_to_clock_t(u64 x);
 extern u64 nsec_to_clock_t(u64 x);
-- 
1.9.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH 00/13] drm/msm: Hook up the DRM gpu scheduler

2018-10-01 Thread Sharat Masetty
This series is an effort to have the msm drm driver start using the DRM gpu
scheduler for its general job scheduling needs. Immediate benefits to the msm
drm driver include async fencing and improved driver performance.

Testing is still under progress, but general GPU submissions, preemption,
timeout, fault recovery and slumber all work fine with these changes when
tested using my test apps. My workspace is based on ~4.18.rc6, so these changes
should apply on tip relatively easily.

Please review and share your valuable feedback :-).

Note that I backported a change from Linus's 4.19.rc1 tree to handle jiffie
wrapping. If it's not desirable, I will squash the last few changes of the
series under drm/msm into one single commit.

Matteo Croce (1):
  jiffies: add utility function to calculate delta in ms

Sharat Masetty (12):
  drm/msm: Track GPU fences with idr
  drm/msm: Change msm_gpu_submit() API
  drm/msm: Save the ring name in the ring structure
  drm/msm: Change the name of the fence to hw_fence
  drm/msm: rearrange submit buffer objects clean up
  drm/msm: Use kzalloc for submit struct allocation
  drm/msm: Fix leak in submitqueue create
  drm/scheduler: set sched->thread to NULL in failure
  drm/msm: Use the DRM common Scheduler
  msm/drm: Remove unused code
  drm/scheduler: Add a timeout_start_notify function op
  drm/msm: Implement better timeout detection

 drivers/gpu/drm/msm/Kconfig   |   1 +
 drivers/gpu/drm/msm/Makefile  |   3 +-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c |   3 -
 drivers/gpu/drm/msm/adreno/a5xx_preempt.c |  29 +++
 drivers/gpu/drm/msm/adreno/adreno_gpu.c   |   3 +
 drivers/gpu/drm/msm/msm_drv.c |   3 +-
 drivers/gpu/drm/msm/msm_drv.h |   5 +-
 drivers/gpu/drm/msm/msm_fence.c   |  52 ++---
 drivers/gpu/drm/msm/msm_fence.h   |   5 +-
 drivers/gpu/drm/msm/msm_gem.c |  44 +---
 drivers/gpu/drm/msm/msm_gem.h |  10 +-
 drivers/gpu/drm/msm/msm_gem_submit.c  | 170 +-
 drivers/gpu/drm/msm/msm_gpu.c | 264 --
 drivers/gpu/drm/msm/msm_gpu.h |  11 +-
 drivers/gpu/drm/msm/msm_ringbuffer.c  |  17 +-
 drivers/gpu/drm/msm/msm_ringbuffer.h  |   7 +
 drivers/gpu/drm/msm/msm_sched.c   | 355 ++
 drivers/gpu/drm/msm/msm_sched.h   |  18 ++
 drivers/gpu/drm/msm/msm_submitqueue.c |  16 +-
 drivers/gpu/drm/scheduler/gpu_scheduler.c |  22 +-
 include/drm/gpu_scheduler.h   |   6 +
 include/linux/jiffies.h   |   5 +
 22 files changed, 662 insertions(+), 387 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/msm_sched.c
 create mode 100644 drivers/gpu/drm/msm/msm_sched.h

--
1.9.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH 10/13] msm/drm: Remove unused code

2018-10-01 Thread Sharat Masetty
With the introduction of the scheduler, most of the code related to
timeout detection, recovery and submission retirement are no longer
needed in the msm driver. This patch simply removes the no longer used
code.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c |   3 -
 drivers/gpu/drm/msm/msm_drv.h |   2 -
 drivers/gpu/drm/msm/msm_fence.c   |  22 
 drivers/gpu/drm/msm/msm_gem.c |  36 --
 drivers/gpu/drm/msm/msm_gpu.c | 204 --
 drivers/gpu/drm/msm/msm_gpu.h |   6 -
 6 files changed, 273 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index d39400e..6f5a4c5 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1034,9 +1034,6 @@ static void a5xx_fault_detect_irq(struct msm_gpu *gpu)
gpu_read64(gpu, REG_A5XX_CP_IB2_BASE, REG_A5XX_CP_IB2_BASE_HI),
gpu_read(gpu, REG_A5XX_CP_IB2_BUFSZ));
 
-   /* Turn off the hangcheck timer to keep it from bothering us */
-   del_timer(>hangcheck_timer);
-
queue_work(priv->wq, >recover_work);
 }
 
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index e461a9c..9004738 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -211,8 +211,6 @@ struct drm_gem_object *msm_gem_prime_import_sg_table(struct 
drm_device *dev,
 void *msm_gem_get_vaddr_active(struct drm_gem_object *obj);
 void msm_gem_put_vaddr(struct drm_gem_object *obj);
 int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv);
-int msm_gem_sync_object(struct drm_gem_object *obj,
-   struct msm_fence_context *fctx, bool exclusive);
 void msm_gem_move_to_active(struct drm_gem_object *obj, struct msm_gpu *gpu);
 void msm_gem_move_to_inactive(struct drm_gem_object *obj);
 int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t 
*timeout);
diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 0e7912b..d5bba25 100644
--- a/drivers/gpu/drm/msm/msm_fence.c
+++ b/drivers/gpu/drm/msm/msm_fence.c
@@ -44,11 +44,6 @@ void msm_fence_context_free(struct msm_fence_context *fctx)
kfree(fctx);
 }
 
-static inline bool fence_completed(struct msm_fence_context *fctx, uint32_t 
fence)
-{
-   return (int32_t)(fctx->completed_fence - fence) >= 0;
-}
-
 /* legacy path for WAIT_FENCE ioctl: */
 int msm_wait_fence(struct msm_ringbuffer *ring, uint32_t fence_id,
ktime_t *timeout)
@@ -86,16 +81,6 @@ int msm_wait_fence(struct msm_ringbuffer *ring, uint32_t 
fence_id,
return ret;
 }
 
-/* called from workqueue */
-void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence)
-{
-   spin_lock(>spinlock);
-   fctx->completed_fence = max(fence, fctx->completed_fence);
-   spin_unlock(>spinlock);
-
-   wake_up_all(>event);
-}
-
 struct msm_fence {
struct dma_fence base;
struct msm_fence_context *fctx;
@@ -122,17 +107,10 @@ static bool msm_fence_enable_signaling(struct dma_fence 
*fence)
return true;
 }
 
-static bool msm_fence_signaled(struct dma_fence *fence)
-{
-   struct msm_fence *f = to_msm_fence(fence);
-   return fence_completed(f->fctx, f->base.seqno);
-}
-
 static const struct dma_fence_ops msm_fence_ops = {
.get_driver_name = msm_fence_get_driver_name,
.get_timeline_name = msm_fence_get_timeline_name,
.enable_signaling = msm_fence_enable_signaling,
-   .signaled = msm_fence_signaled,
.wait = dma_fence_default_wait,
.release = dma_fence_free,
 };
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 7a12f30..e916c00 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -627,42 +627,6 @@ void msm_gem_vunmap(struct drm_gem_object *obj, enum 
msm_gem_lock subclass)
mutex_unlock(_obj->lock);
 }
 
-/* must be called before _move_to_active().. */
-int msm_gem_sync_object(struct drm_gem_object *obj,
-   struct msm_fence_context *fctx, bool exclusive)
-{
-   struct msm_gem_object *msm_obj = to_msm_bo(obj);
-   struct reservation_object_list *fobj;
-   struct dma_fence *fence;
-   int i, ret;
-
-   fobj = reservation_object_get_list(msm_obj->resv);
-   if (!fobj || (fobj->shared_count == 0)) {
-   fence = reservation_object_get_excl(msm_obj->resv);
-   /* don't need to wait on our own fences, since ring is fifo */
-   if (fence && (fence->context != fctx->context)) {
-   ret = dma_fence_wait(fence, true);
-   if (ret)
-   return ret;
-   }
-   }
-
-   if (!exclusive || !fobj)
-   return 0;
-
-   for (i = 0; i < fobj->shared_count; i++) {
-   fence = rcu_dereference_protected(fobj->shared[i],
-   

[Freedreno] [PATCH 08/13] drm/scheduler: set sched->thread to NULL in failure

2018-10-01 Thread Sharat Masetty
In cases where the scheduler instance is used a base object of another
vendor driver object, it's not clear if the driver can call sched cleanup on
the fail path. Set the sched->thread to NULL, so that the vendor driver
can safely call drm_sched_fini() during cleanup.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c 
b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 44d4807..bf0e0c9 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -760,7 +760,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
   long timeout,
   const char *name)
 {
-   int i;
+   int i, ret;
sched->ops = ops;
sched->hw_submission_limit = hw_submission;
sched->name = name;
@@ -779,8 +779,10 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
/* Each scheduler will run on a seperate kernel thread */
sched->thread = kthread_run(drm_sched_main, sched, sched->name);
if (IS_ERR(sched->thread)) {
+   ret = PTR_ERR(sched->thread);
+   sched->thread = NULL;
DRM_ERROR("Failed to create scheduler for %s.\n", name);
-   return PTR_ERR(sched->thread);
+   return ret;
}
 
return 0;
-- 
1.9.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH 01/13] drm/msm: Track GPU fences with idr

2018-10-01 Thread Sharat Masetty
Track the GPU fences created at submit time with idr instead of the ring
the sequence number. This helps with easily changing the underlying
fence to something we don't truly own, like the scheduler fence.

Also move the GPU fence allocation to msm_gpu_submit() and have
the function return the fence. This suits well when integrating with the
GPU scheduler.

Additionally remove the non-interruptible wait variant from msm_wait_fence()
as it is not used.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/msm/msm_drv.c|  3 +--
 drivers/gpu/drm/msm/msm_fence.c  | 30 ++
 drivers/gpu/drm/msm/msm_fence.h  |  5 +++--
 drivers/gpu/drm/msm/msm_gem.h|  1 +
 drivers/gpu/drm/msm/msm_gem_submit.c | 26 ++
 drivers/gpu/drm/msm/msm_gpu.c| 10 --
 drivers/gpu/drm/msm/msm_gpu.h|  4 ++--
 drivers/gpu/drm/msm/msm_ringbuffer.c |  5 +
 drivers/gpu/drm/msm/msm_ringbuffer.h |  1 +
 9 files changed, 53 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 021a0b6..8eaa1bd 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -746,8 +746,7 @@ static int msm_ioctl_wait_fence(struct drm_device *dev, 
void *data,
if (!queue)
return -ENOENT;
 
-   ret = msm_wait_fence(gpu->rb[queue->prio]->fctx, args->fence, ,
-   true);
+   ret = msm_wait_fence(gpu->rb[queue->prio], args->fence, );
 
msm_submitqueue_put(queue);
return ret;
diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 349c12f..0e7912b 100644
--- a/drivers/gpu/drm/msm/msm_fence.c
+++ b/drivers/gpu/drm/msm/msm_fence.c
@@ -50,41 +50,39 @@ static inline bool fence_completed(struct msm_fence_context 
*fctx, uint32_t fenc
 }
 
 /* legacy path for WAIT_FENCE ioctl: */
-int msm_wait_fence(struct msm_fence_context *fctx, uint32_t fence,
-   ktime_t *timeout, bool interruptible)
+int msm_wait_fence(struct msm_ringbuffer *ring, uint32_t fence_id,
+   ktime_t *timeout)
 {
+   struct dma_fence *fence;
int ret;
 
-   if (fence > fctx->last_fence) {
-   DRM_ERROR("%s: waiting on invalid fence: %u (of %u)\n",
-   fctx->name, fence, fctx->last_fence);
-   return -EINVAL;
+   rcu_read_lock();
+   fence = idr_find(>fence_idr, fence_id);
+
+   if (!fence || !dma_fence_get_rcu(fence)) {
+   rcu_read_unlock();
+   return 0;
}
+   rcu_read_unlock();
 
if (!timeout) {
/* no-wait: */
-   ret = fence_completed(fctx, fence) ? 0 : -EBUSY;
+   ret = dma_fence_is_signaled(fence) ? 0 : -EBUSY;
} else {
unsigned long remaining_jiffies = timeout_to_jiffies(timeout);
 
-   if (interruptible)
-   ret = wait_event_interruptible_timeout(fctx->event,
-   fence_completed(fctx, fence),
-   remaining_jiffies);
-   else
-   ret = wait_event_timeout(fctx->event,
-   fence_completed(fctx, fence),
-   remaining_jiffies);
+   ret = dma_fence_wait_timeout(fence, true, remaining_jiffies);
 
if (ret == 0) {
DBG("timeout waiting for fence: %u (completed: %u)",
-   fence, fctx->completed_fence);
+   fence_id, ring->memptrs->fence);
ret = -ETIMEDOUT;
} else if (ret != -ERESTARTSYS) {
ret = 0;
}
}
 
+   dma_fence_put(fence);
return ret;
 }
 
diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h
index b9fe059..a8133f7 100644
--- a/drivers/gpu/drm/msm/msm_fence.h
+++ b/drivers/gpu/drm/msm/msm_fence.h
@@ -19,6 +19,7 @@
 #define __MSM_FENCE_H__
 
 #include "msm_drv.h"
+#include "msm_ringbuffer.h"
 
 struct msm_fence_context {
struct drm_device *dev;
@@ -35,8 +36,8 @@ struct msm_fence_context * msm_fence_context_alloc(struct 
drm_device *dev,
const char *name);
 void msm_fence_context_free(struct msm_fence_context *fctx);
 
-int msm_wait_fence(struct msm_fence_context *fctx, uint32_t fence,
-   ktime_t *timeout, bool interruptible);
+int msm_wait_fence(struct msm_ringbuffer *ring, uint32_t fence_id,
+   ktime_t *timeout);
 void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence);
 
 struct dma_fence * msm_fence_alloc(struct msm_fence_context *fctx);
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index c5d9bd3..287f974 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -143,6 +143,7 @@ struct msm_gem_submit {
   

[Freedreno] [PATCH 05/13] drm/msm: rearrange submit buffer objects clean up

2018-10-01 Thread Sharat Masetty
The submit path in msm_gpu_submit() takes a reference to the buffer
object and the iova. This should not be needed with a little bit of
code rearrangement. We still keep the same semantic of a valid GPU
submission holding a reference to the  base object and the iova until
retirement.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/msm/msm_gem_submit.c | 20 ++--
 drivers/gpu/drm/msm/msm_gpu.c|  8 
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c 
b/drivers/gpu/drm/msm/msm_gem_submit.c
index fd28ace..a7c8cbc 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -68,9 +68,21 @@ static struct msm_gem_submit *submit_create(struct 
drm_device *dev,
 
 void msm_gem_submit_free(struct msm_gem_submit *submit)
 {
+   int i;
+
idr_remove(>ring->fence_idr, submit->out_fence_id);
 
dma_fence_put(submit->hw_fence);
+
+   for (i = 0; i < submit->nr_bos; i++) {
+   struct msm_gem_object *msm_obj = submit->bos[i].obj;
+
+   if (submit->bos[i].flags & BO_PINNED)
+   msm_gem_put_iova(_obj->base, submit->gpu->aspace);
+
+   drm_gem_object_put(_obj->base);
+   }
+
list_del(>node);
put_pid(submit->pid);
msm_submitqueue_put(submit->queue);
@@ -398,9 +410,13 @@ static void submit_cleanup(struct msm_gem_submit *submit)
 
for (i = 0; i < submit->nr_bos; i++) {
struct msm_gem_object *msm_obj = submit->bos[i].obj;
-   submit_unlock_unpin_bo(submit, i, false);
+
+   if (submit->bos[i].flags & BO_LOCKED) {
+   ww_mutex_unlock(_obj->resv->lock);
+   submit->bos[i].flags &= ~BO_LOCKED;
+   }
+
list_del_init(_obj->submit_entry);
-   drm_gem_object_unreference(_obj->base);
}
 
ww_acquire_fini(>ticket);
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 449cc23..cd5fe49 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -551,8 +551,6 @@ static void retire_submit(struct msm_gpu *gpu, struct 
msm_gem_submit *submit)
struct msm_gem_object *msm_obj = submit->bos[i].obj;
/* move to inactive: */
msm_gem_move_to_inactive(_obj->base);
-   msm_gem_put_iova(_obj->base, gpu->aspace);
-   drm_gem_object_put(_obj->base);
}
 
pm_runtime_mark_last_busy(>pdev->dev);
@@ -630,18 +628,12 @@ struct dma_fence *msm_gpu_submit(struct msm_gem_submit 
*submit)
 
for (i = 0; i < submit->nr_bos; i++) {
struct msm_gem_object *msm_obj = submit->bos[i].obj;
-   uint64_t iova;
 
/* can't happen yet.. but when we add 2d support we'll have
 * to deal w/ cross-ring synchronization:
 */
WARN_ON(is_active(msm_obj) && (msm_obj->gpu != gpu));
 
-   /* submit takes a reference to the bo and iova until retired: */
-   drm_gem_object_get(_obj->base);
-   msm_gem_get_iova(_obj->base,
-   submit->gpu->aspace, );
-
if (submit->bos[i].flags & MSM_SUBMIT_BO_WRITE)
msm_gem_move_to_active(_obj->base, gpu, true, 
submit->hw_fence);
else if (submit->bos[i].flags & MSM_SUBMIT_BO_READ)
-- 
1.9.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH 06/13] drm/msm: Use kzalloc for submit struct allocation

2018-10-01 Thread Sharat Masetty
This patch changes to kzalloc and avoids setting individual submit
struct fields to zero manually.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/msm/msm_gem_submit.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c 
b/drivers/gpu/drm/msm/msm_gem_submit.c
index a7c8cbc..7931c2a 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -41,24 +41,19 @@ static struct msm_gem_submit *submit_create(struct 
drm_device *dev,
if (sz > SIZE_MAX)
return NULL;
 
-   submit = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
+   submit = kzalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
if (!submit)
return NULL;
 
submit->dev = dev;
submit->gpu = gpu;
submit->ctx = ctx;
-   submit->hw_fence = NULL;
submit->out_fence_id = -1;
submit->pid = get_pid(task_pid(current));
submit->cmd = (void *)>bos[nr_bos];
submit->queue = queue;
submit->ring = gpu->rb[queue->prio];
 
-   /* initially, until copy_from_user() and bo lookup succeeds: */
-   submit->nr_bos = 0;
-   submit->nr_cmds = 0;
-
INIT_LIST_HEAD(>node);
INIT_LIST_HEAD(>bo_list);
ww_acquire_init(>ticket, _ww_class);
-- 
1.9.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH 02/13] drm/msm: Change msm_gpu_submit() API

2018-10-01 Thread Sharat Masetty
When the scheduler comes into picture, the msm_gpu_submit() will be
called from the scheduler worker thread. So save the necessary information
into msm job structure for use at the actual GPU submission time. This
also simplifies the msm_gpu_submit() API.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/msm/msm_gem.h| 1 +
 drivers/gpu/drm/msm/msm_gem_submit.c | 8 +---
 drivers/gpu/drm/msm/msm_gpu.c| 8 
 drivers/gpu/drm/msm/msm_gpu.h| 3 +--
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 287f974..289abe5 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -137,6 +137,7 @@ enum msm_gem_lock {
  */
 struct msm_gem_submit {
struct drm_device *dev;
+   struct msm_file_private *ctx;
struct msm_gpu *gpu;
struct list_head node;   /* node in ring submit list */
struct list_head bo_list;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c 
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 00e6347..14906b9 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -32,7 +32,7 @@
 
 static struct msm_gem_submit *submit_create(struct drm_device *dev,
struct msm_gpu *gpu, struct msm_gpu_submitqueue *queue,
-   uint32_t nr_bos, uint32_t nr_cmds)
+   uint32_t nr_bos, uint32_t nr_cmds, struct msm_file_private *ctx)
 {
struct msm_gem_submit *submit;
uint64_t sz = sizeof(*submit) + ((u64)nr_bos * sizeof(submit->bos[0])) +
@@ -47,6 +47,7 @@ static struct msm_gem_submit *submit_create(struct drm_device 
*dev,
 
submit->dev = dev;
submit->gpu = gpu;
+   submit->ctx = ctx;
submit->fence = NULL;
submit->out_fence_id = -1;
submit->pid = get_pid(task_pid(current));
@@ -474,7 +475,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
}
}
 
-   submit = submit_create(dev, gpu, queue, args->nr_bos, args->nr_cmds);
+   submit = submit_create(dev, gpu, queue, args->nr_bos,
+   args->nr_cmds, ctx);
if (!submit) {
ret = -ENOMEM;
goto out_unlock;
@@ -560,7 +562,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 
submit->nr_cmds = i;
 
-   msm_gpu_submit(gpu, submit, ctx);
+   msm_gpu_submit(submit);
if (IS_ERR(submit->fence)) {
ret = PTR_ERR(submit->fence);
submit->fence = NULL;
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index eb67172..5f9cd85 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -602,9 +602,9 @@ void msm_gpu_retire(struct msm_gpu *gpu)
 }
 
 /* add bo's to gpu's ring, and kick gpu: */
-struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu,
-   struct msm_gem_submit *submit, struct msm_file_private *ctx)
+struct dma_fence *msm_gpu_submit(struct msm_gem_submit *submit)
 {
+   struct msm_gpu *gpu = submit->gpu;
struct drm_device *dev = gpu->dev;
struct msm_drm_private *priv = dev->dev_private;
struct msm_ringbuffer *ring = submit->ring;
@@ -648,8 +648,8 @@ struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu,
msm_gem_move_to_active(_obj->base, gpu, false, 
submit->fence);
}
 
-   gpu->funcs->submit(gpu, submit, ctx);
-   priv->lastctx = ctx;
+   gpu->funcs->submit(gpu, submit, submit->ctx);
+   priv->lastctx = submit->ctx;
 
hangcheck_timer_reset(gpu);
 
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index b562b25..dd55979 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -235,8 +235,7 @@ int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint32_t 
*activetime,
uint32_t *totaltime, uint32_t ncntrs, uint32_t *cntrs);
 
 void msm_gpu_retire(struct msm_gpu *gpu);
-struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu,
-   struct msm_gem_submit *submit, struct msm_file_private *ctx);
+struct dma_fence *msm_gpu_submit(struct msm_gem_submit *submit);
 
 int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs,
-- 
1.9.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH 03/13] drm/msm: Save the ring name in the ring structure

2018-10-01 Thread Sharat Masetty
The scheduler needs an instance name mostly for debug purposes. Save the
name in the ringbuffer instead of a stack variable, so that the name
can be shared with the scheduler.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/msm/msm_ringbuffer.c | 5 ++---
 drivers/gpu/drm/msm/msm_ringbuffer.h | 1 +
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c 
b/drivers/gpu/drm/msm/msm_ringbuffer.c
index 734f2b8..0889766 100644
--- a/drivers/gpu/drm/msm/msm_ringbuffer.c
+++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
@@ -22,7 +22,6 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu 
*gpu, int id,
void *memptrs, uint64_t memptrs_iova)
 {
struct msm_ringbuffer *ring;
-   char name[32];
int ret;
 
/* We assume everwhere that MSM_GPU_RINGBUFFER_SZ is a power of 2 */
@@ -55,9 +54,9 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu 
*gpu, int id,
INIT_LIST_HEAD(>submits);
spin_lock_init(>lock);
 
-   snprintf(name, sizeof(name), "gpu-ring-%d", ring->id);
+   snprintf(ring->name, sizeof(ring->name), "msm-gpu-ring-%d", ring->id);
 
-   ring->fctx = msm_fence_context_alloc(gpu->dev, name);
+   ring->fctx = msm_fence_context_alloc(gpu->dev, ring->name);
 
idr_init(>fence_idr);
 
diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h 
b/drivers/gpu/drm/msm/msm_ringbuffer.h
index b74a0a9..523373b 100644
--- a/drivers/gpu/drm/msm/msm_ringbuffer.h
+++ b/drivers/gpu/drm/msm/msm_ringbuffer.h
@@ -31,6 +31,7 @@ struct msm_rbmemptrs {
 struct msm_ringbuffer {
struct msm_gpu *gpu;
int id;
+   char name[16];
struct drm_gem_object *bo;
uint32_t *start, *end, *cur, *next;
struct list_head submits;
-- 
1.9.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH 04/13] drm/msm: Change the name of the fence to hw_fence

2018-10-01 Thread Sharat Masetty
We are going to soon deal with lot more fences, so change the generic
name 'fence' to hw_fence to denote association with an actual hardware
submission.

Signed-off-by: Sharat Masetty 
---
 drivers/gpu/drm/msm/msm_gem.h|  2 +-
 drivers/gpu/drm/msm/msm_gem_submit.c | 17 -
 drivers/gpu/drm/msm/msm_gpu.c| 16 
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 289abe5..cae3aa5 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -143,7 +143,7 @@ struct msm_gem_submit {
struct list_head bo_list;
struct ww_acquire_ctx ticket;
uint32_t seqno; /* Sequence number of the submit on the ring */
-   struct dma_fence *fence;
+   struct dma_fence *hw_fence;
int out_fence_id;
struct msm_gpu_submitqueue *queue;
struct pid *pid;/* submitting process */
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c 
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 14906b9..fd28ace 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -48,7 +48,7 @@ static struct msm_gem_submit *submit_create(struct drm_device 
*dev,
submit->dev = dev;
submit->gpu = gpu;
submit->ctx = ctx;
-   submit->fence = NULL;
+   submit->hw_fence = NULL;
submit->out_fence_id = -1;
submit->pid = get_pid(task_pid(current));
submit->cmd = (void *)>bos[nr_bos];
@@ -70,7 +70,7 @@ void msm_gem_submit_free(struct msm_gem_submit *submit)
 {
idr_remove(>ring->fence_idr, submit->out_fence_id);
 
-   dma_fence_put(submit->fence);
+   dma_fence_put(submit->hw_fence);
list_del(>node);
put_pid(submit->pid);
msm_submitqueue_put(submit->queue);
@@ -563,9 +563,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
submit->nr_cmds = i;
 
msm_gpu_submit(submit);
-   if (IS_ERR(submit->fence)) {
-   ret = PTR_ERR(submit->fence);
-   submit->fence = NULL;
+   if (IS_ERR(submit->hw_fence)) {
+   ret = PTR_ERR(submit->hw_fence);
+   submit->hw_fence = NULL;
goto out;
}
 
@@ -573,9 +573,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 * No protection should be okay here since this is protected by the big
 * GPU lock.
 */
-   submit->out_fence_id = idr_alloc_cyclic(>fence_idr, submit->fence,
-   0, INT_MAX, GFP_KERNEL);
-
+   submit->out_fence_id = idr_alloc_cyclic(>fence_idr,
+   submit->hw_fence, 0, INT_MAX, GFP_KERNEL);
if (submit->out_fence_id < 0) {
ret = -ENOMEM;
goto out;
@@ -584,7 +583,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
args->fence = submit->out_fence_id;
 
if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
-   sync_file = sync_file_create(submit->fence);
+   sync_file = sync_file_create(submit->hw_fence);
if (!sync_file) {
ret = -ENOMEM;
goto out;
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 5f9cd85..449cc23 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -287,7 +287,7 @@ static void update_fences(struct msm_gpu *gpu, struct 
msm_ringbuffer *ring,
break;
 
msm_update_fence(submit->ring->fctx,
-   submit->fence->seqno);
+   submit->hw_fence->seqno);
}
 }
 
@@ -573,7 +573,7 @@ static void retire_submits(struct msm_gpu *gpu)
struct msm_ringbuffer *ring = gpu->rb[i];
 
list_for_each_entry_safe(submit, tmp, >submits, node) {
-   if (dma_fence_is_signaled(submit->fence))
+   if (dma_fence_is_signaled(submit->hw_fence))
retire_submit(gpu, submit);
}
}
@@ -612,9 +612,9 @@ struct dma_fence *msm_gpu_submit(struct msm_gem_submit 
*submit)
 
WARN_ON(!mutex_is_locked(>struct_mutex));
 
-   submit->fence = msm_fence_alloc(ring->fctx);
-   if (IS_ERR(submit->fence))
-   return submit->fence;
+   submit->hw_fence = msm_fence_alloc(ring->fctx);
+   if (IS_ERR(submit->hw_fence))
+   return submit->hw_fence;
 
pm_runtime_get_sync(>pdev->dev);
 
@@ -643,9 +643,9 @@ struct dma_fence *msm_gpu_submit(struct msm_gem_submit 
*submit)
submit->gpu->aspace, );
 
if (submit->bos[i].flags & MSM_SUBMIT_BO_WRITE)
-   msm_gem_move_to_active(_obj->base, gpu, true, 
submit->fence);
+   msm_gem_move_to_active(_obj->base, gpu, true, 
submit->hw_fence);
else if 

[Freedreno] [PATCH 07/13] drm/msm: Fix leak in submitqueue create

2018-10-01 Thread Sharat Masetty
This patch fixes a trivial leak when trying to create a submitqueue.

Signed-off-by: Sharat Masetty 
Reviewed-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/msm_submitqueue.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c 
b/drivers/gpu/drm/msm/msm_submitqueue.c
index 5115f75..325da44 100644
--- a/drivers/gpu/drm/msm/msm_submitqueue.c
+++ b/drivers/gpu/drm/msm/msm_submitqueue.c
@@ -78,8 +78,10 @@ int msm_submitqueue_create(struct drm_device *drm, struct 
msm_file_private *ctx,
queue->flags = flags;
 
if (priv->gpu) {
-   if (prio >= priv->gpu->nr_rings)
+   if (prio >= priv->gpu->nr_rings) {
+   kfree(queue);
return -EINVAL;
+   }
 
queue->prio = prio;
}
-- 
1.9.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2

2018-10-01 Thread Will Deacon
On Thu, Aug 30, 2018 at 08:15:40PM +0530, Vivek Gautam wrote:
> Add bindings doc for Qcom's smmu-v2 implementation.
> 
> Signed-off-by: Vivek Gautam 
> Reviewed-by: Tomasz Figa 
> Tested-by: Srinivas Kandagatla 
> ---
>  .../devicetree/bindings/iommu/arm,smmu.txt | 39 
> ++
>  1 file changed, 39 insertions(+)

It would be nice to have an Ack from a DT maintainer on this, since it's
adding new compatible strings...

Will

> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt 
> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 8a6ffce12af5..a6504b37cc21 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -17,10 +17,16 @@ conditions.
>  "arm,mmu-401"
>  "arm,mmu-500"
>  "cavium,smmu-v2"
> +"qcom,smmu-v2"
>  
>depending on the particular implementation and/or the
>version of the architecture implemented.
>  
> +  Qcom SoCs must contain, as below, SoC-specific compatibles
> +  along with "qcom,smmu-v2":
> +  "qcom,msm8996-smmu-v2", "qcom,smmu-v2",
> +  "qcom,sdm845-smmu-v2", "qcom,smmu-v2".
> +
>  - reg   : Base address and size of the SMMU.
>  
>  - #global-interrupts : The number of global interrupts exposed by the
> @@ -71,6 +77,22 @@ conditions.
>or using stream matching with #iommu-cells = <2>, and
>may be ignored if present in such cases.
>  
> +- clock-names:List of the names of clocks input to the device. The
> +  required list depends on particular implementation and
> +  is as follows:
> +  - for "qcom,smmu-v2":
> +- "bus": clock required for downstream bus access and
> + for the smmu ptw,
> +- "iface": clock required to access smmu's registers
> +   through the TCU's programming interface.
> +  - unspecified for other implementations.
> +
> +- clocks: Specifiers for all clocks listed in the clock-names 
> property,
> +  as per generic clock bindings.
> +
> +- power-domains:  Specifiers for power domains required to be powered on for
> +  the SMMU to operate, as per generic power domain bindings.
> +
>  ** Deprecated properties:
>  
>  - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -137,3 +159,20 @@ conditions.
>  iommu-map = <0  0 0x400>;
>  ...
>  };
> +
> + /* Qcom's arm,smmu-v2 implementation */
> + smmu4: iommu@d0 {
> + compatible = "qcom,msm8996-smmu-v2", "qcom,smmu-v2";
> + reg = <0xd0 0x1>;
> +
> + #global-interrupts = <1>;
> + interrupts = ,
> +  ,
> +  ;
> + #iommu-cells = <1>;
> + power-domains = < MDSS_GDSC>;
> +
> + clocks = < SMMU_MDP_AXI_CLK>,
> +  < SMMU_MDP_AHB_CLK>;
> + clock-names = "bus", "iface";
> + };
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v16 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops

2018-10-01 Thread Ulf Hansson
On 1 October 2018 at 12:32, Vivek Gautam  wrote:
> On Mon, Oct 1, 2018 at 11:19 AM Vivek Gautam
>  wrote:
>>
>> HI Ulf,
>>
>> On Fri, Sep 28, 2018 at 5:30 PM Ulf Hansson  wrote:
>> >
>> > On 30 August 2018 at 16:45, Vivek Gautam  
>> > wrote:
>> > > From: Sricharan R 
>> > >
>> > > The smmu needs to be functional only when the respective
>> > > master's using it are active. The device_link feature
>> > > helps to track such functional dependencies, so that the
>> > > iommu gets powered when the master device enables itself
>> > > using pm_runtime. So by adapting the smmu driver for
>> > > runtime pm, above said dependency can be addressed.
>> > >
>> > > This patch adds the pm runtime/sleep callbacks to the
>> > > driver and also the functions to parse the smmu clocks
>> > > from DT and enable them in resume/suspend.
>> > >
>> > > Also, while we enable the runtime pm add a pm sleep suspend
>> > > callback that pushes devices to low power state by turning
>> > > the clocks off in a system sleep.
>> > > Also add corresponding clock enable path in resume callback.
>> > >
>> > > Signed-off-by: Sricharan R 
>> > > Signed-off-by: Archit Taneja 
>> > > [vivek: rework for clock and pm ops]
>> > > Signed-off-by: Vivek Gautam 
>> > > Reviewed-by: Tomasz Figa 
>> > > Tested-by: Srinivas Kandagatla 
>> > > ---
>> > >  drivers/iommu/arm-smmu.c | 77 
>> > > ++--
>> > >  1 file changed, 74 insertions(+), 3 deletions(-)
>> > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> >
>> > [...]
>> >
>> > > -static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
>> > > +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
>> > >  {
>> > > struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> > > +   int ret;
>> > > +
>> > > +   ret = clk_bulk_enable(smmu->num_clks, smmu->clks);
>> > > +   if (ret)
>> > > +   return ret;
>> > >
>> > > arm_smmu_device_reset(smmu);
>> > > +
>> > > return 0;
>> > >  }
>> > >
>> > > -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
>> > > +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
>> > > +{
>> > > +   struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> > > +
>> > > +   clk_bulk_disable(smmu->num_clks, smmu->clks);
>> > > +
>> > > +   return 0;
>> > > +}
>> > > +
>> > > +static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
>> > > +{
>> > > +   if (pm_runtime_suspended(dev))
>> > > +   return 0;
>> >
>> > Looks like you should be able use pm_runtime_force_resume(), instead
>> > of using this local trick. Unless I am missing something, of course.
>> >
>> > In other words, just assign the system sleep callbacks for resume, to
>> > pm_runtime_force_resume(). And vice verse for the system suspend
>> > callbacks, pm_runtime_force_suspend(), of course.
>>
>> Thanks for the review. I will change this as suggested.
>
> Coming back at this - actually Rafael suggested _not_ to use
> pm_runtime_force_suspend/resume() when Marek had suggested
> the same [1].

I see.

> He also mentioned few caveats/limitations of using these APIs
> for system sleep ops.
> Let me know your opinion. Thanks.

>
> [1] https://lkml.org/lkml/2018/7/11/978
> [2] https://lkml.org/lkml/2018/7/23/334

Me and Rafael have been discussing these topics historically as well.
I don't want to get that discussion started again here.

If your device is attached to the PCI bus or the ACPI PM domain (and
also gets runtime PM enabled), then I suggest you to stick to the
currently suggested approach. Otherwise it should be perfectly fine to
switch to the *force helpers.

Kind regards
Uffe
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v16 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops

2018-10-01 Thread Vivek Gautam
On Mon, Oct 1, 2018 at 11:19 AM Vivek Gautam
 wrote:
>
> HI Ulf,
>
> On Fri, Sep 28, 2018 at 5:30 PM Ulf Hansson  wrote:
> >
> > On 30 August 2018 at 16:45, Vivek Gautam  
> > wrote:
> > > From: Sricharan R 
> > >
> > > The smmu needs to be functional only when the respective
> > > master's using it are active. The device_link feature
> > > helps to track such functional dependencies, so that the
> > > iommu gets powered when the master device enables itself
> > > using pm_runtime. So by adapting the smmu driver for
> > > runtime pm, above said dependency can be addressed.
> > >
> > > This patch adds the pm runtime/sleep callbacks to the
> > > driver and also the functions to parse the smmu clocks
> > > from DT and enable them in resume/suspend.
> > >
> > > Also, while we enable the runtime pm add a pm sleep suspend
> > > callback that pushes devices to low power state by turning
> > > the clocks off in a system sleep.
> > > Also add corresponding clock enable path in resume callback.
> > >
> > > Signed-off-by: Sricharan R 
> > > Signed-off-by: Archit Taneja 
> > > [vivek: rework for clock and pm ops]
> > > Signed-off-by: Vivek Gautam 
> > > Reviewed-by: Tomasz Figa 
> > > Tested-by: Srinivas Kandagatla 
> > > ---
> > >  drivers/iommu/arm-smmu.c | 77 
> > > ++--
> > >  1 file changed, 74 insertions(+), 3 deletions(-)
> > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >
> > [...]
> >
> > > -static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
> > > +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
> > >  {
> > > struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> > > +   int ret;
> > > +
> > > +   ret = clk_bulk_enable(smmu->num_clks, smmu->clks);
> > > +   if (ret)
> > > +   return ret;
> > >
> > > arm_smmu_device_reset(smmu);
> > > +
> > > return 0;
> > >  }
> > >
> > > -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
> > > +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
> > > +{
> > > +   struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> > > +
> > > +   clk_bulk_disable(smmu->num_clks, smmu->clks);
> > > +
> > > +   return 0;
> > > +}
> > > +
> > > +static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
> > > +{
> > > +   if (pm_runtime_suspended(dev))
> > > +   return 0;
> >
> > Looks like you should be able use pm_runtime_force_resume(), instead
> > of using this local trick. Unless I am missing something, of course.
> >
> > In other words, just assign the system sleep callbacks for resume, to
> > pm_runtime_force_resume(). And vice verse for the system suspend
> > callbacks, pm_runtime_force_suspend(), of course.
>
> Thanks for the review. I will change this as suggested.

Coming back at this - actually Rafael suggested _not_ to use
pm_runtime_force_suspend/resume() when Marek had suggested
the same [1].
He also mentioned few caveats/limitations of using these APIs
for system sleep ops.
Let me know your opinion. Thanks.

[1] https://lkml.org/lkml/2018/7/11/978
[2] https://lkml.org/lkml/2018/7/23/334

Best regards
Vivek
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v16 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops

2018-10-01 Thread Vivek Gautam
On Mon, Oct 1, 2018 at 3:09 PM Ulf Hansson  wrote:
>
> On 1 October 2018 at 07:49, Vivek Gautam  wrote:
> > HI Ulf,
> >
> > On Fri, Sep 28, 2018 at 5:30 PM Ulf Hansson  wrote:
> >>
> >> On 30 August 2018 at 16:45, Vivek Gautam  
> >> wrote:
> >> > From: Sricharan R 
> >> >
> >> > The smmu needs to be functional only when the respective
> >> > master's using it are active. The device_link feature
> >> > helps to track such functional dependencies, so that the
> >> > iommu gets powered when the master device enables itself
> >> > using pm_runtime. So by adapting the smmu driver for
> >> > runtime pm, above said dependency can be addressed.
> >> >
> >> > This patch adds the pm runtime/sleep callbacks to the
> >> > driver and also the functions to parse the smmu clocks
> >> > from DT and enable them in resume/suspend.
> >> >
> >> > Also, while we enable the runtime pm add a pm sleep suspend
> >> > callback that pushes devices to low power state by turning
> >> > the clocks off in a system sleep.
> >> > Also add corresponding clock enable path in resume callback.
> >> >
> >> > Signed-off-by: Sricharan R 
> >> > Signed-off-by: Archit Taneja 
> >> > [vivek: rework for clock and pm ops]
> >> > Signed-off-by: Vivek Gautam 
> >> > Reviewed-by: Tomasz Figa 
> >> > Tested-by: Srinivas Kandagatla 
> >> > ---
> >> >  drivers/iommu/arm-smmu.c | 77 
> >> > ++--
> >> >  1 file changed, 74 insertions(+), 3 deletions(-)
> >> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >>
> >> [...]
> >>
> >> > -static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
> >> > +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
> >> >  {
> >> > struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> >> > +   int ret;
> >> > +
> >> > +   ret = clk_bulk_enable(smmu->num_clks, smmu->clks);
> >> > +   if (ret)
> >> > +   return ret;
> >> >
> >> > arm_smmu_device_reset(smmu);
> >> > +
> >> > return 0;
> >> >  }
> >> >
> >> > -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
> >> > +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
> >> > +{
> >> > +   struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> >> > +
> >> > +   clk_bulk_disable(smmu->num_clks, smmu->clks);
> >> > +
> >> > +   return 0;
> >> > +}
> >> > +
> >> > +static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
> >> > +{
> >> > +   if (pm_runtime_suspended(dev))
> >> > +   return 0;
> >>
> >> Looks like you should be able use pm_runtime_force_resume(), instead
> >> of using this local trick. Unless I am missing something, of course.
> >>
> >> In other words, just assign the system sleep callbacks for resume, to
> >> pm_runtime_force_resume(). And vice verse for the system suspend
> >> callbacks, pm_runtime_force_suspend(), of course.
> >
> > Thanks for the review. I will change this as suggested.
> >
> >>
> >> > +
> >> > +   return arm_smmu_runtime_resume(dev);
> >> > +}
> >> > +
> >> > +static int __maybe_unused arm_smmu_pm_suspend(struct device *dev)
> >> > +{
> >> > +   if (pm_runtime_suspended(dev))
> >> > +   return 0;
> >> > +
> >> > +   return arm_smmu_runtime_suspend(dev);
> >> > +}
> >> > +
> >> > +static const struct dev_pm_ops arm_smmu_pm_ops = {
> >> > +   SET_SYSTEM_SLEEP_PM_OPS(arm_smmu_pm_suspend, arm_smmu_pm_resume)
> >>
> >> I am wondering if using the ->suspend|resume() callback is really
> >> "late/early" enough in the device suspend phase?
> >>
> >> Others is using the noirq phase and some is even using the syscore
> >> ops. Of course it depends on the behavior of the consumers of iommu
> >> device, and I guess not everyone is using device links, which for sure
> >> improves things in this regards as well.
> >
> > Well yes, as you said the device links should be able to take care of
> > maintaining the correct suspend/resume order of smmu and its clients,
> > or am I missing your point here?
> > Let me know and I will be happy to incorporate any suggestions.
> > Thanks
>
> If it works fine, then you may keep it as is.
>
> Just wanted to point out that if any consumers relies on the iommu to
> operational to say until the suspend-late phase, then this doesn't
> play. Then you need to move your callbacks to the corresponding same
> phase.

Although I have no means to test the suspend-late phase, tests with graphics
and display on db820 haven't shown any anomaly.

[snip]

Best regards
Vivek

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v16 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops

2018-10-01 Thread Ulf Hansson
On 1 October 2018 at 07:49, Vivek Gautam  wrote:
> HI Ulf,
>
> On Fri, Sep 28, 2018 at 5:30 PM Ulf Hansson  wrote:
>>
>> On 30 August 2018 at 16:45, Vivek Gautam  wrote:
>> > From: Sricharan R 
>> >
>> > The smmu needs to be functional only when the respective
>> > master's using it are active. The device_link feature
>> > helps to track such functional dependencies, so that the
>> > iommu gets powered when the master device enables itself
>> > using pm_runtime. So by adapting the smmu driver for
>> > runtime pm, above said dependency can be addressed.
>> >
>> > This patch adds the pm runtime/sleep callbacks to the
>> > driver and also the functions to parse the smmu clocks
>> > from DT and enable them in resume/suspend.
>> >
>> > Also, while we enable the runtime pm add a pm sleep suspend
>> > callback that pushes devices to low power state by turning
>> > the clocks off in a system sleep.
>> > Also add corresponding clock enable path in resume callback.
>> >
>> > Signed-off-by: Sricharan R 
>> > Signed-off-by: Archit Taneja 
>> > [vivek: rework for clock and pm ops]
>> > Signed-off-by: Vivek Gautam 
>> > Reviewed-by: Tomasz Figa 
>> > Tested-by: Srinivas Kandagatla 
>> > ---
>> >  drivers/iommu/arm-smmu.c | 77 
>> > ++--
>> >  1 file changed, 74 insertions(+), 3 deletions(-)
>> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>
>> [...]
>>
>> > -static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
>> > +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
>> >  {
>> > struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> > +   int ret;
>> > +
>> > +   ret = clk_bulk_enable(smmu->num_clks, smmu->clks);
>> > +   if (ret)
>> > +   return ret;
>> >
>> > arm_smmu_device_reset(smmu);
>> > +
>> > return 0;
>> >  }
>> >
>> > -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
>> > +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
>> > +{
>> > +   struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> > +
>> > +   clk_bulk_disable(smmu->num_clks, smmu->clks);
>> > +
>> > +   return 0;
>> > +}
>> > +
>> > +static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
>> > +{
>> > +   if (pm_runtime_suspended(dev))
>> > +   return 0;
>>
>> Looks like you should be able use pm_runtime_force_resume(), instead
>> of using this local trick. Unless I am missing something, of course.
>>
>> In other words, just assign the system sleep callbacks for resume, to
>> pm_runtime_force_resume(). And vice verse for the system suspend
>> callbacks, pm_runtime_force_suspend(), of course.
>
> Thanks for the review. I will change this as suggested.
>
>>
>> > +
>> > +   return arm_smmu_runtime_resume(dev);
>> > +}
>> > +
>> > +static int __maybe_unused arm_smmu_pm_suspend(struct device *dev)
>> > +{
>> > +   if (pm_runtime_suspended(dev))
>> > +   return 0;
>> > +
>> > +   return arm_smmu_runtime_suspend(dev);
>> > +}
>> > +
>> > +static const struct dev_pm_ops arm_smmu_pm_ops = {
>> > +   SET_SYSTEM_SLEEP_PM_OPS(arm_smmu_pm_suspend, arm_smmu_pm_resume)
>>
>> I am wondering if using the ->suspend|resume() callback is really
>> "late/early" enough in the device suspend phase?
>>
>> Others is using the noirq phase and some is even using the syscore
>> ops. Of course it depends on the behavior of the consumers of iommu
>> device, and I guess not everyone is using device links, which for sure
>> improves things in this regards as well.
>
> Well yes, as you said the device links should be able to take care of
> maintaining the correct suspend/resume order of smmu and its clients,
> or am I missing your point here?
> Let me know and I will be happy to incorporate any suggestions.
> Thanks

If it works fine, then you may keep it as is.

Just wanted to point out that if any consumers relies on the iommu to
operational to say until the suspend-late phase, then this doesn't
play. Then you need to move your callbacks to the corresponding same
phase.

[...]

Kind regards
Uffe
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v16 0/5] iommu/arm-smmu: Add runtime pm/sleep support

2018-10-01 Thread Vivek Gautam
Hi Will,

On Fri, Sep 28, 2018 at 7:27 PM Will Deacon  wrote:
>
> Hi Vivek,
>
> On Thu, Aug 30, 2018 at 08:15:36PM +0530, Vivek Gautam wrote:
> > This series provides the support for turning on the arm-smmu's
> > clocks/power domains using runtime pm. This is done using
> > device links between smmu and client devices. The device link
> > framework keeps the two devices in correct order for power-cycling
> > across runtime PM or across system-wide PM.
> >
> > With addition of a new device link flag DL_FLAG_AUTOREMOVE_SUPPLIER [7],
> > the device links created between arm-smmu and its clients will be
> > automatically purged when arm-smmu driver unbinds from its device.
> >
> > As not all implementations support clock/power gating, we are checking
> > for a valid 'smmu->dev's pm_domain' to conditionally enable the runtime
> > power management for such smmu implementations that can support it.
> > Otherwise, the clocks are turned to be always on in .probe until .remove.
> > With conditional runtime pm now, we avoid touching dev->power.lock
> > in fastpaths for smmu implementations that don't need to do anything
> > useful with pm_runtime.
> > This lets us to use the much-argued pm_runtime_get_sync/put_sync()
> > calls in map/unmap callbacks so that the clients do not have to
> > worry about handling any of the arm-smmu's power.
> >
> > This series also adds support for Qcom's arm-smmu-v2 variant that
> > has different clocks and power requirements.
> >
> > Previous version of this patch series is @ [1].
> >
> > Build tested the series based on 4.19-rc1.
>
> I'm going to send my pull request to Joerg early next week (probably
> Monday), but I'm not keen to include this whilst it has outstanding comments
> from Ulf. Your errata workaround patch is in a similar situation, with
> outstanding comments from Robin.

I am going to address Ulf's comments for pm_runtime_force_suspend/resume()
calls in system sleep callbacks and respin the series unless he has any more
comments regarding the early/late nature of suspend/resume.
So will it do if I respin the series today after waiting for Ulf?

The workaround series is going for a discussion now, so i think it can wait.
Thanks

Best regards
Vivek
>
> Will
> ___
> iommu mailing list
> io...@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu


--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v2] drm: Convert to using %pOFn instead of device_node.name

2018-10-01 Thread Daniel Vetter
On Fri, Sep 28, 2018 at 05:50:44PM -0500, Rob Herring wrote:
> In preparation to remove the node name pointer from struct device_node,
> convert printf users to use the %pOFn format specifier.
> 
> For drm_modes.c, the full node path is already printed out, so printing
> just the node name a 2nd time is redundant and can be removed.
> 
> Cc: Gustavo Padovan 
> Cc: Maarten Lankhorst 
> Cc: Sean Paul 
> Cc: David Airlie 
> Acked-by: Rob Clark 
> Cc: dri-de...@lists.freedesktop.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Signed-off-by: Rob Herring 
> ---
> v2:
> - Add to commit msg that we're dropping redundant printing of node name.

Applied, thanks for the patch.

Aside, still don't want drm-misc commit rights so you can offload these
yourself?

Cheers, Daniel

> 
>  drivers/gpu/drm/drm_modes.c | 4 ++--
>  drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 02db9ac82d7a..24a750436559 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -716,8 +716,8 @@ int of_get_drm_display_mode(struct device_node *np,
>   if (bus_flags)
>   drm_bus_flags_from_videomode(, bus_flags);
>  
> - pr_debug("%pOF: got %dx%d display mode from %s\n",
> - np, vm.hactive, vm.vactive, np->name);
> + pr_debug("%pOF: got %dx%d display mode\n",
> + np, vm.hactive, vm.vactive);
>   drm_mode_debug_printmodeline(dmode);
>  
>   return 0;
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index c79659ca5706..23670907a29d 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -579,7 +579,7 @@ static int msm_hdmi_bind(struct device *dev, struct 
> device *master, void *data)
>   hdmi_cfg = (struct hdmi_platform_config *)
>   of_device_get_match_data(dev);
>   if (!hdmi_cfg) {
> - dev_err(dev, "unknown hdmi_cfg: %s\n", of_node->name);
> + dev_err(dev, "unknown hdmi_cfg: %pOFn\n", of_node);
>   return -ENXIO;
>   }
>  
> -- 
> 2.17.1
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v3 2/3] drm/msm: Replace drm_gem_object_{un/reference} with put, get functions

2018-10-01 Thread Daniel Vetter
On Wed, Sep 26, 2018 at 10:23:17AM -0600, Jordan Crouse wrote:
> On Wed, Sep 26, 2018 at 01:48:58PM +0200, Thomas Zimmermann wrote:
> > This patch unifies the naming of DRM functions for reference counting
> > of struct drm_gem_object. The resulting code is more aligned with the
> > rest of the Linux kernel interfaces.
> 
> Reviewed-by: Jordan Crouse 

Care to review all 3 and then push them? Or is msm still not yet group
maintained :-)

Cheers, Daniel

> 
> > Signed-off-by: Thomas Zimmermann 
> > ---
> >  drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 4 ++--
> >  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +-
> >  drivers/gpu/drm/msm/adreno/a5xx_power.c   | 2 +-
> >  drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 2 +-
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> >  drivers/gpu/drm/msm/msm_gem_submit.c  | 4 ++--
> >  6 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c 
> > b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
> > index 059ec7d394d0..d2127b1c4ece 100644
> > --- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
> > +++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
> > @@ -132,14 +132,14 @@ reset_set(void *data, u64 val)
> > if (a5xx_gpu->pm4_bo) {
> > if (a5xx_gpu->pm4_iova)
> > msm_gem_put_iova(a5xx_gpu->pm4_bo, gpu->aspace);
> > -   drm_gem_object_unreference(a5xx_gpu->pm4_bo);
> > +   drm_gem_object_put(a5xx_gpu->pm4_bo);
> > a5xx_gpu->pm4_bo = NULL;
> > }
> >  
> > if (a5xx_gpu->pfp_bo) {
> > if (a5xx_gpu->pfp_iova)
> > msm_gem_put_iova(a5xx_gpu->pfp_bo, gpu->aspace);
> > -   drm_gem_object_unreference(a5xx_gpu->pfp_bo);
> > +   drm_gem_object_put(a5xx_gpu->pfp_bo);
> > a5xx_gpu->pfp_bo = NULL;
> > }
> >  
> > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c 
> > b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > index ab1d9308c311..6a6849309b6a 100644
> > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > @@ -1234,7 +1234,7 @@ static void a5xx_crashdumper_free(struct msm_gpu *gpu,
> > msm_gem_put_iova(dumper->bo, gpu->aspace);
> > msm_gem_put_vaddr(dumper->bo);
> >  
> > -   drm_gem_object_unreference(dumper->bo);
> > +   drm_gem_object_put(dumper->bo);
> >  }
> >  
> >  static int a5xx_crashdumper_run(struct msm_gpu *gpu,
> > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c 
> > b/drivers/gpu/drm/msm/adreno/a5xx_power.c
> > index e9c0e56dbec0..7a41e1c147e4 100644
> > --- a/drivers/gpu/drm/msm/adreno/a5xx_power.c
> > +++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c
> > @@ -323,7 +323,7 @@ void a5xx_gpmu_ucode_init(struct msm_gpu *gpu)
> > if (a5xx_gpu->gpmu_iova)
> > msm_gem_put_iova(a5xx_gpu->gpmu_bo, gpu->aspace);
> > if (a5xx_gpu->gpmu_bo)
> > -   drm_gem_object_unreference(a5xx_gpu->gpmu_bo);
> > +   drm_gem_object_put(a5xx_gpu->gpmu_bo);
> >  
> > a5xx_gpu->gpmu_bo = NULL;
> > a5xx_gpu->gpmu_iova = 0;
> > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c 
> > b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> > index 970c7963ae29..f3c21f827a4d 100644
> > --- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> > +++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> > @@ -272,7 +272,7 @@ void a5xx_preempt_fini(struct msm_gpu *gpu)
> > if (a5xx_gpu->preempt_iova[i])
> > msm_gem_put_iova(a5xx_gpu->preempt_bo[i], gpu->aspace);
> >  
> > -   drm_gem_object_unreference(a5xx_gpu->preempt_bo[i]);
> > +   drm_gem_object_put(a5xx_gpu->preempt_bo[i]);
> > a5xx_gpu->preempt_bo[i] = NULL;
> > }
> >  }
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
> > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > index c629f742a1d1..1a5d7a53752c 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > @@ -744,7 +744,7 @@ static void a6xx_destroy(struct msm_gpu *gpu)
> > if (a6xx_gpu->sqe_bo) {
> > if (a6xx_gpu->sqe_iova)
> > msm_gem_put_iova(a6xx_gpu->sqe_bo, gpu->aspace);
> > -   drm_gem_object_unreference_unlocked(a6xx_gpu->sqe_bo);
> > +   drm_gem_object_put_unlocked(a6xx_gpu->sqe_bo);
> > }
> >  
> > a6xx_gmu_remove(a6xx_gpu);
> > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c 
> > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > index 7bd83e0afa97..7a7923e6220d 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > @@ -144,7 +144,7 @@ static int submit_lookup_objects(struct msm_gem_submit 
> > *submit,
> > goto out_unlock;
> > }
> >  
> > -   drm_gem_object_reference(obj);
> > +   drm_gem_object_get(obj);
> >  
> > submit->bos[i].obj = msm_obj;
> >  
> > @@ -396,7 +396,7 @@ static void submit_cleanup(struct msm_gem_submit 
> > *submit)
> >