Re: [Intel-gfx] [PATCH 11/11] dma-buf: Do a fast lockless check for poll with timeout=0

[Christian König]

Am 23.09.2016 um 17:20 schrieb Chris Wilson:

On Fri, Sep 23, 2016 at 03:50:44PM +0200, Daniel Vetter wrote:

On Mon, Aug 29, 2016 at 08:08:34AM +0100, Chris Wilson wrote:

Currently we install a callback for performing poll on a dma-buf,
irrespective of the timeout. This involves taking a spinlock, as well as
unnecessary work, and greatly reduces scaling of poll(.timeout=0) across
multiple threads.

We can query whether the poll will block prior to installing the
callback to make the busy-query fast.

Single thread: 60% faster
8 threads on 4 (+4 HT) cores: 600% faster

Still not quite the perfect scaling we get with a native busy ioctl, but
poll(dmabuf) is faster due to the quicker lookup of the object and
avoiding drm_ioctl().

Signed-off-by: Chris Wilson 
Cc: Sumit Semwal 
Cc: linux-me...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Reviewed-by: Daniel Vetter 

Need to strike the r-b here, since Christian König pointed out that
objects won't magically switch signalling on.

Oh, it also means that

commit fb8b7d2b9d80e1e71f379e57355936bd2b024be9
Author: Jammy Zhou 
Date:   Wed Jan 21 18:35:47 2015 +0800

 reservation: wait only with non-zero timeout specified (v3)
 
 When the timeout value passed to reservation_object_wait_timeout_rcu

 is zero, no wait should be done if the fences are not signaled.
 
 Return '1' for idle and '0' for busy if the specified timeout is '0'

 to keep consistent with the case of non-zero timeout.
 
 v2: call fence_put if not signaled in the case of timeout==0
 
 v3: switch to reservation_object_test_signaled_rcu
 
 Signed-off-by: Jammy Zhou 

 Reviewed-by: Christian König 
 Reviewed-by: Alex Deucher 
 Reviewed-By: Maarten Lankhorst 
 Signed-off-by: Sumit Semwal 

is wrong. And reservation_object_test_signaled_rcu() is unreliable.


Ups indeed, that patch is wrong as well.

I suggest that we just enable the signaling in this case as well.

Regards,
Christian.


-Chris



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/11] dma-buf: Do a fast lockless check for poll with timeout=0

[Chris Wilson]

On Fri, Sep 23, 2016 at 03:50:44PM +0200, Daniel Vetter wrote:
> On Mon, Aug 29, 2016 at 08:08:34AM +0100, Chris Wilson wrote:
> > Currently we install a callback for performing poll on a dma-buf,
> > irrespective of the timeout. This involves taking a spinlock, as well as
> > unnecessary work, and greatly reduces scaling of poll(.timeout=0) across
> > multiple threads.
> > 
> > We can query whether the poll will block prior to installing the
> > callback to make the busy-query fast.
> > 
> > Single thread: 60% faster
> > 8 threads on 4 (+4 HT) cores: 600% faster
> > 
> > Still not quite the perfect scaling we get with a native busy ioctl, but
> > poll(dmabuf) is faster due to the quicker lookup of the object and
> > avoiding drm_ioctl().
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Sumit Semwal 
> > Cc: linux-me...@vger.kernel.org
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: linaro-mm-...@lists.linaro.org
> > Reviewed-by: Daniel Vetter 
> 
> Need to strike the r-b here, since Christian König pointed out that
> objects won't magically switch signalling on.

Propagating a flag through to sync_file is trivial, but not through to
the dma_buf->resv. Looks like dma-buf will be without a fast busy query,
which I guess in the grand scheme of things (i.e. dma-buf itself is not
intended to be used as a fence) is not that important.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/11] dma-buf: Do a fast lockless check for poll with timeout=0

[Chris Wilson]

On Fri, Sep 23, 2016 at 03:50:44PM +0200, Daniel Vetter wrote:
> On Mon, Aug 29, 2016 at 08:08:34AM +0100, Chris Wilson wrote:
> > Currently we install a callback for performing poll on a dma-buf,
> > irrespective of the timeout. This involves taking a spinlock, as well as
> > unnecessary work, and greatly reduces scaling of poll(.timeout=0) across
> > multiple threads.
> > 
> > We can query whether the poll will block prior to installing the
> > callback to make the busy-query fast.
> > 
> > Single thread: 60% faster
> > 8 threads on 4 (+4 HT) cores: 600% faster
> > 
> > Still not quite the perfect scaling we get with a native busy ioctl, but
> > poll(dmabuf) is faster due to the quicker lookup of the object and
> > avoiding drm_ioctl().
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Sumit Semwal 
> > Cc: linux-me...@vger.kernel.org
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: linaro-mm-...@lists.linaro.org
> > Reviewed-by: Daniel Vetter 
> 
> Need to strike the r-b here, since Christian König pointed out that
> objects won't magically switch signalling on.

Oh, it also means that

commit fb8b7d2b9d80e1e71f379e57355936bd2b024be9
Author: Jammy Zhou 
Date:   Wed Jan 21 18:35:47 2015 +0800

reservation: wait only with non-zero timeout specified (v3)

When the timeout value passed to reservation_object_wait_timeout_rcu
is zero, no wait should be done if the fences are not signaled.

Return '1' for idle and '0' for busy if the specified timeout is '0'
to keep consistent with the case of non-zero timeout.

v2: call fence_put if not signaled in the case of timeout==0

v3: switch to reservation_object_test_signaled_rcu

Signed-off-by: Jammy Zhou 
Reviewed-by: Christian König 
Reviewed-by: Alex Deucher 
Reviewed-By: Maarten Lankhorst 
Signed-off-by: Sumit Semwal 

is wrong. And reservation_object_test_signaled_rcu() is unreliable.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/11] dma-buf: Do a fast lockless check for poll with timeout=0

[Chris Wilson]

On Fri, Sep 23, 2016 at 03:50:44PM +0200, Daniel Vetter wrote:
> On Mon, Aug 29, 2016 at 08:08:34AM +0100, Chris Wilson wrote:
> > Currently we install a callback for performing poll on a dma-buf,
> > irrespective of the timeout. This involves taking a spinlock, as well as
> > unnecessary work, and greatly reduces scaling of poll(.timeout=0) across
> > multiple threads.
> > 
> > We can query whether the poll will block prior to installing the
> > callback to make the busy-query fast.
> > 
> > Single thread: 60% faster
> > 8 threads on 4 (+4 HT) cores: 600% faster
> > 
> > Still not quite the perfect scaling we get with a native busy ioctl, but
> > poll(dmabuf) is faster due to the quicker lookup of the object and
> > avoiding drm_ioctl().
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Sumit Semwal 
> > Cc: linux-me...@vger.kernel.org
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: linaro-mm-...@lists.linaro.org
> > Reviewed-by: Daniel Vetter 
> 
> Need to strike the r-b here, since Christian König pointed out that
> objects won't magically switch signalling on.

The point being here that we don't even want to switch signaling on! :)

Christian's point was that not all fences guarantee forward progress
irrespective of whether signaling is enabled or not, and fences are not
required to guarantee forward progress without signaling even if they
provide an ops->signaled().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Linaro-mm-sig] [PATCH 10/11] dma-buf: Use seqlock to close RCU race in test_signaled_single

[Chris Wilson]

On Fri, Sep 23, 2016 at 03:49:26PM +0200, Daniel Vetter wrote:
> On Mon, Aug 29, 2016 at 08:08:33AM +0100, Chris Wilson wrote:
> > With the seqlock now extended to cover the lookup of the fence and its
> > testing, we can perform that testing solely under the seqlock guard and
> > avoid the effective locking and serialisation of acquiring a reference to
> > the request.  As the fence is RCU protected we know it cannot disappear
> > as we test it, the same guarantee that made it safe to acquire the
> > reference previously.  The seqlock tests whether the fence was replaced
> > as we are testing it telling us whether or not we can trust the result
> > (if not, we just repeat the test until stable).
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Sumit Semwal 
> > Cc: linux-me...@vger.kernel.org
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: linaro-mm-...@lists.linaro.org
> 
> Not entirely sure this is safe for non-i915 drivers. We might now call
> ->signalled on a zombie fence (i.e. refcount == 0, but not yet kfreed).
> i915 can do that, but other drivers might go boom.

All fences must be under RCU guard, or is that the sticking point? Given
that, the problem is fence reallocation within the RCU grace period. If
we can mandate that fence reallocation must be safe for concurrent
fence->ops->*(), we can use this technique to avoid the serialisation
barrier otherwise required. In the simple stress test, the difference is
an order of magnitude, and test_signaled_rcu is often on a path where
every memory barrier quickly adds up (at least for us).

So is it just that you worry that others using SLAB_DESTROY_BY_RCU won't
ensure their fence is safe during the reallocation?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/11] dma-buf: Do a fast lockless check for poll with timeout=0

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:34AM +0100, Chris Wilson wrote:
> Currently we install a callback for performing poll on a dma-buf,
> irrespective of the timeout. This involves taking a spinlock, as well as
> unnecessary work, and greatly reduces scaling of poll(.timeout=0) across
> multiple threads.
> 
> We can query whether the poll will block prior to installing the
> callback to make the busy-query fast.
> 
> Single thread: 60% faster
> 8 threads on 4 (+4 HT) cores: 600% faster
> 
> Still not quite the perfect scaling we get with a native busy ioctl, but
> poll(dmabuf) is faster due to the quicker lookup of the object and
> avoiding drm_ioctl().
> 
> Signed-off-by: Chris Wilson 
> Cc: Sumit Semwal 
> Cc: linux-me...@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> Reviewed-by: Daniel Vetter 

Need to strike the r-b here, since Christian König pointed out that
objects won't magically switch signalling on.
-Daniel

> ---
>  drivers/dma-buf/dma-buf.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index cf04d249a6a4..c7a7bc579941 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -156,6 +156,18 @@ static unsigned int dma_buf_poll(struct file *file, 
> poll_table *poll)
>   if (!events)
>   return 0;
>  
> + if (poll_does_not_wait(poll)) {
> + if (events & POLLOUT &&
> + !reservation_object_test_signaled_rcu(resv, true))
> + events &= ~(POLLOUT | POLLIN);
> +
> + if (events & POLLIN &&
> + !reservation_object_test_signaled_rcu(resv, false))
> + events &= ~POLLIN;
> +
> + return events;
> + }
> +
>  retry:
>   seq = read_seqcount_begin(>seq);
>   rcu_read_lock();
> -- 
> 2.9.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [Linaro-mm-sig] [PATCH 10/11] dma-buf: Use seqlock to close RCU race in test_signaled_single

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:33AM +0100, Chris Wilson wrote:
> With the seqlock now extended to cover the lookup of the fence and its
> testing, we can perform that testing solely under the seqlock guard and
> avoid the effective locking and serialisation of acquiring a reference to
> the request.  As the fence is RCU protected we know it cannot disappear
> as we test it, the same guarantee that made it safe to acquire the
> reference previously.  The seqlock tests whether the fence was replaced
> as we are testing it telling us whether or not we can trust the result
> (if not, we just repeat the test until stable).
> 
> Signed-off-by: Chris Wilson 
> Cc: Sumit Semwal 
> Cc: linux-me...@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org

Not entirely sure this is safe for non-i915 drivers. We might now call
->signalled on a zombie fence (i.e. refcount == 0, but not yet kfreed).
i915 can do that, but other drivers might go boom.

I think in generic code we can't do these kind of tricks unfortunately.
-Daniel

> ---
>  drivers/dma-buf/reservation.c | 32 
>  1 file changed, 4 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index e74493e7332b..1ddffa5adb5a 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -442,24 +442,6 @@ unlock_retry:
>  }
>  EXPORT_SYMBOL_GPL(reservation_object_wait_timeout_rcu);
>  
> -
> -static inline int
> -reservation_object_test_signaled_single(struct fence *passed_fence)
> -{
> - struct fence *fence, *lfence = passed_fence;
> - int ret = 1;
> -
> - if (!test_bit(FENCE_FLAG_SIGNALED_BIT, >flags)) {
> - fence = fence_get_rcu(lfence);
> - if (!fence)
> - return -1;
> -
> - ret = !!fence_is_signaled(fence);
> - fence_put(fence);
> - }
> - return ret;
> -}
> -
>  /**
>   * reservation_object_test_signaled_rcu - Test if a reservation object's
>   * fences have been signaled.
> @@ -474,7 +456,7 @@ bool reservation_object_test_signaled_rcu(struct 
> reservation_object *obj,
> bool test_all)
>  {
>   unsigned seq, shared_count;
> - int ret;
> + bool ret;
>  
>   rcu_read_lock();
>  retry:
> @@ -494,10 +476,8 @@ retry:
>   for (i = 0; i < shared_count; ++i) {
>   struct fence *fence = rcu_dereference(fobj->shared[i]);
>  
> - ret = reservation_object_test_signaled_single(fence);
> - if (ret < 0)
> - goto retry;
> - else if (!ret)
> + ret = fence_is_signaled(fence);
> + if (!ret)
>   break;
>   }
>  
> @@ -509,11 +489,7 @@ retry:
>   struct fence *fence_excl = rcu_dereference(obj->fence_excl);
>  
>   if (fence_excl) {
> - ret = reservation_object_test_signaled_single(
> - fence_excl);
> - if (ret < 0)
> - goto retry;
> -
> + ret = fence_is_signaled(fence_excl);
>   if (read_seqcount_retry(>seq, seq))
>   goto retry;
>   }
> -- 
> 2.9.3
> 
> ___
> Linaro-mm-sig mailing list
> linaro-mm-...@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig

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

Re: [Intel-gfx] [PATCH 06/11] dma-buf: Introduce fence_get_rcu_safe()

[Markus Heiser]

Am 23.09.2016 um 14:59 schrieb Daniel Vetter :

>> 
>> /**
>> - * fence_put - decreases refcount of the fence
>> - * @fence:  [in]fence to reduce refcount of
>> + * fence_get_rcu_safe  - acquire a reference to an RCU tracked fence
>> + * @fence:  [in]pointer to fence to increase refcount of
>> + *
>> + * Function returns NULL if no refcount could be obtained, or the fence.
>> + * This function handles acquiring a reference to a fence that may be
>> + * reallocated within the RCU grace period (such as with 
>> SLAB_DESTROY_BY_RCU),
>> + * so long as the caller is using RCU on the pointer to the fence.
>> + *
>> + * An alternative mechanism is to employ a seqlock to protect a bunch of
>> + * fences, such as used by struct reservation_object. When using a seqlock,
>> + * the seqlock must be taken before and checked after a reference to the
>> + * fence is acquired (as shown here).
>> + *
>> + * The caller is required to hold the RCU read lock.
> 
> Would be good to cross reference the various fence_get functions a bit
> better in the docs. But since the docs aren't yet pulled into the rst/html
> output, that doesn't matter that much

Hi Daniel ... I'am working on ;-)

* 
http://return42.github.io/sphkerneldoc/linux_src_doc/include/linux/fence_h.html

-- Markus --
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/11] dma-buf: Restart reservation_object_test_signaled_rcu() after writes

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:32AM +0100, Chris Wilson wrote:
> In order to be completely generic, we have to double check the read
> seqlock after acquiring a reference to the fence. If the driver is
> allocating fences from a SLAB_DESTROY_BY_RCU, or similar freelist, then
> within an RCU grace period a fence may be freed and reallocated. The RCU
> read side critical section does not prevent this reallocation, instead
> we have to inspect the reservation's seqlock to double check if the
> fences have been reassigned as we were acquiring our reference.
> 
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Maarten Lankhorst 
> Cc: Christian König 
> Cc: Alex Deucher 
> Cc: Sumit Semwal 
> Cc: linux-me...@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org

Reviewed-by: Daniel Vetter 

> ---
>  drivers/dma-buf/reservation.c | 30 ++
>  1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 3369e4668e96..e74493e7332b 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -474,12 +474,13 @@ bool reservation_object_test_signaled_rcu(struct 
> reservation_object *obj,
> bool test_all)
>  {
>   unsigned seq, shared_count;
> - int ret = true;
> + int ret;
>  
> + rcu_read_lock();
>  retry:
> + ret = true;
>   shared_count = 0;
>   seq = read_seqcount_begin(>seq);
> - rcu_read_lock();
>  
>   if (test_all) {
>   unsigned i;
> @@ -490,46 +491,35 @@ retry:
>   if (fobj)
>   shared_count = fobj->shared_count;
>  
> - if (read_seqcount_retry(>seq, seq))
> - goto unlock_retry;
> -
>   for (i = 0; i < shared_count; ++i) {
>   struct fence *fence = rcu_dereference(fobj->shared[i]);
>  
>   ret = reservation_object_test_signaled_single(fence);
>   if (ret < 0)
> - goto unlock_retry;
> + goto retry;
>   else if (!ret)
>   break;
>   }
>  
> - /*
> -  * There could be a read_seqcount_retry here, but nothing cares
> -  * about whether it's the old or newer fence pointers that are
> -  * signaled. That race could still have happened after checking
> -  * read_seqcount_retry. If you care, use ww_mutex_lock.
> -  */
> + if (read_seqcount_retry(>seq, seq))
> + goto retry;
>   }
>  
>   if (!shared_count) {
>   struct fence *fence_excl = rcu_dereference(obj->fence_excl);
>  
> - if (read_seqcount_retry(>seq, seq))
> - goto unlock_retry;
> -
>   if (fence_excl) {
>   ret = reservation_object_test_signaled_single(
>   fence_excl);
>   if (ret < 0)
> - goto unlock_retry;
> + goto retry;
> +
> + if (read_seqcount_retry(>seq, seq))
> + goto retry;
>   }
>   }
>  
>   rcu_read_unlock();
>   return ret;
> -
> -unlock_retry:
> - rcu_read_unlock();
> - goto retry;
>  }
>  EXPORT_SYMBOL_GPL(reservation_object_test_signaled_rcu);
> -- 
> 2.9.3
> 

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

Re: [Intel-gfx] [PATCH v2] Idleness DRRS test

[Chris Wilson]

On Fri, Sep 23, 2016 at 06:10:29PM +0530, Nautiyal Ankit wrote:
> From: Ramalingam C 
> 
> Idleness DRRS:
>   By default the DRRS state will be at DRRS_HIGH_RR. When a Display
>   content is Idle for more than 1Sec Idleness will be declared and
>   DRRS_LOW_RR will be invoked, changing the refresh rate to the
>   lower most refresh rate supported by the panel. As soon as there
>   is a display content change there will be a DRRS state transition
>   as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
>   highest refresh rate supported by the panel.
> 
> To test this, Idleness DRRS IGT will probe the DRRS state at below
> instances and compare with the expected state.
> 
>   InstanceExpected State
> 1. Immediately after rendering the still imageDRRS_HIGH_RR
> 2. After a delay of 1.2SecDRRS_LOW_RR
> 3. After changing the frame bufferDRRS_HIGH_RR
> 4. After a delay of 1.2SecDRRS_LOW_RR
> 5. After changing the frame bufferDRRS_HIGH_RR
> 6. After a delay of 1.2SecDRRS_LOW_RR
> 
> The test checks the driver DRRS state from the debugfs entry. To check the
> actual refresh-rate, a separate thread counts the number of vblanks
> received per sec. The refresh-rate calculated is checked against the
> expected refresh-rate with a tolerance value of 2.
> 
> This patch is a continuation of the earlier work
> https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness
> 
> DRRS. The code is tested on Broxton BXT_T platform.
> 
> v2: Addressed the comments and suggestions from Vlad, Marius.
> The signoff details from the earlier work are also included.
> 
> Signed-off-by: Ramalingam C 
> Signed-off-by: Vandana Kannan 
> Signed-off-by: aknautiy 
> ---
>  tests/Makefile.sources |   1 +
>  tests/kms_drrs.c   | 612 
> +
>  2 files changed, 613 insertions(+)
>  create mode 100644 tests/kms_drrs.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index a837977..5f31521 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -91,6 +91,7 @@ TESTS_progs_M = \
>   kms_cursor_crc \
>   kms_cursor_legacy \
>   kms_draw_crc \
> + kms_drrs \
>   kms_fbc_crc \
>   kms_fbcon_fbt \
>   kms_flip \
> diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
> new file mode 100644
> index 000..69f8b06
> --- /dev/null
> +++ b/tests/kms_drrs.c
> @@ -0,0 +1,612 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "drmtest.h"
> +#include "igt_debugfs.h"
> +#include "igt_kms.h"
> +#include "intel_chipset.h"
> +#include "intel_batchbuffer.h"
> +#include "ioctl_wrappers.h"
> +#include 
> +#include 
> +#include 
> +#include 
> +IGT_TEST_DESCRIPTION(
> +"Performs write operations and then waits for DRRS to invoke the"
> +"Low Refresh Rate and then disturbs the contents of the screen once"
> +"again hence DRRS revert back to High Refresh Rate(Default).");
> +
> +#define DRRS_STATUS_BYTES_CNT1000
> +#define SET  1
> +#define RESET0
> +
> +/*
> + * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a 
> given
> + * display. To disturb the content of the screen, we replace fb[0] by fb[1] 
> and
> + * vice versa.
> + */
> +typedef struct {
> + int drm_fd;
> + uint32_t devid;
> + uint32_t handle[2];
> + igt_display_t display;
> + igt_output_t *output;
> + enum pipe pipe;
> + igt_plane_t *primary;
> + struct igt_fb fb[2];
> + uint32_t fb_id[2];
> +} data_t;
> +
> +/*
> + * 

Re: [Intel-gfx] [PATCH 08/11] dma-buf: Restart reservation_object_wait_timeout_rcu() after writes

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:31AM +0100, Chris Wilson wrote:
> In order to be completely generic, we have to double check the read
> seqlock after acquiring a reference to the fence. If the driver is
> allocating fences from a SLAB_DESTROY_BY_RCU, or similar freelist, then
> within an RCU grace period a fence may be freed and reallocated. The RCU
> read side critical section does not prevent this reallocation, instead
> we have to inspect the reservation's seqlock to double check if the
> fences have been reassigned as we were acquiring our reference.
> 
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Maarten Lankhorst 
> Cc: Christian König 
> Cc: Alex Deucher 
> Cc: Sumit Semwal 
> Cc: linux-me...@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org

Reviewed-by: Daniel Vetter 

> ---
>  drivers/dma-buf/reservation.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 10fd441dd4ed..3369e4668e96 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -388,9 +388,6 @@ retry:
>   if (fobj)
>   shared_count = fobj->shared_count;
>  
> - if (read_seqcount_retry(>seq, seq))
> - goto unlock_retry;
> -
>   for (i = 0; i < shared_count; ++i) {
>   struct fence *lfence = rcu_dereference(fobj->shared[i]);
>  
> @@ -413,9 +410,6 @@ retry:
>   if (!shared_count) {
>   struct fence *fence_excl = rcu_dereference(obj->fence_excl);
>  
> - if (read_seqcount_retry(>seq, seq))
> - goto unlock_retry;
> -
>   if (fence_excl &&
>   !test_bit(FENCE_FLAG_SIGNALED_BIT, _excl->flags)) {
>   if (!fence_get_rcu(fence_excl))
> @@ -430,6 +424,11 @@ retry:
>  
>   rcu_read_unlock();
>   if (fence) {
> + if (read_seqcount_retry(>seq, seq)) {
> + fence_put(fence);
> + goto retry;
> + }
> +
>   ret = fence_wait_timeout(fence, intr, ret);
>   fence_put(fence);
>   if (ret > 0 && wait_all && (i + 1 < shared_count))
> -- 
> 2.9.3
> 

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

Re: [Intel-gfx] [PATCH 03/11] drm/msm: Remove call to reservation_object_test_signaled_rcu before wait

[Rob Clark]

On Fri, Sep 23, 2016 at 8:55 AM, Daniel Vetter  wrote:
> On Mon, Aug 29, 2016 at 08:08:26AM +0100, Chris Wilson wrote:
>> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
>> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
>> need to handle such conversion in the caller. The only challenge are
>> those callers that wish to differentiate the error code between the
>> nonblocking busy check and potentially blocking wait.
>>
>> v2: 9 is only 0 in German.
>>
>> Signed-off-by: Chris Wilson 
>> Cc: Rob Clark 
>
> Reviewed-by: Daniel Vetter 

fyi, this is in my msm-next pull-req sent last week..  (with one small
s/MSG_PREP_NOSYNC/MSM_PREP_NOSYNC/ fixup ;-))

BR,
-R


>> ---
>>  drivers/gpu/drm/msm/msm_gem.c | 22 ++
>>  1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
>> index 6cd4af443139..45796a88d353 100644
>> --- a/drivers/gpu/drm/msm/msm_gem.c
>> +++ b/drivers/gpu/drm/msm/msm_gem.c
>> @@ -584,18 +584,16 @@ int msm_gem_cpu_prep(struct drm_gem_object *obj, 
>> uint32_t op, ktime_t *timeout)
>>  {
>>   struct msm_gem_object *msm_obj = to_msm_bo(obj);
>>   bool write = !!(op & MSM_PREP_WRITE);
>> -
>> - if (op & MSM_PREP_NOSYNC) {
>> - if (!reservation_object_test_signaled_rcu(msm_obj->resv, 
>> write))
>> - return -EBUSY;
>> - } else {
>> - int ret;
>> -
>> - ret = reservation_object_wait_timeout_rcu(msm_obj->resv, write,
>> - true, timeout_to_jiffies(timeout));
>> - if (ret <= 0)
>> - return ret == 0 ? -ETIMEDOUT : ret;
>> - }
>> + unsigned long remain =
>> + op & MSG_PREP_NOSYNC ? 0 : timeout_to_jiffies(timeout);
>> + long ret;
>> +
>> + ret = reservation_object_wait_timeout_rcu(msm_obj->resv, write,
>> +   true,  remain);
>> + if (ret == 0)
>> + return remain == 0 ? -EBUSY : -ETIMEDOUT;
>> + else if (ret < 0)
>> + return ret;
>>
>>   /* TODO cache maintenance */
>>
>> --
>> 2.9.3
>>
>> ___
>> 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
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/11] dma-buf: Restart reservation_object_get_fences_rcu() after writes

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:30AM +0100, Chris Wilson wrote:
> In order to be completely generic, we have to double check the read
> seqlock after acquiring a reference to the fence. If the driver is
> allocating fences from a SLAB_DESTROY_BY_RCU, or similar freelist, then
> within an RCU grace period a fence may be freed and reallocated. The RCU
> read side critical section does not prevent this reallocation, instead
> we have to inspect the reservation's seqlock to double check if the
> fences have been reassigned as we were acquiring our reference.
> 
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Maarten Lankhorst 
> Cc: Christian König 
> Cc: Alex Deucher 
> Cc: Sumit Semwal 
> Cc: linux-me...@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> ---
>  drivers/dma-buf/reservation.c | 71 
> +++
>  1 file changed, 31 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 723d8af988e5..10fd441dd4ed 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -280,18 +280,24 @@ int reservation_object_get_fences_rcu(struct 
> reservation_object *obj,
> unsigned *pshared_count,
> struct fence ***pshared)
>  {
> - unsigned shared_count = 0;
> - unsigned retry = 1;
> - struct fence **shared = NULL, *fence_excl = NULL;
> - int ret = 0;
> + struct fence **shared = NULL;
> + struct fence *fence_excl;
> + unsigned shared_count;
> + int ret = 1;

Personally I'd go with ret = -EBUSY here, but that's a bikeshed.

Reviewed-by: Daniel Vetter 
>  
> - while (retry) {
> + do {
>   struct reservation_object_list *fobj;
>   unsigned seq;
> + unsigned i;
>  
> - seq = read_seqcount_begin(>seq);
> + shared_count = i = 0;
>  
>   rcu_read_lock();
> + seq = read_seqcount_begin(>seq);
> +
> + fence_excl = rcu_dereference(obj->fence_excl);
> + if (fence_excl && !fence_get_rcu(fence_excl))
> + goto unlock;
>  
>   fobj = rcu_dereference(obj->fence);
>   if (fobj) {
> @@ -309,52 +315,37 @@ int reservation_object_get_fences_rcu(struct 
> reservation_object *obj,
>   }
>  
>   ret = -ENOMEM;
> - shared_count = 0;
>   break;
>   }
>   shared = nshared;
> - memcpy(shared, fobj->shared, sz);
>   shared_count = fobj->shared_count;
> - } else
> - shared_count = 0;
> - fence_excl = rcu_dereference(obj->fence_excl);
> -
> - retry = read_seqcount_retry(>seq, seq);
> - if (retry)
> - goto unlock;
> -
> - if (!fence_excl || fence_get_rcu(fence_excl)) {
> - unsigned i;
>  
>   for (i = 0; i < shared_count; ++i) {
> - if (fence_get_rcu(shared[i]))
> - continue;
> -
> - /* uh oh, refcount failed, abort and retry */
> - while (i--)
> - fence_put(shared[i]);
> -
> - if (fence_excl) {
> - fence_put(fence_excl);
> - fence_excl = NULL;
> - }
> -
> - retry = 1;
> - break;
> + shared[i] = rcu_dereference(fobj->shared[i]);
> + if (!fence_get_rcu(shared[i]))
> + break;
>   }
> - } else
> - retry = 1;
> + }
> +
> + if (i != shared_count || read_seqcount_retry(>seq, seq)) {
> + while (i--)
> + fence_put(shared[i]);
> + fence_put(fence_excl);
> + goto unlock;
> + }
>  
> + ret = 0;
>  unlock:
>   rcu_read_unlock();
> - }
> - *pshared_count = shared_count;
> - if (shared_count)
> - *pshared = shared;
> - else {
> - *pshared = NULL;
> + } while (ret);
> +
> + if (!shared_count) {
>   kfree(shared);
> + shared = NULL;
>   }
> +
> + *pshared_count = shared_count;
> + *pshared = shared;
>   *pfence_excl = 

Re: [Intel-gfx] [PATCH 06/11] dma-buf: Introduce fence_get_rcu_safe()

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:29AM +0100, Chris Wilson wrote:
> This variant of fence_get_rcu() takes an RCU protected pointer to a
> fence and carefully returns a reference to the fence ensuring that it is
> not reallocated as it does. This is required when mixing fences and
> SLAB_DESTROY_BY_RCU - although it serves a more pedagogical function atm
> 
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Sumit Semwal 
> Cc: linux-me...@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> ---
>  include/linux/fence.h | 56 
> ++-
>  1 file changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/fence.h b/include/linux/fence.h
> index 0d763053f97a..c9c5ba98c302 100644
> --- a/include/linux/fence.h
> +++ b/include/linux/fence.h
> @@ -183,6 +183,16 @@ void fence_release(struct kref *kref);
>  void fence_free(struct fence *fence);
>  
>  /**
> + * fence_put - decreases refcount of the fence
> + * @fence:   [in]fence to reduce refcount of
> + */
> +static inline void fence_put(struct fence *fence)
> +{
> + if (fence)
> + kref_put(>refcount, fence_release);
> +}
> +
> +/**
>   * fence_get - increases refcount of the fence
>   * @fence:   [in]fence to increase refcount of
>   *
> @@ -210,13 +220,49 @@ static inline struct fence *fence_get_rcu(struct fence 
> *fence)
>  }
>  
>  /**
> - * fence_put - decreases refcount of the fence
> - * @fence:   [in]fence to reduce refcount of
> + * fence_get_rcu_safe  - acquire a reference to an RCU tracked fence
> + * @fence:   [in]pointer to fence to increase refcount of
> + *
> + * Function returns NULL if no refcount could be obtained, or the fence.
> + * This function handles acquiring a reference to a fence that may be
> + * reallocated within the RCU grace period (such as with 
> SLAB_DESTROY_BY_RCU),
> + * so long as the caller is using RCU on the pointer to the fence.
> + *
> + * An alternative mechanism is to employ a seqlock to protect a bunch of
> + * fences, such as used by struct reservation_object. When using a seqlock,
> + * the seqlock must be taken before and checked after a reference to the
> + * fence is acquired (as shown here).
> + *
> + * The caller is required to hold the RCU read lock.

Would be good to cross reference the various fence_get functions a bit
better in the docs. But since the docs aren't yet pulled into the rst/html
output, that doesn't matter that much. Hence as-is:

Reviewed-by: Daniel Vetter 

>   */
> -static inline void fence_put(struct fence *fence)
> +static inline struct fence *fence_get_rcu_safe(struct fence * __rcu *fencep)
>  {
> - if (fence)
> - kref_put(>refcount, fence_release);
> + do {
> + struct fence *fence;
> +
> + fence = rcu_dereference(*fencep);
> + if (!fence || !fence_get_rcu(fence))
> + return NULL;
> +
> + /* The atomic_inc_not_zero() inside fence_get_rcu()
> +  * provides a full memory barrier upon success (such as now).
> +  * This is paired with the write barrier from assigning
> +  * to the __rcu protected fence pointer so that if that
> +  * pointer still matches the current fence, we know we
> +  * have successfully acquire a reference to it. If it no
> +  * longer matches, we are holding a reference to some other
> +  * reallocated pointer. This is possible if the allocator
> +  * is using a freelist like SLAB_DESTROY_BY_RCU where the
> +  * fence remains valid for the RCU grace period, but it
> +  * may be reallocated. When using such allocators, we are
> +  * responsible for ensuring the reference we get is to
> +  * the right fence, as below.
> +  */
> + if (fence == rcu_access_pointer(*fencep))
> + return rcu_pointer_handoff(fence);
> +
> + fence_put(fence);
> + } while (1);
>  }
>  
>  int fence_signal(struct fence *fence);
> -- 
> 2.9.3
> 

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

Re: [Intel-gfx] [PATCH 05/11] drm/vmwgfx: Remove call to reservation_object_test_signaled_rcu before wait

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:28AM +0100, Chris Wilson wrote:
> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
> need to handle such conversion in the caller. The only challenge are
> those callers that wish to differentiate the error code between the
> nonblocking busy check and potentially blocking wait.
> 
> Signed-off-by: Chris Wilson 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Reviewed-by: Sinclair Yeh 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index 6a328d507a28..1a85fb2d4dc6 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -574,10 +574,8 @@ static int vmw_user_dmabuf_synccpu_grab(struct 
> vmw_user_dma_buffer *user_bo,
>   bool nonblock = !!(flags & drm_vmw_synccpu_dontblock);
>   long lret;
>  
> - if (nonblock)
> - return reservation_object_test_signaled_rcu(bo->resv, 
> true) ? 0 : -EBUSY;
> -
> - lret = reservation_object_wait_timeout_rcu(bo->resv, true, 
> true, MAX_SCHEDULE_TIMEOUT);
> + lret = reservation_object_wait_timeout_rcu(bo->resv, true, true,
> +nonblock ? 0 : 
> MAX_SCHEDULE_TIMEOUT);
>   if (!lret)
>   return -EBUSY;
>   else if (lret < 0)
> -- 
> 2.9.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 03/11] drm/msm: Remove call to reservation_object_test_signaled_rcu before wait

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:26AM +0100, Chris Wilson wrote:
> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
> need to handle such conversion in the caller. The only challenge are
> those callers that wish to differentiate the error code between the
> nonblocking busy check and potentially blocking wait.
> 
> v2: 9 is only 0 in German.
> 
> Signed-off-by: Chris Wilson 
> Cc: Rob Clark 

Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/msm/msm_gem.c | 22 ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 6cd4af443139..45796a88d353 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -584,18 +584,16 @@ int msm_gem_cpu_prep(struct drm_gem_object *obj, 
> uint32_t op, ktime_t *timeout)
>  {
>   struct msm_gem_object *msm_obj = to_msm_bo(obj);
>   bool write = !!(op & MSM_PREP_WRITE);
> -
> - if (op & MSM_PREP_NOSYNC) {
> - if (!reservation_object_test_signaled_rcu(msm_obj->resv, write))
> - return -EBUSY;
> - } else {
> - int ret;
> -
> - ret = reservation_object_wait_timeout_rcu(msm_obj->resv, write,
> - true, timeout_to_jiffies(timeout));
> - if (ret <= 0)
> - return ret == 0 ? -ETIMEDOUT : ret;
> - }
> + unsigned long remain =
> + op & MSG_PREP_NOSYNC ? 0 : timeout_to_jiffies(timeout);
> + long ret;
> +
> + ret = reservation_object_wait_timeout_rcu(msm_obj->resv, write,
> +   true,  remain);
> + if (ret == 0)
> + return remain == 0 ? -EBUSY : -ETIMEDOUT;
> + else if (ret < 0)
> + return ret;
>  
>   /* TODO cache maintenance */
>  
> -- 
> 2.9.3
> 
> ___
> 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/11] drm/nouveau: Remove call to reservation_object_test_signaled_rcu before wait

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:27AM +0100, Chris Wilson wrote:
> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
> need to handle such conversion in the caller. The only challenge are
> those callers that wish to differentiate the error code between the
> nonblocking busy check and potentially blocking wait.
> 
> Signed-off-by: Chris Wilson 
> Cc: Ben Skeggs 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/nouveau/nouveau_gem.c | 21 +
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
> b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index 72e2399bce39..b90e21ff1ed8 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> @@ -861,6 +861,7 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void 
> *data,
>   struct nouveau_bo *nvbo;
>   bool no_wait = !!(req->flags & NOUVEAU_GEM_CPU_PREP_NOWAIT);
>   bool write = !!(req->flags & NOUVEAU_GEM_CPU_PREP_WRITE);
> + long lret;
>   int ret;
>  
>   gem = drm_gem_object_lookup(file_priv, req->handle);
> @@ -868,19 +869,15 @@ nouveau_gem_ioctl_cpu_prep(struct drm_device *dev, void 
> *data,
>   return -ENOENT;
>   nvbo = nouveau_gem_object(gem);
>  
> - if (no_wait)
> - ret = reservation_object_test_signaled_rcu(nvbo->bo.resv, 
> write) ? 0 : -EBUSY;
> - else {
> - long lret;
> + lret = reservation_object_wait_timeout_rcu(nvbo->bo.resv, write, true,
> +no_wait ? 0 :30 * HZ);
> + if (!lret)
> + ret = -EBUSY;
> + else if (lret > 0)
> + ret = 0;
> + else
> + ret = lret;
>  
> - lret = reservation_object_wait_timeout_rcu(nvbo->bo.resv, 
> write, true, 30 * HZ);
> - if (!lret)
> - ret = -EBUSY;
> - else if (lret > 0)
> - ret = 0;
> - else
> - ret = lret;
> - }
>   nouveau_bo_sync_for_cpu(nvbo);
>   drm_gem_object_unreference_unlocked(gem);
>  
> -- 
> 2.9.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 02/11] drm/etnaviv: Remove manual call to reservation_object_test_signaled_rcu before wait

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:25AM +0100, Chris Wilson wrote:
> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
> need to handle such conversion in the caller. The only challenge are
> those callers that wish to differentiate the error code between the
> nonblocking busy check and potentially blocking wait.
> 
> Signed-off-by: Chris Wilson 
> Cc: Lucas Stach 
> Cc: Russell King 
> Cc: Christian Gmeiner 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 24 ++--
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 5ce3603e6eac..9ffca2478e02 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -409,20 +409,16 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, 
> u32 op,
>   struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>   struct drm_device *dev = obj->dev;
>   bool write = !!(op & ETNA_PREP_WRITE);
> - int ret;
> -
> - if (op & ETNA_PREP_NOSYNC) {
> - if (!reservation_object_test_signaled_rcu(etnaviv_obj->resv,
> -   write))
> - return -EBUSY;
> - } else {
> - unsigned long remain = etnaviv_timeout_to_jiffies(timeout);
> -
> - ret = reservation_object_wait_timeout_rcu(etnaviv_obj->resv,
> -   write, true, remain);
> - if (ret <= 0)
> - return ret == 0 ? -ETIMEDOUT : ret;
> - }
> + unsigned long remain =
> + op & ETNA_PREP_NOSYNC ? 0 : etnaviv_timeout_to_jiffies(timeout);
> + long lret;
> +
> + lret = reservation_object_wait_timeout_rcu(etnaviv_obj->resv,
> +write, true, remain);
> + if (lret < 0)
> + return lret;
> + else if (lret == 0)
> + return remain == 0 ? -EBUSY : -ETIMEDOUT;
>  
>   if (etnaviv_obj->flags & ETNA_BO_CACHED) {
>   if (!etnaviv_obj->sgt) {
> -- 
> 2.9.3
> 
> ___
> 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/11] drm/amdgpu: Remove call to reservation_object_test_signaled_rcu before wait

[Daniel Vetter]

On Mon, Aug 29, 2016 at 08:08:24AM +0100, Chris Wilson wrote:
> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
> need to handle such conversion in the caller. The only challenge are
> those callers that wish to differentiate the error code between the
> nonblocking busy check and potentially blocking wait.
> 
> Signed-off-by: Chris Wilson 
> Cc: Alex Deucher 
> Cc: Christian König 

Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 88fbed2389c0..a3e6f883ac2c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -407,10 +407,8 @@ int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, 
> void *data,
>   return -ENOENT;
>   }
>   robj = gem_to_amdgpu_bo(gobj);
> - if (timeout == 0)
> - ret = reservation_object_test_signaled_rcu(robj->tbo.resv, 
> true);
> - else
> - ret = reservation_object_wait_timeout_rcu(robj->tbo.resv, true, 
> true, timeout);
> + ret = reservation_object_wait_timeout_rcu(robj->tbo.resv, true, true,
> +   timeout);
>  
>   /* ret == 0 means not signaled,
>* ret > 0 means signaled
> -- 
> 2.9.3
> 
> ___
> 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/audio: audio cleanups, 4k fixes (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/audio: audio cleanups, 4k fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/12754/
State : success

== Summary ==

Series 12754v2 drm/i915/audio: audio cleanups, 4k fixes
https://patchwork.freedesktop.org/api/1.0/series/12754/revisions/2/mbox/

Test kms_cursor_legacy:
Subgroup basic-flip-after-cursor-legacy:
dmesg-warn -> PASS   (fi-byt-n2820)

fi-bdw-5557u total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050 total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-byt-n2820 total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770  total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650   total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hqtotal:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
fi-skl-6700k total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hqtotal:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600  total:244  pass:206  dwarn:0   dfail:0   fail:0   skip:38 

Results at /archive/results/CI_IGT_test/Patchwork_2572/

c9f6c206fe35021c45419a30deb1cba85910453a drm-intel-nightly: 
2016y-09m-23d-10h-30m-14s UTC integration manifest
741 drm/i915: set proper N/M in modeset
48018c7 drm/i915/audio: rename N value getter to emphasize it's for hdmi
36bc4b0 drm/i915/audio: add register macros for audio config N value
b095203 drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock
13e0bf3 drm/i915/audio: set proper N/MCTS on more platforms
5363f9e drm/i915/audio: split dp and hdmi audio config update
6a8d52e drm/i915/audio: use the same code for updating audio config
326321f drm/i915/audio: port is going to be just fine, simplify checks
29fa361 drm/i915/audio: abstract audio config update

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] Idleness DRRS test

[Nautiyal Ankit]

From: Ramalingam C 

Idleness DRRS:
By default the DRRS state will be at DRRS_HIGH_RR. When a Display
content is Idle for more than 1Sec Idleness will be declared and
DRRS_LOW_RR will be invoked, changing the refresh rate to the
lower most refresh rate supported by the panel. As soon as there
is a display content change there will be a DRRS state transition
as DRRS_LOW_RR--> DRRS_HIGH_RR, changing the refresh rate to the
highest refresh rate supported by the panel.

To test this, Idleness DRRS IGT will probe the DRRS state at below
instances and compare with the expected state.

InstanceExpected State
1. Immediately after rendering the still image  DRRS_HIGH_RR
2. After a delay of 1.2Sec  DRRS_LOW_RR
3. After changing the frame buffer  DRRS_HIGH_RR
4. After a delay of 1.2Sec  DRRS_LOW_RR
5. After changing the frame buffer  DRRS_HIGH_RR
6. After a delay of 1.2Sec  DRRS_LOW_RR

The test checks the driver DRRS state from the debugfs entry. To check the
actual refresh-rate, a separate thread counts the number of vblanks
received per sec. The refresh-rate calculated is checked against the
expected refresh-rate with a tolerance value of 2.

This patch is a continuation of the earlier work
https://patchwork.freedesktop.org/patch/45472/ towards igt for idleness

DRRS. The code is tested on Broxton BXT_T platform.

v2: Addressed the comments and suggestions from Vlad, Marius.
The signoff details from the earlier work are also included.

Signed-off-by: Ramalingam C 
Signed-off-by: Vandana Kannan 
Signed-off-by: aknautiy 
---
 tests/Makefile.sources |   1 +
 tests/kms_drrs.c   | 612 +
 2 files changed, 613 insertions(+)
 create mode 100644 tests/kms_drrs.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a837977..5f31521 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -91,6 +91,7 @@ TESTS_progs_M = \
kms_cursor_crc \
kms_cursor_legacy \
kms_draw_crc \
+   kms_drrs \
kms_fbc_crc \
kms_fbcon_fbt \
kms_flip \
diff --git a/tests/kms_drrs.c b/tests/kms_drrs.c
new file mode 100644
index 000..69f8b06
--- /dev/null
+++ b/tests/kms_drrs.c
@@ -0,0 +1,612 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+#include "intel_chipset.h"
+#include "intel_batchbuffer.h"
+#include "ioctl_wrappers.h"
+#include 
+#include 
+#include 
+#include 
+IGT_TEST_DESCRIPTION(
+"Performs write operations and then waits for DRRS to invoke the"
+"Low Refresh Rate and then disturbs the contents of the screen once"
+"again hence DRRS revert back to High Refresh Rate(Default).");
+
+#define DRRS_STATUS_BYTES_CNT  1000
+#define SET1
+#define RESET  0
+
+/*
+ * Structure to store data to create 2 framebuffers, fb[0] and fb[1] on a given
+ * display. To disturb the content of the screen, we replace fb[0] by fb[1] and
+ * vice versa.
+ */
+typedef struct {
+   int drm_fd;
+   uint32_t devid;
+   uint32_t handle[2];
+   igt_display_t display;
+   igt_output_t *output;
+   enum pipe pipe;
+   igt_plane_t *primary;
+   struct igt_fb fb[2];
+   uint32_t fb_id[2];
+} data_t;
+
+/*
+ * Structure to count vblank and note the starting time of the counter
+ */
+typedef struct {
+   unsigned int vbl_count;
+   struct timeval start;
+} vbl_info;
+
+/*
+ * Condition variables,mutex, and shared variables for thread synchronization
+ */
+pthread_mutex_t rr_mutex;

[Intel-gfx] [PATCH v2 7/9] drm/i915/audio: add register macros for audio config N value

[Jani Nikula]

Have generic macros in line with the rest of the register bit definition
macros instead of a dedicated function in intel_audio.c, and use them.
No functional changes.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_reg.h|  4 
 drivers/gpu/drm/i915/intel_audio.c | 23 ++-
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8d44cee710f0..dc04ce68f8b6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7331,6 +7331,10 @@ enum {
 #define   AUD_CONFIG_UPPER_N_MASK  (0xff << 20)
 #define   AUD_CONFIG_LOWER_N_SHIFT 4
 #define   AUD_CONFIG_LOWER_N_MASK  (0xfff << 4)
+#define   AUD_CONFIG_N_MASK(AUD_CONFIG_UPPER_N_MASK | 
AUD_CONFIG_LOWER_N_MASK)
+#define   AUD_CONFIG_N(n) \
+   (n) >> 12) & 0xff) << AUD_CONFIG_UPPER_N_SHIFT) |   \
+(((n) & 0xfff) << AUD_CONFIG_LOWER_N_SHIFT))
 #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT16
 #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK (0xf << 16)
 #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_25175(0 << 16)
diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 6aa619a84439..d2c6227f72b8 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -135,20 +135,6 @@ static int audio_config_get_n(const struct 
drm_display_mode *adjusted_mode,
return 0;
 }
 
-static uint32_t audio_config_setup_n_reg(int n, uint32_t val)
-{
-   int n_low, n_up;
-   uint32_t tmp = val;
-
-   n_low = n & 0xfff;
-   n_up = (n >> 12) & 0xff;
-   tmp &= ~(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK);
-   tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
-   (n_low << AUD_CONFIG_LOWER_N_SHIFT) |
-   AUD_CONFIG_N_PROG_ENABLE);
-   return tmp;
-}
-
 static bool intel_eld_uptodate(struct drm_connector *connector,
   i915_reg_t reg_eldv, uint32_t bits_eldv,
   i915_reg_t reg_elda, uint32_t bits_elda,
@@ -271,10 +257,13 @@ hsw_hdmi_audio_config_update(struct intel_crtc 
*intel_crtc, enum port port,
if (adjusted_mode->crtc_clock == TMDS_296M ||
adjusted_mode->crtc_clock == TMDS_297M) {
n = audio_config_get_n(adjusted_mode, rate);
-   if (n != 0)
-   tmp = audio_config_setup_n_reg(n, tmp);
-   else
+   if (n != 0) {
+   tmp &= ~AUD_CONFIG_N_MASK;
+   tmp |= AUD_CONFIG_N(n);
+   tmp |= AUD_CONFIG_N_PROG_ENABLE;
+   } else {
DRM_DEBUG_KMS("no suitable N value is found\n");
+   }
}
 
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi

[Jani Nikula]

We'll be getting a function and a table for dp parameters soon enough,
so rename the function and table for hdmi. No functional changes.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index d2c6227f72b8..81df29ca4947 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -81,7 +81,7 @@ static const struct {
int clock;
int n;
int cts;
-} aud_ncts[] = {
+} hdmi_aud_ncts[] = {
{ 44100, TMDS_296M, 4459, 234375 },
{ 44100, TMDS_297M, 4704, 247500 },
{ 48000, TMDS_296M, 5824, 281250 },
@@ -121,15 +121,15 @@ static u32 audio_config_hdmi_pixel_clock(const struct 
drm_display_mode *adjusted
return hdmi_audio_clock[i].config;
 }
 
-static int audio_config_get_n(const struct drm_display_mode *adjusted_mode,
- int rate)
+static int audio_config_hdmi_get_n(const struct drm_display_mode 
*adjusted_mode,
+  int rate)
 {
int i;
 
-   for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
-   if ((rate == aud_ncts[i].sample_rate) &&
-   (adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
-   return aud_ncts[i].n;
+   for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
+   if (rate == hdmi_aud_ncts[i].sample_rate &&
+   adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
+   return hdmi_aud_ncts[i].n;
}
}
return 0;
@@ -256,7 +256,7 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, 
enum port port,
 
if (adjusted_mode->crtc_clock == TMDS_296M ||
adjusted_mode->crtc_clock == TMDS_297M) {
-   n = audio_config_get_n(adjusted_mode, rate);
+   n = audio_config_hdmi_get_n(adjusted_mode, rate);
if (n != 0) {
tmp &= ~AUD_CONFIG_N_MASK;
tmp |= AUD_CONFIG_N(n);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 3/9] drm/i915/audio: use the same code for updating audio config

[Jani Nikula]

It gets fragile to duplicate the code for updating HSW_AUD_CFG. The only
change should be that the hdmi pixel clock is also updated in
i915_audio_component_sync_audio_rate(), but it should not be any
different.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 29 +++--
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 5d0bd07afa51..4d62b3e8ac19 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -671,10 +671,8 @@ static int i915_audio_component_sync_audio_rate(struct 
device *kdev, int port,
struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
struct intel_encoder *intel_encoder;
struct intel_crtc *crtc;
-   struct drm_display_mode *mode;
+   struct drm_display_mode *adjusted_mode;
struct i915_audio_component *acomp = dev_priv->audio_component;
-   u32 tmp;
-   int n;
int err = 0;
 
/* HSW, BDW, SKL, KBL need this fix */
@@ -700,33 +698,12 @@ static int i915_audio_component_sync_audio_rate(struct 
device *kdev, int port,
crtc = to_intel_crtc(intel_encoder->base.crtc);
pipe = crtc->pipe;
 
-   mode = >config->base.adjusted_mode;
+   adjusted_mode = >config->base.adjusted_mode;
 
/* port must be valid now, otherwise the pipe will be invalid */
acomp->aud_sample_rate[port] = rate;
 
-   /* 2. check whether to set the N/CTS/M manually or not */
-   if (!audio_rate_need_prog(crtc, mode)) {
-   tmp = I915_READ(HSW_AUD_CFG(pipe));
-   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
-   goto unlock;
-   }
-
-   n = audio_config_get_n(mode, rate);
-   if (n == 0) {
-   DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
- port_name(port));
-   tmp = I915_READ(HSW_AUD_CFG(pipe));
-   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
-   goto unlock;
-   }
-
-   /* 3. set the N/CTS/M */
-   tmp = I915_READ(HSW_AUD_CFG(pipe));
-   tmp = audio_config_setup_n_reg(n, tmp);
-   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+   hsw_audio_config_update(crtc, port, adjusted_mode);
 
  unlock:
mutex_unlock(_priv->av_mutex);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 1/9] drm/i915/audio: abstract audio config update

[Jani Nikula]

Prepare for using the same code for updating HSW_AUD_CFG register. No
functional changes.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 68 ++
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 9583f432e02e..0a54f7cdce37 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -245,6 +245,45 @@ static void g4x_audio_codec_enable(struct drm_connector 
*connector,
I915_WRITE(G4X_AUD_CNTL_ST, tmp);
 }
 
+static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
+   enum port port,
+   const struct drm_display_mode 
*adjusted_mode)
+{
+   struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+   struct i915_audio_component *acomp = dev_priv->audio_component;
+   enum pipe pipe = intel_crtc->pipe;
+   int n, rate;
+   u32 tmp;
+
+   tmp = I915_READ(HSW_AUD_CFG(pipe));
+   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
+   tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
+   if (intel_crtc_has_dp_encoder(intel_crtc->config))
+   tmp |= AUD_CONFIG_N_VALUE_INDEX;
+   else
+   tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
+
+   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+   if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
+   if (!acomp)
+   rate = 0;
+   else if (port >= PORT_A && port <= PORT_E)
+   rate = acomp->aud_sample_rate[port];
+   else {
+   DRM_ERROR("invalid port: %d\n", port);
+   rate = 0;
+   }
+
+   n = audio_config_get_n(adjusted_mode, rate);
+   if (n != 0)
+   tmp = audio_config_setup_n_reg(n, tmp);
+   else
+   DRM_DEBUG_KMS("no suitable N value is found\n");
+   }
+
+   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+}
+
 static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 {
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -283,11 +322,9 @@ static void hsw_audio_codec_enable(struct drm_connector 
*connector,
struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
enum pipe pipe = intel_crtc->pipe;
enum port port = intel_encoder->port;
-   struct i915_audio_component *acomp = dev_priv->audio_component;
const uint8_t *eld = connector->eld;
uint32_t tmp;
int len, i;
-   int n, rate;
 
DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
  pipe_name(pipe), drm_eld_size(eld));
@@ -323,32 +360,7 @@ static void hsw_audio_codec_enable(struct drm_connector 
*connector,
I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
 
/* Enable timestamps */
-   tmp = I915_READ(HSW_AUD_CFG(pipe));
-   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
-   tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-   if (intel_crtc_has_dp_encoder(intel_crtc->config))
-   tmp |= AUD_CONFIG_N_VALUE_INDEX;
-   else
-   tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
-
-   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-   if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
-   if (!acomp)
-   rate = 0;
-   else if (port >= PORT_A && port <= PORT_E)
-   rate = acomp->aud_sample_rate[port];
-   else {
-   DRM_ERROR("invalid port: %d\n", port);
-   rate = 0;
-   }
-   n = audio_config_get_n(adjusted_mode, rate);
-   if (n != 0)
-   tmp = audio_config_setup_n_reg(n, tmp);
-   else
-   DRM_DEBUG_KMS("no suitable N value is found\n");
-   }
-
-   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+   hsw_audio_config_update(intel_crtc, port, adjusted_mode);
 
mutex_unlock(_priv->av_mutex);
 }
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 6/9] drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock

[Jani Nikula]

From: Libin Yang 

HDMI audio should use crtc_clock to get the TMDS clock.

This patch renames mode to adjusted_mode to unify the name.

Reviewed-by: Ville Syrjälä 
Signed-off-by: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index bce3ad2eb86d..6aa619a84439 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -121,13 +121,14 @@ static u32 audio_config_hdmi_pixel_clock(const struct 
drm_display_mode *adjusted
return hdmi_audio_clock[i].config;
 }
 
-static int audio_config_get_n(const struct drm_display_mode *mode, int rate)
+static int audio_config_get_n(const struct drm_display_mode *adjusted_mode,
+ int rate)
 {
int i;
 
for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
if ((rate == aud_ncts[i].sample_rate) &&
-   (mode->clock == aud_ncts[i].clock)) {
+   (adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
return aud_ncts[i].n;
}
}
@@ -267,8 +268,8 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, 
enum port port,
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
 
-   if (adjusted_mode->clock == TMDS_296M ||
-   adjusted_mode->clock == TMDS_297M) {
+   if (adjusted_mode->crtc_clock == TMDS_296M ||
+   adjusted_mode->crtc_clock == TMDS_297M) {
n = audio_config_get_n(adjusted_mode, rate);
if (n != 0)
tmp = audio_config_setup_n_reg(n, tmp);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 9/9] drm/i915: set proper N/M in modeset

[Jani Nikula]

When modeset occurs and the LS_CLK is set to some
special values in DP mode, the N/M need to be set
manually if audio is playing. Otherwise the first
several seconds may be silent in audio playback.

The relationship of Maud and Naud is expressed in
the following equation:
Maud/Naud = 512 * fs / f_LS_Clk

Please refer VESA DisplayPort Standard spec for details.

Signed-off-by: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_reg.h|   7 +++
 drivers/gpu/drm/i915/intel_audio.c | 100 -
 2 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc04ce68f8b6..2cde9513 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7358,6 +7358,13 @@ enum {
 #define _HSW_AUD_MISC_CTRL_B   0x65110
 #define HSW_AUD_MISC_CTRL(pipe)_MMIO_PIPE(pipe, 
_HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
 
+#define _HSW_AUD_M_CTS_ENABLE_A0x65028
+#define _HSW_AUD_M_CTS_ENABLE_B0x65128
+#define HSW_AUD_M_CTS_ENABLE(pipe) _MMIO_PIPE(pipe, 
_HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
+#define   AUD_M_CTS_M_VALUE_INDEX  (1 << 21)
+#define   AUD_M_CTS_M_PROG_ENABLE  (1 << 20)
+#define   AUD_CONFIG_M_MASK0xf
+
 #define _HSW_AUD_DIP_ELD_CTRL_ST_A 0x650b4
 #define _HSW_AUD_DIP_ELD_CTRL_ST_B 0x651b4
 #define HSW_AUD_DIP_ELD_CTRL(pipe) _MMIO_PIPE(pipe, 
_HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 81df29ca4947..0bc2701b6c9c 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -57,6 +57,70 @@
  * struct _audio_component_audio_ops @audio_ops is called from i915 
driver.
  */
 
+/* DP N/M table */
+#define LC_540M 54
+#define LC_270M 27
+#define LC_162M 162000
+
+struct dp_aud_n_m {
+   int sample_rate;
+   int clock;
+   u16 n;
+   u16 m;
+};
+
+static const struct dp_aud_n_m dp_aud_n_m[] = {
+   { 192000, LC_540M, 5625, 1024 },
+   { 176400, LC_540M, 9375, 1568 },
+   { 96000, LC_540M, 5625, 512 },
+   { 88200, LC_540M, 9375, 784 },
+   { 48000, LC_540M, 5625, 256 },
+   { 44100, LC_540M, 9375, 392 },
+   { 32000, LC_540M, 16875, 512 },
+   { 192000, LC_270M, 5625, 2048 },
+   { 176400, LC_270M, 9375, 3136 },
+   { 96000, LC_270M, 5625, 1024 },
+   { 88200, LC_270M, 9375, 1568 },
+   { 48000, LC_270M, 5625, 512 },
+   { 44100, LC_270M, 9375, 784 },
+   { 32000, LC_270M, 16875, 1024 },
+   { 192000, LC_162M, 3375, 2048 },
+   { 176400, LC_162M, 5625, 3136 },
+   { 96000, LC_162M, 3375, 1024 },
+   { 88200, LC_162M, 5625, 1568 },
+   { 48000, LC_162M, 3375, 512 },
+   { 44100, LC_162M, 5625, 784 },
+   { 32000, LC_162M, 10125, 1024 },
+};
+
+static const struct dp_aud_n_m *
+audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
+   if (rate == dp_aud_n_m[i].sample_rate &&
+   intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
+   return _aud_n_m[i];
+   }
+
+   return NULL;
+}
+
+static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int rate)
+{
+   const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, rate);
+
+   return nm ? nm->m : 0;
+}
+
+static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int rate)
+{
+   const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, rate);
+
+   return nm ? nm->n : 0;
+}
+
 static const struct {
int clock;
u32 config;
@@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, 
enum port port,
   const struct drm_display_mode *adjusted_mode)
 {
struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+   struct i915_audio_component *acomp = dev_priv->audio_component;
+   int rate = acomp ? acomp->aud_sample_rate[port] : 0;
enum pipe pipe = intel_crtc->pipe;
-   u32 tmp;
+   u32 tmp, n, m;
 
tmp = I915_READ(HSW_AUD_CFG(pipe));
tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
@@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, 
enum port port,
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
tmp |= AUD_CONFIG_N_VALUE_INDEX;
 
+   if (intel_crtc->config->port_clock == LC_540M ||
+   intel_crtc->config->port_clock == LC_270M ||
+   intel_crtc->config->port_clock == LC_162M) {
+   n = audio_config_dp_get_n(intel_crtc, rate);
+   if (n != 0) {
+   tmp &= ~AUD_CONFIG_N_MASK;
+   tmp |= AUD_CONFIG_N(n);
+   tmp |= 

[Intel-gfx] [PATCH v2 2/9] drm/i915/audio: port is going to be just fine, simplify checks

[Jani Nikula]

If it was wrong, we'd be screwed already.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 0a54f7cdce37..5d0bd07afa51 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -251,8 +251,9 @@ static void hsw_audio_config_update(struct intel_crtc 
*intel_crtc,
 {
struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
struct i915_audio_component *acomp = dev_priv->audio_component;
+   int rate = acomp ? acomp->aud_sample_rate[port] : 0;
enum pipe pipe = intel_crtc->pipe;
-   int n, rate;
+   int n;
u32 tmp;
 
tmp = I915_READ(HSW_AUD_CFG(pipe));
@@ -265,15 +266,6 @@ static void hsw_audio_config_update(struct intel_crtc 
*intel_crtc,
 
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
-   if (!acomp)
-   rate = 0;
-   else if (port >= PORT_A && port <= PORT_E)
-   rate = acomp->aud_sample_rate[port];
-   else {
-   DRM_ERROR("invalid port: %d\n", port);
-   rate = 0;
-   }
-
n = audio_config_get_n(adjusted_mode, rate);
if (n != 0)
tmp = audio_config_setup_n_reg(n, tmp);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 5/9] drm/i915/audio: set proper N/MCTS on more platforms

[Jani Nikula]

From: Libin Yang 

This patch applies setting proper N/M, N/CTS on more platforms.

Reviewed-by: Ville Syrjälä 
Signed-off-by: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index db7fd6d8333f..bce3ad2eb86d 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -688,11 +688,7 @@ static int i915_audio_component_sync_audio_rate(struct 
device *kdev, int port,
struct i915_audio_component *acomp = dev_priv->audio_component;
int err = 0;
 
-   /* HSW, BDW, SKL, KBL need this fix */
-   if (!IS_SKYLAKE(dev_priv) &&
-   !IS_KABYLAKE(dev_priv) &&
-   !IS_BROADWELL(dev_priv) &&
-   !IS_HASWELL(dev_priv))
+   if (!HAS_DDI(dev_priv))
return 0;
 
i915_audio_component_get_power(kdev);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 4/9] drm/i915/audio: split dp and hdmi audio config update

[Jani Nikula]

The code for dp and hdmi are already different, and they're about to
diverge even more. Split them for clarity in future work. No functional
changes.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 55 +++---
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 4d62b3e8ac19..db7fd6d8333f 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -148,18 +148,6 @@ static uint32_t audio_config_setup_n_reg(int n, uint32_t 
val)
return tmp;
 }
 
-/* check whether N/CTS/M need be set manually */
-static bool audio_rate_need_prog(struct intel_crtc *crtc,
-const struct drm_display_mode *mode)
-{
-   if (((mode->clock == TMDS_297M) ||
-(mode->clock == TMDS_296M)) &&
-   intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
-   return true;
-   else
-   return false;
-}
-
 static bool intel_eld_uptodate(struct drm_connector *connector,
   i915_reg_t reg_eldv, uint32_t bits_eldv,
   i915_reg_t reg_elda, uint32_t bits_elda,
@@ -245,9 +233,26 @@ static void g4x_audio_codec_enable(struct drm_connector 
*connector,
I915_WRITE(G4X_AUD_CNTL_ST, tmp);
 }
 
-static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
-   enum port port,
-   const struct drm_display_mode 
*adjusted_mode)
+static void
+hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+  const struct drm_display_mode *adjusted_mode)
+{
+   struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+   enum pipe pipe = intel_crtc->pipe;
+   u32 tmp;
+
+   tmp = I915_READ(HSW_AUD_CFG(pipe));
+   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
+   tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
+   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+   tmp |= AUD_CONFIG_N_VALUE_INDEX;
+
+   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+}
+
+static void
+hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+const struct drm_display_mode *adjusted_mode)
 {
struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
struct i915_audio_component *acomp = dev_priv->audio_component;
@@ -259,13 +264,11 @@ static void hsw_audio_config_update(struct intel_crtc 
*intel_crtc,
tmp = I915_READ(HSW_AUD_CFG(pipe));
tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-   if (intel_crtc_has_dp_encoder(intel_crtc->config))
-   tmp |= AUD_CONFIG_N_VALUE_INDEX;
-   else
-   tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
-
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-   if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
+   tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
+
+   if (adjusted_mode->clock == TMDS_296M ||
+   adjusted_mode->clock == TMDS_297M) {
n = audio_config_get_n(adjusted_mode, rate);
if (n != 0)
tmp = audio_config_setup_n_reg(n, tmp);
@@ -276,6 +279,16 @@ static void hsw_audio_config_update(struct intel_crtc 
*intel_crtc,
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 }
 
+static void
+hsw_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+   const struct drm_display_mode *adjusted_mode)
+{
+   if (intel_crtc_has_dp_encoder(intel_crtc->config))
+   hsw_dp_audio_config_update(intel_crtc, port, adjusted_mode);
+   else
+   hsw_hdmi_audio_config_update(intel_crtc, port, adjusted_mode);
+}
+
 static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 {
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 0/9] drm/i915/audio: audio cleanups, 4k fixes

[Jani Nikula]

This is v2 of [1], basically just a rebase on top of current nightly.

BR,
Jani.


[1] https://patchwork.freedesktop.org/series/12754/

Jani Nikula (7):
  drm/i915/audio: abstract audio config update
  drm/i915/audio: port is going to be just fine, simplify checks
  drm/i915/audio: use the same code for updating audio config
  drm/i915/audio: split dp and hdmi audio config update
  drm/i915/audio: add register macros for audio config N value
  drm/i915/audio: rename N value getter to emphasize it's for hdmi
  drm/i915: set proper N/M in modeset

Libin Yang (2):
  drm/i915/audio: set proper N/MCTS on more platforms
  drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock

 drivers/gpu/drm/i915/i915_reg.h|  11 ++
 drivers/gpu/drm/i915/intel_audio.c | 260 -
 2 files changed, 179 insertions(+), 92 deletions(-)

-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/15] drm/omap: Use per-plane rotation property

[Tomi Valkeinen]

On 12/08/16 19:04, Ville Syrjälä wrote:
> On Thu, Aug 11, 2016 at 04:33:32PM +0300, Ville Syrjälä wrote:
>> On Thu, Aug 11, 2016 at 02:32:44PM +0300, Tomi Valkeinen wrote:
>>> Hi,
>>>
>>> On 22/07/16 16:43, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä 

 The global mode_config.rotation_property is going away, switch over to
 per-plane rotation_property.

 Not sure I got the annoying crtc rotation_property handling right.
 Might work, or migth not.
>>>
>>> I think something is funny with this patch or the series. I fetched your
>>> branch, and with your series, it looks like the primary planes lose all
>>> their props. modetest says:
>>>
>>> could not get plane 26 properties: Invalid argument
>>> could not get plane 30 properties: Invalid argument
>>
>> Hmm. Weird. Is it really the get props ioctl that fails?
>>
>> The first EINVAL I can spot there is
>> if (!obj->properties) {
>>   ret = -EINVAL;
>>   goto out_unref;
>>  }
>> which definitely makes no sense since this is assigned
>> as plane->base.properties = >properties. So can't be that unless
>> we manage to clear the pointer somehow after the init.
>>
>> The only other direct EINVAL I see there is if
>>  drm_object_property_get_value(obj->properties->properties[i])
>> fails to find the passed prop in the properties array. Which clearly
>> can't happen since we got it from the array in the first place. Also,
>> clearly that code is rather inefficient, perhaps someone should rewrite
>> it a bit.
>>
>> Can't quite see how this could fail for the plane in other ways. But I
>> might be blind.
> 
> I tried to think on this a bit more, and the only think I came up with was
> that we end up doing the drm_plane_create_rotation_property() twice for the
> primary planes. I tried that on i915 but it'd didn't result in anything bad
> AFAICS. Would leak a bit, but so what :P
> 
> Dunno, I guess you could try something like:
> 
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -211,11 +211,12 @@ void omap_plane_install_properties(struct drm_plane 
> *plane,
> struct omap_drm_private *priv = dev->dev_private;
>  
> if (priv->has_dmm) {
> -   drm_plane_create_rotation_property(plane,
> -  BIT(DRM_ROTATE_0),
> -  BIT(DRM_ROTATE_0) | 
> BIT(DRM_ROTATE_90) |
> -  BIT(DRM_ROTATE_180) | 
> BIT(DRM_ROTATE_270) |
> -  BIT(DRM_REFLECT_X) | 
> BIT(DRM_REFLECT_Y));
> +   if (!plane->rotation_property)
> +   drm_plane_create_rotation_property(plane,
> +  BIT(DRM_ROTATE_0),
> +  BIT(DRM_ROTATE_0) 
> | BIT(DRM_ROTATE_90) |
> +  
> BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
> +  BIT(DRM_REFLECT_X) 
> | BIT(DRM_REFLECT_Y));

This fixes the problem... Obviously something gets confused if the
property is created twice. Perhaps the first one gets stored somewhere,
and gets used somehow, even if the latter property is the "real" one,
attached to the plane? Just a guess...

 Tomi



signature.asc
Description: OpenPGP digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Convert prime dma-buf <-> handle to rhashtable

[Sean Paul]

On Thu, Sep 22, 2016 at 7:43 AM, Chris Wilson  wrote:
> Currently we use a linear walk to lookup a handle and return a dma-buf,
> and vice versa. A long overdue TODO task is to convert that to a
> hashtable. Since the initial implementation of dma-buf/prime, we now
> have resizeable hashtables we can use (and now a future task is to RCU
> enable the lookup!).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94631
> Signed-off-by: Chris Wilson 

Looks like a nice improvement

Reviewed-by: Sean Paul 

> ---
>  drivers/gpu/drm/drm_prime.c | 94 
> +++--
>  include/drm/drmP.h  |  5 ++-
>  2 files changed, 77 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 780589b420a4..ad077def660d 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -28,6 +28,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -61,9 +62,11 @@
>   */
>
>  struct drm_prime_member {
> -   struct list_head entry;
> struct dma_buf *dma_buf;
> uint32_t handle;
> +
> +   struct rhash_head dma_buf_rht;
> +   struct rhash_head handle_rht;
>  };
>
>  struct drm_prime_attachment {
> @@ -71,10 +74,31 @@ struct drm_prime_attachment {
> enum dma_data_direction dir;
>  };
>
> +static const struct rhashtable_params dma_buf_params = {
> +   .head_offset = offsetof(struct drm_prime_member, dma_buf_rht),
> +   .key_len = sizeof(struct dma_buf *),
> +   .key_offset = offsetof(struct drm_prime_member, dma_buf),
> +   .hashfn = jhash,
> +   .nulls_base = 1u << RHT_BASE_SHIFT,
> +   .automatic_shrinking = true,
> +   .nelem_hint = 2,
> +};
> +
> +static const struct rhashtable_params handle_params = {
> +   .head_offset = offsetof(struct drm_prime_member, handle_rht),
> +   .key_len = sizeof(uint32_t),
> +   .key_offset = offsetof(struct drm_prime_member, handle),
> +   .hashfn = jhash,
> +   .nulls_base = 1u << RHT_BASE_SHIFT,
> +   .automatic_shrinking = true,
> +   .nelem_hint = 2,
> +};
> +
>  static int drm_prime_add_buf_handle(struct drm_prime_file_private 
> *prime_fpriv,
> struct dma_buf *dma_buf, uint32_t handle)
>  {
> struct drm_prime_member *member;
> +   int err;
>
> member = kmalloc(sizeof(*member), GFP_KERNEL);
> if (!member)
> @@ -83,8 +107,28 @@ static int drm_prime_add_buf_handle(struct 
> drm_prime_file_private *prime_fpriv,
> get_dma_buf(dma_buf);
> member->dma_buf = dma_buf;
> member->handle = handle;
> -   list_add(>entry, _fpriv->head);
> +
> +   err = rhashtable_insert_fast(_fpriv->dma_bufs,
> +>dma_buf_rht,
> +dma_buf_params);
> +   if (err)
> +   goto err_dma_buf;
> +
> +   err = rhashtable_insert_fast(_fpriv->handles,
> +>handle_rht,
> +handle_params);
> +   if (err)
> +   goto err_dma_rht;
> +
> return 0;
> +
> +err_dma_rht:
> +   rhashtable_remove_fast(_fpriv->dma_bufs,
> +  >dma_buf_rht,
> +  dma_buf_params);
> +err_dma_buf:
> +   dma_buf_put(dma_buf);
> +   return err;
>  }
>
>  static struct dma_buf *drm_prime_lookup_buf_by_handle(struct 
> drm_prime_file_private *prime_fpriv,
> @@ -92,10 +136,10 @@ static struct dma_buf 
> *drm_prime_lookup_buf_by_handle(struct drm_prime_file_priv
>  {
> struct drm_prime_member *member;
>
> -   list_for_each_entry(member, _fpriv->head, entry) {
> -   if (member->handle == handle)
> -   return member->dma_buf;
> -   }
> +   member = rhashtable_lookup_fast(_fpriv->handles,
> +   , handle_params);
> +   if (member)
> +   return member->dma_buf;
>
> return NULL;
>  }
> @@ -106,12 +150,13 @@ static int drm_prime_lookup_buf_handle(struct 
> drm_prime_file_private *prime_fpri
>  {
> struct drm_prime_member *member;
>
> -   list_for_each_entry(member, _fpriv->head, entry) {
> -   if (member->dma_buf == dma_buf) {
> -   *handle = member->handle;
> -   return 0;
> -   }
> +   member = rhashtable_lookup_fast(_fpriv->dma_bufs,
> +   _buf, dma_buf_params);
> +   if (member) {
> +   *handle = member->handle;
> +   return 0;
> }
> +
> return -ENOENT;
>  }
>
> @@ -166,14 +211,21 @@ static void drm_gem_map_detach(struct dma_buf *dma_buf,
>  void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private 
> *prime_fpriv,
>

Re: [Intel-gfx] [PATCH v2] drm/i915/bxt: Broxton decoupled MMIO

[Tvrtko Ursulin]

Hi,

On 19/09/2016 18:15, Praveen Paneri wrote:


Decoupled MMIO is an alternative way to access forcewake domain
registers, which requires less cycles and avoids frequent software
forcewake.


I would like to see a sentence or two on how it works here.


v2:
- Moved platform check out of the function and got rid of duplicate
functions to find out decoupled power domain.
- Added a check for forcewake already held and skipped decoupled access
- Skipped writing 64 bit register through decoupled MMIO

Signed-off-by: Zhe Wang 
Signed-off-by: Damien Lespiau 
Signed-off-by: Ankitprasad Sharma 
Signed-off-by: Praveen Paneri 


How come this list of SoBs?


---
  drivers/gpu/drm/i915/i915_drv.h |  11 
  drivers/gpu/drm/i915/i915_reg.h |   7 +++
  drivers/gpu/drm/i915/intel_uncore.c | 118 +++-
  3 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4dd307e..065247b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -558,6 +558,16 @@ enum forcewake_domains {
  #define FW_REG_READ  (1)
  #define FW_REG_WRITE (2)
  
+enum power_domains {


If this is specific to decouples mmio maybe call it decoupled_power_domains?


+   GEN9_DECOUPLED_PD_BLITTER = 0,
+   GEN9_DECOUPLED_PD_RENDER,
+   GEN9_DECOUPLED_PD_MEDIA,
+   GEN9_DECOUPLED_PD_ALL
+};
+
+#define GEN9_DECOUPLED_OP_WRITE (0)
+#define GEN9_DECOUPLED_OP_READ (1)


Would it be more consistent to make these ones enums as well?


+
  enum forcewake_domains
  intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
   i915_reg_t reg, unsigned int op);
@@ -2854,6 +2864,7 @@ struct drm_i915_cmd_table {
  #define GT_FREQUENCY_MULTIPLIER 50
  #define GEN9_FREQ_SCALER 3
  
+#define HAS_DECOUPLED_MMIO(dev_priv) (IS_BROXTON(dev_priv) && IS_BXT_REVID(dev_priv, BXT_REVID_C0, REVID_FOREVER))


There is a recent patch series from Carlos Santa which moved all these 
type of things to device info. So I think you have to make this 
compliant with that new style.



  #include "i915_trace.h"
  
  static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 70d9616..be59803 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7394,6 +7394,13 @@ enum {
  #define  SKL_FUSE_PG1_DIST_STATUS  (1<<26)
  #define  SKL_FUSE_PG2_DIST_STATUS  (1<<25)
  
+/* Decoupled MMIO register pair for kernel driver */

+#define GEN9_DECOUPLED_REG0_DW0_MMIO(0xF00)
+#define GEN9_DECOUPLED_REG0_DW1_MMIO(0xF04)
+#define GEN9_DECOUPLED_DW1_GO  (1<<31)
+#define GEN9_DECOUPLED_PD_SHIFT28
+#define GEN9_DECOUPLED_OP_SHIFT24
+
  /* Per-pipe DDI Function Control */
  #define _TRANS_DDI_FUNC_CTL_A 0x60400
  #define _TRANS_DDI_FUNC_CTL_B 0x61400
diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index a9b6c93..06fffba 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -816,6 +816,42 @@ unclaimed_reg_debug(struct drm_i915_private *dev_priv,
__unclaimed_reg_debug(dev_priv, reg, read, before);
  }
  
+/*

+ * Decoupled MMIO access for only 1 DWORD
+ */
+static void __gen9_decoupled_mmio_access(struct drm_i915_private *dev_priv,
+uint32_t reg, u32 *ptr_data,


Seeing uint32_t and u32 on the same line looks a bit untidy to me. I 
think you should only use one and that in kernel u32 is preferred.



+enum forcewake_domains fw_engine, int 
operation)
+{
+   enum power_domains pd_engine;
+   u32 ctrl_reg_data = 0;
+
+   if (operation == GEN9_DECOUPLED_OP_WRITE)
+   __raw_i915_write32(dev_priv,
+   GEN9_DECOUPLED_REG0_DW0,
+   *ptr_data);
+
+   pd_engine = (fw_engine == (FORCEWAKE_RENDER || FORCEWAKE_BLITTER)) ? \
+!(fw_engine >> 1) : (fw_engine >> 1); \


Feels that look up table would be better.


+
+   ctrl_reg_data |= reg;
+   ctrl_reg_data |= (operation << GEN9_DECOUPLED_OP_SHIFT);
+   ctrl_reg_data |= (pd_engine << GEN9_DECOUPLED_PD_SHIFT);
+   __raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, ctrl_reg_data);
+
+   ctrl_reg_data |= GEN9_DECOUPLED_DW1_GO;
+   __raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, ctrl_reg_data);
+
+   if (wait_for_atomic((__raw_i915_read32(dev_priv,
+   GEN9_DECOUPLED_REG0_DW1) & GEN9_DECOUPLED_DW1_GO) == 0,
+   FORCEWAKE_ACK_TIMEOUT_MS))
+   

Re: [Intel-gfx] [CI 10/21] drm/i915: Mark up all locked waiters

[ch...@chris-wilson.co.uk]

On Thu, Sep 22, 2016 at 04:05:51PM -0300, Paulo Zanoni wrote:
> Em Qua, 2016-09-14 às 09:40 +0100, ch...@chris-wilson.co.uk escreveu:
> > On Tue, Sep 13, 2016 at 03:21:48PM +0300, Mika Kuoppala wrote:
> > > 
> > > Mika Kuoppala  writes:
> > > > 
> > > > "Zanoni, Paulo R"  writes:
> > > > > 
> > > > > > 
> > > > > > +#if IS_ENABLED(CONFIG_LOCKDEP)
> > > > > > +   GEM_BUG_ON(!!lockdep_is_held(>i915-
> > > > > > >drm.struct_mutex)
> > > > > > !=
> > > > > > +      !!(flags & I915_WAIT_LOCKED));
> > > > > > +#endif
> > > > > 
> > > > > I'm hitting this on my SKL. It usually happens right after the
> > > > > login,
> > > > > when I click the terminal icon on the toolbar in order to open
> > > > > it (on
> > > > > Cinnamon). When it doesn't happen at that time, I just open a
> > > > > browser
> > > > > and browse for like 30 seconds until it happens.
> > > > > 
> > > > > I did manual reverts of this series up to this patch and
> > > > > obviously the
> > > > > problem goes away (no more GEM_BUG_ON to hit). I didn't try to
> > > > > do a
> > > > > real bisect to check if this is the first bad commit or if it's
> > > > > something later on the series. It could even be an older
> > > > > problem that
> > > > > just got exposed by the new BUG(). I'm available to test
> > > > > patches, just
> > > > > send them to me.
> > > > 
> > > > No patches yet but there is a comment on 
> > > > intel_prepare_plane_fb() that says that it must be called with
> > > > mutex
> > > > held.
> > > > 
> > > > Quite likely the new GEM_BUG_ON catches this not being true.
> > > > 
> > > 
> > > As Chris pointed out in irc, its about the reverse. Waiting
> > > with mutex when you shouldn't.
> > 
> > Fwiw, the fix is now on the list.
> 
> Can you please tell us which patch it is? I see we still have this
> problem.

https://patchwork.freedesktop.org/series/12703/

Note that the problem is only shown when you enable the BUG_ON. The
impact otherwise is very low.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm: Fix plane type uabi breakage

[Patchwork]

== Series Details ==

Series: drm: Fix plane type uabi breakage
URL   : https://patchwork.freedesktop.org/series/12816/
State : success

== Summary ==

Series 12816v1 drm: Fix plane type uabi breakage
https://patchwork.freedesktop.org/api/1.0/series/12816/revisions/1/mbox/

Test kms_busy:
Subgroup basic-flip-default-a:
fail   -> PASS   (fi-ivb-3520m)
fail   -> PASS   (fi-skl-6700k)
fail   -> PASS   (fi-skl-6260u)
fail   -> PASS   (fi-hsw-4770r)
fail   -> PASS   (fi-skl-6770hq)
fail   -> PASS   (fi-byt-n2820)
fail   -> PASS   (fi-snb-2520m)
fail   -> PASS   (fi-ilk-650)
fail   -> PASS   (fi-snb-2600)
fail   -> PASS   (fi-skl-6700hq)
fail   -> PASS   (fi-bdw-5557u)
fail   -> PASS   (fi-hsw-4770k)
Subgroup basic-flip-default-b:
fail   -> PASS   (fi-ivb-3520m)
fail   -> PASS   (fi-skl-6700k)
fail   -> PASS   (fi-skl-6260u)
fail   -> PASS   (fi-hsw-4770r)
fail   -> PASS   (fi-skl-6770hq)
fail   -> PASS   (fi-byt-n2820)
fail   -> PASS   (fi-snb-2520m)
fail   -> PASS   (fi-ilk-650)
fail   -> PASS   (fi-snb-2600)
fail   -> PASS   (fi-skl-6700hq)
fail   -> PASS   (fi-bdw-5557u)
fail   -> PASS   (fi-hsw-4770k)
Subgroup basic-flip-default-c:
fail   -> PASS   (fi-skl-6770hq)
fail   -> PASS   (fi-bsw-n3050)
fail   -> PASS   (fi-ivb-3520m)
fail   -> PASS   (fi-hsw-4770k)
fail   -> PASS   (fi-skl-6260u)
fail   -> PASS   (fi-hsw-4770r)
fail   -> PASS   (fi-skl-6700k)
fail   -> PASS   (fi-bdw-5557u)
fail   -> PASS   (fi-skl-6700hq)
Test kms_cursor_legacy:
Subgroup basic-flip-after-cursor-legacy:
fail   -> PASS   (fi-ivb-3520m)
fail   -> PASS   (fi-skl-6700hq)
fail   -> PASS   (fi-skl-6260u)
fail   -> PASS   (fi-hsw-4770r)
fail   -> PASS   (fi-skl-6770hq)
fail   -> PASS   (fi-byt-n2820)
fail   -> PASS   (fi-snb-2520m)
fail   -> PASS   (fi-ilk-650)
fail   -> PASS   (fi-snb-2600)
fail   -> PASS   (fi-bsw-n3050)
fail   -> PASS   (fi-skl-6700k)
fail   -> PASS   (fi-bdw-5557u)
fail   -> PASS   (fi-hsw-4770k)
Subgroup basic-flip-after-cursor-varying-size:
fail   -> PASS   (fi-ivb-3520m)
fail   -> PASS   (fi-skl-6700hq)
fail   -> PASS   (fi-skl-6260u)
fail   -> PASS   (fi-hsw-4770r)
fail   -> PASS   (fi-skl-6770hq)
fail   -> PASS   (fi-byt-n2820)
fail   -> PASS   (fi-snb-2520m)
fail   -> PASS   (fi-ilk-650)
fail   -> PASS   (fi-snb-2600)
fail   -> PASS   (fi-bsw-n3050)
fail   -> PASS   (fi-skl-6700k)
fail   -> PASS   (fi-bdw-5557u)
fail   -> PASS   (fi-hsw-4770k)
Subgroup basic-flip-before-cursor-legacy:
fail   -> PASS   (fi-ivb-3520m)
fail   -> PASS   (fi-skl-6700hq)
fail   -> PASS   (fi-skl-6260u)
fail   -> PASS   (fi-hsw-4770r)
fail   -> PASS   (fi-skl-6770hq)
fail   -> PASS   (fi-byt-n2820)
fail   -> PASS   (fi-snb-2520m)
fail   -> PASS   (fi-ilk-650)
fail   -> PASS   (fi-snb-2600)
fail   -> PASS   (fi-bsw-n3050)
fail   -> PASS   (fi-skl-6700k)
fail   -> PASS   (fi-bdw-5557u)
fail   -> PASS   (fi-hsw-4770k)
Subgroup basic-flip-before-cursor-varying-size:
fail   -> PASS   (fi-ivb-3520m)
fail   -> PASS   (fi-skl-6700hq)
fail   -> PASS   (fi-skl-6260u)
fail   -> PASS   (fi-hsw-4770r)
fail   -> PASS   (fi-skl-6770hq)
fail   -> PASS   

Re: [Intel-gfx] [PATCH] drm: Fix plane type uabi breakage

[Sean Paul]

On Thu, Sep 22, 2016 at 11:44 PM, Jani Nikula
 wrote:
> On Fri, 23 Sep 2016, Daniel Vetter  wrote:
>> Turns out assuming that only stuff in uabi is uabi is a bit naive, and
>> we have a bunch of properties for which the enum values are placed in
>> random headers. A proper fix would be to split out uapi include
>> headers, but meanwhile sprinkle at least some warning over them.
>>
>> Fixes: 532b36712ddf ("drm/doc: Polish for drm_plane.[hc]")
>> Cc: Archit Taneja 
>> Cc: Sean Paul 
>> Signed-off-by: Daniel Vetter 
>
> Reviewed-by: Jani Nikula 
>

Thanks, applied to drm-misc

Sean

>
>> ---
>>  include/drm/drm_blend.h |  3 +++
>>  include/drm/drm_plane.h | 19 +++
>>  2 files changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
>> index 868f0364e939..36baa175de99 100644
>> --- a/include/drm/drm_blend.h
>> +++ b/include/drm/drm_blend.h
>> @@ -33,6 +33,9 @@ struct drm_atomic_state;
>>   * Rotation property bits. DRM_ROTATE_ rotates the image by the
>>   * specified amount in degrees in counter clockwise direction. 
>> DRM_REFLECT_X and
>>   * DRM_REFLECT_Y reflects the image along the specified axis prior to 
>> rotation
>> + *
>> + * WARNING: These defines are UABI since they're exposed in the rotation
>> + * property.
>>   */
>>  #define DRM_ROTATE_0 BIT(0)
>>  #define DRM_ROTATE_90BIT(1)
>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>> index 256219bfd07b..43cf193e54d6 100644
>> --- a/include/drm/drm_plane.h
>> +++ b/include/drm/drm_plane.h
>> @@ -333,9 +333,20 @@ struct drm_plane_funcs {
>>   * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that 
>> they
>>   * wish to receive a universal plane list containing all plane types. See 
>> also
>>   * drm_for_each_legacy_plane().
>> + *
>> + * WARNING: The values of this enum is UABI since they're exposed in the 
>> "type"
>> + * property.
>>   */
>>  enum drm_plane_type {
>>   /**
>> +  * @DRM_PLANE_TYPE_OVERLAY:
>> +  *
>> +  * Overlay planes represent all non-primary, non-cursor planes. Some
>> +  * drivers refer to these types of planes as "sprites" internally.
>> +  */
>> + DRM_PLANE_TYPE_OVERLAY,
>> +
>> + /**
>>* @DRM_PLANE_TYPE_PRIMARY:
>>*
>>* Primary planes represent a "main" plane for a CRTC.  Primary planes
>> @@ -353,14 +364,6 @@ enum drm_plane_type {
>>* DRM_IOCTL_MODE_CURSOR2 IOCTLs.
>>*/
>>   DRM_PLANE_TYPE_CURSOR,
>> -
>> - /**
>> -  * @DRM_PLANE_TYPE_OVERLAY:
>> -  *
>> -  * Overlay planes represent all non-primary, non-cursor planes. Some
>> -  * drivers refer to these types of planes as "sprites" internally.
>> -  */
>> - DRM_PLANE_TYPE_OVERLAY,
>>  };
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Fix plane type uabi breakage

[Archit Taneja]

On 09/23/2016 12:05 PM, Daniel Vetter wrote:

Turns out assuming that only stuff in uabi is uabi is a bit naive, and
we have a bunch of properties for which the enum values are placed in
random headers. A proper fix would be to split out uapi include
headers, but meanwhile sprinkle at least some warning over them.

Fixes: 532b36712ddf ("drm/doc: Polish for drm_plane.[hc]")
Cc: Archit Taneja 
Cc: Sean Paul 
Signed-off-by: Daniel Vetter 


Reviewed-by: Archit Taneja 


---
 include/drm/drm_blend.h |  3 +++
 include/drm/drm_plane.h | 19 +++
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 868f0364e939..36baa175de99 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -33,6 +33,9 @@ struct drm_atomic_state;
  * Rotation property bits. DRM_ROTATE_ rotates the image by the
  * specified amount in degrees in counter clockwise direction. DRM_REFLECT_X 
and
  * DRM_REFLECT_Y reflects the image along the specified axis prior to rotation
+ *
+ * WARNING: These defines are UABI since they're exposed in the rotation
+ * property.
  */
 #define DRM_ROTATE_0   BIT(0)
 #define DRM_ROTATE_90  BIT(1)
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 256219bfd07b..43cf193e54d6 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -333,9 +333,20 @@ struct drm_plane_funcs {
  * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
  * wish to receive a universal plane list containing all plane types. See also
  * drm_for_each_legacy_plane().
+ *
+ * WARNING: The values of this enum is UABI since they're exposed in the "type"
+ * property.
  */
 enum drm_plane_type {
/**
+* @DRM_PLANE_TYPE_OVERLAY:
+*
+* Overlay planes represent all non-primary, non-cursor planes. Some
+* drivers refer to these types of planes as "sprites" internally.
+*/
+   DRM_PLANE_TYPE_OVERLAY,
+
+   /**
 * @DRM_PLANE_TYPE_PRIMARY:
 *
 * Primary planes represent a "main" plane for a CRTC.  Primary planes
@@ -353,14 +364,6 @@ enum drm_plane_type {
 * DRM_IOCTL_MODE_CURSOR2 IOCTLs.
 */
DRM_PLANE_TYPE_CURSOR,
-
-   /**
-* @DRM_PLANE_TYPE_OVERLAY:
-*
-* Overlay planes represent all non-primary, non-cursor planes. Some
-* drivers refer to these types of planes as "sprites" internally.
-*/
-   DRM_PLANE_TYPE_OVERLAY,
 };





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

Re: [Intel-gfx] [PATCH] drm: Fix plane type uabi breakage

[Jani Nikula]

On Fri, 23 Sep 2016, Daniel Vetter  wrote:
> Turns out assuming that only stuff in uabi is uabi is a bit naive, and
> we have a bunch of properties for which the enum values are placed in
> random headers. A proper fix would be to split out uapi include
> headers, but meanwhile sprinkle at least some warning over them.
>
> Fixes: 532b36712ddf ("drm/doc: Polish for drm_plane.[hc]")
> Cc: Archit Taneja 
> Cc: Sean Paul 
> Signed-off-by: Daniel Vetter 

Reviewed-by: Jani Nikula 


> ---
>  include/drm/drm_blend.h |  3 +++
>  include/drm/drm_plane.h | 19 +++
>  2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 868f0364e939..36baa175de99 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -33,6 +33,9 @@ struct drm_atomic_state;
>   * Rotation property bits. DRM_ROTATE_ rotates the image by the
>   * specified amount in degrees in counter clockwise direction. DRM_REFLECT_X 
> and
>   * DRM_REFLECT_Y reflects the image along the specified axis prior to 
> rotation
> + *
> + * WARNING: These defines are UABI since they're exposed in the rotation
> + * property.
>   */
>  #define DRM_ROTATE_0 BIT(0)
>  #define DRM_ROTATE_90BIT(1)
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 256219bfd07b..43cf193e54d6 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -333,9 +333,20 @@ struct drm_plane_funcs {
>   * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that 
> they
>   * wish to receive a universal plane list containing all plane types. See 
> also
>   * drm_for_each_legacy_plane().
> + *
> + * WARNING: The values of this enum is UABI since they're exposed in the 
> "type"
> + * property.
>   */
>  enum drm_plane_type {
>   /**
> +  * @DRM_PLANE_TYPE_OVERLAY:
> +  *
> +  * Overlay planes represent all non-primary, non-cursor planes. Some
> +  * drivers refer to these types of planes as "sprites" internally.
> +  */
> + DRM_PLANE_TYPE_OVERLAY,
> +
> + /**
>* @DRM_PLANE_TYPE_PRIMARY:
>*
>* Primary planes represent a "main" plane for a CRTC.  Primary planes
> @@ -353,14 +364,6 @@ enum drm_plane_type {
>* DRM_IOCTL_MODE_CURSOR2 IOCTLs.
>*/
>   DRM_PLANE_TYPE_CURSOR,
> -
> - /**
> -  * @DRM_PLANE_TYPE_OVERLAY:
> -  *
> -  * Overlay planes represent all non-primary, non-cursor planes. Some
> -  * drivers refer to these types of planes as "sprites" internally.
> -  */
> - DRM_PLANE_TYPE_OVERLAY,
>  };

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm: Fix plane type uabi breakage

[Daniel Vetter]

Turns out assuming that only stuff in uabi is uabi is a bit naive, and
we have a bunch of properties for which the enum values are placed in
random headers. A proper fix would be to split out uapi include
headers, but meanwhile sprinkle at least some warning over them.

Fixes: 532b36712ddf ("drm/doc: Polish for drm_plane.[hc]")
Cc: Archit Taneja 
Cc: Sean Paul 
Signed-off-by: Daniel Vetter 
---
 include/drm/drm_blend.h |  3 +++
 include/drm/drm_plane.h | 19 +++
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 868f0364e939..36baa175de99 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -33,6 +33,9 @@ struct drm_atomic_state;
  * Rotation property bits. DRM_ROTATE_ rotates the image by the
  * specified amount in degrees in counter clockwise direction. DRM_REFLECT_X 
and
  * DRM_REFLECT_Y reflects the image along the specified axis prior to rotation
+ *
+ * WARNING: These defines are UABI since they're exposed in the rotation
+ * property.
  */
 #define DRM_ROTATE_0   BIT(0)
 #define DRM_ROTATE_90  BIT(1)
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 256219bfd07b..43cf193e54d6 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -333,9 +333,20 @@ struct drm_plane_funcs {
  * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
  * wish to receive a universal plane list containing all plane types. See also
  * drm_for_each_legacy_plane().
+ *
+ * WARNING: The values of this enum is UABI since they're exposed in the "type"
+ * property.
  */
 enum drm_plane_type {
/**
+* @DRM_PLANE_TYPE_OVERLAY:
+*
+* Overlay planes represent all non-primary, non-cursor planes. Some
+* drivers refer to these types of planes as "sprites" internally.
+*/
+   DRM_PLANE_TYPE_OVERLAY,
+
+   /**
 * @DRM_PLANE_TYPE_PRIMARY:
 *
 * Primary planes represent a "main" plane for a CRTC.  Primary planes
@@ -353,14 +364,6 @@ enum drm_plane_type {
 * DRM_IOCTL_MODE_CURSOR2 IOCTLs.
 */
DRM_PLANE_TYPE_CURSOR,
-
-   /**
-* @DRM_PLANE_TYPE_OVERLAY:
-*
-* Overlay planes represent all non-primary, non-cursor planes. Some
-* drivers refer to these types of planes as "sprites" internally.
-*/
-   DRM_PLANE_TYPE_OVERLAY,
 };
 
 
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx