[PATCH] drm/udl: Fix for the X server screen update v3

2016-09-21 Thread Jani Nikula
On Wed, 21 Sep 2016, poma  wrote:
> On 21.09.2016 13:33, David Herrmann wrote:
>> The author of a patch must provide the S-o-b (see
>> Documentation/SubmittingPatches if you want details). So simply reply
>> with a "S-o-b: foo " line to this mail. And please include it in
>> all patches you submit (preferably use `git commit --sign-off`).
>> 
>> Thanks
>> David
>> 
>
> Patches are nothing but direct suggestions from Noralf and Daniel.
> I wrote a patch to show what is actually tested, tested successfully,
> but I'm not the author of these corrections.

For the third and last time, see Documentation/SubmittingPatches.

The Signed-off-by tag is not about authorship, it's about "Developer's
Certificate of Origin". The text is also available at
http://developercertificate.org/. If you send a patch, no matter whose
patch it is, we can't apply it without your Signed-off-by.

Thanks for your understanding.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center


[PATCH] dma-buf/fence-array: get signaled state when signaling is disabled

2016-09-21 Thread Christian König
Am 21.09.2016 um 13:36 schrieb Gustavo Padovan:
> From: Gustavo Padovan 
>
> If the fences in the fence_array signal on the fence_array does not have
> signalling enabled num_pending will not be updated accordingly.
>
> So when signaling is disabled check the signal of every fence with
> fence_is_signaled() and then compare with num_pending to learn if the
> fence_array was signalled or not.
>
> If we want to keep the poll_does_not_wait optimization I think we need
> something like this. It keeps the same behaviour if signalling is enabled
> but tries to calculated the state otherwise.
>
> Signed-off-by: Gustavo Padovan 
> Reviewed-by: Chris Wilson 

First of all the patch is horrible wrong because fence_array_signaled() 
is called without any locks held. So you can run into a race condition 
between checking the fences here and enable signaling.

Additional to that I'm not sure if that is such a good idea or not, 
cause fence_array_signaled() should be light weight and without calling 
enable_signaling there is not guarantee that fences will ever signal.

Regards,
Christian.

> ---
>   drivers/dma-buf/fence-array.c | 19 ++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/fence-array.c b/drivers/dma-buf/fence-array.c
> index f1989fc..1eec271 100644
> --- a/drivers/dma-buf/fence-array.c
> +++ b/drivers/dma-buf/fence-array.c
> @@ -75,8 +75,25 @@ static bool fence_array_enable_signaling(struct fence 
> *fence)
>   static bool fence_array_signaled(struct fence *fence)
>   {
>   struct fence_array *array = to_fence_array(fence);
> + int i, num_pending;
> +
> + num_pending = atomic_read(>num_pending);
> +
> + /*
> +  * Before signaling is enabled, num_pending is static (set during array
> +  * construction as a count of all fences or set to 1 if signal_on_any
> +  * flag is passed. To ensure forward progress, i.e. a while
> +  * (!fence_is_signaled()) ; busy-loop eventually proceeds, we need to
> +  * check the current status of our fences.
> +  */
> + if (!test_bit(FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) {
> + for (i = 0 ; i < array->num_fences; ++i) {
> + if (fence_is_signaled(array->fences[i]))
> + num_pending--;
> + }
> + }
>   
> - return atomic_read(>num_pending) <= 0;
> + return num_pending <= 0;
>   }
>   
>   static void fence_array_release(struct fence *fence)




[Bug 172421] radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #9 from Elmar Stellnberger  ---
any other opinion by someone else on this issue?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[Bug 172421] radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #8 from Alex Deucher  ---
Running the hardware out of spec is not something we want to support.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[Bug 172421] radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #7 from Elmar Stellnberger  ---
  It obviously works stable over HDMI as well. Besides this I have just tested
an ATI Mobility Radeon HD 2600 XT/2700 and it can provide 3840x2160_22.00 over
its HDMI port with a radeon.hdmimhz of 250. With a hdmimhz of 297 I get some
screen distortions but that does not harm the card.
  Why not just unlock a new feature of so many radeon cards? Nouveau developers
did so for long with their device driver.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97879

--- Comment #9 from Tobias Droste  ---
Ah should have mentioned it:

This is with radeon+radeonsi (no amdpgu) so it's probably not the kernel.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/09c75186/attachment.html>


[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97879

--- Comment #8 from Tobias Droste  ---
On the first screenshot you can clearly see the FPS going down to 0 multiple
times. 
Though looking at the graph only the first 2 drops could be explained by shader
compiles if I interpret this correctly. I can't tell what's happening after
that.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/11e93e23/attachment.html>


[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97879

--- Comment #7 from Tobias Droste  ---
Created attachment 126717
  --> https://bugs.freedesktop.org/attachment.cgi?id=126717=edit
Screenshot with GALLIUM_HUD of the first 2min30s of the game (good)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/d1183b44/attachment.html>


[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97879

--- Comment #6 from Tobias Droste  ---
Created attachment 126716
  --> https://bugs.freedesktop.org/attachment.cgi?id=126716=edit
Screenshot with GALLIUM_HUD of the first 1min30s of the game (good)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/e6b10902/attachment-0001.html>


[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97879

--- Comment #5 from Tobias Droste  ---
Created attachment 126715
  --> https://bugs.freedesktop.org/attachment.cgi?id=126715=edit
Screenshot with GALLIUM_HUD of the first 10s of the game (bad)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/91322a92/attachment.html>


[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

2016-09-21 Thread Michel Dänzer
On 13/09/16 09:52 PM, Christian König wrote:
> Am 13.09.2016 um 11:39 schrieb Chris Wilson:
>> On Tue, Sep 13, 2016 at 10:44:11AM +0200, Christian König wrote:
>>> Am 09.09.2016 um 03:15 schrieb Michel Dänzer:
 On 09/09/16 01:23 AM, Chris Wilson wrote:
> On Thu, Sep 08, 2016 at 05:21:42PM +0200, Mario Kleiner wrote:
>> On 09/08/2016 08:30 AM, Chris Wilson wrote:
>>> On Thu, Sep 08, 2016 at 02:14:43AM +0200, Mario Kleiner wrote:
 amdgpu-kms uses shared fences for its prime exported dmabufs,
 instead of an exclusive fence. Therefore we need to wait for
 all fences of the dmabuf reservation object to prevent
 unsynchronized rendering and flipping.
>>> No. Fix the root cause as this affects not just flips but copies -
>>> this implies that everybody using the resv object must wait for all
>>> fences. The resv object is not just used for prime, but all
>>> fencing, so
>>> this breaks the ability to schedule parallel operations across
>>> engine.
>>> -Chris
>>>
>> Ok. I think i now understand the difference, but let's check: The
>> exclusive fence is essentially acting a bit like a write-lock, and
>> the shared fences as readers-locks? So you can have multiple readers
>> but only one writer at a time?
> That's how we (i915.ko and I hope the rest of the world) are using
> them.
> In the model where here is just one reservation object on the GEM
> object, that reservation object is then shared between internal driver
> scheduling and external. We are reliant on being able to use
> buffers on
> multiple engines through the virtue of the shared fences, and to
> serialise
> after writes by waiting on the exclusive fence. (So we can have
> concurrent
> reads on the display engine, render engines and on the CPU - or
> alternatively an exclusive writer.)
>
> In the near future, i915 flips will wait on the common reservation
> object
> not only for dma-bufs, but also its own GEM objects.
>> Ie.:
>>
>> Writer must wait for all fences before starting write access to a
>> buffer, then attach the exclusive fence and signal it on end of
>> write access. E.g., write to renderbuffer, write to texture etc.
> Yes.
>> Readers must wait for exclusive fence, then attach a shared fence
>> per reader and signal it on end of read access? E.g., read from
>> texture, fb, scanout?
> Yes.
>> Is that correct? In that case we'd have a missing exclusive fence in
>> amdgpu for the linear target dmabuf? Probably beyond my level of
>> knowledge to fix this?
> i915.ko requires the client to mark which buffers are written to.
>
> In ttm, there are ttm_validate_buffer objects which mark whether they
> should be using shared or exclusive fences. Afaict, in amdgpu they are
> all set to shared, the relevant user interface seems to be
> amdgpu_bo_list_set().
 This all makes sense to me.

 Christian, why is amdgpu setting only shared fences? Can we fix that?
>>> No, amdgpu relies on the fact that we even allow concurrent write
>>> accesses by userspace.
>>>
>>> E.g. one part of the buffer can be rendered by one engine while
>>> another part could be rendered by another engine.
>>>
>>> Just imagine X which is composing a buffer with both the 3D engine
>>> as well as the DMA engine.
>>>
>>> All engines need to run in parallel and you need to wait for all of
>>> them to finish before scanout.
>>>
>>> Everybody which needs exclusive access to the reservation object
>>> (like scanouts do) needs to wait for all fences, not just the
>>> exclusive one.
>>>
>>> The Intel driver clearly needs to be fixed here.
>> If you are not using implicit fencing, you have to pass explicit fences
>> instead.
> 
> Which is exactly what we do, but only for the driver internally command
> submissions.
> 
> All command submissions from the same process can run concurrently with
> amdgpu, only when we see a fence from another driver or process we wait
> for it to complete before starting to run a command submission.
> 
> Other drivers can't make any assumption on what a shared access is
> actually doing (e.g. writing or reading) with a buffer.
> 
> So the model i915.ko is using the reservation object and it's shared
> fences is certainly not correct and needs to be fixed.

Looks like there are different interpretations of the semantics of
exclusive vs. shared fences. Where are these semantics documented?


FWIW, we seem to have the same issue with radeon vs. amdgpu: radeon only
seems to wait for exclusive fences, so e.g. running Xorg on amdgpu and
using PRIME slave scanout on radeon leaves artifacts.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer


[PATCH 14/14] GPU-DRM-OMAP: Rename a jump label in four functions

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 18:00:23 +0200

Adjust jump labels according to the current Linux coding style convention.
Thus replace the identifier "fail" by "unlock" for this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 27 +++
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 92510de..ea7ad1c 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -549,7 +549,7 @@ int omap_gem_fault(struct vm_area_struct *vma, struct 
vm_fault *vmf)
/* if a shmem backed object, make sure we have pages attached now */
ret = get_pages(obj, );
if (ret)
-   goto fail;
+   goto unlock;

/* where should we do corresponding put_pages().. we are mapping
 * the original page, rather than thru a GART, so we can't rely
@@ -561,9 +561,7 @@ int omap_gem_fault(struct vm_area_struct *vma, struct 
vm_fault *vmf)
ret = fault_2d(obj, vma, vmf);
else
ret = fault_1d(obj, vma, vmf);
-
-
-fail:
+ unlock:
mutex_unlock(>struct_mutex);
switch (ret) {
case 0:
@@ -682,14 +680,13 @@ int omap_gem_dumb_map_offset(struct drm_file *file, 
struct drm_device *dev,
obj = drm_gem_object_lookup(file, handle);
if (obj == NULL) {
ret = -ENOENT;
-   goto fail;
+   goto unlock;
}

*offset = omap_gem_mmap_offset(obj);

drm_gem_object_unreference_unlocked(obj);
-
-fail:
+ unlock:
return ret;
 }

@@ -719,13 +716,12 @@ int omap_gem_roll(struct drm_gem_object *obj, uint32_t 
roll)
struct page **pages;
ret = get_pages(obj, );
if (ret)
-   goto fail;
+   goto unlock;
ret = tiler_pin(omap_obj->block, pages, npages, roll, true);
if (ret)
dev_err(obj->dev->dev, "could not repin: %d\n", ret);
}
-
-fail:
+ unlock:
mutex_unlock(>dev->struct_mutex);

return ret;
@@ -825,7 +821,7 @@ int omap_gem_get_paddr(struct drm_gem_object *obj,

ret = get_pages(obj, );
if (ret)
-   goto fail;
+   goto unlock;

if (omap_obj->flags & OMAP_BO_TILED) {
block = tiler_reserve_2d(fmt,
@@ -839,7 +835,7 @@ int omap_gem_get_paddr(struct drm_gem_object *obj,
ret = PTR_ERR(block);
dev_err(obj->dev->dev,
"could not remap: %d (%d)\n", ret, fmt);
-   goto fail;
+   goto unlock;
}

/* TODO: enable async refill.. */
@@ -849,7 +845,7 @@ int omap_gem_get_paddr(struct drm_gem_object *obj,
tiler_release(block);
dev_err(obj->dev->dev,
"could not pin: %d\n", ret);
-   goto fail;
+   goto unlock;
}

omap_obj->paddr = tiler_ssptr(block);
@@ -865,10 +861,9 @@ int omap_gem_get_paddr(struct drm_gem_object *obj,
*paddr = omap_obj->paddr;
} else {
ret = -EINVAL;
-   goto fail;
+   goto unlock;
}
-
-fail:
+ unlock:
mutex_unlock(>dev->struct_mutex);

return ret;
-- 
2.10.0



[PATCH 13/14] GPU-DRM-OMAP: Rename a jump label in omap_gem_new_dmabuf()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 17:45:04 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 3c49ad9..92510de 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1442,7 +1442,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct 
drm_device *dev, size_t size,
obj = omap_gem_new(dev, gsize, OMAP_BO_MEM_DMABUF | OMAP_BO_WC);
if (!obj) {
obj = ERR_PTR(-ENOMEM);
-   goto done;
+   goto unlock;
}

omap_obj = to_omap_bo(obj);
@@ -1462,7 +1462,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct 
drm_device *dev, size_t size,
if (!pages) {
omap_gem_free_object(obj);
obj = ERR_PTR(-ENOMEM);
-   goto done;
+   goto unlock;
}

omap_obj->pages = pages;
@@ -1476,11 +1476,10 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct 
drm_device *dev, size_t size,
if (WARN_ON(i != npages)) {
omap_gem_free_object(obj);
obj = ERR_PTR(-ENOMEM);
-   goto done;
+   goto unlock;
}
}
-
-done:
+ unlock:
mutex_unlock(>struct_mutex);
return obj;
 }
-- 
2.10.0



[PATCH 12/14] GPU-DRM-OMAP: Move a variable assignment in omap_gem_attach_pages()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 17:42:28 +0200

Move one assignment for the local variable "npages" so that its setting
will only be performed after a call of the function "drm_gem_get_pages"
succeeded by this function.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 26f1212..3c49ad9 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -243,7 +243,7 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
struct drm_device *dev = obj->dev;
struct omap_gem_object *omap_obj = to_omap_bo(obj);
struct page **pages;
-   int npages = obj->size >> PAGE_SHIFT;
+   int npages;
int i, ret;
dma_addr_t *addrs;

@@ -255,6 +255,8 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
return PTR_ERR(pages);
}

+   npages = obj->size >> PAGE_SHIFT;
+
/* for non-cached buffers, ensure the new pages are clean because
 * DSS, GPU, etc. are not cache coherent:
 */
-- 
2.10.0



[PATCH 11/14] GPU-DRM-OMAP: Replace a kzalloc() call by kcalloc() in omap_gem_attach_pages()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 17:40:20 +0200

The script "checkpatch.pl" can point information out like the following.

WARNING: Prefer kcalloc over kzalloc with multiply

Thus fix the affected source code place.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index e4f1924..26f1212 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -283,7 +283,7 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
}
}
} else {
-   addrs = kzalloc(npages * sizeof(*addrs), GFP_KERNEL);
+   addrs = kcalloc(npages, sizeof(*addrs), GFP_KERNEL);
if (!addrs) {
ret = -ENOMEM;
goto free_pages;
-- 
2.10.0



[PATCH 10/14] GPU-DRM-OMAP: Use kmalloc_array() in omap_gem_attach_pages()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 17:37:04 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 505dee0..e4f1924 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -259,7 +259,7 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
 * DSS, GPU, etc. are not cache coherent:
 */
if (omap_obj->flags & (OMAP_BO_WC|OMAP_BO_UNCACHED)) {
-   addrs = kmalloc(npages * sizeof(*addrs), GFP_KERNEL);
+   addrs = kmalloc_array(npages, sizeof(*addrs), GFP_KERNEL);
if (!addrs) {
ret = -ENOMEM;
goto free_pages;
-- 
2.10.0



[PATCH 09/14] GPU-DRM-OMAP: Delete an unnecessary variable initialisation in dmm_txn_commit()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 17:34:40 +0200

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index c8ced158..c5c3793 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -262,7 +262,7 @@ static void dmm_txn_append(struct dmm_txn *txn, struct 
pat_area *area,
  */
 static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
 {
-   int ret = 0;
+   int ret;
struct refill_engine *engine = txn->engine_handle;
struct dmm *dmm = engine->dmm;

-- 
2.10.0



[PATCH 08/14] GPU-DRM-OMAP: Rename a jump label in dmm_txn_commit()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 17:32:42 +0200

Adjust a jump target so that redundant checks can be avoided at the end.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 5f6f21b..c8ced158 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -269,7 +269,7 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
if (!txn->last_pat) {
dev_err(engine->dmm->dev, "need at least one txn\n");
ret = -EINVAL;
-   goto cleanup;
+   goto release_engine;
}

txn->last_pat->next_pa = 0;
@@ -281,7 +281,7 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
ret = wait_status(engine, DMM_PATSTATUS_READY);
if (ret) {
ret = -EFAULT;
-   goto cleanup;
+   goto release_engine;
}

/* mark whether it is async to denote list management in IRQ handler */
@@ -301,9 +301,9 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
}
}

-cleanup:
/* only place engine back on list if we are done with it */
if (ret || wait)
+ release_engine:
release_engine(engine);

return ret;
-- 
2.10.0



[Bug 172421] radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #6 from Alex Deucher  ---
The asic supports 4K over DP or duallink DVI.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[PATCH 07/14] GPU-DRM-OMAP: Rename a jump label in omap_dmm_probe()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 17:30:25 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index c6a7197..5f6f21b 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -624,7 +624,7 @@ static int omap_dmm_probe(struct platform_device *dev)

omap_dmm = kzalloc(sizeof(*omap_dmm), GFP_KERNEL);
if (!omap_dmm)
-   goto fail;
+   goto check_dmm_removal;

/* initialize lists */
INIT_LIST_HEAD(_dmm->alloc_head);
@@ -648,20 +648,20 @@ static int omap_dmm_probe(struct platform_device *dev)
mem = platform_get_resource(dev, IORESOURCE_MEM, 0);
if (!mem) {
dev_err(>dev, "failed to get base address resource\n");
-   goto fail;
+   goto check_dmm_removal;
}

omap_dmm->base = ioremap(mem->start, SZ_2K);

if (!omap_dmm->base) {
dev_err(>dev, "failed to get dmm base address\n");
-   goto fail;
+   goto check_dmm_removal;
}

omap_dmm->irq = platform_get_irq(dev, 0);
if (omap_dmm->irq < 0) {
dev_err(>dev, "failed to get IRQ resource\n");
-   goto fail;
+   goto check_dmm_removal;
}

omap_dmm->dev = >dev;
@@ -699,7 +699,7 @@ static int omap_dmm_probe(struct platform_device *dev)
dev_err(>dev, "couldn't register IRQ %d, error %d\n",
omap_dmm->irq, ret);
omap_dmm->irq = -1;
-   goto fail;
+   goto check_dmm_removal;
}

/* Enable all interrupts for each refill engine except
@@ -714,13 +714,13 @@ static int omap_dmm_probe(struct platform_device *dev)
if (!omap_dmm->dummy_page) {
dev_err(>dev, "could not allocate dummy page\n");
ret = -ENOMEM;
-   goto fail;
+   goto check_dmm_removal;
}

/* set dma mask for device */
ret = dma_set_coherent_mask(>dev, DMA_BIT_MASK(32));
if (ret)
-   goto fail;
+   goto check_dmm_removal;

omap_dmm->dummy_pa = page_to_phys(omap_dmm->dummy_page);

@@ -730,7 +730,7 @@ static int omap_dmm_probe(struct platform_device *dev)
   _dmm->refill_pa, GFP_KERNEL);
if (!omap_dmm->refill_va) {
dev_err(>dev, "could not allocate refill memory\n");
-   goto fail;
+   goto check_dmm_removal;
}

/* alloc engines */
@@ -739,7 +739,7 @@ static int omap_dmm_probe(struct platform_device *dev)
GFP_KERNEL);
if (!omap_dmm->engines) {
ret = -ENOMEM;
-   goto fail;
+   goto check_dmm_removal;
}

for (i = 0; i < omap_dmm->num_engines; i++) {
@@ -758,7 +758,7 @@ static int omap_dmm_probe(struct platform_device *dev)
GFP_KERNEL);
if (!omap_dmm->tcm) {
ret = -ENOMEM;
-   goto fail;
+   goto check_dmm_removal;
}

/* init containers */
@@ -772,7 +772,7 @@ static int omap_dmm_probe(struct platform_device *dev)
if (!omap_dmm->tcm[i]) {
dev_err(>dev, "failed to allocate container\n");
ret = -ENOMEM;
-   goto fail;
+   goto check_dmm_removal;
}

omap_dmm->tcm[i]->lut_id = i;
@@ -812,8 +812,7 @@ static int omap_dmm_probe(struct platform_device *dev)
dev_info(omap_dmm->dev, "initialized all PAT entries\n");

return 0;
-
-fail:
+ check_dmm_removal:
if (omap_dmm_remove(dev))
dev_err(>dev, "cleanup failed\n");
return ret;
-- 
2.10.0



[PATCH 06/14] GPU-DRM-OMAP: Improve a size determination in omap_dmm_probe()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 17:21:57 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index f110965..c6a7197 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -735,7 +735,8 @@ static int omap_dmm_probe(struct platform_device *dev)

/* alloc engines */
omap_dmm->engines = kcalloc(omap_dmm->num_engines,
-   sizeof(struct refill_engine), GFP_KERNEL);
+   sizeof(*omap_dmm->engines),
+   GFP_KERNEL);
if (!omap_dmm->engines) {
ret = -ENOMEM;
goto fail;
-- 
2.10.0



[PATCH 05/14] GPU-DRM-OMAP: Improve a size determination in dmm_txn_append()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 13:53:11 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index c262ef5..f110965 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -224,7 +224,7 @@ static void dmm_txn_append(struct dmm_txn *txn, struct 
pat_area *area,
int rows = (1 + area->y1 - area->y0);
int i = columns*rows;

-   pat = alloc_dma(txn, sizeof(struct pat), _pa);
+   pat = alloc_dma(txn, sizeof(*pat), _pa);

if (txn->last_pat)
txn->last_pat->next_pa = (uint32_t)pat_pa;
-- 
2.10.0



[PATCH 04/14] GPU-DRM-OMAP: Delete an unnecessary variable initialisation in tiler_map_show()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 13:31:45 +0200

The local variable "map" will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 60beeb9..c262ef5 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -896,7 +896,7 @@ static void map_2d_info(char **map, int xdiv, int ydiv, 
char *nice,
 int tiler_map_show(struct seq_file *s, void *arg)
 {
int xdiv = 2, ydiv = 1;
-   char **map = NULL, *global_map;
+   char **map, *global_map;
struct tiler_block *block;
struct tcm_area a, p;
int i;
-- 
2.10.0



[PATCH 04/14] GPU-DRM-OMAP: Delete an unnecessary variable initialisation in tiler_map_show()

2016-09-21 Thread Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 13:31:45 +0200

The local variable "map" will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 60beeb9..c262ef5 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -896,7 +896,7 @@ static void map_2d_info(char **map, int xdiv, int ydiv, 
char *nice,
 int tiler_map_show(struct seq_file *s, void *arg)
 {
int xdiv = 2, ydiv = 1;
-   char **map = NULL, *global_map;
+   char **map, *global_map;
struct tiler_block *block;
struct tcm_area a, p;
int i;
-- 
2.10.0



[PATCH 03/14] GPU-DRM-OMAP: Less function calls in tiler_map_show() after error detection

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 13:16:20 +0200

The kfree() function was called in up to two cases
by the tiler_map_show() function during error handling even if
the passed variable contained a null pointer.

* Adjust jump targets according to the Linux coding style convention.

* Split a condition check for memory allocation failures so that
  each pointer from these function calls will be checked immediately.

  See also background information:
  Topic "CWE-754: Improper check for unusual or exceptional conditions"
  Link: https://cwe.mitre.org/data/definitions/754.html

* Return directly after a call of the function "kmalloc_array" failed
  at the beginning.

* Move an assignment for the local variable "w_adj" behind the first
  memory allocation.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 3a4f91b..60beeb9 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -916,11 +916,14 @@ int tiler_map_show(struct seq_file *s, void *arg)
}

h_adj = omap_dmm->container_height / ydiv;
-   w_adj = omap_dmm->container_width / xdiv;
map = kmalloc_array(h_adj, sizeof(*map), GFP_KERNEL);
+   if (!map)
+   return 0;
+
+   w_adj = omap_dmm->container_width / xdiv;
global_map = kmalloc_array(h_adj, w_adj + 1, GFP_KERNEL);
-   if (!map || !global_map)
-   goto error;
+   if (!global_map)
+   goto free_map;

for (lut_idx = 0; lut_idx < omap_dmm->num_lut; lut_idx++) {
memset(map, 0, h_adj * sizeof(*map));
@@ -982,10 +985,9 @@ int tiler_map_show(struct seq_file *s, void *arg)
}
}

-error:
-   kfree(map);
kfree(global_map);
-
+ free_map:
+   kfree(map);
return 0;
 }
 #endif
-- 
2.10.0



[PATCH 02/14] GPU-DRM-OMAP: Replace another kmalloc() call by kmalloc_array() in tiler_map_show()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 12:54:07 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array" at another place.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 7b32dd3..3a4f91b 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -918,8 +918,7 @@ int tiler_map_show(struct seq_file *s, void *arg)
h_adj = omap_dmm->container_height / ydiv;
w_adj = omap_dmm->container_width / xdiv;
map = kmalloc_array(h_adj, sizeof(*map), GFP_KERNEL);
-   global_map = kmalloc((w_adj + 1) * h_adj, GFP_KERNEL);
-
+   global_map = kmalloc_array(h_adj, w_adj + 1, GFP_KERNEL);
if (!map || !global_map)
goto error;

-- 
2.10.0



[Bug 172421] radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #5 from Elmar Stellnberger  ---
... and VGA

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[PATCH 01/14] GPU-DRM-OMAP: Use kmalloc_array() in tiler_map_show()

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 12:23:46 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 4ceed7a9..7b32dd3 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -917,8 +917,7 @@ int tiler_map_show(struct seq_file *s, void *arg)

h_adj = omap_dmm->container_height / ydiv;
w_adj = omap_dmm->container_width / xdiv;
-
-   map = kmalloc(h_adj * sizeof(*map), GFP_KERNEL);
+   map = kmalloc_array(h_adj, sizeof(*map), GFP_KERNEL);
global_map = kmalloc((w_adj + 1) * h_adj, GFP_KERNEL);

if (!map || !global_map)
-- 
2.10.0



[Bug 172421] radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #4 from Elmar Stellnberger  ---
  It is not true that this card would only feature a TMDS of 297 over DP. The
card I have bought does only have HDMI, DVI and 4K and it was sold as 4K-ready
as you can see from the first attachement.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[PATCH 00/14] GPU-DRM-OMAP: Fine-tuning for several function implementations

2016-09-21 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 21 Sep 2016 18:28:38 +0200

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (14):
  Use kmalloc_array() in tiler_map_show()
  Replace another kmalloc() call by kmalloc_array() in tiler_map_show()
  Less function calls in tiler_map_show() after error detection
  Delete an unnecessary variable initialisation in tiler_map_show()
  Improve a size determination in dmm_txn_append()
  Improve a size determination in omap_dmm_probe()
  Rename a jump label in omap_dmm_probe()
  Rename a jump label in dmm_txn_commit()
  Delete an unnecessary variable initialisation in dmm_txn_commit()
  Use kmalloc_array() in omap_gem_attach_pages()
  Replace a kzalloc() call by kcalloc() in omap_gem_attach_pages()
  Move a variable assignment in omap_gem_attach_pages()
  Rename a jump label in omap_gem_new_dmabuf()
  Rename a jump label in four functions

 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 58 
 drivers/gpu/drm/omapdrm/omap_gem.c   | 44 +++-
 2 files changed, 49 insertions(+), 53 deletions(-)

-- 
2.10.0



[Bug 172421] radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

Alex Deucher  changed:

   What|Removed |Added

 CC||alexdeucher at gmail.com

--- Comment #3 from Alex Deucher  ---
NACK.  I rejected this the last time you brought this up.  You are running the
hw outside of it's validated specs.  See this discussion:
https://bugs.freedesktop.org/show_bug.cgi?id=93885

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

2016-09-21 Thread Christian König
Am 21.09.2016 um 17:29 schrieb Michel Dänzer:
> On 22/09/16 12:15 AM, Christian König wrote:
>> Am 21.09.2016 um 17:07 schrieb Michel Dänzer:
>>> On 21/09/16 09:56 PM, Daniel Vetter wrote:
 On Wed, Sep 21, 2016 at 1:19 PM, Christian König
  wrote:
> We use multiple writers without implicit syncing between processes
> in the
> amdgpu stack perfectly fine.
>
> See amdgpu_sync.c for the implementation. What we do there is taking
> a look
> at all the fences associated with a reservation object and only sync to
> those who are from another process.
>
> Then we use implicit syncing for command submissions in the form of
> "dependencies". E.g. for each CS we report back an identifier of that
> submission to user space and on the next submission you can give this
> identifier as dependency which needs to be satisfied before the command
> submission can start running.
 This is called explicit fencing. Implemented with a driver-private
 primitive (and not sync_file fds like on android), but still
 conceptually explicit fencing. Implicit fencing really only can handle
 one writer, at least as currently implemented by struct
 reservation_object.

> This was done to allow multiple engines (3D, DMA, Compute) to compose a
> buffer while still allow compatibility with protocols like DRI2/DRI3.
 Instead of the current solution you need to stop attaching exclusive
 fences to non-shared buffers (which are coordinated using the
 driver-private explicit fencing you're describing),
>>> Err, the current issue is actually that amdgpu never sets an exclusive
>>> fence, only ever shared ones. :)
>> Actually amdgpu does set the exclusive fence for buffer migrations,
>> cause that is an operation user space doesn't know about and so it needs
>> to be "exclusive" access to the buffer.
>>
>>
 and only attach exclusive fences to shared buffers (DRI2/3, PRIME,
 whatever).
>>> Still, it occurred to me in the meantime that amdgpu setting the
>>> exclusive fence for buffers shared via PRIME (no matter if it's a write
>>> or read operation) might be a solution. Christian, what do you think?
>> The problem is that we don't have one fence, but many.
>>
>> E.g. there can be many operation on a buffer at the same time and we
>> need to wait for all of them to complete before it can be displayed.
> Maybe in theory, but with the problem we have in practice right now, the
> amdgpu GPU should only ever access the shared BO with the same engine.

That clearly won't work. Take a look at what both Mesa and the pro stack 
do with the BO before it is displayed makes it mandatory to execute 
things in parallel (at least for the not shared case).

> Anyway, this should be solvable by setting some kind of meta-fence as
> the exclusive fence, which can internally be mapped to multiple fences,
> maybe up to one for each ring which can access the BO?

I've thought about that as well, but this approach would also only work 
when we keep a collection of fences and not just an array because of the 
scheduler.

For a quick workaround I suggest to just serialize all accesses to BO 
shared with different drivers, but essentially I think it is a perfectly 
valid requirement to have multiple writers to one BO.

Christian.



[Bug 97806] GPU lockup with mesa-git and llvm-svn with rx 470 on Unigine Heaven and TombRaider 2013

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97806

--- Comment #12 from Laurent carlier  ---
Created attachment 126713
  --> https://bugs.freedesktop.org/attachment.cgi?id=126713=edit
GALLIUM_DDEBUG="pipelined 1000" ./heaven

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/9a43a4d7/attachment.html>


[Bug 172421] radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #2 from Elmar Stellnberger  ---
Created attachment 239381
  --> https://bugzilla.kernel.org/attachment.cgi?id=239381=edit
patch introducing radeon.hdmimhz for kernel 4.8.0-rc2

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[Bug 172421] radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #1 from Elmar Stellnberger  ---
Created attachment 239371
  --> https://bugzilla.kernel.org/attachment.cgi?id=239371=edit
shipment notification: R5 230 marketed as 4K-ready

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[Bug 172421] New: radeon: allow to set the TMDS frequency by a special kernel parameter

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=172421

Bug ID: 172421
   Summary: radeon: allow to set the TMDS frequency by a special
kernel parameter
   Product: Drivers
   Version: 2.5
Kernel Version: 4.8.0-rc7+
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: enhancement
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-dri at kernel-bugs.osdl.org
  Reporter: estellnb at elstel.org
Regression: No

Despite different claims by ATI in 2016 Radeon R5 230 graphics cards featuring
HDMI, DVI and VGA had originally been sold as 4K-ready up to the year 2015. The
only prove I have for this is a comercial invoice from Nexus Mobile and a
packaging card. As far as I know Radeon R5 230 cards currently sold can still
be run stable and reliable under UltraHD provided that you apply the right
kernel patch (see for the attachement). Unfortunately neither my former
merchant nor ATI have responded about my questions concerning this change in
trade and marketing policy.

  My request would now be to integrate the provided patch into the mainline
kernel. It does not change the behaviour of the radeon driver unless you
specify a nonzero value for the radeon.hdmimhz parameter. If you do I have
tested the R5 230 cards I have to run stable and reliably for days. At least
the 2GB variant of this card has largely sufficient resources for proficient
desktop computing under UltraHD including image manipulation in 3840x2160.

  While never officially discussed for the radeon driver nouveau is already
implementing a similar parameter called nouveau.hdmimhz since kernel 4.5.x.
Though it thereby becomes possible to specify a hdmimhz that is far above the
cards technical possibilties the nouveau developers I have talked with say that
it would rarely be possible to damage any card by overcloking the TMDS. In deed
I have successfully been overclocking my GeForce 9600M GT to feature 4K/2160p. 

  Even specifying values considerable higher than 225MHz did not damage my
GeForce 9600M GT though the screen stayed black upon the nouveau driver
initialization. While the Radeon R5 230 works well at 297MHz (as long as you
specify that via radeon.hdmimhz) I have similarly to the GeForce 9600M GT tried
to overclock a Radeon R7 240. It did produce stable images at a higher hdmimhz
like 330 though the HDMI input of my monitor features no more than 30Hz at
3840x2160 (tested with or without a DP-adapter).

  While it remains questionable if the provided patch can improve things for
newer Radeon cards I would believe it to be beneficial for some elder cards. At
least it is known to be beneficial for the R5 230 initially marketed as
4K-ready. The according radeon patch provided with this report has so far
already been accepted by the Mageia 6 distribution. Though the attached patch
is for application at the current 4.8.0-rcX+ kernels most of my machines that
rely on it still run with 4.6.0.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[PATCH 7/7] selftest: sync: stress test for merges

2016-09-21 Thread Emilio López
This test is based on the libsync test suite from Android.
This commit includes a test to stress merge operations.

Signed-off-by: Emilio López 
---
 tools/testing/selftests/sync/Makefile|   1 +
 tools/testing/selftests/sync/sync_stress_merge.c | 116 +++
 tools/testing/selftests/sync/sync_test.c |   1 +
 tools/testing/selftests/sync/synctest.h  |   3 +
 4 files changed, 121 insertions(+)
 create mode 100644 tools/testing/selftests/sync/sync_stress_merge.c

diff --git a/tools/testing/selftests/sync/Makefile 
b/tools/testing/selftests/sync/Makefile
index 910e3f9..6e05749 100644
--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -20,6 +20,7 @@ TESTS += sync_merge.o
 TESTS += sync_wait.o
 TESTS += sync_stress_parallelism.o
 TESTS += sync_stress_consumer.o
+TESTS += sync_stress_merge.o

 sync_test: $(SRC) $(TESTS)

diff --git a/tools/testing/selftests/sync/sync_stress_merge.c 
b/tools/testing/selftests/sync/sync_stress_merge.c
new file mode 100644
index 000..0e57360
--- /dev/null
+++ b/tools/testing/selftests/sync/sync_stress_merge.c
@@ -0,0 +1,116 @@
+/*
+ *  sync stress test: merging
+ *  Copyright 2015-2016 Collabora Ltd.
+ *
+ *  Based on the implementation from the Android Open Source Project,
+ *
+ *  Copyright 2012 Google, Inc
+ *
+ *  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 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 
+#include 
+#include 
+
+#include "sync.h"
+#include "sw_sync.h"
+#include "synctest.h"
+
+static int fence_map[1024 * 32];
+
+int test_merge_stress_random_merge(void)
+{
+   int i, size, ret;
+   int timeline_count = 32;
+   int merge_count = 1024 * 32;
+   int timelines[timeline_count];
+   int fence, tmpfence, merged, valid;
+   int timeline, timeline_offset, sync_point;
+
+   srand(time(NULL));
+
+   for (i = 0; i < timeline_count; i++)
+   timelines[i] = sw_sync_timeline_create();
+
+   fence = sw_sync_fence_create(timelines[0], "fence", 0);
+   valid = sw_sync_fence_is_valid(fence);
+   ASSERT(valid, "Failure creating fence\n");
+
+   memset(fence_map, -1, sizeof(fence_map));
+   fence_map[0] = 0;
+
+   /*
+* Randomly create sync_points out of a fixed set of timelines,
+* and merge them together
+*/
+   for (i = 0; i < merge_count; i++) {
+   /* Generate sync_point. */
+   timeline_offset = rand() % timeline_count;
+   timeline = timelines[timeline_offset];
+   sync_point = rand();
+
+   /* Keep track of the latest sync_point in each timeline. */
+   if (fence_map[timeline_offset] == -1)
+   fence_map[timeline_offset] = sync_point;
+   else if (fence_map[timeline_offset] < sync_point)
+   fence_map[timeline_offset] = sync_point;
+
+   /* Merge */
+   tmpfence = sw_sync_fence_create(timeline, "fence", sync_point);
+   merged = sync_merge("merge", tmpfence, fence);
+   sw_sync_fence_destroy(tmpfence);
+   sw_sync_fence_destroy(fence);
+   fence = merged;
+
+   valid = sw_sync_fence_is_valid(merged);
+   ASSERT(valid, "Failure creating fence i\n");
+   }
+
+   size = 0;
+   for (i = 0; i < timeline_count; i++)
+   if (fence_map[i] != -1)
+   size++;
+
+   /* Confirm our map matches the fence. */
+   ASSERT(sync_fence_size(fence) == size,
+  "Quantity of elements not matching\n");
+
+   /* Trigger the merged fence */
+   for (i = 0; i < timeline_count; i++) {
+   if (fence_map[i] != -1) {
+   ret = sync_wait(fence, 0);
+   ASSERT(ret == 0,
+  "Failure waiting on fence until timeout\n");
+   /* Increment 

[PATCH 6/7] selftest: sync: stress consumer/producer test

2016-09-21 Thread Emilio López
This test is based on the libsync test suite from Android.
This commit includes a stress test that replicates a
consumer/producer pattern.

Signed-off-by: Emilio López 
---
 tools/testing/selftests/sync/Makefile  |   1 +
 .../testing/selftests/sync/sync_stress_consumer.c  | 185 +
 tools/testing/selftests/sync/sync_test.c   |   1 +
 tools/testing/selftests/sync/synctest.h|   3 +
 4 files changed, 190 insertions(+)
 create mode 100644 tools/testing/selftests/sync/sync_stress_consumer.c

diff --git a/tools/testing/selftests/sync/Makefile 
b/tools/testing/selftests/sync/Makefile
index 1ed2864..910e3f9 100644
--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -19,6 +19,7 @@ TESTS += sync_fence.o
 TESTS += sync_merge.o
 TESTS += sync_wait.o
 TESTS += sync_stress_parallelism.o
+TESTS += sync_stress_consumer.o

 sync_test: $(SRC) $(TESTS)

diff --git a/tools/testing/selftests/sync/sync_stress_consumer.c 
b/tools/testing/selftests/sync/sync_stress_consumer.c
new file mode 100644
index 000..d9eff8d
--- /dev/null
+++ b/tools/testing/selftests/sync/sync_stress_consumer.c
@@ -0,0 +1,185 @@
+/*
+ *  sync stress test: producer/consumer
+ *  Copyright 2015-2016 Collabora Ltd.
+ *
+ *  Based on the implementation from the Android Open Source Project,
+ *
+ *  Copyright 2012 Google, Inc
+ *
+ *  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 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 
+
+#include "sync.h"
+#include "sw_sync.h"
+#include "synctest.h"
+
+/* IMPORTANT NOTE: if you see this test failing on your system, it may be
+ * due to a shortage of file descriptors. Please ensure your system has
+ * a sensible limit for this test to finish correctly.
+ */
+
+/* Returns 1 on error, 0 on success */
+static int busy_wait_on_fence(int fence)
+{
+   int error, active;
+
+   do {
+   error = sync_fence_count_with_status(fence, FENCE_STATUS_ERROR);
+   ASSERT(error == 0, "Error occurred on fence\n");
+   active = sync_fence_count_with_status(fence,
+ FENCE_STATUS_ACTIVE);
+   } while (active);
+
+   return 0;
+}
+
+static struct {
+   int iterations;
+   int threads;
+   int counter;
+   int consumer_timeline;
+   int *producer_timelines;
+   pthread_mutex_t lock;
+} test_data_mpsc;
+
+static int mpsc_producer_thread(void *d)
+{
+   int id = (long)d;
+   int fence, valid, i;
+   int *producer_timelines = test_data_mpsc.producer_timelines;
+   int consumer_timeline = test_data_mpsc.consumer_timeline;
+   int iterations = test_data_mpsc.iterations;
+
+   for (i = 0; i < iterations; i++) {
+   fence = sw_sync_fence_create(consumer_timeline, "fence", i);
+   valid = sw_sync_fence_is_valid(fence);
+   ASSERT(valid, "Failure creating fence\n");
+
+   /*
+* Wait for the consumer to finish. Use alternate
+* means of waiting on the fence
+*/
+
+   if ((iterations + id) % 8 != 0) {
+   ASSERT(sync_wait(fence, -1) > 0,
+  "Failure waiting on fence\n");
+   } else {
+   ASSERT(busy_wait_on_fence(fence) == 0,
+  "Failure waiting on fence\n");
+   }
+
+   /*
+* Every producer increments the counter, the consumer
+* checks and erases it
+*/
+   pthread_mutex_lock(_data_mpsc.lock);
+   test_data_mpsc.counter++;
+   pthread_mutex_unlock(_data_mpsc.lock);
+
+   ASSERT(sw_sync_timeline_inc(producer_timelines[id], 1) == 0,
+  "Error advancing producer timeline\n");
+
+   sw_sync_fence_destroy(fence);
+   }
+
+   return 0;
+}
+

[PATCH 5/7] selftest: sync: stress test for parallelism

2016-09-21 Thread Emilio López
This test is based on the libsync test suite from Android.
This commit includes a stress test that invokes operations
in parallel.

Signed-off-by: Emilio López 
---
 tools/testing/selftests/sync/Makefile  |   1 +
 .../selftests/sync/sync_stress_parallelism.c   | 111 +
 tools/testing/selftests/sync/sync_test.c   |   1 +
 tools/testing/selftests/sync/synctest.h|   3 +
 4 files changed, 116 insertions(+)
 create mode 100644 tools/testing/selftests/sync/sync_stress_parallelism.c

diff --git a/tools/testing/selftests/sync/Makefile 
b/tools/testing/selftests/sync/Makefile
index 19731b9..1ed2864 100644
--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -18,6 +18,7 @@ TESTS += sync_alloc.o
 TESTS += sync_fence.o
 TESTS += sync_merge.o
 TESTS += sync_wait.o
+TESTS += sync_stress_parallelism.o

 sync_test: $(SRC) $(TESTS)

diff --git a/tools/testing/selftests/sync/sync_stress_parallelism.c 
b/tools/testing/selftests/sync/sync_stress_parallelism.c
new file mode 100644
index 000..e6c9be6
--- /dev/null
+++ b/tools/testing/selftests/sync/sync_stress_parallelism.c
@@ -0,0 +1,111 @@
+/*
+ *  sync stress test: parallelism
+ *  Copyright 2015-2016 Collabora Ltd.
+ *
+ *  Based on the implementation from the Android Open Source Project,
+ *
+ *  Copyright 2012 Google, Inc
+ *
+ *  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 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 
+
+#include "sync.h"
+#include "sw_sync.h"
+#include "synctest.h"
+
+static struct {
+   int iterations;
+   int timeline;
+   int counter;
+} test_data_two_threads;
+
+static int test_stress_two_threads_shared_timeline_thread(void *d)
+{
+   int thread_id = (long)d;
+   int timeline = test_data_two_threads.timeline;
+   int iterations = test_data_two_threads.iterations;
+   int fence, valid, ret, i;
+
+   for (i = 0; i < iterations; i++) {
+   fence = sw_sync_fence_create(timeline, "fence",
+i * 2 + thread_id);
+   valid = sw_sync_fence_is_valid(fence);
+   ASSERT(valid, "Failure allocating fence\n");
+
+   /* Wait on the prior thread to complete */
+   ret = sync_wait(fence, -1);
+   ASSERT(ret > 0, "Problem occurred on prior thread\n");
+
+   /*
+* Confirm the previous thread's writes are visible
+* and then increment
+*/
+   ASSERT(test_data_two_threads.counter == i * 2 + thread_id,
+  "Counter got damaged!\n");
+   test_data_two_threads.counter++;
+
+   /* Kick off the other thread */
+   ret = sw_sync_timeline_inc(timeline, 1);
+   ASSERT(ret == 0, "Advancing timeline failed\n");
+
+   sw_sync_fence_destroy(fence);
+   }
+
+   return 0;
+}
+
+int test_stress_two_threads_shared_timeline(void)
+{
+   pthread_t a, b;
+   int valid;
+   int timeline = sw_sync_timeline_create();
+
+   valid = sw_sync_timeline_is_valid(timeline);
+   ASSERT(valid, "Failure allocating timeline\n");
+
+   test_data_two_threads.iterations = 1 << 16;
+   test_data_two_threads.counter = 0;
+   test_data_two_threads.timeline = timeline;
+
+   /*
+* Use a single timeline to synchronize two threads
+* hammmering on the same counter.
+*/
+
+   pthread_create(, NULL, (void *(*)(void *))
+  test_stress_two_threads_shared_timeline_thread,
+  (void *)0);
+   pthread_create(, NULL, (void *(*)(void *))
+  test_stress_two_threads_shared_timeline_thread,
+  (void *)1);
+
+   pthread_join(a, NULL);
+   pthread_join(b, NULL);
+
+   /* make sure the threads did not trample on one another */
+   

[PATCH 4/7] selftest: sync: wait tests for sw_sync framework

2016-09-21 Thread Emilio López
These tests are based on the libsync test suite from Android.
This commit includes tests for waiting on fences.

Signed-off-by: Emilio López 
---
 tools/testing/selftests/sync/Makefile|  1 +
 tools/testing/selftests/sync/sync_test.c |  1 +
 tools/testing/selftests/sync/sync_wait.c | 91 
 tools/testing/selftests/sync/synctest.h  |  3 ++
 4 files changed, 96 insertions(+)
 create mode 100644 tools/testing/selftests/sync/sync_wait.c

diff --git a/tools/testing/selftests/sync/Makefile 
b/tools/testing/selftests/sync/Makefile
index 53b716f..19731b9 100644
--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -17,6 +17,7 @@ SRC = sync_test.o sync.o
 TESTS += sync_alloc.o
 TESTS += sync_fence.o
 TESTS += sync_merge.o
+TESTS += sync_wait.o

 sync_test: $(SRC) $(TESTS)

diff --git a/tools/testing/selftests/sync/sync_test.c 
b/tools/testing/selftests/sync/sync_test.c
index ab37eee..eab5ceb 100644
--- a/tools/testing/selftests/sync/sync_test.c
+++ b/tools/testing/selftests/sync/sync_test.c
@@ -65,6 +65,7 @@ int main(void)
err += RUN_TEST(test_fence_one_timeline_wait);
err += RUN_TEST(test_fence_one_timeline_merge);
err += RUN_TEST(test_fence_merge_same_fence);
+   err += RUN_TEST(test_fence_multi_timeline_wait);

if (err)
printf("[FAIL]\tsync errors: %d\n", err);
diff --git a/tools/testing/selftests/sync/sync_wait.c 
b/tools/testing/selftests/sync/sync_wait.c
new file mode 100644
index 000..d69b752
--- /dev/null
+++ b/tools/testing/selftests/sync/sync_wait.c
@@ -0,0 +1,91 @@
+/*
+ *  sync fence wait tests
+ *  Copyright 2015-2016 Collabora Ltd.
+ *
+ *  Based on the implementation from the Android Open Source Project,
+ *
+ *  Copyright 2012 Google, Inc
+ *
+ *  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 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 "sync.h"
+#include "sw_sync.h"
+#include "synctest.h"
+
+int test_fence_multi_timeline_wait(void)
+{
+   int timelineA, timelineB, timelineC;
+   int fenceA, fenceB, fenceC, merged;
+   int valid, active, signaled, ret;
+
+   timelineA = sw_sync_timeline_create();
+   timelineB = sw_sync_timeline_create();
+   timelineC = sw_sync_timeline_create();
+
+   fenceA = sw_sync_fence_create(timelineA, "fenceA", 5);
+   fenceB = sw_sync_fence_create(timelineB, "fenceB", 5);
+   fenceC = sw_sync_fence_create(timelineC, "fenceC", 5);
+
+   merged = sync_merge("mergeFence", fenceB, fenceA);
+   merged = sync_merge("mergeFence", fenceC, merged);
+
+   valid = sw_sync_fence_is_valid(merged);
+   ASSERT(valid, "Failure merging fence from various timelines\n");
+
+   /* Confirm fence isn't signaled */
+   active = sync_fence_count_with_status(merged, FENCE_STATUS_ACTIVE);
+   ASSERT(active == 3, "Fence signaled too early!\n");
+
+   ret = sync_wait(merged, 0);
+   ASSERT(ret == 0,
+  "Failure waiting on fence until timeout\n");
+
+   ret = sw_sync_timeline_inc(timelineA, 5);
+   active = sync_fence_count_with_status(merged, FENCE_STATUS_ACTIVE);
+   signaled = sync_fence_count_with_status(merged, FENCE_STATUS_SIGNALED);
+   ASSERT(active == 2 && signaled == 1,
+  "Fence did not signal properly!\n");
+
+   ret = sw_sync_timeline_inc(timelineB, 5);
+   active = sync_fence_count_with_status(merged, FENCE_STATUS_ACTIVE);
+   signaled = sync_fence_count_with_status(merged, FENCE_STATUS_SIGNALED);
+   ASSERT(active == 1 && signaled == 2,
+  "Fence did not signal properly!\n");
+
+   ret = sw_sync_timeline_inc(timelineC, 5);
+   active = sync_fence_count_with_status(merged, FENCE_STATUS_ACTIVE);
+   signaled = sync_fence_count_with_status(merged, FENCE_STATUS_SIGNALED);
+   ASSERT(active == 0 && signaled == 3,
+  "Fence did not signal properly!\n");
+
+   /* confirm you can successfully 

[PATCH 3/7] selftest: sync: merge tests for sw_sync framework

2016-09-21 Thread Emilio López
These tests are based on the libsync test suite from Android.
This commit includes tests for basic merge operations.

Signed-off-by: Emilio López 
---
 tools/testing/selftests/sync/Makefile |  1 +
 tools/testing/selftests/sync/sync_merge.c | 60 +++
 tools/testing/selftests/sync/sync_test.c  |  1 +
 tools/testing/selftests/sync/synctest.h   |  3 ++
 4 files changed, 65 insertions(+)
 create mode 100644 tools/testing/selftests/sync/sync_merge.c

diff --git a/tools/testing/selftests/sync/Makefile 
b/tools/testing/selftests/sync/Makefile
index 2bba52e..53b716f 100644
--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -16,6 +16,7 @@ SRC = sync_test.o sync.o

 TESTS += sync_alloc.o
 TESTS += sync_fence.o
+TESTS += sync_merge.o

 sync_test: $(SRC) $(TESTS)

diff --git a/tools/testing/selftests/sync/sync_merge.c 
b/tools/testing/selftests/sync/sync_merge.c
new file mode 100644
index 000..8914d43
--- /dev/null
+++ b/tools/testing/selftests/sync/sync_merge.c
@@ -0,0 +1,60 @@
+/*
+ *  sync fence merge tests
+ *  Copyright 2015-2016 Collabora Ltd.
+ *
+ *  Based on the implementation from the Android Open Source Project,
+ *
+ *  Copyright 2012 Google, Inc
+ *
+ *  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 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 "sync.h"
+#include "sw_sync.h"
+#include "synctest.h"
+
+int test_fence_merge_same_fence(void)
+{
+   int fence, valid, merged;
+   int timeline = sw_sync_timeline_create();
+
+   valid = sw_sync_timeline_is_valid(timeline);
+   ASSERT(valid, "Failure allocating timeline\n");
+
+   fence = sw_sync_fence_create(timeline, "allocFence", 5);
+   valid = sw_sync_fence_is_valid(fence);
+   ASSERT(valid, "Failure allocating fence\n");
+
+   merged = sync_merge("mergeFence", fence, fence);
+   valid = sw_sync_fence_is_valid(fence);
+   ASSERT(valid, "Failure merging fence\n");
+
+   ASSERT(sync_fence_count_with_status(merged, FENCE_STATUS_SIGNALED) == 0,
+  "fence signaled too early!\n");
+
+   sw_sync_timeline_inc(timeline, 5);
+   ASSERT(sync_fence_count_with_status(merged, FENCE_STATUS_SIGNALED) == 1,
+  "fence did not signal!\n");
+
+   sw_sync_fence_destroy(merged);
+   sw_sync_fence_destroy(fence);
+   sw_sync_timeline_destroy(timeline);
+
+   return 0;
+}
diff --git a/tools/testing/selftests/sync/sync_test.c 
b/tools/testing/selftests/sync/sync_test.c
index b442292b..ab37eee 100644
--- a/tools/testing/selftests/sync/sync_test.c
+++ b/tools/testing/selftests/sync/sync_test.c
@@ -64,6 +64,7 @@ int main(void)

err += RUN_TEST(test_fence_one_timeline_wait);
err += RUN_TEST(test_fence_one_timeline_merge);
+   err += RUN_TEST(test_fence_merge_same_fence);

if (err)
printf("[FAIL]\tsync errors: %d\n", err);
diff --git a/tools/testing/selftests/sync/synctest.h 
b/tools/testing/selftests/sync/synctest.h
index 6505c28..c3b0b5e 100644
--- a/tools/testing/selftests/sync/synctest.h
+++ b/tools/testing/selftests/sync/synctest.h
@@ -48,4 +48,7 @@ int test_alloc_fence_negative(void);
 int test_fence_one_timeline_wait(void);
 int test_fence_one_timeline_merge(void);

+/* Fence merge tests */
+int test_fence_merge_same_fence(void);
+
 #endif
-- 
2.9.3



[PATCH 2/7] selftest: sync: fence tests for sw_sync framework

2016-09-21 Thread Emilio López
These tests are based on the libsync test suite from Android.
This commit includes tests for basic fence creation.

Signed-off-by: Emilio López 
---
 tools/testing/selftests/sync/Makefile |   1 +
 tools/testing/selftests/sync/sync_fence.c | 132 ++
 tools/testing/selftests/sync/sync_test.c  |   3 +
 tools/testing/selftests/sync/synctest.h   |   4 +
 4 files changed, 140 insertions(+)
 create mode 100644 tools/testing/selftests/sync/sync_fence.c

diff --git a/tools/testing/selftests/sync/Makefile 
b/tools/testing/selftests/sync/Makefile
index f67827f..2bba52e 100644
--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -15,6 +15,7 @@ include ../lib.mk
 SRC = sync_test.o sync.o

 TESTS += sync_alloc.o
+TESTS += sync_fence.o

 sync_test: $(SRC) $(TESTS)

diff --git a/tools/testing/selftests/sync/sync_fence.c 
b/tools/testing/selftests/sync/sync_fence.c
new file mode 100644
index 000..13f1752
--- /dev/null
+++ b/tools/testing/selftests/sync/sync_fence.c
@@ -0,0 +1,132 @@
+/*
+ *  sync fence tests with one timeline
+ *  Copyright 2015-2016 Collabora Ltd.
+ *
+ *  Based on the implementation from the Android Open Source Project,
+ *
+ *  Copyright 2012 Google, Inc
+ *
+ *  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 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 "sync.h"
+#include "sw_sync.h"
+#include "synctest.h"
+
+int test_fence_one_timeline_wait(void)
+{
+   int fence, valid, ret;
+   int timeline = sw_sync_timeline_create();
+
+   valid = sw_sync_timeline_is_valid(timeline);
+   ASSERT(valid, "Failure allocating timeline\n");
+
+   fence = sw_sync_fence_create(timeline, "allocFence", 5);
+   valid = sw_sync_fence_is_valid(fence);
+   ASSERT(valid, "Failure allocating fence\n");
+
+   /* Wait on fence until timeout */
+   ret = sync_wait(fence, 0);
+   ASSERT(ret == 0, "Failure waiting on fence until timeout\n");
+
+   /* Advance timeline from 0 -> 1 */
+   ret = sw_sync_timeline_inc(timeline, 1);
+   ASSERT(ret == 0, "Failure advancing timeline\n");
+
+   /* Wait on fence until timeout */
+   ret = sync_wait(fence, 0);
+   ASSERT(ret == 0, "Failure waiting on fence until timeout\n");
+
+   /* Signal the fence */
+   ret = sw_sync_timeline_inc(timeline, 4);
+   ASSERT(ret == 0, "Failure signaling the fence\n");
+
+   /* Wait successfully */
+   ret = sync_wait(fence, 0);
+   ASSERT(ret > 0, "Failure waiting on fence\n");
+
+   /* Go even further, and confirm wait still succeeds */
+   ret = sw_sync_timeline_inc(timeline, 10);
+   ASSERT(ret == 0, "Failure going further\n");
+   ret = sync_wait(fence, 0);
+   ASSERT(ret > 0, "Failure waiting ahead\n");
+
+   sw_sync_fence_destroy(fence);
+   sw_sync_timeline_destroy(timeline);
+
+   return 0;
+}
+
+int test_fence_one_timeline_merge(void)
+{
+   int a, b, c, d, valid;
+   int timeline = sw_sync_timeline_create();
+
+   /* create fence a,b,c and then merge them all into fence d */
+   a = sw_sync_fence_create(timeline, "allocFence", 1);
+   b = sw_sync_fence_create(timeline, "allocFence", 2);
+   c = sw_sync_fence_create(timeline, "allocFence", 3);
+
+   valid = sw_sync_fence_is_valid(a) &&
+   sw_sync_fence_is_valid(b) &&
+   sw_sync_fence_is_valid(c);
+   ASSERT(valid, "Failure allocating fences\n");
+
+   d = sync_merge("mergeFence", b, a);
+   d = sync_merge("mergeFence", c, d);
+   valid = sw_sync_fence_is_valid(d);
+   ASSERT(valid, "Failure merging fences\n");
+
+   /* confirm all fences have one active point (even d) */
+   ASSERT(sync_fence_count_with_status(a, FENCE_STATUS_ACTIVE) == 1,
+  "a has too many active fences!\n");
+   ASSERT(sync_fence_count_with_status(a, FENCE_STATUS_ACTIVE) == 1,
+  "b has too many active fences!\n");
+   

[PATCH 1/7] selftest: sync: basic tests for sw_sync framework

2016-09-21 Thread Emilio López
These tests are based on the libsync test suite from Android.
This commit lays the ground for future tests, as well as includes
tests for a variety of basic allocation commands.

Signed-off-by: Emilio López 
---
 tools/testing/selftests/Makefile  |   1 +
 tools/testing/selftests/sync/.gitignore   |   1 +
 tools/testing/selftests/sync/Makefile |  24 
 tools/testing/selftests/sync/sw_sync.h|  46 +++
 tools/testing/selftests/sync/sync.c   | 222 ++
 tools/testing/selftests/sync/sync.h   |  40 ++
 tools/testing/selftests/sync/sync_alloc.c |  74 ++
 tools/testing/selftests/sync/sync_test.c  |  71 ++
 tools/testing/selftests/sync/synctest.h   |  47 +++
 9 files changed, 526 insertions(+)
 create mode 100644 tools/testing/selftests/sync/.gitignore
 create mode 100644 tools/testing/selftests/sync/Makefile
 create mode 100644 tools/testing/selftests/sync/sw_sync.h
 create mode 100644 tools/testing/selftests/sync/sync.c
 create mode 100644 tools/testing/selftests/sync/sync.h
 create mode 100644 tools/testing/selftests/sync/sync_alloc.c
 create mode 100644 tools/testing/selftests/sync/sync_test.c
 create mode 100644 tools/testing/selftests/sync/synctest.h

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index ff9e5f2..a72e3c7 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -22,6 +22,7 @@ TARGETS += seccomp
 TARGETS += sigaltstack
 TARGETS += size
 TARGETS += static_keys
+TARGETS += sync
 TARGETS += sysctl
 ifneq (1, $(quicktest))
 TARGETS += timers
diff --git a/tools/testing/selftests/sync/.gitignore 
b/tools/testing/selftests/sync/.gitignore
new file mode 100644
index 000..f5091e7
--- /dev/null
+++ b/tools/testing/selftests/sync/.gitignore
@@ -0,0 +1 @@
+sync_test
diff --git a/tools/testing/selftests/sync/Makefile 
b/tools/testing/selftests/sync/Makefile
new file mode 100644
index 000..f67827f
--- /dev/null
+++ b/tools/testing/selftests/sync/Makefile
@@ -0,0 +1,24 @@
+CC = $(CROSS_COMPILE)gcc
+CFLAGS := -O2 -g -std=gnu89 -pthread -Wall -Wextra
+CFLAGS += -I../../../../include/uapi/
+CFLAGS += -I../../../../include/
+CFLAGS += -I../../../../usr/include/
+CFLAGS += -I../../../../drivers/dma-buf/
+LDFLAGS += -pthread
+
+TEST_PROGS = sync_test
+
+all: $(TEST_PROGS)
+
+include ../lib.mk
+
+SRC = sync_test.o sync.o
+
+TESTS += sync_alloc.o
+
+sync_test: $(SRC) $(TESTS)
+
+.PHONY: clean
+
+clean:
+   $(RM) sync_test $(SRC) $(TESTS)
diff --git a/tools/testing/selftests/sync/sw_sync.h 
b/tools/testing/selftests/sync/sw_sync.h
new file mode 100644
index 000..e2cfc6ba
--- /dev/null
+++ b/tools/testing/selftests/sync/sw_sync.h
@@ -0,0 +1,46 @@
+/*
+ *  sw_sync abstraction
+ *
+ *  Copyright 2015-2016 Collabora Ltd.
+ *
+ *  Based on the implementation from the Android Open Source Project,
+ *
+ *  Copyright 2013 Google, Inc
+ *
+ *  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 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.
+ */
+
+#ifndef SELFTESTS_SW_SYNC_H
+#define SELFTESTS_SW_SYNC_H
+
+/*
+ * sw_sync is mainly intended for testing and should not be compiled into
+ * production kernels
+ */
+
+int sw_sync_timeline_create(void);
+int sw_sync_timeline_is_valid(int fd);
+int sw_sync_timeline_inc(int fd, unsigned int count);
+void sw_sync_timeline_destroy(int fd);
+
+int sw_sync_fence_create(int fd, const char *name, unsigned int value);
+int sw_sync_fence_is_valid(int fd);
+void sw_sync_fence_destroy(int fd);
+
+#endif
diff --git a/tools/testing/selftests/sync/sync.c 
b/tools/testing/selftests/sync/sync.c
new file mode 100644
index 000..99368be
--- /dev/null
+++ b/tools/testing/selftests/sync/sync.c
@@ -0,0 +1,222 @@
+/*
+ *  sync / sw_sync abstraction
+ *  Copyright 2015-2016 Collabora Ltd.
+ *
+ *  Based on the implementation from the Android Open Source Project,
+ *
+ *  Copyright 2012 Google, Inc
+ *
+ *  Permission is hereby granted, free of charge, to any 

[PATCH 0/7] Tests for sync infrastructure

2016-09-21 Thread Emilio López
Hello everyone,

This is a series of tests to exercise the sync kernel infrastructure. It is
meant to be a test suite for the work Gustavo has been doing to destage it.

These tests were originally part of a battery of tests shipping with
Android's libsync that were rewritten to use the new userspace interfaces.

An older version of this set was sent as an RFC series back in March. Now
that the framework has been destaged, I'm resending them with a few
changes - some tests were removed, and some bugs were squashed. See [0]
if you wish to see the the old set.

As usual, all comments are welcome.

Cheers!
Emilio

[0] 
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-March/086932.html

Emilio López (7):
  selftest: sync: basic tests for sw_sync framework
  selftest: sync: fence tests for sw_sync framework
  selftest: sync: merge tests for sw_sync framework
  selftest: sync: wait tests for sw_sync framework
  selftest: sync: stress test for parallelism
  selftest: sync: stress consumer/producer test
  selftest: sync: stress test for merges

 tools/testing/selftests/Makefile   |   1 +
 tools/testing/selftests/sync/.gitignore|   1 +
 tools/testing/selftests/sync/Makefile  |  30 +++
 tools/testing/selftests/sync/sw_sync.h |  46 +
 tools/testing/selftests/sync/sync.c| 222 +
 tools/testing/selftests/sync/sync.h|  40 
 tools/testing/selftests/sync/sync_alloc.c  |  74 +++
 tools/testing/selftests/sync/sync_fence.c  | 132 
 tools/testing/selftests/sync/sync_merge.c  |  60 ++
 .../testing/selftests/sync/sync_stress_consumer.c  | 185 +
 tools/testing/selftests/sync/sync_stress_merge.c   | 116 +++
 .../selftests/sync/sync_stress_parallelism.c   | 111 +++
 tools/testing/selftests/sync/sync_test.c   |  79 
 tools/testing/selftests/sync/sync_wait.c   |  91 +
 tools/testing/selftests/sync/synctest.h|  66 ++
 15 files changed, 1254 insertions(+)
 create mode 100644 tools/testing/selftests/sync/.gitignore
 create mode 100644 tools/testing/selftests/sync/Makefile
 create mode 100644 tools/testing/selftests/sync/sw_sync.h
 create mode 100644 tools/testing/selftests/sync/sync.c
 create mode 100644 tools/testing/selftests/sync/sync.h
 create mode 100644 tools/testing/selftests/sync/sync_alloc.c
 create mode 100644 tools/testing/selftests/sync/sync_fence.c
 create mode 100644 tools/testing/selftests/sync/sync_merge.c
 create mode 100644 tools/testing/selftests/sync/sync_stress_consumer.c
 create mode 100644 tools/testing/selftests/sync/sync_stress_merge.c
 create mode 100644 tools/testing/selftests/sync/sync_stress_parallelism.c
 create mode 100644 tools/testing/selftests/sync/sync_test.c
 create mode 100644 tools/testing/selftests/sync/sync_wait.c
 create mode 100644 tools/testing/selftests/sync/synctest.h

-- 
2.9.3



[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97879

--- Comment #4 from Andreas Hartmetz  ---
I have the same problem. Assuming the problem was shader compilation, what
could be done about it, though? Optimizing shader compilation by a factor of 10
seems unrealistic, a disk cache for shaders has been rejected (right?) and
would not always help, e.g. when somebody with a car that uses a new asset
joins.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/e29a4e5e/attachment.html>


[PATCH] drm/i2c: tda998x: don't register the connector

2016-09-21 Thread Russell King - ARM Linux
On Wed, Sep 21, 2016 at 09:57:38AM +0100, Brian Starkey wrote:
> Hi Russell,
> 
> Are you in a position to be able to test this now?

Normally, I'd say no, because I'd normally wait for 4.8 to be out before
moving the cubox tree up.  However, as we're close to 4.8, I've merged
4.8-rc7 in (and fixed the multitude of conflicts), and manually made the
changes in your patch.  Nothing seems to have broken, so I think we're
good.

Acked-by: Russell King 

Daniel, please take this change through the drm-misc tree as I'm unlikely
to have a branch which I can apply it to until after the merge window
opens.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.


[Bug 97806] GPU lockup with mesa-git and llvm-svn with rx 470 on Unigine Heaven and TombRaider 2013

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97806

--- Comment #11 from Nicolai Hähnle  ---
Could you please also run the hanging setup with GALLIUM_DDEBUG="pipelined
1000"? That should produce a file in ~/ddebug_dumps/ which will allow isolating
the problematic shader.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/69a0e66f/attachment.html>


[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

2016-09-21 Thread Christian König
Am 21.09.2016 um 17:13 schrieb Michel Dänzer:
> On 21/09/16 07:30 PM, Christian König wrote:
>> Am 21.09.2016 um 11:56 schrieb Michel Dänzer:
>>> FWIW, we seem to have the same issue with radeon vs. amdgpu: radeon only
>>> seems to wait for exclusive fences, so e.g. running Xorg on amdgpu and
>>> using PRIME slave scanout on radeon leaves artifacts.
>> Yeah, I know. See radeon_display.c radeon_flip_work_func().
>>
>> We pretty much need the same patch here I've done for amdgpu as well.
> Actually, the PRIME slave can't scan out from the shared BOs directly
> (recall the recent discussion we had about that with Mario), but has to
> copy from the shared BO to a dedicated scanout BO. These copies need to
> be synchronized with the primary GPU's copies to the shared BO.

Yeah, that thought came to my mind before as well.

Buffer migrations by the kernel caused by a prime export actually set 
the exclusive fence.

So this shouldn't be an issue in practice when the displaying GPU needs 
to copy from the BO again anyway.

The only case I can see when this can happen is when the BO is composed 
directly in system memory by the engines and not migrated there.

Could be that we run into this issue more often in the future, because 
that is pretty much what we want to have for 4K UVD decode.

Christian.



[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

2016-09-21 Thread Christian König
Am 21.09.2016 um 17:07 schrieb Michel Dänzer:
> On 21/09/16 09:56 PM, Daniel Vetter wrote:
>> On Wed, Sep 21, 2016 at 1:19 PM, Christian König
>>  wrote:
>>> Am 21.09.2016 um 13:04 schrieb Daniel Vetter:
 On Wed, Sep 21, 2016 at 12:30 PM, Christian König
  wrote:
> Am 21.09.2016 um 11:56 schrieb Michel Dänzer:
>>
>> Looks like there are different interpretations of the semantics of
>> exclusive vs. shared fences. Where are these semantics documented?
>
> Yeah, I think as well that this is the primary question here.
>
> IIRC the fences were explicitly called exclusive/shared instead of
> writing/reading on purpose.
>
> I absolutely don't mind switching to them to writing/reading semantics,
> but
> amdgpu really needs multiple writers at the same time.
>
> So in this case the writing side of a reservation object needs to be a
> collection of fences as well.
 You can't have multiple writers with implicit syncing. That confusion
 is exactly why we called them shared/exclusive. Multiple writers
 generally means that you do some form of fencing in userspace
 (unsync'ed gl buffer access is the common one). What you do for
 private buffers doesn't matter, but when you render into a
 shared/winsys buffer you really need to set the exclusive fence (and
 there can only ever be one). So probably needs some userspace
 adjustments to make sure you don't accidentally set an exclusive write
 hazard when you don't really want that implicit sync.
>>>
>>> Nope, that isn't true.
>>>
>>> We use multiple writers without implicit syncing between processes in the
>>> amdgpu stack perfectly fine.
>>>
>>> See amdgpu_sync.c for the implementation. What we do there is taking a look
>>> at all the fences associated with a reservation object and only sync to
>>> those who are from another process.
>>>
>>> Then we use implicit syncing for command submissions in the form of
>>> "dependencies". E.g. for each CS we report back an identifier of that
>>> submission to user space and on the next submission you can give this
>>> identifier as dependency which needs to be satisfied before the command
>>> submission can start running.
>> This is called explicit fencing. Implemented with a driver-private
>> primitive (and not sync_file fds like on android), but still
>> conceptually explicit fencing. Implicit fencing really only can handle
>> one writer, at least as currently implemented by struct
>> reservation_object.
>>
>>> This was done to allow multiple engines (3D, DMA, Compute) to compose a
>>> buffer while still allow compatibility with protocols like DRI2/DRI3.
>> Instead of the current solution you need to stop attaching exclusive
>> fences to non-shared buffers (which are coordinated using the
>> driver-private explicit fencing you're describing),
> Err, the current issue is actually that amdgpu never sets an exclusive
> fence, only ever shared ones. :)

Actually amdgpu does set the exclusive fence for buffer migrations, 
cause that is an operation user space doesn't know about and so it needs 
to be "exclusive" access to the buffer.


>> and only attach exclusive fences to shared buffers (DRI2/3, PRIME,
>> whatever).
> Still, it occurred to me in the meantime that amdgpu setting the
> exclusive fence for buffers shared via PRIME (no matter if it's a write
> or read operation) might be a solution. Christian, what do you think?

The problem is that we don't have one fence, but many.

E.g. there can be many operation on a buffer at the same time and we 
need to wait for all of them to complete before it can be displayed.

Regards,
Christian.


[PATCH v4 10/14] drm: amdgpu: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

2016-09-21 Thread Laurent Pinchart
Hi Daniel,

On Wednesday 21 Sep 2016 14:46:07 Daniel Vetter wrote:
> On Wed, Sep 21, 2016 at 2:39 PM, Laurent Pinchart wrote:
> >> > @@ -82,7 +82,7 @@ int amdgpu_align_pitch(struct amdgpu_device *adev,
> >> > int width, int bpp, bool tile
> >> > 
> >> > aligned += pitch_mask;
> >> > aligned &= ~pitch_mask;
> >> > 
> >> > -   return aligned;
> >> > +   return aligned * cpp;
> >> 
> >> Now you multiply by cpp after the rounding.
> > 
> > That's right, but I don't think that's a problem, as all bpp values
> > returned by drm_fb_get_bpp_depth() are multiple of 8 bits.
> 
> Before we have ALIGN(width * cpp, pitch_mask + 1). With your patch we
> have ALIGN(width, pitch_mask + 1) * cpp. In short we overalign, and
> instead of e.g. aligning to 256bytes we now align to 256*4 (for
> xrgb).

The current code is

mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, bpp,
  fb_tiled) * ((bpp + 1) / 8);

As bpp is a multiple of 8, this is equivalent to

mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, bpp,
  fb_tiled) * (bpp / 8);

The patch replaces the code with

mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
  fb_tiled);

with cpp = bpp / 8, and amdgpu_align_pitch() now returns

-   return aligned;
+   return aligned * cpp;

So the patch just moves * (bpp / 8) inside the amdgpu_align_pitch() function.

The other code path is changed as follows:

-   args->pitch = amdgpu_align_pitch(adev, args->width, args->bpp, 0) * 
((args->bpp + 1) / 8);
+   args->pitch = amdgpu_align_pitch(adev, args->width,
+DIV_ROUND_UP(args->bpp, 8), 0);

DIV_ROUND_UP(args->bpp, 8) is equal to ((args->bpp + 1) / 8) for all supported 
bpp values, so this also moves the multiplication inside the function, without 
changing the result as far as I can see.

Note that amdgpu_align_width() is also changed as follows

-   switch (bpp / 8) {
+   switch (cpp) {
case 1:
pitch_mask = 255;
break;
case 2:
pitch_mask = 127;
break;
case 3:
case 4:
pitch_mask = 63;
break;
}

This will change the pitch mask if the bpp value isn't a multiple of 8. 
However, all the formats we support use multiples of 8 as bpp values, so I 
don't see a problem here.

-- 
Regards,

Laurent Pinchart



[PATCH 2/2] drm: Don't swallow error codes in drm_dev_alloc()

2016-09-21 Thread Tom Gundersen
There are many reasons other than ENOMEM that drm_dev_init() can
fail. Return ERR_PTR rather than NULL to be able to distinguish
these in the caller.

Signed-off-by: Tom Gundersen 
---
 drivers/gpu/drm/arc/arcpgu_drv.c| 4 ++--
 drivers/gpu/drm/arm/hdlcd_drv.c | 4 ++--
 drivers/gpu/drm/arm/malidp_drv.c| 4 ++--
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 4 ++--
 drivers/gpu/drm/drm_drv.c   | 6 +++---
 drivers/gpu/drm/drm_pci.c   | 4 ++--
 drivers/gpu/drm/drm_platform.c  | 4 ++--
 drivers/gpu/drm/etnaviv/etnaviv_drv.c   | 4 ++--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   | 4 ++--
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 ++--
 drivers/gpu/drm/mediatek/mtk_drm_drv.c  | 4 ++--
 drivers/gpu/drm/msm/msm_drv.c   | 4 ++--
 drivers/gpu/drm/nouveau/nouveau_drm.c   | 4 ++--
 drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 4 ++--
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 ++--
 drivers/gpu/drm/sti/sti_drv.c   | 4 ++--
 drivers/gpu/drm/sun4i/sun4i_drv.c   | 4 ++--
 drivers/gpu/drm/tegra/drm.c | 4 ++--
 drivers/gpu/drm/udl/udl_drv.c   | 4 ++--
 drivers/gpu/drm/vc4/vc4_drv.c   | 4 ++--
 drivers/gpu/drm/vgem/vgem_drv.c | 4 ++--
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c| 4 ++--
 22 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 6d4ff34..28e6471 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -198,8 +198,8 @@ static int arcpgu_probe(struct platform_device *pdev)
int ret;

drm = drm_dev_alloc(_drm_driver, >dev);
-   if (!drm)
-   return -ENOMEM;
+   if (IS_ERR(drm))
+   return PTR_ERR(drm);

ret = arcpgu_load(drm);
if (ret)
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index d83b46a..fb6a418 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -326,8 +326,8 @@ static int hdlcd_drm_bind(struct device *dev)
return -ENOMEM;

drm = drm_dev_alloc(_driver, dev);
-   if (!drm)
-   return -ENOMEM;
+   if (IS_ERR(drm))
+   return PTR_ERR(drm);

drm->dev_private = hdlcd;
dev_set_drvdata(dev, drm);
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index c383d72..9280358 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -311,8 +311,8 @@ static int malidp_bind(struct device *dev)
return ret;

drm = drm_dev_alloc(_driver, dev);
-   if (!drm) {
-   ret = -ENOMEM;
+   if (IS_ERR(drm)) {
+   ret = PTR_ERR(drm);
goto alloc_fail;
}

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 8e7483d..5f48431 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -797,8 +797,8 @@ static int atmel_hlcdc_dc_drm_probe(struct platform_device 
*pdev)
int ret;

ddev = drm_dev_alloc(_hlcdc_dc_driver, >dev);
-   if (!ddev)
-   return -ENOMEM;
+   if (IS_ERR(ddev))
+   return PTR_ERR(ddev);

ret = atmel_hlcdc_dc_load(ddev);
if (ret)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 99e6751..80c7f25 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -591,7 +591,7 @@ EXPORT_SYMBOL(drm_dev_init);
  * own struct should look at using drm_dev_init() instead.
  *
  * RETURNS:
- * Pointer to new DRM device, or NULL if out of memory.
+ * Pointer to new DRM device, or ERR_PTR on failure.
  */
 struct drm_device *drm_dev_alloc(struct drm_driver *driver,
 struct device *parent)
@@ -601,12 +601,12 @@ struct drm_device *drm_dev_alloc(struct drm_driver 
*driver,

dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev)
-   return NULL;
+   return ERR_PTR(-ENOMEM);

ret = drm_dev_init(dev, driver, parent);
if (ret) {
kfree(dev);
-   return NULL;
+   return ERR_PTR(ret);
}

return dev;
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index d86362f..3ceea9c 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -236,8 +236,8 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct 
pci_device_id *ent,
DRM_DEBUG("\n");

dev = drm_dev_alloc(driver, >dev);
-   if (!dev)
-   return -ENOMEM;
+   if (IS_ERR(dev))
+   return PTR_ERR(dev);

ret = pci_enable_device(pdev);
if 

[PATCH 1/2] drm: Distinguish no name from ENOMEM in set_unique()

2016-09-21 Thread Tom Gundersen
If passing name == NULL to drm_drv_set_unique() we now get -ENOMEM
as kstrdup() returns NULL. Instead check for this explicitly and
return -EINVAL if no name is provided.

Signed-off-by: Tom Gundersen 
---
 drivers/gpu/drm/drm_drv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index f2f6429..99e6751 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -338,6 +338,9 @@ void drm_minor_release(struct drm_minor *minor)

 static int drm_dev_set_unique(struct drm_device *dev, const char *name)
 {
+   if (!name)
+   return -EINVAL;
+
kfree(dev->unique);
dev->unique = kstrdup(name, GFP_KERNEL);

-- 
2.9.3



[Bug 97806] GPU lockup with mesa-git and llvm-svn with rx 470 on Unigine Heaven and TombRaider 2013

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97806

--- Comment #10 from Laurent carlier  ---
Created attachment 126712
  --> https://bugs.freedesktop.org/attachment.cgi?id=126712=edit
output of R600_DEBUG=ps,gs,vs,cs,tcs,tes ./heaven with tesselation extreme and
llvm-3.9.0 -> good

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/47f5e747/attachment.html>


[PATCH] drm/udl: Fix for the X server screen update v3

2016-09-21 Thread poma
On 21.09.2016 13:33, David Herrmann wrote:
> Hi
> 
> On Wed, Sep 21, 2016 at 1:19 PM, poma  wrote:
>> On 21.09.2016 09:34, David Herrmann wrote:
>>> Hi
>>>
>>> On Wed, Sep 21, 2016 at 6:47 AM, poma  wrote:
 Within X server, on top of DisplayLink GPU USB2.0 device,
 screen content is not refreshed i.e. updated.

 This fixes commit:

 - e375882406d0cc24030746638592004755ed4ae0
   "drm/udl: Use drm_fb_helper deferred_io support"

 Thanks Noralf and Daniel for the comments.

 Tested-by: poma 
>>>
>>> Can you provide your Signed-off-by: line?
>>>
>>> Reviewed-by: David Herrmann 
>>>
>>> Thanks
>>> David
>>>
>>
>> S-o-b should be performed by the actual kernel developer.
>> R-b & T-b, I've already written.
> 
> The author of a patch must provide the S-o-b (see
> Documentation/SubmittingPatches if you want details). So simply reply
> with a "S-o-b: foo " line to this mail. And please include it in
> all patches you submit (preferably use `git commit --sign-off`).
> 
> Thanks
> David
> 

Patches are nothing but direct suggestions from Noralf and Daniel.
I wrote a patch to show what is actually tested, tested successfully,
but I'm not the author of these corrections.




[PATCH] drm/rockchip: Cleanup dangling devm pointers

2016-09-21 Thread Daniel Kurtz
On Wed, Sep 21, 2016 at 3:36 PM, Sean Paul  wrote:
> On Mon, Sep 19, 2016 at 7:14 AM, Daniel Kurtz  wrote:
>> Hi Sean,
>>
>> On Sat, Sep 17, 2016 at 2:22 AM, Sean Paul  wrote:
>>>
>>> Instead of assigning device managed resources to local variables,
>>> keep track of them in the vop struct.
>>
>> Why this patch?
>> Is it fixing an issue?
>> Or, is it preparing for some future use of ahb_rst outside of vop_initial?
>>
>
> Nah, this is just me being pedantic.
>
>> I thought that one of the nice features of using devm is you do not
>> need to carry around pointers to devm allocated resources in the
>> driver local device struct.
>>
>
> True, but it feels a bit weird to allocate something on driver load
> and intentionally abandon it for the lifetime of the driver. I'd feel
> better either keeping it around, or perhaps it should be put once
> we're done using it in vop_initial.

Yeah... if we don't ever use it again, I agree - no reason to devm it,
just use reset_control_get / _put here in vop_initial.

>
> Sean
>
>
>> -Dan
>>
>>>
>>> Signed-off-by: Sean Paul 
>>> ---
>>>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 12 ++--
>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
>>> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>> index 131ae0f..bed782e 100644
>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>> @@ -142,6 +142,7 @@ struct vop {
>>>
>>> /* vop dclk reset */
>>> struct reset_control *dclk_rst;
>>> +   struct reset_control *ahb_rst;
>>>
>>> struct vop_win win[];
>>>  };
>>> @@ -1333,7 +1334,6 @@ static int vop_initial(struct vop *vop)
>>>  {
>>> const struct vop_data *vop_data = vop->data;
>>> const struct vop_reg_data *init_table = vop_data->init_table;
>>> -   struct reset_control *ahb_rst;
>>> int i, ret;
>>>
>>> vop->hclk = devm_clk_get(vop->dev, "hclk_vop");
>>> @@ -1374,15 +1374,15 @@ static int vop_initial(struct vop *vop)
>>> /*
>>>  * do hclk_reset, reset all vop registers.
>>>  */
>>> -   ahb_rst = devm_reset_control_get(vop->dev, "ahb");
>>> -   if (IS_ERR(ahb_rst)) {
>>> +   vop->ahb_rst = devm_reset_control_get(vop->dev, "ahb");
>>> +   if (IS_ERR(vop->ahb_rst)) {
>>> dev_err(vop->dev, "failed to get ahb reset\n");
>>> -   ret = PTR_ERR(ahb_rst);
>>> +   ret = PTR_ERR(vop->ahb_rst);
>>> goto err_disable_aclk;
>>> }
>>> -   reset_control_assert(ahb_rst);
>>> +   reset_control_assert(vop->ahb_rst);
>>> usleep_range(10, 20);
>>> -   reset_control_deassert(ahb_rst);
>>> +   reset_control_deassert(vop->ahb_rst);
>>>
>>> memcpy(vop->regsbak, vop->regs, vop->len);
>>>
>>> --
>>> 2.8.0.rc3.226.g39d4020
>>>
>>> ___
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 97806] GPU lockup with mesa-git and llvm-svn with rx 470 on Unigine Heaven and TombRaider 2013

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97806

--- Comment #9 from Laurent carlier  ---
Created attachment 126709
  --> https://bugs.freedesktop.org/attachment.cgi?id=126709=edit
output of R600_DEBUG=ps,gs,vs,cs,tcs,tes ./heaven with tesselation extreme ->
lockup

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/3440560a/attachment-0001.html>


[ADV7393] DRM Encoder Slave or DRM Bridge

2016-09-21 Thread Vikas Patil
On Thu, Sep 15, 2016 at 3:23 PM, Tomi Valkeinen  
wrote:
>
>
> On 15/09/16 12:44, Vikas Patil wrote:
>> On Wed, Sep 14, 2016 at 3:04 PM, Tomi Valkeinen  
>> wrote:
>>>
>>>
>>> On 13/09/16 16:13, Vikas Patil wrote:
 Thanks Tomi for quick comment.

 I am thinking to base adv7393 driver on
 "drivers\gpu\drm\omapdrm\displays\encoder-tc358768.c" as I don't think
 any similar to adv7393 chip driver available. Could you please comment
 if this will help to get adv chip running?
>>>
>>> I presume you're not using mainline kernel, as that driver is not there.
>>> I'm not familiar with adv7393, but yes, I think you can use that as an
>>> example.
>>>
>>
>> Thanks a lot for your comments. I am using latest (i.e. 3.00.00.03 )
>> Processor SDK Linux Automotive which is based on linux 4.4.14.
>>
>> As my display panel is connected as follows. I am little confused over
>> the values I need to set for following properties in probe function.
>>
>> DPI1/VOUT1 -16bit DRGB---> ADV7393 (Digital to Analog video
>> encoder) --> CVBS Out --> Display Panel
>>
>>
>>  dssdev->ops.dpi = _dpi_ops; (atv?)
>> dssdev->type = OMAP_DISPLAY_TYPE_DPI;
>> dssdev->output_type = OMAP_DISPLAY_TYPE_DPI; (Do I need to use
>> OMAP_DISPLAY_TYPE_VENC, but DRA74x do not have VENC Encoder I think)
>> dssdev->phy.dpi.data_lines = ddata->dpi_ndl;
>> dssdev->port_num = 1;
>>
>>
>> As adv7393 takes 16-bit DRGB as input and gives composite as output,
>> does above configuration looks correct? or Do I need to change to
>> something else (e.g. dpi,sdi,dvi, hdmi, atv, dsi)?
>
> The API is quite messy (full of legacy)...
>
> But the "ops" there are for the "downstream" direction, i.e. towards the
> connector. So here you should have atv ops. You should then have
> connector-analog-tv as a device after adv7393, and that connector driver
> will be calling those atv ops.
>
> adv7393 itself will be calling dpi ops, offered by the DSS.
>
> You should set dssdev->type to DPI (that's the input).
> dssdev->output_type to OMAP_DISPLAY_TYPE_VENC (output, although "venc"
> is not quite correct here, but closest match we have). DRA74x doesn't
> have VENC, but this is what the adv7393 outputs.
>

Thanks a lot for explaining it. It really helped. I have created the
driver and configured the registers of ADV7393 as required. Even
though encoders and connectors are visible using "modetest" now, but
it seems adv7393 encoder and cvbs-out connector not connected (sttaus
in modetest shows unknown) and might be failing in mode setting (from
DRM logs).

Could you please suggest where should I need to look for fixing this?
Do I need to configure the display timing as per the ADV7393
configuration or as per the attached panel in
\omapdrm\displays\connector-analog-tv.c ?

Also one doubt I have is,  as I know DRA74x has 1 GFX pipeline/overlay
and 3 video pipeline/overlay and now with my first LCD display GFX
pipeline would have been connected to vout3/lcd3/dpi3 to LCD panel.
How could I now configure the GFX overlay to vout1/lcd1/dpi1 and
further to adv7393 so that weston could show up on both the displays?
Will second display show kmscube, weston etc?

DRM log snippets: (Attached here the complete drm log when i run modetest)

[  226.862242] [drm:drm_ioctl] pid=1853, dev=0xe200, auth=1,
DRM_IOCTL_MODE_GETRESOURCES
[  226.862254] [drm:drm_mode_getresources] CRTC[2] CONNECTORS[2] ENCODERS[2]
[  226.862266] [drm:drm_ioctl] pid=1853, dev=0xe200, auth=1,
DRM_IOCTL_MODE_GETRESOURCES
[  226.862276] [drm:drm_mode_getresources] [CRTC:34]
[  226.862284] [drm:drm_mode_getresources] [CRTC:38]
[  226.862292] [drm:drm_mode_getresources] [ENCODER:31:TMDS-31]
[  226.862301] [drm:drm_mode_getresources] [ENCODER:35:TMDS-35]
[  226.862309] [drm:drm_mode_getresources] [CONNECTOR:32:Unknown-1]
[  226.862317] [drm:drm_mode_getresources] [CONNECTOR:36:Unknown-2]
[  226.862325] [drm:drm_mode_getresources] CRTC[2] CONNECTORS[2] ENCODERS[2]
[  226.862456] [drm:drm_ioctl] pid=1853, dev=0xe200, auth=1,
DRM_IOCTL_MODE_GETCRTC
[  226.862475] [drm:drm_ioctl] pid=1853, dev=0xe200, auth=1,
DRM_IOCTL_MODE_GETCRTC
[  226.862498] [drm:drm_ioctl] pid=1853, dev=0xe200, auth=1,
DRM_IOCTL_MODE_GETENCODER
[  226.862512] [drm:drm_ioctl] pid=1853, dev=0xe200, auth=1,
DRM_IOCTL_MODE_GETENCODER
[  226.862527] [drm:drm_ioctl] pid=1853, dev=0xe200, auth=1,
DRM_IOCTL_MODE_GETCONNECTOR
[  226.862536] [drm:drm_mode_getconnector] [CONNECTOR:32:?]
[  226.862547] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:32:Unknown-1]
[  226.862557] [drm:omap_connector_get_modes] cvbs_out
[  226.862571] -->adv7393_check_timings: start
[  226.870369] [drm:omap_connector_mode_valid] connector: mode
invalid: 42:"720x574i" 50 13500 720 732 796 864 574 579 584 625 0x48
0x2a1a
[  226.870383] [drm:drm_mode_debug_printmodeline] Modeline
42:"720x574i" 50 13500 720 732 796 864 574 579 584 625 0x48 0x2a1a
[  226.870392] [drm:drm_mode_prune_invalid] Not using 720x574i 

[Bug 97887] llvm segfault in janusvr -render vive

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97887

--- Comment #1 from Christoph Haag  ---
Created attachment 126706
  --> https://bugs.freedesktop.org/attachment.cgi?id=126706=edit
gdb.txt assertion with full backtrace

With full debugging llvm and mesa I get an assertion instead:

janusvr:
/home/chris/build/llvm-svn/src/llvm/include/llvm/Analysis/LoopInfoImpl.h:247:
void llvm::LoopBase<N, M>::verifyLoop() const [with BlockT = llvm::BasicBlock;
LoopT = llvm::Loop]: Assertion
`std::any_of(GraphTraits<BlockT*>::child_begin(BB),
GraphTraits<BlockT*>::child_end(BB), [&](BlockT *B){return contains(B);}) &&
"Loop block has no in-loop successors!"' failed.

backtrace full attached

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/e644ad06/attachment.html>


[PATCH v4 10/14] drm: amdgpu: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

2016-09-21 Thread Laurent Pinchart
Hi Daniel,

Thank you for the review.

On Wednesday 21 Sep 2016 09:51:44 Daniel Vetter wrote:
> On Thu, Sep 08, 2016 at 05:44:24PM +0300, Laurent Pinchart wrote:
> > The driver needs the number of bytes per pixel, not the bpp and depth
> > info meant for fbdev compatibility. Use the right API.
> > 
> > Signed-off-by: Laurent Pinchart 
> > ---
> > Changes since v3:
> > 
> > - Renamed bpp to cpp
> > ---
> > 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  | 14 +++---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  3 ++-
> >  2 files changed, 9 insertions(+), 8 deletions(-)
> > 
> > Cc: Alex Deucher 
> > Cc: Christian König 
> > Cc: Michel Dänzer 
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c index bf033b58056c..0727946db189
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> > @@ -62,12 +62,12 @@ static struct fb_ops amdgpufb_ops = {
> > 
> >  };
> > 
> > -int amdgpu_align_pitch(struct amdgpu_device *adev, int width, int bpp,
> > bool tiled)
> > +int amdgpu_align_pitch(struct amdgpu_device *adev, int> width, int cpp,
> > bool tiled)
> >  {
> > int aligned = width;
> > int pitch_mask = 0;
> > 
> > -   switch (bpp / 8) {
> > +   switch (cpp) {
> > case 1:
> > pitch_mask = 255;
> > break;
> > @@ -82,7 +82,7 @@ int amdgpu_align_pitch(struct amdgpu_device *adev, int
> > width, int bpp, bool tile
> > aligned += pitch_mask;
> > aligned &= ~pitch_mask;
> > 
> > -   return aligned;
> > +   return aligned * cpp;
> 
> Now you multiply by cpp after the rounding.

That's right, but I don't think that's a problem, as all bpp values returned 
by drm_fb_get_bpp_depth() are multiple of 8 bits.

> Otherwise looks reasonable.
> -Daniel
> 
> >  }
> >  
> >  static void amdgpufb_destroy_pinned_object(struct drm_gem_object *gobj)
> > @@ -111,13 +111,13 @@ static int amdgpufb_create_pinned_object(struct
> > amdgpu_fbdev *rfbdev,
> > int ret;
> > int aligned_size, size;
> > int height = mode_cmd->height;
> > -   u32 bpp, depth;
> > +   u32 cpp;
> > 
> > -   drm_fb_get_bpp_depth(mode_cmd->pixel_format, , );
> > +   cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
> > 
> > /* need to align pitch with crtc limits */
> > 
> > -   mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, bpp,
> > - fb_tiled) * ((bpp + 1) / 8);
> > +   mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
> > + fb_tiled);
> > 
> > height = ALIGN(mode_cmd->height, 8);
> > size = mode_cmd->pitches[0] * height;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index
> > 88fbed2389c0..20a4e569b245 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > @@ -704,7 +704,8 @@ int amdgpu_mode_dumb_create(struct drm_file
> > *file_priv,
> > uint32_t handle;
> > int r;
> > 
> > -   args->pitch = amdgpu_align_pitch(adev, args->width, args->bpp, 0) *
> > ((args->bpp + 1) / 8);
> > +   args->pitch = amdgpu_align_pitch(adev,> args->width,
> > +DIV_ROUND_UP(args->bpp, 8), 0);
> > 
> > args->size = (u64)args->pitch * args->height;
> > args->size = ALIGN(args->size, PAGE_SIZE);

-- 
Regards,

Laurent Pinchart



GPU-DRM-nouveau: Delete unnecessary braces

2016-09-21 Thread SF Markus Elfring
> The original style was correct, the new style is wrong.

I find your feedback interesting for further clarifications.


> Multi-line indents get curly braces for readability.

How do you think about to transform such an information
into an official specification for the the document "CodingStyle"?

Regards,
Markus


DRM: how to support live source

2016-09-21 Thread Ginny Chen
Dear All:

I am Ginny Chen from Mediatek company.I start to work with DRM for a 
while and now I am evaluating our display architecture for our new SoC.
I would like to adopt DRM to develop our display driver. 
However, in our new SoC, the input source of display engine could be 
memory or direct-linked with other hardwares (ex:camera sensor) which 
means the input source of each plane should be able to configure during 
plane updating.
As my understanding,with currently DRM framework, the input source of 
plane could only be framebuffer that implies we still need to allocate 
framebuffer for planes which are direct-linked with other hardware 
engines if I want to control planes without modifying DRM framework.
It will waste memory and doesn't make sense for user to notice/manage 
the framebuffers for direct-linked DRM planes.
Do you have any ideas for such use case?
Any suggestion will be very appreciated.

Regards,
Ginny Chen



[PATCH -next] gpu: ipu-v3: Use ERR_CAST instead of ERR_PTR(PTR_ERR())

2016-09-21 Thread Wei Yongjun
From: Wei Yongjun 

Use ERR_CAST inlined function instead of ERR_PTR(PTR_ERR(...)).

Generated by: scripts/coccinelle/api/err_cast.cocci

Signed-off-by: Wei Yongjun 
---
 drivers/gpu/ipu-v3/ipu-image-convert.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/ipu-v3/ipu-image-convert.c 
b/drivers/gpu/ipu-v3/ipu-image-convert.c
index 2ba7d43..805b6fa 100644
--- a/drivers/gpu/ipu-v3/ipu-image-convert.c
+++ b/drivers/gpu/ipu-v3/ipu-image-convert.c
@@ -1617,7 +1617,7 @@ ipu_image_convert(struct ipu_soc *ipu, enum ipu_ic_task 
ic_task,
ctx = ipu_image_convert_prepare(ipu, ic_task, in, out, rot_mode,
complete, complete_context);
if (IS_ERR(ctx))
-   return ERR_PTR(PTR_ERR(ctx));
+   return ERR_CAST(ctx);

run = kzalloc(sizeof(*run), GFP_KERNEL);
if (!run) {





[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97879

--- Comment #3 from Michel Dänzer  ---
(In reply to Silvan Jegen from comment #2)
> Just to clarify: the trace also includes loading of the game itself (which
> takes a long time too) as well as in-game menu navigation.

Right, replaying the trace crashes for me after the shader compilations on
startup, so my CPU profile only covered that. Maybe you can try getting a CPU
profile of one of the other stalls.

Otherwise, maybe try setting the environment variable
GALLIUM_HUD=.dfps,requested-VRAM+VRAM-usage,requested-GTT+GTT-usage,cpu+temperature+GPU-load,.dnum-bytes-moved,.dbuffer-wait-time,.dnum-compilations+num-shaders-created
for either running the game itself or replaying the trace, and taking a
screenshot within one minute after one of the other stalls. That should allow
at least confirming or ruling out that the other stalls are due to shader
compilation as well.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/5dd75905/attachment-0001.html>


[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

2016-09-21 Thread Daniel Vetter
On Wed, Sep 21, 2016 at 1:19 PM, Christian König
 wrote:
> Am 21.09.2016 um 13:04 schrieb Daniel Vetter:
>>
>> On Wed, Sep 21, 2016 at 12:30 PM, Christian König
>>  wrote:
>>>
>>> Am 21.09.2016 um 11:56 schrieb Michel Dänzer:


 Looks like there are different interpretations of the semantics of
 exclusive vs. shared fences. Where are these semantics documented?
>>>
>>>
>>> Yeah, I think as well that this is the primary question here.
>>>
>>> IIRC the fences were explicitly called exclusive/shared instead of
>>> writing/reading on purpose.
>>>
>>> I absolutely don't mind switching to them to writing/reading semantics,
>>> but
>>> amdgpu really needs multiple writers at the same time.
>>>
>>> So in this case the writing side of a reservation object needs to be a
>>> collection of fences as well.
>>
>> You can't have multiple writers with implicit syncing. That confusion
>> is exactly why we called them shared/exclusive. Multiple writers
>> generally means that you do some form of fencing in userspace
>> (unsync'ed gl buffer access is the common one). What you do for
>> private buffers doesn't matter, but when you render into a
>> shared/winsys buffer you really need to set the exclusive fence (and
>> there can only ever be one). So probably needs some userspace
>> adjustments to make sure you don't accidentally set an exclusive write
>> hazard when you don't really want that implicit sync.
>
>
> Nope, that isn't true.
>
> We use multiple writers without implicit syncing between processes in the
> amdgpu stack perfectly fine.
>
> See amdgpu_sync.c for the implementation. What we do there is taking a look
> at all the fences associated with a reservation object and only sync to
> those who are from another process.
>
> Then we use implicit syncing for command submissions in the form of
> "dependencies". E.g. for each CS we report back an identifier of that
> submission to user space and on the next submission you can give this
> identifier as dependency which needs to be satisfied before the command
> submission can start running.

This is called explicit fencing. Implemented with a driver-private
primitive (and not sync_file fds like on android), but still
conceptually explicit fencing. Implicit fencing really only can handle
one writer, at least as currently implemented by struct
reservation_object.

> This was done to allow multiple engines (3D, DMA, Compute) to compose a
> buffer while still allow compatibility with protocols like DRI2/DRI3.

Instead of the current solution you need to stop attaching exclusive
fences to non-shared buffers (which are coordinated using the
driver-private explicit fencing you're describing), and only attach
exclusive fences to shared buffers (DRI2/3, PRIME, whatever). Since
you're doing explicit syncing for internal stuff anyway you can still
opt to ignore the exclusive fences if you want to (driven by a flag or
something similar).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[radeon-alex:amd-staging-4.7 1432/1444] ERROR: "amd_set_clockgating_by_smu" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!

2016-09-21 Thread kbuild test robot
tree:   git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.7
head:   1dc235cbe74b44f34e0962b86fc548eee7fe6ec0
commit: a5cce7abb3d24dd2bdab47d663e3d21f91ca1d8d [1432/1444] drm/amdgpu: set 
system clock gating for tonga/polaris.
config: i386-randconfig-r0-09190352 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
git checkout a5cce7abb3d24dd2bdab47d663e3d21f91ca1d8d
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "amd_set_clockgating_by_smu" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] 
>> undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
-- next part --
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 28206 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/62d3c810/attachment-0001.gz>


[PATCH v4 10/14] drm: amdgpu: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

2016-09-21 Thread Daniel Vetter
On Wed, Sep 21, 2016 at 2:39 PM, Laurent Pinchart
 wrote:
>> > @@ -82,7 +82,7 @@ int amdgpu_align_pitch(struct amdgpu_device *adev, int
>> > width, int bpp, bool tile
>> > aligned += pitch_mask;
>> > aligned &= ~pitch_mask;
>> >
>> > -   return aligned;
>> > +   return aligned * cpp;
>>
>> Now you multiply by cpp after the rounding.
>
> That's right, but I don't think that's a problem, as all bpp values returned
> by drm_fb_get_bpp_depth() are multiple of 8 bits.

Before we have ALIGN(width * cpp, pitch_mask + 1). With your patch we
have ALIGN(width, pitch_mask + 1) * cpp. In short we overalign, and
instead of e.g. aligning to 256bytes we now align to 256*4 (for
xrgb).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[PATCH v4.1 01/14] drm: Centralize format information

2016-09-21 Thread Daniel Vetter
On Wed, Sep 21, 2016 at 1:21 PM, Laurent Pinchart
 wrote:
>> > +/**
>> > + * struct drm_format_info - information about a DRM format
>> > + * @format: 4CC format identifier (DRM_FORMAT_*)
>> > + * @depth: Color depth (number of bits per pixel excluding padding bits),
>> > + * valid for a subset of RGB formats only. This is a legacy field, do not
>> > + * use in new code and set to 0 for new formats.
>> > + * @num_planes: Number of color planes (1 to 3)
>> > + * @cpp: Number of bytes per pixel (per plane)
>> > + * @hsub: Horizontal chroma subsampling factor
>> > + * @vsub: Vertical chroma subsampling factor
>> > + */
>>
>> Atm we don't include the drm_fourcc.h header in
>> Documentation/gpu/drm-kms.rst, which means we're missing this new
>> kerneldoc.
>
> ... How do you mean exactly ? :-)


I'm blind ;-) r-b holds as-is.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[PATCH v4.1 01/14] drm: Centralize format information

2016-09-21 Thread Eric Engestrom
On Sun, Sep 18, 2016 at 01:17:27PM +0300, Laurent Pinchart wrote:
> Various pieces of information about DRM formats (number of planes, color
> depth, chroma subsampling, ...) are scattered across different helper
> functions in the DRM core. Callers of those functions often need to
> access more than a single parameter of the format, leading to
> inefficiencies due to multiple lookups.
> 
> Centralize all format information in a data structure and create a
> function to look up information based on the format 4CC.
> 
> Signed-off-by: Laurent Pinchart 

I just realized I forgot to give you my r-b's.
Patches #1-3 are:
Reviewed-by: Eric Engestrom 

(One nit-pick below, though :P)

> ---
> Changes since v4:
> 
> - Fixed depth value of DRM_FORMAT_[AXRGB]{4} formats to match
>   current code
> - Documented the depth field as legacy
> ---
>  Documentation/gpu/drm-kms.rst |  3 ++
>  drivers/gpu/drm/drm_fourcc.c  | 84 
> +++
>  include/drm/drm_fourcc.h  | 21 +++
>  3 files changed, 108 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index f9a991bb87d4..85c4c49f4436 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -63,6 +63,9 @@ Frame Buffer Functions Reference
>  DRM Format Handling
>  ===
>  
> +.. kernel-doc:: include/drm/drm_fourcc.h
> +   :internal:
> +
>  .. kernel-doc:: drivers/gpu/drm/drm_fourcc.c
> :export:
>  
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 29c56b4331e0..39f09c564111 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -103,6 +103,90 @@ char *drm_get_format_name(uint32_t format)
>  EXPORT_SYMBOL(drm_get_format_name);
>  
>  /**
> + * drm_format_info - query information for a given format
> + * @format: pixel format (DRM_FORMAT_*)
> + *
> + * Returns:
> + * The instance of struct drm_format_info that describes the pixel format, or
> + * NULL if the format is unsupported.
> + */
> +const struct drm_format_info *drm_format_info(u32 format)
> +{
> + static const struct drm_format_info formats[] = {
> + { .format = DRM_FORMAT_C8,  .depth = 8,  
> .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGB332,  .depth = 8,  
> .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGR233,  .depth = 8,  
> .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_XRGB,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_XBGR,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGBX,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGRX,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_ARGB,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_ABGR,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGBA,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGRA,.depth = 0,  
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_XRGB1555,.depth = 15, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_XBGR1555,.depth = 15, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGBX5551,.depth = 15, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGRX5551,.depth = 15, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_ARGB1555,.depth = 15, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_ABGR1555,.depth = 15, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGBA5551,.depth = 15, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGRA5551,.depth = 15, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGB565,  .depth = 16, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_BGR565,  .depth = 16, 
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format 

[PATCH] dma-buf/fence-array: get signaled state when signaling is disabled

2016-09-21 Thread Gustavo Padovan
From: Gustavo Padovan 

If the fences in the fence_array signal on the fence_array does not have
signalling enabled num_pending will not be updated accordingly.

So when signaling is disabled check the signal of every fence with
fence_is_signaled() and then compare with num_pending to learn if the
fence_array was signalled or not.

If we want to keep the poll_does_not_wait optimization I think we need
something like this. It keeps the same behaviour if signalling is enabled
but tries to calculated the state otherwise.

Signed-off-by: Gustavo Padovan 
Reviewed-by: Chris Wilson 
---
 drivers/dma-buf/fence-array.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/fence-array.c b/drivers/dma-buf/fence-array.c
index f1989fc..1eec271 100644
--- a/drivers/dma-buf/fence-array.c
+++ b/drivers/dma-buf/fence-array.c
@@ -75,8 +75,25 @@ static bool fence_array_enable_signaling(struct fence *fence)
 static bool fence_array_signaled(struct fence *fence)
 {
struct fence_array *array = to_fence_array(fence);
+   int i, num_pending;
+
+   num_pending = atomic_read(>num_pending);
+
+   /*
+* Before signaling is enabled, num_pending is static (set during array
+* construction as a count of all fences or set to 1 if signal_on_any
+* flag is passed. To ensure forward progress, i.e. a while
+* (!fence_is_signaled()) ; busy-loop eventually proceeds, we need to
+* check the current status of our fences.
+*/
+   if (!test_bit(FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) {
+   for (i = 0 ; i < array->num_fences; ++i) {
+   if (fence_is_signaled(array->fences[i]))
+   num_pending--;
+   }
+   }

-   return atomic_read(>num_pending) <= 0;
+   return num_pending <= 0;
 }

 static void fence_array_release(struct fence *fence)
-- 
2.5.5



[PATCH] drm/udl: Fix for the X server screen update v3

2016-09-21 Thread Jani Nikula
On Wed, 21 Sep 2016, poma  wrote:
> On 21.09.2016 09:34, David Herrmann wrote:
>> Hi
>> 
>> On Wed, Sep 21, 2016 at 6:47 AM, poma  wrote:
>>> Within X server, on top of DisplayLink GPU USB2.0 device,
>>> screen content is not refreshed i.e. updated.
>>>
>>> This fixes commit:
>>>
>>> - e375882406d0cc24030746638592004755ed4ae0
>>>   "drm/udl: Use drm_fb_helper deferred_io support"
>>>
>>> Thanks Noralf and Daniel for the comments.
>>>
>>> Tested-by: poma 
>> 
>> Can you provide your Signed-off-by: line?
>> 
>> Reviewed-by: David Herrmann 
>> 
>> Thanks
>> David
>> 
>
> S-o-b should be performed by the actual kernel developer.
> R-b & T-b, I've already written.

Please read section 11 of Documentation/SubmittingPatches.

In short, you need to add your Signed-off-by with your real name.

BR,
Jani.




-- 
Jani Nikula, Intel Open Source Technology Center


[PATCH v4 02/14] drm: Implement the drm_format_*() helpers as drm_format_info() wrappers

2016-09-21 Thread Laurent Pinchart
Hi Daniel,

Thanks for the review.

On Wednesday 21 Sep 2016 09:34:13 Daniel Vetter wrote:
> On Thu, Sep 08, 2016 at 05:44:16PM +0300, Laurent Pinchart wrote:
> > Turn the drm_format_*() helpers into wrappers around the drm_format_info
> > lookup function to centralize all format information in a single place.
> > 
> > Signed-off-by: Laurent Pinchart 
> > ---
> > 
> >  drivers/gpu/drm/drm_fourcc.c | 186 +++--
> >  1 file changed, 37 insertions(+), 149 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index 6b91bd8a510d..bf91c5044d84 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -198,69 +198,22 @@ EXPORT_SYMBOL(drm_format_info);
> > 
> >  void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> >   int *bpp)

[snip]

> > +
> > +   *depth = info->depth;
> > +   *bpp = info->cpp[0] << 3;
> 
> Bikeshed: This is a funny way to write * 8 ... Would be nice to fix imo.

Fixed.

> Reviewed-by: Daniel Vetter 

-- 
Regards,

Laurent Pinchart



[Bug 97806] GPU lockup with mesa-git and llvm-svn with rx 470 on Unigine Heaven and TombRaider 2013

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97806

--- Comment #8 from Tom Stellard  ---
Can you post good/bad shader logs: R600_DEBUG=ps,gs,vs,cs,tcs,tes

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/a55a25fd/attachment.html>


[PATCH v4.1 01/14] drm: Centralize format information

2016-09-21 Thread Laurent Pinchart
Hi Daniel,

Thanks for the review.

On Wednesday 21 Sep 2016 09:23:14 Daniel Vetter wrote:
> On Sun, Sep 18, 2016 at 01:17:27PM +0300, Laurent Pinchart wrote:
> > Various pieces of information about DRM formats (number of planes, color
> > depth, chroma subsampling, ...) are scattered across different helper
> > functions in the DRM core. Callers of those functions often need to
> > access more than a single parameter of the format, leading to
> > inefficiencies due to multiple lookups.
> > 
> > Centralize all format information in a data structure and create a
> > function to look up information based on the format 4CC.
> > 
> > Signed-off-by: Laurent Pinchart 
> > ---
> > Changes since v4:
> > 
> > - Fixed depth value of DRM_FORMAT_[AXRGB]{4} formats to match
> >   current code
> > - Documented the depth field as legacy
> > ---
> > 
> >  Documentation/gpu/drm-kms.rst |  3 ++
> >  drivers/gpu/drm/drm_fourcc.c  | 84 ++
> >  include/drm/drm_fourcc.h  | 21 +++
> >  3 files changed, 108 insertions(+)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index f9a991bb87d4..85c4c49f4436 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -63,6 +63,9 @@ Frame Buffer Functions Reference
> >  DRM Format Handling
> >  ===
> > 
> > +.. kernel-doc:: include/drm/drm_fourcc.h
> > +   :internal:
> > +
> >  .. kernel-doc:: drivers/gpu/drm/drm_fourcc.c
> > :export:
> >  

Given this...

[snip]

> > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> > index 30c30fa87ee8..135fef050ee6 100644
> > --- a/include/drm/drm_fourcc.h
> > +++ b/include/drm/drm_fourcc.h
> > @@ -25,6 +25,27 @@
> > 
> >  #include 
> >  #include 
> > 
> > +/**
> > + * struct drm_format_info - information about a DRM format
> > + * @format: 4CC format identifier (DRM_FORMAT_*)
> > + * @depth: Color depth (number of bits per pixel excluding padding bits),
> > + * valid for a subset of RGB formats only. This is a legacy field, do not
> > + * use in new code and set to 0 for new formats.
> > + * @num_planes: Number of color planes (1 to 3)
> > + * @cpp: Number of bytes per pixel (per plane)
> > + * @hsub: Horizontal chroma subsampling factor
> > + * @vsub: Vertical chroma subsampling factor
> > + */
> 
> Atm we don't include the drm_fourcc.h header in
> Documentation/gpu/drm-kms.rst, which means we're missing this new
> kerneldoc.

... How do you mean exactly ? :-)

> With that fixed:
> 
> Reviewed-by: Daniel Vetter 

-- 
Regards,

Laurent Pinchart



[PATCH] drm: Fix typo in encoder docs

2016-09-21 Thread Daniel Vetter
On Mon, Sep 19, 2016 at 03:40:48PM -0700, Dhinakaran Pandiyan wrote:
> Corrected typo in bridge and encoder comparison. Also, added a one-line
> encoder description from the previous documentation.
> 
> Cc: Daniel Vetter 
> Cc: Archit Taneja 
> 
> Signed-off-by: Dhinakaran Pandiyan 

Thanks for spotting these and creating the patch, applied to drm-misc.
-Daniel

> ---
>  drivers/gpu/drm/drm_encoder.c | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 998a674..5c06771 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -31,20 +31,21 @@
>   *
>   * Encoders represent the connecting element between the CRTC (as the overall
>   * pixel pipeline, represented by struct _crtc) and the connectors (as 
> the
> - * generic sink entity, represented by struct _connector). Encoders are
> - * objects exposed to userspace, originally to allow userspace to infer 
> cloning
> - * and connector/CRTC restrictions. Unfortunately almost all drivers get this
> - * wrong, making the uabi pretty much useless. On top of that the exposed
> - * restrictions are too simple for todays hardware, and the recommend way to
> - * infer restrictions is by using the DRM_MODE_ATOMIC_TEST_ONLY flag for the
> - * atomic IOCTL.
> + * generic sink entity, represented by struct _connector). An encoder 
> takes
> + * pixel data from a CRTC and converts it to a format suitable for any 
> attached
> + * connector. Encoders are objects exposed to userspace, originally to allow
> + * userspace to infer cloning and connector/CRTC restrictions. Unfortunately
> + * almost all drivers get this wrong, making the uabi pretty much useless. On
> + * top of that the exposed restrictions are too simple for today's hardware, 
> and
> + * the recommended way to infer restrictions is by using the
> + * DRM_MODE_ATOMIC_TEST_ONLY flag for the atomic IOCTL.
>   *
>   * Otherwise encoders aren't used in the uapi at all (any modeset request 
> from
>   * userspace directly connects a connector with a CRTC), drivers are 
> therefore
>   * free to use them however they wish. Modeset helper libraries make strong 
> use
>   * of encoders to facilitate code sharing. But for more complex settings it 
> is
>   * usually better to move shared code into a separate _bridge. Compared 
> to
> - * encoders bridges also have the benefit of not being purely an internal
> + * encoders, bridges also have the benefit of being purely an internal
>   * abstraction since they are not exposed to userspace at all.
>   *
>   * Encoders are initialized with drm_encoder_init() and cleaned up using
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH] drm/udl: Fix for the X server screen update v3

2016-09-21 Thread David Herrmann
Hi

On Wed, Sep 21, 2016 at 1:19 PM, poma  wrote:
> On 21.09.2016 09:34, David Herrmann wrote:
>> Hi
>>
>> On Wed, Sep 21, 2016 at 6:47 AM, poma  wrote:
>>> Within X server, on top of DisplayLink GPU USB2.0 device,
>>> screen content is not refreshed i.e. updated.
>>>
>>> This fixes commit:
>>>
>>> - e375882406d0cc24030746638592004755ed4ae0
>>>   "drm/udl: Use drm_fb_helper deferred_io support"
>>>
>>> Thanks Noralf and Daniel for the comments.
>>>
>>> Tested-by: poma 
>>
>> Can you provide your Signed-off-by: line?
>>
>> Reviewed-by: David Herrmann 
>>
>> Thanks
>> David
>>
>
> S-o-b should be performed by the actual kernel developer.
> R-b & T-b, I've already written.

The author of a patch must provide the S-o-b (see
Documentation/SubmittingPatches if you want details). So simply reply
with a "S-o-b: foo " line to this mail. And please include it in
all patches you submit (preferably use `git commit --sign-off`).

Thanks
David


[PATCH 4/5] GPU-DRM: Replace a kzalloc() call by kcalloc() in drm_legacy_addbufs_sg()

2016-09-21 Thread Daniel Vetter
On Mon, Sep 19, 2016 at 05:56:49PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Mon, 19 Sep 2016 17:30:31 +0200
> 
> The script "checkpatch.pl" can point information out like the following.
> 
> WARNING: Prefer kcalloc over kzalloc with multiply
> 
> Thus fix the affected source code place.
> 
> Signed-off-by: Markus Elfring 

Merged patches 1-4. I agree with Jani that changing jump labels is a pure
bikeshed, so didn't apply them.

Also I'll repeat that imo checkpatch patches are great to get started, but
it's much better to do more involved work. Both since that tends to be
more interesting, and fixing all the checkpatch issues will rob the next
newbies of some great starting opportunity. Which means from now on I'll
only selectively apply your checkpatch patches.

We have todo list with some ideas at:

https://www.x.org/wiki/DRMJanitors/

Thanks, Daniel
> ---
>  drivers/gpu/drm/drm_bufs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
> index 36dd685..adb1dd7 100644
> --- a/drivers/gpu/drm/drm_bufs.c
> +++ b/drivers/gpu/drm/drm_bufs.c
> @@ -1117,8 +1117,7 @@ static int drm_legacy_addbufs_sg(struct drm_device *dev,
>   return -EINVAL;
>   }
>  
> - entry->buflist = kzalloc(count * sizeof(*entry->buflist),
> - GFP_KERNEL);
> + entry->buflist = kcalloc(count, sizeof(*entry->buflist), GFP_KERNEL);
>   if (!entry->buflist) {
>   mutex_unlock(>struct_mutex);
>   atomic_dec(>buf_alloc);
> -- 
> 2.10.0
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH] drm/udl: Fix for the X server screen update v3

2016-09-21 Thread poma
On 21.09.2016 09:34, David Herrmann wrote:
> Hi
> 
> On Wed, Sep 21, 2016 at 6:47 AM, poma  wrote:
>> Within X server, on top of DisplayLink GPU USB2.0 device,
>> screen content is not refreshed i.e. updated.
>>
>> This fixes commit:
>>
>> - e375882406d0cc24030746638592004755ed4ae0
>>   "drm/udl: Use drm_fb_helper deferred_io support"
>>
>> Thanks Noralf and Daniel for the comments.
>>
>> Tested-by: poma 
> 
> Can you provide your Signed-off-by: line?
> 
> Reviewed-by: David Herrmann 
> 
> Thanks
> David
> 

S-o-b should be performed by the actual kernel developer.
R-b & T-b, I've already written.




[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

2016-09-21 Thread Christian König
Am 21.09.2016 um 13:04 schrieb Daniel Vetter:
> On Wed, Sep 21, 2016 at 12:30 PM, Christian König
>  wrote:
>> Am 21.09.2016 um 11:56 schrieb Michel Dänzer:
>>>
>>> Looks like there are different interpretations of the semantics of
>>> exclusive vs. shared fences. Where are these semantics documented?
>>
>> Yeah, I think as well that this is the primary question here.
>>
>> IIRC the fences were explicitly called exclusive/shared instead of
>> writing/reading on purpose.
>>
>> I absolutely don't mind switching to them to writing/reading semantics, but
>> amdgpu really needs multiple writers at the same time.
>>
>> So in this case the writing side of a reservation object needs to be a
>> collection of fences as well.
> You can't have multiple writers with implicit syncing. That confusion
> is exactly why we called them shared/exclusive. Multiple writers
> generally means that you do some form of fencing in userspace
> (unsync'ed gl buffer access is the common one). What you do for
> private buffers doesn't matter, but when you render into a
> shared/winsys buffer you really need to set the exclusive fence (and
> there can only ever be one). So probably needs some userspace
> adjustments to make sure you don't accidentally set an exclusive write
> hazard when you don't really want that implicit sync.

Nope, that isn't true.

We use multiple writers without implicit syncing between processes in 
the amdgpu stack perfectly fine.

See amdgpu_sync.c for the implementation. What we do there is taking a 
look at all the fences associated with a reservation object and only 
sync to those who are from another process.

Then we use implicit syncing for command submissions in the form of 
"dependencies". E.g. for each CS we report back an identifier of that 
submission to user space and on the next submission you can give this 
identifier as dependency which needs to be satisfied before the command 
submission can start running.

This was done to allow multiple engines (3D, DMA, Compute) to compose a 
buffer while still allow compatibility with protocols like DRI2/DRI3.

Regards,
Christian.

> -Daniel




[Bug 141741] drm:radeon_get_bios [radeon]] *ERROR* Unable to locate a BIOS ROM

2016-09-21 Thread bugzilla-dae...@bugzilla.kernel.org
https://bugzilla.kernel.org/show_bug.cgi?id=141741

Alex Deucher  changed:

   What|Removed |Added

 CC||alexdeucher at gmail.com

--- Comment #15 from Alex Deucher  ---
Can you bisect?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.


[PATCH] drm/tilcdc: mark tilcdc_atomic_check() static

2016-09-21 Thread Baoyou Xie
We get 1 warning when building kernel with W=1:
drivers/gpu/drm/tilcdc/tilcdc_drv.c:64:5: warning: no previous prototype for 
'tilcdc_atomic_check' [-Wmissing-prototypes]

In fact, this function is only used in the file in which it is
declared and don't need a declaration, but can be made static.
So this patch marks it 'static'.

Signed-off-by: Baoyou Xie 
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 4405e4b..1d26bc9 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -61,7 +61,7 @@ static void tilcdc_fb_output_poll_changed(struct drm_device 
*dev)
drm_fbdev_cma_hotplug_event(priv->fbdev);
 }

-int tilcdc_atomic_check(struct drm_device *dev,
+static int tilcdc_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
 {
int ret;
-- 
2.7.4



[PATCH] drm/tilcdc: add missing header dependencies

2016-09-21 Thread Baoyou Xie
We get 4 warnings when building kernel with W=1:
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c:397:12: warning: no previous prototype 
for 'tilcdc_tfp410_init' [-Wmissing-prototypes]
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c:402:13: warning: no previous prototype 
for 'tilcdc_tfp410_fini' [-Wmissing-prototypes]
drivers/gpu/drm/tilcdc/tilcdc_panel.c:448:12: warning: no previous prototype 
for 'tilcdc_panel_init' [-Wmissing-prototypes]
drivers/gpu/drm/tilcdc/tilcdc_panel.c:453:13: warning: no previous prototype 
for 'tilcdc_panel_fini' [-Wmissing-prototypes]

In fact, these functions are declared in
drivers/gpu/drm/tilcdc/tilcdc_tfp410.h,
drivers/gpu/drm/tilcdc/tilcdc_panel.h,
so this patch adds missing header dependencies.

Signed-off-by: Baoyou Xie 
---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c  | 1 +
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c 
b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 4ac1d25..186cf23 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -25,6 +25,7 @@
 #include 

 #include "tilcdc_drv.h"
+#include "tilcdc_panel.h"

 struct panel_module {
struct tilcdc_module base;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c 
b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index 741c7b5..8692671 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -23,6 +23,7 @@
 #include 

 #include "tilcdc_drv.h"
+#include "tilcdc_tfp410.h"

 struct tfp410_module {
struct tilcdc_module base;
-- 
2.7.4



[PATCH v9 17/19] drm/virtio: kconfig: Fix recursive dependency issue.

2016-09-21 Thread Emil Velikov
On 20 September 2016 at 09:32, Peter Griffin  
wrote:
> Hi Emil,
>
> On Tue, 20 Sep 2016, Emil Velikov wrote:
>
>> On 5 September 2016 at 14:16, Peter Griffin  
>> wrote:
>> > ST_SLIM_REMOTEPROC must select REMOTEPROC, which exposes the following
>> > recursive dependency.
>
>
>> >
>> From a humble skim through remoteproc, drm and a few other subsystems
>> I think the above is wrong. All the drivers (outside of remoteproc),
>> that I've seen, depend on the core component, they don't select it.
>
> I will let Bjorn comment on the remoteproc subsystem Kconfig design, and
> why it is like it is.
>
> For this particular SLIM_RPROC I have added it to Kconfig in keeping with all
> the other drivers in the remoteproc subsystem which has exposed this recursive
> dependency issue.
>
> For this particular kconfig symbol a quick grep reveals more drivers in
> the kernel using 'select', than 'depend on'
>
> git grep "select VIRTIO" | wc -l
> 14
>
> git grep "depends on VIRTIO" | wc -l
> 10
>
Might be worth taking a closer look into these at some point.

>
>> Furthermore most places explicitly hide the drivers from the menu if
>> the core component isn't enabled.
>
> Remoteproc subsystem takes a different approach, the core code is only enabled
> if a driver which relies on it is enabled. This IMHO makes sense, as
> remoteproc is not widely used (only a few particular ARM SoC's).
>
> It is true that for subsystems which rely on the core component being
> explicitly enabled, they often tend to hide drivers which depend on it
> from the menu unless it is. This also makes sense.
>
>>
>> Is there something that requires such a different/unusual behaviour in
>> remoteproc ?
>>
>
> I'm not sure it is that unusual...looking at config USB, it selects 
> USB_COMMON in
> mfd subsystem, client drivers select MFD_CORE.
>
On the USB case I'm not sure what the reasoning behind the USB vs
USB_COMMON split. In seems that one could just fold them, but that's
another topic. On the MFD side... it follows the select {,mis,ab}use.
With one (the only one?) MFD driver not using/selecting MFD_CORE doing
it's own version of mfd_add_devices... which could be reworked,
possibly.

> I've added Arnd to this thread, as I've seen lots of Kconfig patches from him
> recently, maybe he has some thoughts on whether this is the correct fix or 
> not?
>
Ack. Fwiw, I believe that the reasoning put by Jani is perfeclty
reasonable, but it'll be great to hear others as well.

Thanks
Emil


[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

2016-09-21 Thread Daniel Vetter
On Wed, Sep 21, 2016 at 12:30 PM, Christian König
 wrote:
> Am 21.09.2016 um 11:56 schrieb Michel Dänzer:
>>
>>
>> Looks like there are different interpretations of the semantics of
>> exclusive vs. shared fences. Where are these semantics documented?
>
>
> Yeah, I think as well that this is the primary question here.
>
> IIRC the fences were explicitly called exclusive/shared instead of
> writing/reading on purpose.
>
> I absolutely don't mind switching to them to writing/reading semantics, but
> amdgpu really needs multiple writers at the same time.
>
> So in this case the writing side of a reservation object needs to be a
> collection of fences as well.

You can't have multiple writers with implicit syncing. That confusion
is exactly why we called them shared/exclusive. Multiple writers
generally means that you do some form of fencing in userspace
(unsync'ed gl buffer access is the common one). What you do for
private buffers doesn't matter, but when you render into a
shared/winsys buffer you really need to set the exclusive fence (and
there can only ever be one). So probably needs some userspace
adjustments to make sure you don't accidentally set an exclusive write
hazard when you don't really want that implicit sync.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[PATCH v12 5/7] drm/i915/skl: Ensure pipes with changed wms get added to the state

2016-09-21 Thread Mike Lothian
I've raised https://bugs.freedesktop.org/show_bug.cgi?id=97888 I'll
attach the info you requested once I get back to my machine

On 21 September 2016 at 07:56, Maarten Lankhorst
 wrote:
> Hey,
>
> Op 20-09-16 om 20:45 schreef Mike Lothian:
>> Hi
>>
>> I've bisected back to this commit in the drm-intel-nightly branch
>>
>> 05a76d3d6ad1ee9f9814f88949cc9305fc165460 is the first bad commit
>> commit 05a76d3d6ad1ee9f9814f88949cc9305fc165460
>> Author: Lyude 
>> Date:   Wed Aug 17 15:55:57 2016 -0400
>>
>>drm/i915/skl: Ensure pipes with changed wms get added to the state
>>
>>If we're enabling a pipe, we'll need to modify the watermarks on all
>>active planes. Since those planes won't be added to the state on
>>their own, we need to add them ourselves.
>>
>>Signed-off-by: Lyude 
>>Reviewed-by: Matt Roper 
>>Cc: stable at vger.kernel.org
>>Cc: Ville Syrjälä 
>>Cc: Daniel Vetter 
>>Cc: Radhakrishna Sripada 
>>Cc: Hans de Goede 
>>Signed-off-by: Maarten Lankhorst 
>>Link: 
>> http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-6-git-send-email-cpaul
>>  at redhat.com
>>
>> The symptoms I'm seeing look like tearing at the top of the screen and
>> it's especially noticeable in Chrome - reverting this commit makes the
>> issue go away
>>
>> Let me know if you'd like me to raise a bug
> Please do so, it's nice to refer to when making a fix for it.
>
> Could you attach the contents of /sys/kernel/debug/dri/0/i915_ddb_info for 
> working and not-working in it?
>
> ~Maarten


[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

2016-09-21 Thread Christian König
Am 21.09.2016 um 11:56 schrieb Michel Dänzer:
>
> Looks like there are different interpretations of the semantics of
> exclusive vs. shared fences. Where are these semantics documented?

Yeah, I think as well that this is the primary question here.

IIRC the fences were explicitly called exclusive/shared instead of 
writing/reading on purpose.

I absolutely don't mind switching to them to writing/reading semantics, 
but amdgpu really needs multiple writers at the same time.

So in this case the writing side of a reservation object needs to be a 
collection of fences as well.

> FWIW, we seem to have the same issue with radeon vs. amdgpu: radeon only
> seems to wait for exclusive fences, so e.g. running Xorg on amdgpu and
> using PRIME slave scanout on radeon leaves artifacts.

Yeah, I know. See radeon_display.c radeon_flip_work_func().

We pretty much need the same patch here I've done for amdgpu as well.

Regards,
Christian.


[PATCH] dma-buf/sync_file: free fences array in num_fences is 1

2016-09-21 Thread Chris Wilson
On Wed, Sep 21, 2016 at 10:20:19AM +0300, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> When merging sync_files there is a case when we can end up with only one
> fence in the merged sync_file: when all fences belong to the same
> timeline.
> 
> So for this case a fence_array is not created instead we just assigned the
> fence to sync_file->fence. Then we do not use the fences array anymore nor
> does free it.
> 
> This patch frees the array.
> 
> Signed-off-by: Gustavo Padovan 
> Reported-by:  Chris Wilson 
> ---
>  drivers/dma-buf/sync_file.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 706eea9..9ed4f9f 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -150,6 +150,7 @@ static int sync_file_set_fence(struct sync_file 
> *sync_file,
>*/
>   if (num_fences == 1) {
>   sync_file->fence = fences[0];
> + kfree(fences);

Ok, that makes sense wrt the code. I don't see any particular advantage
in a warning comment, so
Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


[PATCH] dma-buf/sync-file: Avoid enable fence signaling if poll(.timeout=0)

2016-09-21 Thread Chris Wilson
On Wed, Sep 21, 2016 at 10:26:25AM +0300, Gustavo Padovan wrote:
> Hi Rafael,
> 
> 2016-09-14 Rafael Antognolli :
> 
> > Hi Chris and Gustavo,
> > 
> > On Mon, Aug 29, 2016 at 07:16:13PM +0100, Chris Wilson wrote:
> > > If we being polled with a timeout of zero, a nonblocking busy query,
> > > we don't need to install any fence callbacks as we will not be waiting.
> > > As we only install the callback once, the overhead comes from the atomic
> > > bit test that also causes serialisation between threads.
> > > 
> > > Signed-off-by: Chris Wilson 
> > > Cc: Sumit Semwal 
> > > Cc: Gustavo Padovan 
> > > Cc: linux-media at vger.kernel.org
> > > Cc: dri-devel at lists.freedesktop.org
> > > Cc: linaro-mm-sig at lists.linaro.org
> > > ---
> > >  drivers/dma-buf/sync_file.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> > > index 486d29c1a830..abb5fdab75fd 100644
> > > --- a/drivers/dma-buf/sync_file.c
> > > +++ b/drivers/dma-buf/sync_file.c
> > > @@ -306,7 +306,8 @@ static unsigned int sync_file_poll(struct file *file, 
> > > poll_table *wait)
> > >  
> > >   poll_wait(file, _file->wq, wait);
> > >  
> > > - if (!test_and_set_bit(POLL_ENABLED, _file->fence->flags)) {
> > > + if (!poll_does_not_wait(wait) &&
> > > + !test_and_set_bit(POLL_ENABLED, _file->fence->flags)) {
> > >   if (fence_add_callback(sync_file->fence, _file->cb,
> > >  fence_check_cb_func) < 0)
> > >   wake_up_all(_file->wq);
> > 
> > This commit is causing an error on one of the tests that Robert Foss
> > submitted for i-g-t. The one that does random merge of fences from
> > different timelines. A simple version of the test that still triggers
> > this is:
> > 
> > static void test_sync_simple_merge(void)
> > {
> > int fence1, fence2, fence_merge, timeline1, timeline2;
> > int ret;
> > 
> > timeline1 = sw_sync_timeline_create();
> > timeline2 = sw_sync_timeline_create();
> > fence1 = sw_sync_fence_create(timeline1, 1);
> > fence2 = sw_sync_fence_create(timeline2, 2);
> > fence_merge = sw_sync_merge(fence1, fence2);
> > sw_sync_timeline_inc(timeline1, 5);
> > sw_sync_timeline_inc(timeline2, 5);
> > 
> > ret = sw_sync_wait(fence_merge, 0);
> > igt_assert_f(ret > 0, "Failure triggering fence\n");
> > 
> > sw_sync_fence_destroy(fence_merge);
> > sw_sync_fence_destroy(fence1);
> > sw_sync_fence_destroy(fence2);
> > sw_sync_timeline_destroy(timeline1);
> > sw_sync_timeline_destroy(timeline2);
> > }
> > 
> > It looks like you cannot trust fence_is_signaled() without a
> > fence_add_callback(). I think the fence_array->num_pending won't get
> > updated. Although I couldn't figure out why it only happens if you merge
> > fences from different timelines.
> 
> Yes, num_pending is only updated when signaling is enabled. It only
> happens with different timelines because when you merge fences that are
> on the same timeline your final sync_file has only one fence and thus 
> a fence_array is not created.
> 
> If we want to keep the poll_does_not_wait optimization we need a way
> to count the pending fences during fence_is_signaled(). I'd propose
> something like this:
> 
> 
> Author: Gustavo Padovan 
> Date:   Tue Sep 20 16:43:06 2016 +0200
> 
> dma-buf/fence-array: get signaled state when signaling is disabled
> 
> If the fences in the fence_array signal on the fence_array does not have
> signalling enabled num_pending will not be updated accordingly.
> 
> So when signaling is disabled check the signal of every fence with
> fence_is_signaled() and then compare with num_pending to learn if the
> fence_array was signalled or not.
> 
> If we want to keep the poll_does_not_wait optimization I think we need
> something like this. It keeps the same behaviour if signalling is enabled
> but tries to calculated the state otherwise.
> 
> Signed-off-by: Gustavo Padovan 

We need this regardless, so yay for uncovering a bug!
> 
> diff --git a/drivers/dma-buf/fence-array.c b/drivers/dma-buf/fence-array.c
> index f1989fc..34c9209 100644
> --- a/drivers/dma-buf/fence-array.c
> +++ b/drivers/dma-buf/fence-array.c
> @@ -75,8 +75,18 @@ static bool fence_array_enable_signaling(struct fence 
> *fence)
>  static bool fence_array_signaled(struct fence *fence)
>  {
> struct fence_array *array = to_fence_array(fence);
> +   int i, num_pending;
>  
> -   return atomic_read(>num_pending) <= 0;
> +   num_pending = atomic_read(>num_pending);
> +
> +   if (!test_bit(FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) {

Oh, very sneaky. I thought this was FENCE_FLAG_SIGNALED_BIT!

Throw in a comment like:

/* Before signaling is enabled, num_pending is static (set during array
 * construction as a count of *all* fences. To ensure 

[PATCH 05/10] drm/doc: Polish for drm_plane.[hc]

2016-09-21 Thread Archit Taneja


On 09/19/2016 06:43 PM, Daniel Vetter wrote:
> On Fri, Sep 02, 2016 at 03:00:38PM +0530, Archit Taneja wrote:
>>
>>
>> On 8/31/2016 9:39 PM, Daniel Vetter wrote:
>>> Big thing is untangling and carefully documenting the different uapi
>>> types of planes. I also sprinkled a few more cross references around
>>> to make this easier to discover.
>>>
>>> As usual, remove the kerneldoc for internal functions which are not
>>> exported. Aside: We should probably go OCD on all the ioctl handlers
>>> and consistenly give them an _ioctl postfix.
>>>
>>> Signed-off-by: Daniel Vetter 
>>> ---
>>>   Documentation/gpu/drm-kms.rst |  47 +--
>>>   drivers/gpu/drm/drm_crtc.c|   6 +-
>>>   drivers/gpu/drm/drm_plane.c   | 132 
>>> --
>>>   include/drm/drm_plane.h   |  57 +-
>>>   4 files changed, 86 insertions(+), 156 deletions(-)
>>>
>> 
>>
>>> +/**
>>> + * enum drm_plane_type - uapi plane type enumeration
>>> + *
>>> + * For historical reasons not all planes are made the same. This 
>>> enumeration is
>>> + * used to tell the different types of planes apart to implement the 
>>> different
>>> + * uapi semantics for them. For userspace which is universal plane aware 
>>> and
>>> + * which is using that atomic IOCTL there's no difference between these 
>>> planes
>>> + * (beyong what the driver and hardware can support of course).
>>> + *
>>> + * For compatibility with legacy userspace, only overlay planes are made
>>> + * available to userspace by default. Userspace clients may set the
>>> + * 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().
>>> + */
>>>   enum drm_plane_type {
>>> -   DRM_PLANE_TYPE_OVERLAY,
>>
>> Any reason why you moved this down? I guess there is no harm, but people
>> might be printing plane type while debugging, and they'd assume
>> DRM_PLANE_TYPE_OVERLAY=0
>
> I think starting out with 0 for the primary plane makes a lot more sense,
> and since it's an internal thing we can change it however we want. I also
> think from a documentation pov it reads better if the 2 special planes
> (primary and cursor) are first.
>
> But I'm happy to shuffle it back if you feel strongly the other way round.

No, it's fine. The series looks good too.

Thanks,
Archit

> -Daniel
>
>>
>> Thanks,
>> Archit
>>
>>> +   /**
>>> +* @DRM_PLANE_TYPE_PRIMARY:
>>> +*
>>> +* Primary planes represent a "main" plane for a CRTC.  Primary planes
>>> +* are the planes operated upon by CRTC modesetting and flipping
>>> +* operations described in the page_flip and set_config hooks in struct
>>> +* _crtc_funcs.
>>> +*/
>>> DRM_PLANE_TYPE_PRIMARY,
>>> +
>>> +   /**
>>> +* @DRM_PLANE_TYPE_CURSOR:
>>> +*
>>> +* Cursor planes represent a "cursor" plane for a CRTC.  Cursor planes
>>> +* are the planes operated upon by the DRM_IOCTL_MODE_CURSOR and
>>> +* 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,
>>>   };
>>>
>>>
>>> @@ -458,11 +496,26 @@ static inline struct drm_plane *drm_plane_find(struct 
>>> drm_device *dev,
>>> list_for_each_entry((plane), &(dev)->mode_config.plane_list, head) \
>>> for_each_if ((plane_mask) & (1 << drm_plane_index(plane)))
>>>
>>> -/* Plane list iterator for legacy (overlay only) planes. */
>>> +/**
>>> + * drm_for_each_legacy_plane - iterate over all planes for legacy userspace
>>> + * @plane: the loop cursor
>>> + * @dev: the DRM device
>>> + *
>>> + * Iterate over all legacy planes of @dev, excluding primary and cursor 
>>> planes.
>>> + * This is useful for implementing userspace apis when userspace is not
>>> + * universal plane aware. See also enum _plane_type.
>>> + */
>>>   #define drm_for_each_legacy_plane(plane, dev) \
>>> list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
>>> for_each_if (plane->type == DRM_PLANE_TYPE_OVERLAY)
>>>
>>> +/**
>>> + * drm_for_each_plane - iterate over all planes
>>> + * @plane: the loop cursor
>>> + * @dev: the DRM device
>>> + *
>>> + * Iterate over all planes of @dev, include primary and cursor planes.
>>> + */
>>>   #define drm_for_each_plane(plane, dev) \
>>> list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
>>>
>>>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project


[PULL] qemu drm driver fixes

2016-09-21 Thread Gerd Hoffmann
  Hi Dave,

Here is a pull request for the next merge window with a bunch of fixes
for the qemu drm drivers (bochs, qxl, virtio-gpu).

cheers,
  Gerd

PS: In case anyone has pending patches which are not included here
(probably slipped through due to summer vacation season):  Please
resend and feel free to Cc: me.

The following changes since commit
3be7988674ab33565700a37b210f502563d932e6:

  Linux 4.8-rc7 (2016-09-18 17:27:41 -0700)

are available in the git repository at:

  git://git.kraxel.org/linux tags/drm-qemu-20160921

for you to fetch changes up to 30b9c96cf7b44d53b9165649c8be34ac234be324:

  drm/virtio: add real fence context and seqno (2016-09-20 14:25:43
+0200)


bugfixes for qemu (bochs, qxl and virtio-gpu) drm drivers


Gerd Hoffmann (1):
  bochs: ignore device if there isn't enougth memory

Gustavo Padovan (2):
  drm/virtio: drop virtio_gpu_execbuffer_ioctl() wrapping
  drm/virtio: add real fence context and seqno

Heinrich Schuchardt (1):
  virtio-gpu: avoid possible NULL pointer dereference

Ray Strode (1):
  drm/qxl: reapply cursor after SetCrtc calls

 drivers/gpu/drm/bochs/bochs_drv.c  |  7 
 drivers/gpu/drm/qxl/qxl_display.c  | 56 +-
 drivers/gpu/drm/qxl/qxl_drv.h  |  1 +
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  1 +
 drivers/gpu/drm/virtio/virtgpu_fence.c |  2 +-
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 24 ---
 drivers/gpu/drm/virtio/virtgpu_kms.c   |  1 +
 drivers/gpu/drm/virtio/virtgpu_plane.c |  6 ++-
 8 files changed, 78 insertions(+), 20 deletions(-)



[PATCH 3/5] GPU-DRM-nouveau: Delete unnecessary braces

2016-09-21 Thread Dan Carpenter
The original style was correct, the new style is wrong. Multi-line
indents get curly braces for readability.

regards,
dan carpenter



[Bug 97887] llvm segfault in janusvr -render vive

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97887

Bug ID: 97887
   Summary: llvm segfault in janusvr -render vive
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: haagch at frickel.club
QA Contact: dri-devel at lists.freedesktop.org

Created attachment 126700
  --> https://bugs.freedesktop.org/attachment.cgi?id=126700=edit
stderr output with R600_DEBUG=vs,tcs,tes,gs,ps,cs

01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
Ellesmere [Radeon RX 480] (rev c7)
mesa git, linux drm-next-4.9-wip, llvm 4.0.0svn_r282018

So I'm trying to start janusvr -render vive with the steamvr-osvr steamvr
plugin. Because osvr-rendermanager still doesn't do core profile, I start it
with

MESA_GL_VERSION_OVERRIDE=3.3COMPAT MESA_GLSL_VERSION_OVERRIDE=330 ./janusvr
-render vive

But llvm segfaults:

Thread 12 "si_shader:1" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffc3568700 (LWP 28734)]
0x7fffc59ee0c6 in
llvm::SSAUpdaterImpl::BuildBlockList(llvm::BasicBlock*,
llvm::SmallVectorImpl<llvm::SSAUpdaterImpl::BBInfo*>*) ()
from /usr/lib/libLLVM-4.0svn.so
#0  0x7fffc59ee0c6 in
llvm::SSAUpdaterImpl::BuildBlockList(llvm::BasicBlock*,
llvm::SmallVectorImpl<llvm::SSAUpdaterImpl::BBInfo*>*) () at
/usr/lib/libLLVM-4.0svn.so
#1  0x7fffc59f2acf in
llvm::SSAUpdater::GetValueAtEndOfBlockInternal(llvm::BasicBlock*) () at
/usr/lib/libLLVM-4.0svn.so
#2  0x7fffc59f942a in
llvm::SSAUpdater::RewriteUseAfterInsertions(llvm::Use&) () at
/usr/lib/libLLVM-4.0svn.so
#3  0x7fffc5cb7551 in (anonymous
namespace)::StructurizeCFG::runOnRegion(llvm::Region*, llvm::RGPassManager&) ()
at /usr/lib/libLLVM-4.0svn.so
#4  0x7fffc5f3571c in llvm::RGPassManager::runOnFunction(llvm::Function&)
() at /usr/lib/libLLVM-4.0svn.so
#5  0x7fffc53c03a2 in llvm::FPPassManager::runOnFunction(llvm::Function&)
() at /usr/lib/libLLVM-4.0svn.so
#6  0x7fffc53c0443 in llvm::FPPassManager::runOnModule(llvm::Module&) () at
/usr/lib/libLLVM-4.0svn.so
#7  0x7fffc53c0a54 in llvm::legacy::PassManagerImpl::run(llvm::Module&) ()
at /usr/lib/libLLVM-4.0svn.so
#8  0x7fffc61a5d57 in LLVMTargetMachineEmit(LLVMOpaqueTargetMachine*,
LLVMOpaqueModule*, llvm::raw_pwrite_stream&, LLVMCodeGenFileType, char**) () at
/usr/lib/libLLVM-4.0svn.so
#9  0x7fffc61a6139 in LLVMTargetMachineEmitToMemoryBuffer () at
/usr/lib/libLLVM-4.0svn.so
#10 0x7fffc889e7c3 in  () at /usr/lib/xorg/modules/dri/radeonsi_dri.so
#11 0x7fffc8814605 in  () at /usr/lib/xorg/modules/dri/radeonsi_dri.so
#12 0x7fffc88161d2 in  () at /usr/lib/xorg/modules/dri/radeonsi_dri.so
#13 0x7fffc8822f73 in  () at /usr/lib/xorg/modules/dri/radeonsi_dri.so
#14 0x7fffc868ec34 in  () at /usr/lib/xorg/modules/dri/radeonsi_dri.so
#15 0x7fffc868e9f6 in  () at /usr/lib/xorg/modules/dri/radeonsi_dri.so
#16 0x72636454 in start_thread () at /usr/lib/libpthread.so.0
#17 0x71ad67df in clone () at /usr/lib/libc.so.6

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/7240cc8e/attachment.html>


[Bug 97857] card detects non-existent monitor on display port

2016-09-21 Thread bugzilla-dae...@freedesktop.org
https://bugs.freedesktop.org/show_bug.cgi?id=97857

--- Comment #7 from Daniel  ---
> can you narrow it down to at least the kernel version which introduced it

But I did, in my description: "Since kernel 4.5.0-2 my Xorg.0.log shows that
something is also connected to display port...".

Up to 4.5.0-1 everything was fine.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160921/ff2c0fcb/attachment.html>


[PATCH 0/4] drm/fsl-dcu: add overlay and cursor plane support

2016-09-21 Thread Stefan Agner
On 2016-09-07 01:43, Meng Yi wrote:
> Hi Stefan,
> 
> I had tested the patches on LS1021A-TWR board using drmlib.
> 
> Like set three overlays:
> root at ls1021atwr:~# ./modetest -P 39:900x100+10+10 at RG24 -P
> 39:200x200+300+0 at RG24 -P 39:200x200+400+300 at RG24
> 
> How did you test the overlays and cursor layer, I mean I see you using
> x-window like thing in the  video.

I did use X with the modesetting driver. You just need to choose
modesetting in your xorg.conf, and it should make use of the cursor
layer automatically.

>> This patchset adds overlay and cursor plane support. It also fixes some 
>> issues
>> uncovered during implementation of this.
>>
>> However, the plane updates currently causes the display to flicker for 
>> unknown
>> reasons. As far as I can tell, the CRTC atomic_flush should trigger the 
>> update
>> correctly via READREG, which according to
>> documentation:
>> The READREG bit causes a single transfer to begin at the next frame blanking
>> period. This bit is cleared when the transfer is complete.
>>
>> I made a video how that looks:
>> https://cloud.agner.ch/index.php/s/Yfqa2u7UBEWUT8N

It would be interesting whether you see that on LS1021a too.

--
Stefan

>>
>> Any ideas?
>>
>> Stefan Agner (4):
>>   drm/fsl-dcu: support overlay and cursor planes
>>   drm/fsl-dcu: respect pos/size register sizes
>>   drm/fsl-dcu: update all registers on flush
>>   drm/fsl-dcu: do not update when modifying irq registers
>>
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 50
>> -
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   |  4 ---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  8 ++---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 42 +++-
>> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  3 +-
>>  5 files changed, 67 insertions(+), 40 deletions(-)
>>
>> --
>> 2.9.3


[PATCH v3] drm/fsl-dcu: Implement gamma_lut atomic crtc properties

2016-09-21 Thread Stefan Agner
On 2016-09-13 01:49, Meng Yi wrote:
>> > diff --git a/drivers/gpu/drm/fsl-dcu/Kconfig
>> > b/drivers/gpu/drm/fsl-dcu/Kconfig index 14a72c4..f9c76b1 100644
>> > --- a/drivers/gpu/drm/fsl-dcu/Kconfig
>> > +++ b/drivers/gpu/drm/fsl-dcu/Kconfig
>> > @@ -11,3 +11,9 @@ config DRM_FSL_DCU
>> >help
>> >  Choose this option if you have an Freescale DCU chipset.
>> >  If M is selected the module will be called fsl-dcu-drm.
>> > +
>> > +config DRM_FSL_DCU_GAMMA
>> > +  bool "Gamma Correction Support for NXP/Freescale DCU"
>> > +  depends on DRM_FSL_DCU
>> > +  help
>> > +Enable support for gamma correction.
>>
>> What is the reason for making this a configuration option? Are there
>> implementation without support for the Gamma tables?
>>
> When gamma correction is enabled, the color won't display normally since
> The gamma tables are not filled with correct data. So I give a choice
> to not using
> The gamma correction when you don't want to use it.
> 
> Should I remove this configuration?

Yeah making this a compile time configuration seems wrong to me.

I guess we should fill the table with a reasonable default then. The
omapdrm driver seems to do something similar.

Best regards,
Stefan


[PATCH 7/7] drm: Remove dirty property from docs

2016-09-21 Thread Daniel Vetter
We removed it in

commit 6ab10b76ff6252bd9be0849c40f5865e39a29961
Author: Daniel Vetter 
Date:   Fri Aug 12 22:48:45 2016 +0200

drm/kms: Nuke dirty_info property

Reviewed-by: Sean Paul 
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/kms-properties.csv | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/gpu/kms-properties.csv 
b/Documentation/gpu/kms-properties.csv
index 1a5729c4af65..981873a05d14 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -23,7 +23,6 @@ Owner Module/Drivers,Group,Property Name,Type,Property 
Values,Object attached,De
 ,Virtual GPU,“suggested X”,RANGE,"Min=0, 
Max=0x",Connector,property to suggest an X offset for a connector
 ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to 
suggest an Y offset for a connector
 ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" 
}",Connector,TDB
-,,“dirty”,ENUM | IMMUTABLE,"{ ""Off"", ""On"", ""Annotate"" 
}",Connector,TBD
 i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited 
16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is 
set, the hardware will be programmed with the result of the multiplication of 
CTM by the limited range matrix to ensure the pixels normaly in the range 
0..1.0 are remapped to the range 16/255..235/255."
 ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
 ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } 
etc.",Connector,TBD
-- 
2.7.4



[PATCH 6/7] drm/doc: Document color space handling

2016-09-21 Thread Daniel Vetter
Again move it from the unmaintainable csv into DOC free-form overview
sections.

v2: Types Lionel spotted.

Cc: Lionel Landwerlin 
Reviewed-by: Sean Paul 
Reviewed-by: Lionel Landwerlin 
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/drm-kms.rst| 12 +
 Documentation/gpu/kms-properties.csv |  5 
 drivers/gpu/drm/drm_color_mgmt.c | 48 
 include/drm/drm_color_mgmt.h | 11 ++---
 4 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 6be8d3359620..53b872c105d2 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -296,6 +296,18 @@ Plane Composition Properties
 .. kernel-doc:: drivers/gpu/drm/drm_blend.c
:export:

+Color Management Properties
+---
+
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :doc: overview
+
+.. kernel-doc:: include/drm/drm_color_mgmt.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :export:
+
 Existing KMS Properties
 ---

diff --git a/Documentation/gpu/kms-properties.csv 
b/Documentation/gpu/kms-properties.csv
index 1aa2493d1ef9..1a5729c4af65 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -24,11 +24,6 @@ Owner Module/Drivers,Group,Property Name,Type,Property 
Values,Object attached,De
 ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to 
suggest an Y offset for a connector
 ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" 
}",Connector,TDB
 ,,“dirty”,ENUM | IMMUTABLE,"{ ""Off"", ""On"", ""Annotate"" 
}",Connector,TBD
-,,“DEGAMMA_LUT”,BLOB,0,CRTC,DRM property to set the degamma lookup table 
(LUT) mapping pixel data from the framebuffer before it is given to the 
transformation matrix. The data is an interpreted as an array of struct 
drm_color_lut elements. Hardware might choose not to use the full precision of 
the LUT elements nor use all the elements of the LUT (for example the hardware 
might choose to interpolate between LUT[0] and LUT[4]).
-,,“DEGAMMA_LUT_SIZE”,RANGE | IMMUTABLE,"Min=0, Max=UINT_MAX",CRTC,DRM 
property to gives the size of the lookup table to be set on the DEGAMMA_LUT 
property (the size depends on the underlying hardware).
-,,“CTM”,BLOB,0,CRTC,DRM property to set the current transformation matrix 
(CTM) apply to pixel data after the lookup through the degamma LUT and before 
the lookup through the gamma LUT. The data is an interpreted as a struct 
drm_color_ctm.
-,,“GAMMA_LUT”,BLOB,0,CRTC,DRM property to set the gamma lookup table (LUT) 
mapping pixel data after to the transformation matrix to data sent to the 
connector. The data is an interpreted as an array of struct drm_color_lut 
elements. Hardware might choose not to use the full precision of the LUT 
elements nor use all the elements of the LUT (for example the hardware might 
choose to interpolate between LUT[0] and LUT[4]).
-,,“GAMMA_LUT_SIZE”,RANGE | IMMUTABLE,"Min=0, Max=UINT_MAX",CRTC,DRM 
property to gives the size of the lookup table to be set on the GAMMA_LUT 
property (the size depends on the underlying hardware).
 i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited 
16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is 
set, the hardware will be programmed with the result of the multiplication of 
CTM by the limited range matrix to ensure the pixels normaly in the range 
0..1.0 are remapped to the range 16/255..235/255."
 ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
 ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } 
etc.",Connector,TBD
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index aca1b7a6397c..d28ffdd2b929 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -26,6 +26,54 @@

 #include "drm_crtc_internal.h"

+/**
+ * DOC: overview
+ *
+ * Color management or color space adjustments is supported through a set of 5
+ * properties on the _crtc object. They are set up by calling
+ * drm_crtc_enable_color_mgmt().
+ *
+ * "DEGAMMA_LUT”:
+ * Blob property to set the degamma lookup table (LUT) mapping pixel data
+ * from the framebuffer before it is given to the transformation matrix.
+ * The data is interpreted as an array of struct _color_lut elements.
+ * Hardware might choose not to use the full precision of the LUT elements
+ * nor use all the elements of the LUT (for example the hardware might
+ * choose to interpolate between LUT[0] and LUT[4]).
+ *
+ * “DEGAMMA_LUT_SIZE”:
+ * Unsinged range property to give the size of the lookup table to be set
+ * on the DEGAMMA_LUT property (the size depends on the underlying
+ * hardware). If drivers support multiple LUT sizes then they should
+ * publish the largest 

[PATCH 5/7] drm: Extract drm_color_mgmt.[hc]

2016-09-21 Thread Daniel Vetter
For both the new degamm/lut/gamma atomic combo, and the old legacy
gamma tables.

Acked-by: Lionel Landwerlin 
Cc: Lionel Landwerlin 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/Makefile|   2 +-
 drivers/gpu/drm/drm_color_mgmt.c| 248 
 drivers/gpu/drm/drm_crtc.c  | 220 
 drivers/gpu/drm/drm_crtc_internal.h |   4 +
 include/drm/drm_color_mgmt.h|  56 
 include/drm/drm_crtc.h  |  28 +---
 6 files changed, 310 insertions(+), 248 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_color_mgmt.c
 create mode 100644 include/drm/drm_color_mgmt.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 8eeb07a35798..25c720454017 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -15,7 +15,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_modeset_lock.o drm_atomic.o drm_bridge.o \
drm_framebuffer.o drm_connector.o drm_blend.o \
drm_encoder.o drm_mode_object.o drm_property.o \
-   drm_plane.o
+   drm_plane.o drm_color_mgmt.o

 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
new file mode 100644
index ..aca1b7a6397c
--- /dev/null
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -0,0 +1,248 @@
+/*
+ * Copyright (c) 2016 Intel Corporation
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+
+#include "drm_crtc_internal.h"
+
+
+/**
+ * drm_crtc_enable_color_mgmt - enable color management properties
+ * @crtc: DRM CRTC
+ * @degamma_lut_size: the size of the degamma lut (before CSC)
+ * @has_ctm: whether to attach ctm_property for CSC matrix
+ * @gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction
+ * properties on a CRTC. This includes 3 degamma, csc and gamma
+ * properties that userspace can set and 2 size properties to inform
+ * the userspace of the lut sizes. Each of the properties are
+ * optional. The gamma and degamma properties are only attached if
+ * their size is not 0 and ctm_property is only attached if has_ctm is
+ * true.
+ */
+void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+   uint degamma_lut_size,
+   bool has_ctm,
+   uint gamma_lut_size)
+{
+   struct drm_device *dev = crtc->dev;
+   struct drm_mode_config *config = >mode_config;
+
+   if (degamma_lut_size) {
+   drm_object_attach_property(>base,
+  config->degamma_lut_property, 0);
+   drm_object_attach_property(>base,
+  config->degamma_lut_size_property,
+  degamma_lut_size);
+   }
+
+   if (has_ctm)
+   drm_object_attach_property(>base,
+  config->ctm_property, 0);
+
+   if (gamma_lut_size) {
+   drm_object_attach_property(>base,
+  config->gamma_lut_property, 0);
+   drm_object_attach_property(>base,
+  config->gamma_lut_size_property,
+  gamma_lut_size);
+   }
+}
+EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
+
+/**
+ * drm_mode_crtc_set_gamma_size - set the gamma table size
+ * @crtc: CRTC to set the gamma table size for
+ * @gamma_size: size of the gamma table
+ *
+ * Drivers which support gamma tables should set this to the supported gamma
+ * table size when initializing the CRTC. Currently the drm core only supports 

[PATCH 4/7] drm/doc: Polish plane composition property docs

2016-09-21 Thread Daniel Vetter
Try to spec a bit more precisely how they all fit together, now that
at least the code is for all the additional properties is in one
place.

Also remove the entries for the standardized properties from the
table, because that thing is supremely unmaintaineable.

v2: Fix typos Sean spotted.

Cc: Ville Syrjälä 
Cc: Sean Paul 
Cc: Benjamin Gaignard 
Reviewed-by: Sean Paul 
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/drm-kms.rst|   7 +-
 Documentation/gpu/kms-properties.csv |  15 
 drivers/gpu/drm/drm_blend.c  | 146 ++-
 drivers/gpu/drm/drm_plane.c  |   3 +
 4 files changed, 136 insertions(+), 35 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index b1029e292e5c..6be8d3359620 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -287,8 +287,11 @@ Property Types and Blob Property Support
 .. kernel-doc:: drivers/gpu/drm/drm_property.c
:export:

-Blending and Z-Position properties
---
+Plane Composition Properties
+
+
+.. kernel-doc:: drivers/gpu/drm/drm_blend.c
+   :doc: overview

 .. kernel-doc:: drivers/gpu/drm/drm_blend.c
:export:
diff --git a/Documentation/gpu/kms-properties.csv 
b/Documentation/gpu/kms-properties.csv
index 4c5ce3edcfd9..1aa2493d1ef9 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -1,23 +1,10 @@
 Owner Module/Drivers,Group,Property Name,Type,Property Values,Object 
attached,Description/Restrictions
-DRM,Generic,“rotation”,BITMASK,"{ 0, ""rotate-0"" }, { 1, ""rotate-90"" }, 
{ 2, ""rotate-180"" }, { 3, ""rotate-270"" }, { 4, ""reflect-x"" }, { 5, 
""reflect-y"" }","CRTC, Plane",rotate-(degrees) rotates the image by the 
specified amount in degrees in counter clockwise direction. reflect-x and 
reflect-y reflects the image along the specified axis prior to rotation
 ,,“scaling mode”,ENUM,"{ ""None"", ""Full"", ""Center"", ""Full aspect"" 
}",Connector,"Supported by: amdgpu, gma500, i915, nouveau and radeon."
 ,Connector,“EDID”,BLOB | IMMUTABLE,0,Connector,Contains id of edid blob 
ptr object.
 ,,“DPMS”,ENUM,"{ “On”, “Standby”, “Suspend”, “Off” 
}",Connector,Contains DPMS operation mode value.
 ,,“PATH”,BLOB | IMMUTABLE,0,Connector,Contains topology path to a 
connector.
 ,,“TILE”,BLOB | IMMUTABLE,0,Connector,Contains tiling information for a 
connector.
 ,,“CRTC_ID”,OBJECT,DRM_MODE_OBJECT_CRTC,Connector,CRTC that connector is 
attached to (atomic)
-,Plane,“type”,ENUM | IMMUTABLE,"{ ""Overlay"", ""Primary"", ""Cursor"" 
}",Plane,Plane type
-,,“SRC_X”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout source x coordinate in 
16.16 fixed point (atomic)
-,,“SRC_Y”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout source y coordinate in 
16.16 fixed point (atomic)
-,,“SRC_W”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout source width in 16.16 
fixed point (atomic)
-,,“SRC_H”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout source height in 16.16 
fixed point (atomic)
-,,“CRTC_X”,SIGNED_RANGE,"Min=INT_MIN, Max=INT_MAX",Plane,Scanout CRTC 
(destination) x coordinate (atomic)
-,,“CRTC_Y”,SIGNED_RANGE,"Min=INT_MIN, Max=INT_MAX",Plane,Scanout CRTC 
(destination) y coordinate (atomic)
-,,“CRTC_W”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout CRTC (destination) 
width (atomic)
-,,“CRTC_H”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout CRTC (destination) 
height (atomic)
-,,“FB_ID”,OBJECT,DRM_MODE_OBJECT_FB,Plane,Scanout framebuffer (atomic)
-,,“CRTC_ID”,OBJECT,DRM_MODE_OBJECT_CRTC,Plane,CRTC that plane is attached 
to (atomic)
-,,“zpos”,RANGE,"Min=0, Max=UINT_MAX","Plane,Z-order of the plane.Planes 
with higher Z-order values are displayed on top, planes with identical Z-order 
values are display in an undefined order"
 ,DVI-I,“subconnector”,ENUM,"{ “Unknown”, “DVI-D”, “DVI-A” 
}",Connector,TBD
 ,,“select subconnector”,ENUM,"{ “Automatic”, “DVI-D”, “DVI-A” 
}",Connector,TBD
 ,TV,“subconnector”,ENUM,"{ ""Unknown"", ""Composite"", ""SVIDEO"", 
""Component"", ""SCART"" }",Connector,TBD
@@ -95,7 +82,6 @@ armada,CRTC,"""CSC_YUV""",ENUM,"{ ""Auto"" , ""CCIR601"", 
""CCIR709"" }",CRTC,TB
 ,,"""contrast""",RANGE,"Min=0, Max=0x7fff",Plane,TBD
 ,,"""saturation""",RANGE,"Min=0, Max=0x7fff",Plane,TBD
 exynos,CRTC,“mode”,ENUM,"{ ""normal"", ""blank"" }",CRTC,TBD
-,Overlay,“zpos”,RANGE,"Min=0, Max=MAX_PLANE-1",Plane,TBD
 i2c/ch7006_drv,Generic,“scale”,RANGE,"Min=0, Max=2",Connector,TBD
 ,TV,“mode”,ENUM,"{ ""PAL"", ""PAL-M"",""PAL-N""}, ”PAL-Nc"" , 
""PAL-60"", ""NTSC-M"", ""NTSC-J"" }",Connector,TBD
 nouveau,NV10 Overlay,"""colorkey""",RANGE,"Min=0, Max=0x01ff",Plane,TBD
@@ -126,4 +112,3 @@ radeon,DVI-I,“coherent”,RANGE,"Min=0, 
Max=1",Connector,TBD
 ,FMT Dithering,“dither”,ENUM,"{ ""off"", ""on"" }",Connector,TBD
 

[PATCH 3/7] drm: Conslidate blending properties in drm_blend.[hc]

2016-09-21 Thread Daniel Vetter
Imo zpos, rotatation, blending eq (once we have it) and all that
should be in drm_blend.c, since those are all about how exactly the
pixels are rendered onto the CRTC's visible area. Also noticed that
one exported function accidentally ended up in drm_crtc_internal.h,
move it to the right place too.

Reviewed-by: Sean Paul 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_blend.c | 51 +++-
 drivers/gpu/drm/drm_crtc.c  | 49 --
 drivers/gpu/drm/drm_crtc_internal.h |  3 --
 include/drm/drm_blend.h | 59 +
 include/drm/drm_crtc.h  | 27 +
 5 files changed, 110 insertions(+), 79 deletions(-)
 create mode 100644 include/drm/drm_blend.h

diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 0a0b9357db35..0b8e227aa175 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -25,13 +25,62 @@
  */
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 

 #include "drm_crtc_internal.h"

+struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
+  unsigned int 
supported_rotations)
+{
+   static const struct drm_prop_enum_list props[] = {
+   { __builtin_ffs(DRM_ROTATE_0) - 1,   "rotate-0" },
+   { __builtin_ffs(DRM_ROTATE_90) - 1,  "rotate-90" },
+   { __builtin_ffs(DRM_ROTATE_180) - 1, "rotate-180" },
+   { __builtin_ffs(DRM_ROTATE_270) - 1, "rotate-270" },
+   { __builtin_ffs(DRM_REFLECT_X) - 1,  "reflect-x" },
+   { __builtin_ffs(DRM_REFLECT_Y) - 1,  "reflect-y" },
+   };
+
+   return drm_property_create_bitmask(dev, 0, "rotation",
+  props, ARRAY_SIZE(props),
+  supported_rotations);
+}
+EXPORT_SYMBOL(drm_mode_create_rotation_property);
+
+/**
+ * drm_rotation_simplify() - Try to simplify the rotation
+ * @rotation: Rotation to be simplified
+ * @supported_rotations: Supported rotations
+ *
+ * Attempt to simplify the rotation to a form that is supported.
+ * Eg. if the hardware supports everything except DRM_REFLECT_X
+ * one could call this function like this:
+ *
+ * drm_rotation_simplify(rotation, DRM_ROTATE_0 |
+ *   DRM_ROTATE_90 | DRM_ROTATE_180 |
+ *   DRM_ROTATE_270 | DRM_REFLECT_Y);
+ *
+ * to eliminate the DRM_ROTATE_X flag. Depending on what kind of
+ * transforms the hardware supports, this function may not
+ * be able to produce a supported transform, so the caller should
+ * check the result afterwards.
+ */
+unsigned int drm_rotation_simplify(unsigned int rotation,
+  unsigned int supported_rotations)
+{
+   if (rotation & ~supported_rotations) {
+   rotation ^= DRM_REFLECT_X | DRM_REFLECT_Y;
+   rotation = (rotation & DRM_REFLECT_MASK) |
+  BIT((ffs(rotation & DRM_ROTATE_MASK) + 1) % 4);
+   }
+
+   return rotation;
+}
+EXPORT_SYMBOL(drm_rotation_simplify);
+
 /**
  * drm_plane_create_zpos_property - create mutable zpos property
  * @plane: drm plane
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9ef7955032db..e5229b48d5d5 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1218,37 +1218,6 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
 }

 /**
- * drm_rotation_simplify() - Try to simplify the rotation
- * @rotation: Rotation to be simplified
- * @supported_rotations: Supported rotations
- *
- * Attempt to simplify the rotation to a form that is supported.
- * Eg. if the hardware supports everything except DRM_REFLECT_X
- * one could call this function like this:
- *
- * drm_rotation_simplify(rotation, DRM_ROTATE_0 |
- *   DRM_ROTATE_90 | DRM_ROTATE_180 |
- *   DRM_ROTATE_270 | DRM_REFLECT_Y);
- *
- * to eliminate the DRM_ROTATE_X flag. Depending on what kind of
- * transforms the hardware supports, this function may not
- * be able to produce a supported transform, so the caller should
- * check the result afterwards.
- */
-unsigned int drm_rotation_simplify(unsigned int rotation,
-  unsigned int supported_rotations)
-{
-   if (rotation & ~supported_rotations) {
-   rotation ^= DRM_REFLECT_X | DRM_REFLECT_Y;
-   rotation = (rotation & DRM_REFLECT_MASK) |
-  BIT((ffs(rotation & DRM_ROTATE_MASK) + 1) % 4);
-   }
-
-   return rotation;
-}
-EXPORT_SYMBOL(drm_rotation_simplify);
-
-/**
  * drm_mode_config_init - initialize DRM mode_configuration structure
  * @dev: DRM device
  *
@@ -1364,24 +1333,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_config_cleanup);

-struct drm_property 

[PATCH 2/7] drm/doc: Polish for drm_plane.[hc]

2016-09-21 Thread Daniel Vetter
Big thing is untangling and carefully documenting the different uapi
types of planes. I also sprinkled a few more cross references around
to make this easier to discover.

As usual, remove the kerneldoc for internal functions which are not
exported. Aside: We should probably go OCD on all the ioctl handlers
and consistenly give them an _ioctl postfix.

Acked-by: Archit Taneja 
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/drm-kms.rst |  47 +--
 drivers/gpu/drm/drm_crtc.c|   6 +-
 drivers/gpu/drm/drm_plane.c   | 132 --
 include/drm/drm_plane.h   |  57 +-
 4 files changed, 86 insertions(+), 156 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 33181be97151..b1029e292e5c 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -113,6 +113,9 @@ a hardware-specific ioctl to allocate suitable buffer 
objects.
 Plane Abstraction
 =

+.. kernel-doc:: drivers/gpu/drm/drm_plane.c
+   :doc: overview
+
 Plane Functions Reference
 -

@@ -189,50 +192,6 @@ allocated and zeroed by the driver, possibly as part of a 
larger
 structure, and registered with a call to :c:func:`drm_crtc_init()`
 with a pointer to CRTC functions.

-Planes (:c:type:`struct drm_plane `)

-
-A plane represents an image source that can be blended with or overlayed
-on top of a CRTC during the scanout process. Planes are associated with
-a frame buffer to crop a portion of the image memory (source) and
-optionally scale it to a destination size. The result is then blended
-with or overlayed on top of a CRTC.
-
-The DRM core recognizes three types of planes:
-
--  DRM_PLANE_TYPE_PRIMARY represents a "main" plane for a CRTC.
-   Primary planes are the planes operated upon by CRTC modesetting and
-   flipping operations described in the page_flip hook in
-   :c:type:`struct drm_crtc_funcs `.
--  DRM_PLANE_TYPE_CURSOR represents a "cursor" plane for a CRTC.
-   Cursor planes are the planes operated upon by the
-   DRM_IOCTL_MODE_CURSOR and DRM_IOCTL_MODE_CURSOR2 ioctls.
--  DRM_PLANE_TYPE_OVERLAY represents all non-primary, non-cursor
-   planes. Some drivers refer to these types of planes as "sprites"
-   internally.
-
-For compatibility with legacy userspace, only overlay planes are made
-available to userspace by default. Userspace clients may set the
-DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate
-that they wish to receive a universal plane list containing all plane
-types.
-
-Plane Initialization
-
-
-To create a plane, a KMS drivers allocates and zeroes an instances of
-:c:type:`struct drm_plane ` (possibly as part of a
-larger structure) and registers it with a call to
-:c:func:`drm_universal_plane_init()`. The function takes a
-bitmask of the CRTCs that can be associated with the plane, a pointer to
-the plane functions, a list of format supported formats, and the type of
-plane (primary, cursor, or overlay) being initialized.
-
-Cursor and overlay planes are optional. All drivers should provide one
-primary plane per CRTC (although this requirement may change in the
-future); drivers that do not wish to provide special handling for
-primary planes may make use of the helper functions described in ? to
-create and register a primary plane with standard capabilities.

 Cleanup
 ---
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 513ab4729683..9ef7955032db 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -151,7 +151,11 @@ static void drm_crtc_unregister_all(struct drm_device *dev)
  * @funcs: callbacks for the new CRTC
  * @name: printf style format string for the CRTC name, or NULL for default 
name
  *
- * Inits a new object created as base part of a driver crtc object.
+ * Inits a new object created as base part of a driver crtc object. Drivers
+ * should use this function instead of drm_crtc_init(), which is only provided
+ * for backwards compatibility with drivers which do not yet support universal
+ * planes). For really simple hardware which has only 1 plane look at
+ * drm_simple_display_pipe_init() instead.
  *
  * Returns:
  * Zero on success, error code on failure.
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 9003b5f835cf..c17c9c2a342e 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -25,6 +25,28 @@

 #include "drm_crtc_internal.h"

+/**
+ * DOC: overview
+ *
+ * A plane represents an image source that can be blended with or overlayed on
+ * top of a CRTC during the scanout process. Planes take their input data from 
a
+ * _framebuffer object. The plane itself specifies the cropping and scaling
+ * of that image, and where it is placed on the visible are of a display
+ * pipeline, represented by _crtc. A plane can also have additional
+ * 

  1   2   >