[PATCH] drm/sun4i: rgb: Enable panel after controller

[Jonathan Liu]

Hi Maxime,

On 23 September 2016 at 23:16, Maxime Ripard
 wrote:
> On Thu, Sep 22, 2016 at 08:03:31AM +1000, Jonathan Liu wrote:
>> Hi Maxime,
>>
>> On Thursday, 22 September 2016, Maxime Ripard > free-electrons.
>> com> wrote:
>>
>> > On Wed, Sep 21, 2016 at 11:03:04PM +1000, Jonathan Liu wrote:
>> > > The panel should be enabled after the controller so that the panel
>> > > prepare/enable delays are properly taken into account. Similarly, the
>> > > panel should be disabled before the controller so that the panel
>> > > unprepare/disable delays are properly taken into account.
>> > >
>> > > This is useful for avoiding visual glitches.
>> >
>> > This is not really taking any delays into account, especially since
>> > drm_panel_enable and prepare are suppose to block until their
>> > operation is complete.
>>
>>
>> drm_panel_prepare turns on power to the LCD using enable-gpios property of
>> the panel and then blocks for prepare delay. The prepare delay for panel
>> can be set to how long it takes between the time the panel is powered to
>> when it is ready to receive images. If backlight property is specified the
>> backlight will be off while the panel is powered on.
>>
>> drm_panel_enable blocks for enable delay and then turns on the backlight.
>> The enable delay can be set to how long it takes for panel to start making
>> the image visible after receiving the first valid frame. For example if the
>> panel starts off as white and the TFT takes some time to initialize to
>> black before it shows the image being received.
>>
>> Refer to drivers/gpu/drm/panel-panel.simple.c for details.
>
> From drm_panel.h:
>
> """
> * drm_panel_enable - enable a panel
> * @panel: DRM panel
> *
> * Calling this function will cause the panel display drivers to be turned on
> * and the backlight to be enabled. Content will be visible on screen after
> * this call completes.
> """
>
> """
> * drm_panel_prepare - power on a panel
> * @panel: DRM panel
> *
> * Calling this function will enable power and deassert any reset signals to
> * the panel. After this has completed it is possible to communicate with any
> * integrated circuitry via a command bus.
> """
>
> Those comments clearly says that the caller should not have to deal
> with the delays, even more so by just moving calls around and hoping
> that the code running in between is adding enough delay for the panel
> to behave properly.
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/drm/drm_panel.h#n34

In comment for struct drm_panel_funcs:
/**
 * struct drm_panel_funcs - perform operations on a given panel
 * @disable: disable panel (turn off back light, etc.)
 * @unprepare: turn off panel
 * @prepare: turn on panel and perform set up
 * @enable: enable panel (turn on back light, etc.)
 * @get_modes: add modes to the connector that the panel is attached to and
 * return the number of modes added
 * @get_timings: copy display timings into the provided array and return
 * the number of display timings available
 *
 * The .prepare() function is typically called before the display controller
 * starts to transmit video data. Panel drivers can use this to turn the panel
 * on and wait for it to become ready. If additional configuration is required
 * (via a control bus such as I2C, SPI or DSI for example) this is a good time
 * to do that.
 *
 * After the display controller has started transmitting video data, it's safe
 * to call the .enable() function. This will typically enable the backlight to
 * make the image on screen visible. Some panels require a certain amount of
 * time or frames before the image is displayed. This function is responsible
 * for taking this into account before enabling the backlight to avoid visual
 * glitches.
 *
 * Before stopping video transmission from the display controller it can be
 * necessary to turn off the panel to avoid visual glitches. This is done in
 * the .disable() function. Analogously to .enable() this typically involves
 * turning off the backlight and waiting for some time to make sure no image
 * is visible on the panel. It is then safe for the display controller to
 * cease transmission of video data.
 *
 * To save power when no video data is transmitted, a driver can power down
 * the panel. This is the job of the .unprepare() function.
 */

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-simple.c?id=refs/tags/v4.8-rc7#n39

In struct panel_desc:
/**
 * @prepare: the time (in milliseconds) that it takes for the panel to
 *   become ready and start receiving video data
 * @enable: the time (in milliseconds) that it takes for the panel to
 *  display the first valid frame after starting to receive
 *  video data
 * @disable: the time (in milliseconds) that it takes for the panel to
 *   turn the disp

[PATCH v2] drm/panel: simple: add support for Sharp LQ150X1LG11 panels

[Peter Rosin]

On 2016-09-23 19:39, Rob Herring wrote:
> On Sat, Sep 17, 2016 at 11:34:22AM +0200, Peter Rosin wrote:
>> From: Gustaf Lindström 
>>
>> The Sharp 15" LQ150X1LG11 panel is an XGA TFT LCD panel.
>>
>> Signed-off-by: Gustaf Lindström 
>> Signed-off-by: Peter Rosin 
>> ---
>>  .../bindings/display/panel/sharp,lq150x1lg11.txt   |  7 ++
>>  drivers/gpu/drm/panel/panel-simple.c   | 27 
>> ++
>>  2 files changed, 34 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
>>
>> v1->v2: correct author
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt 
>> b/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
>> new file mode 100644
>> index ..014428c984c8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
>> @@ -0,0 +1,7 @@
>> +Sharp 15" LQ150X1LG11 XGA TFT LCD panel
>> +
>> +Required properties:
>> +- compatible: should be "sharp,lq150x1lg11"
> 
> Looking at the spec, what about 12V VDD, 3.3V VCC, XSTABY (backlight 
> ctrl), VBR (PWM), RL/UD, SELLVDS signals?

I guess you're saying that simple-panel isn't the best match? Is it
ok to make the DT bindings more complete but still leave it to the
simple-panel driver to support part of it? Or should we just give
up for the time being, and carry a local patch pending a custom
driver?

Cheers,
Peter

[Bug 97909] X-Plane 10 crashes with SIGSEGV on radeonsi

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=97909

--- Comment #2 from Christian Inci  ---
Created attachment 126751
  --> https://bugs.freedesktop.org/attachment.cgi?id=126751&action=edit
glxinfo output

-- 
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/20160923/ded228b2/attachment.html>

[Bug 97909] X-Plane 10 crashes with SIGSEGV on radeonsi

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=97909

--- Comment #1 from Christian Inci  ---
Created attachment 126750
  --> https://bugs.freedesktop.org/attachment.cgi?id=126750&action=edit
gdb backtrace with some previous errors

-- 
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/20160923/8c492fec/attachment-0001.html>

[Bug 97909] X-Plane 10 crashes with SIGSEGV on radeonsi

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=97909

Bug ID: 97909
   Summary: X-Plane 10 crashes with SIGSEGV on radeonsi
   Product: Mesa
   Version: git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: chris.bugsfd at broke-the-inter.net
QA Contact: dri-devel at lists.freedesktop.org

Created attachment 126749
  --> https://bugs.freedesktop.org/attachment.cgi?id=126749&action=edit
Patch/Hack

The SIGSEGV is being raised because ib->buffer is NULL.

I don't know whether my solution is correct or not.

Is it okay to simply return if ib->buffer is NULL, is it okay to run those last
lines of that if block if ib->buffer isn't NULL only, is it okay to call
si_emit_draw_packets with ib->buffer being NULL, ...?

All I can say is that X-Plane is working fine with this patch/hack.

-- 
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/20160923/a1cfcba6/attachment.html>

[PATCH] drm/nouveau/secboot/gm20b: Fix return value in case of error

[Christophe JAILLET]

If 'ioremap()' returns 0, 'gm20b_tegra_read_wpr()' will return 0 as well,
which means success.
Return -ENOMEM instead

Signed-off-by: Christophe JAILLET 
---
Not sure that -ENOMEM is the best value.
I've taken it because it is often used in such a case.
---
 drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c
index d5395ebfe8d3..d88db933b3fd 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c
@@ -142,7 +142,7 @@ gm20b_tegra_read_wpr(struct gm200_secboot *gsb)
mc = ioremap(TEGRA_MC_BASE, 0xd00);
if (!mc) {
nvkm_error(&sb->subdev, "Cannot map Tegra MC registers\n");
-   return PTR_ERR(mc);
+   return -ENOMEM;
}
gsb->wpr_addr = ioread32_native(mc + MC_SECURITY_CARVEOUT2_BOM_0) |
  ((u64)ioread32_native(mc + MC_SECURITY_CARVEOUT2_BOM_HI_0) << 32);
-- 
2.7.4

[PATCH] drm/amdgpu: Removed unneeded variables adev and dev

[Mike Lothian]

Since commit 62336cc1ca1b96f33e3344ca6e910d30adca749a the variables adev
and dev are no longer required

Signed-off-by: Mike Lothian 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 5b592af..2be20b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -480,9 +480,6 @@ amdgpu_pci_remove(struct pci_dev *pdev)
 static void
 amdgpu_pci_shutdown(struct pci_dev *pdev)
 {
-   struct drm_device *dev = pci_get_drvdata(pdev);
-   struct amdgpu_device *adev = dev->dev_private;
-
/* if we are running in a VM, make sure the device
 * torn down properly on reboot/shutdown.
 * unfortunately we can't detect certain
-- 
2.10.0

[PATCH] drm/amdgpu Remove now unneeded variable adev

[Mike Lothian]

Since commit 62336cc1ca1b96f33e3344ca6e910d30adca749a adev is now no
longer required in amdgpu_drv.c

Signed-off-by: Mike Lothian 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 5b592af..a5ec1f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -481,7 +481,6 @@ static void
 amdgpu_pci_shutdown(struct pci_dev *pdev)
 {
struct drm_device *dev = pci_get_drvdata(pdev);
-   struct amdgpu_device *adev = dev->dev_private;

/* if we are running in a VM, make sure the device
 * torn down properly on reboot/shutdown.
-- 
2.10.0

[PATCH] drm/amdgpu Remove now unneeded variable adev

[Mike Lothian]

Sorry please ignore this - follow up momentarily

On Fri, 23 Sep 2016 at 21:14 Mike Lothian  wrote:

> Since commit 62336cc1ca1b96f33e3344ca6e910d30adca749a adev is now no
> longer required in amdgpu_drv.c
>
> Signed-off-by: Mike Lothian 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 5b592af..a5ec1f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -481,7 +481,6 @@ static void
>  amdgpu_pci_shutdown(struct pci_dev *pdev)
>  {
> struct drm_device *dev = pci_get_drvdata(pdev);
> -   struct amdgpu_device *adev = dev->dev_private;
>
> /* if we are running in a VM, make sure the device
>  * torn down properly on reboot/shutdown.
> --
> 2.10.0
>
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160923/8c1be32d/attachment.html>

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

[Christian König]

Am 23.09.2016 um 17:20 schrieb Chris Wilson:
> On Fri, Sep 23, 2016 at 03:50:44PM +0200, Daniel Vetter wrote:
>> On Mon, Aug 29, 2016 at 08:08:34AM +0100, Chris Wilson wrote:
>>> Currently we install a callback for performing poll on a dma-buf,
>>> irrespective of the timeout. This involves taking a spinlock, as well as
>>> unnecessary work, and greatly reduces scaling of poll(.timeout=0) across
>>> multiple threads.
>>>
>>> We can query whether the poll will block prior to installing the
>>> callback to make the busy-query fast.
>>>
>>> Single thread: 60% faster
>>> 8 threads on 4 (+4 HT) cores: 600% faster
>>>
>>> Still not quite the perfect scaling we get with a native busy ioctl, but
>>> poll(dmabuf) is faster due to the quicker lookup of the object and
>>> avoiding drm_ioctl().
>>>
>>> Signed-off-by: Chris Wilson 
>>> Cc: Sumit Semwal 
>>> Cc: linux-media at vger.kernel.org
>>> Cc: dri-devel at lists.freedesktop.org
>>> Cc: linaro-mm-sig at lists.linaro.org
>>> Reviewed-by: Daniel Vetter 
>> Need to strike the r-b here, since Christian König pointed out that
>> objects won't magically switch signalling on.
> Oh, it also means that
>
> commit fb8b7d2b9d80e1e71f379e57355936bd2b024be9
> Author: Jammy Zhou 
> Date:   Wed Jan 21 18:35:47 2015 +0800
>
>  reservation: wait only with non-zero timeout specified (v3)
>  
>  When the timeout value passed to reservation_object_wait_timeout_rcu
>  is zero, no wait should be done if the fences are not signaled.
>  
>  Return '1' for idle and '0' for busy if the specified timeout is '0'
>  to keep consistent with the case of non-zero timeout.
>  
>  v2: call fence_put if not signaled in the case of timeout==0
>  
>  v3: switch to reservation_object_test_signaled_rcu
>  
>  Signed-off-by: Jammy Zhou 
>  Reviewed-by: Christian König 
>  Reviewed-by: Alex Deucher 
>  Reviewed-By: Maarten Lankhorst 
>  Signed-off-by: Sumit Semwal 
>
> is wrong. And reservation_object_test_signaled_rcu() is unreliable.

Ups indeed, that patch is wrong as well.

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

Regards,
Christian.

> -Chris
>

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

[Michel Dänzer]

On 22/09/16 10:22 PM, Christian König wrote:
> Am 22.09.2016 um 15:05 schrieb Daniel Vetter:
>> On Thu, Sep 22, 2016 at 2:44 PM, Christian König
>>  wrote:
 - explicit fencing: Userspace passes around distinct fence objects for
 any work going on on the gpu. The kernel doesn't insert any stall of
 it's own (except for moving buffer objects around ofc). This is what
 Android. This also seems to be what amdgpu is doing within one
 process/owner.
>>>
>>> No, that is clearly not my understanding of explicit fencing.
>>>
>>> Userspace doesn't necessarily need to pass around distinct fence objects
>>> with all of it's protocols and the kernel is still responsible for
>>> inserting
>>> stalls whenever an userspace protocol or application requires this
>>> semantics.
>>>
>>> Otherwise you will never be able to use explicit fencing on the Linux
>>> desktop with protocols like DRI2/DRI3.
>> This is about mixing them. Explicit fencing still means userspace has
>> an explicit piece, separate from buffers, (either sync_file fd, or a
>> driver-specific cookie, or similar).
>>
>>> I would expect that every driver in the system waits for all fences of a
>>> reservation object as long as it isn't told otherwise by providing a
>>> distinct fence object with the IOCTL in question.
>> Yup agreed. This way if your explicitly-fencing driver reads a shared
>> buffer passed over a protocol that does implicit fencing (like
>> DRI2/3), then it will work.
>>
>> The other interop direction is explicitly-fencing driver passes a
>> buffer to a consumer which expects implicit fencing. In that case you
>> must attach the right fence to the exclusive slot, but _only_ in that
>> case.
> 
> Ok well sounds like you are close to understand why I can't do exactly
> this: There simply is no right fence I could attach.
> 
> When amdgpu makes the command submissions it doesn't necessarily know
> that the buffer will be exported and shared with another device later on.
> 
> So when the buffer is exported and given to the other device you might
> have a whole bunch of fences which run concurrently and not in any
> serial order.

I feel like you're thinking too much of buffers shared between GPUs as
being short-lived and only shared late. In the use-cases I know about,
shared buffers are created separately and shared ahead of time, the
actual rendering work is done to non-shared buffers and then just copied
to the shared buffers for transfer between GPUs. These copies are always
performed by the same context in such a way that they should always be
performed by the same HW engine and thus implicitly serialized.

Do you have any specific use-cases in mind where buffers are only shared
between GPUs after the rendering operations creating the buffer contents
to be shared have already been submitted?


>> Otherwise you end up stalling your explicitly-fencing userspace,
>> since implicit fencing doesn't allow more than 1 writer. For amdgpu
>> one possible way to implement this might be to count how many users a
>> dma-buf has, and if it's more than just the current context set the
>> exclusive fence. Or do an uabi revision and let userspace decide (or
>> at least overwrite it).
> 
> I mean I can pick one fence and wait for the rest to finish manually,
> but that would certainly defeat the whole effort, doesn't it?

I'm afraid it's not clear to me why it would. Can you elaborate?


>> But the current approach in amdgpu_sync.c of declaring a fence as
>> exclusive after the fact (if owners don't match) just isn't how
>> reservation_object works. You can of course change that, but that
>> means you must change all drivers implementing support for implicit
>> fencing of dma-buf. Fixing amdgpu will be easier ;-)
> 
> Well as far as I can see there is no way I can fix amdgpu in this case.
> 
> The handling clearly needs to be changed on the receiving side of the
> reservation objects if I don't completely want to disable concurrent
> access to BOs in amdgpu.

Anyway, we need a solution for this between radeon and amdgpu, and I
don't think a solution which involves those drivers using reservation
object semantics between them which are different from all other drivers
is a good idea.


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

[PATCH v2] drm/panel: simple: add support for Sharp LQ150X1LG11 panels

[Rob Herring]

On Fri, Sep 23, 2016 at 4:39 PM, Peter Rosin  wrote:
> On 2016-09-23 19:39, Rob Herring wrote:
>> On Sat, Sep 17, 2016 at 11:34:22AM +0200, Peter Rosin wrote:
>>> From: Gustaf Lindström 
>>>
>>> The Sharp 15" LQ150X1LG11 panel is an XGA TFT LCD panel.
>>>
>>> Signed-off-by: Gustaf Lindström 
>>> Signed-off-by: Peter Rosin 
>>> ---
>>>  .../bindings/display/panel/sharp,lq150x1lg11.txt   |  7 ++
>>>  drivers/gpu/drm/panel/panel-simple.c   | 27 
>>> ++
>>>  2 files changed, 34 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
>>>
>>> v1->v2: correct author
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt 
>>> b/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
>>> new file mode 100644
>>> index ..014428c984c8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
>>> @@ -0,0 +1,7 @@
>>> +Sharp 15" LQ150X1LG11 XGA TFT LCD panel
>>> +
>>> +Required properties:
>>> +- compatible: should be "sharp,lq150x1lg11"
>>
>> Looking at the spec, what about 12V VDD, 3.3V VCC, XSTABY (backlight
>> ctrl), VBR (PWM), RL/UD, SELLVDS signals?
>
> I guess you're saying that simple-panel isn't the best match?

No, I'm only saying the h/w description should be complete.

> Is it
> ok to make the DT bindings more complete but still leave it to the
> simple-panel driver to support part of it?

Sure, all the properties can be optional though you should define the
default if not present. You're the first one, so you get to pick
defaults.

> Or should we just give
> up for the time being, and carry a local patch pending a custom
> driver?

Not at all.

Rob

[PATCH 3/3] drm/vmwgfx: Adjust checks for null pointers in 13 functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 23 Sep 2016 17:53:49 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

Comparison to NULL could be written !…

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 31 +++
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 15504c6..b445ce9 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -324,7 +324,7 @@ static void vmw_hw_surface_destroy(struct vmw_resource *res)
if (res->id != -1) {

cmd = vmw_fifo_reserve(dev_priv, vmw_surface_destroy_size());
-   if (unlikely(cmd == NULL)) {
+   if (unlikely(!cmd)) {
DRM_ERROR("Failed reserving FIFO space for surface "
  "destruction.\n");
return;
@@ -397,7 +397,7 @@ static int vmw_legacy_srf_create(struct vmw_resource *res)

submit_size = vmw_surface_define_size(srf);
cmd = vmw_fifo_reserve(dev_priv, submit_size);
-   if (unlikely(cmd == NULL)) {
+   if (unlikely(!cmd)) {
DRM_ERROR("Failed reserving FIFO space for surface "
  "creation.\n");
ret = -ENOMEM;
@@ -446,11 +446,10 @@ static int vmw_legacy_srf_dma(struct vmw_resource *res,
uint8_t *cmd;
struct vmw_private *dev_priv = res->dev_priv;

-   BUG_ON(val_buf->bo == NULL);
-
+   BUG_ON(!val_buf->bo);
submit_size = vmw_surface_dma_size(srf);
cmd = vmw_fifo_reserve(dev_priv, submit_size);
-   if (unlikely(cmd == NULL)) {
+   if (unlikely(!cmd)) {
DRM_ERROR("Failed reserving FIFO space for surface "
  "DMA.\n");
return -ENOMEM;
@@ -538,7 +537,7 @@ static int vmw_legacy_srf_destroy(struct vmw_resource *res)

submit_size = vmw_surface_destroy_size();
cmd = vmw_fifo_reserve(dev_priv, submit_size);
-   if (unlikely(cmd == NULL)) {
+   if (unlikely(!cmd)) {
DRM_ERROR("Failed reserving FIFO space for surface "
  "eviction.\n");
return -ENOMEM;
@@ -578,7 +577,7 @@ static int vmw_surface_init(struct vmw_private *dev_priv,
int ret;
struct vmw_resource *res = &srf->res;

-   BUG_ON(res_free == NULL);
+   BUG_ON(!res_free);
if (!dev_priv->has_mob)
vmw_fifo_resource_inc(dev_priv);
ret = vmw_resource_init(dev_priv, res, true, res_free,
@@ -747,7 +746,7 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
}

user_srf = kzalloc(sizeof(*user_srf), GFP_KERNEL);
-   if (unlikely(user_srf == NULL)) {
+   if (unlikely(!user_srf)) {
ret = -ENOMEM;
goto out_no_user_srf;
}
@@ -772,7 +771,7 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
srf->offsets = kmalloc_array(srf->num_sizes,
 sizeof(*srf->offsets),
 GFP_KERNEL);
-   if (unlikely(srf->offsets == NULL)) {
+   if (unlikely(!srf->offsets)) {
ret = -ENOMEM;
goto out_no_offsets;
}
@@ -914,7 +913,7 @@ vmw_surface_handle_reference(struct vmw_private *dev_priv,

ret = -EINVAL;
base = ttm_base_object_lookup_for_ref(dev_priv->tdev, handle);
-   if (unlikely(base == NULL)) {
+   if (unlikely(!base)) {
DRM_ERROR("Could not find surface to reference.\n");
goto out_no_lookup;
}
@@ -1060,7 +1059,7 @@ static int vmw_gb_surface_create(struct vmw_resource *res)

cmd = vmw_fifo_reserve(dev_priv, submit_len);
cmd2 = (typeof(cmd2))cmd;
-   if (unlikely(cmd == NULL)) {
+   if (unlikely(!cmd)) {
DRM_ERROR("Failed reserving FIFO space for surface "
  "creation.\n");
ret = -ENOMEM;
@@ -1126,7 +1125,7 @@ static int vmw_gb_surface_bind(struct vmw_resource *res,
submit_size = sizeof(*cmd1) + (res->backup_dirty ? sizeof(*cmd2) : 0);

cmd1 = vmw_fifo_reserve(dev_priv, submit_size);
-   if (unlikely(cmd1 == NULL)) {
+   if (unlikely(!cmd1)) {
DRM_ERROR("Failed reserving FIFO space for surface "
  "binding.\n");
return -ENOMEM;
@@ -1176,7 +1175,7 @@ static int vmw_gb_surface_unbind(struct vmw_resource *res,

submit_size = sizeof(*cmd3) + (readback ? sizeof(*cmd1) : 
sizeof(*cmd2));
cmd = vmw_fifo_reserve(dev_priv, submit_size);
-   if (unlikely(cmd == NULL)) {
+   if (unlikely(!cmd)) {
DRM_ERROR(

[PATCH 2/3] drm/vmwgfx: Use memdup_user() rather than duplicating its implementation

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 23 Sep 2016 17:26:02 +0200

* Reuse existing functionality from memdup_user() instead of keeping
  duplicate source code.

* Try this copy operation before allocating memory for the data structure
  member "offsets".

* Delete the local variable "user_sizes" which became unnecessary
  with this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 21 +
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index f557549..15504c6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -700,7 +700,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
struct drm_vmw_surface_create_req *req = &arg->req;
struct drm_vmw_surface_arg *rep = &arg->rep;
struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
-   struct drm_vmw_size __user *user_sizes;
int ret;
int i, j;
uint32_t cur_bo_offset;
@@ -763,11 +762,11 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
memcpy(srf->mip_levels, req->mip_levels, sizeof(srf->mip_levels));
srf->num_sizes = num_sizes;
user_srf->size = size;
-   srf->sizes = kmalloc_array(srf->num_sizes,
-  sizeof(*srf->sizes),
-  GFP_KERNEL);
-   if (unlikely(srf->sizes == NULL)) {
-   ret = -ENOMEM;
+   srf->sizes = memdup_user((struct drm_vmw_size __user *)(unsigned long)
+req->size_addr,
+sizeof(*srf->sizes) * srf->num_sizes);
+   if (IS_ERR(srf->sizes)) {
+   ret = PTR_ERR(srf->sizes);
goto out_no_sizes;
}
srf->offsets = kmalloc_array(srf->num_sizes,
@@ -778,16 +777,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
goto out_no_offsets;
}

-   user_sizes = (struct drm_vmw_size __user *)(unsigned long)
-   req->size_addr;
-
-   ret = copy_from_user(srf->sizes, user_sizes,
-srf->num_sizes * sizeof(*srf->sizes));
-   if (unlikely(ret != 0)) {
-   ret = -EFAULT;
-   goto out_no_copy;
-   }
-
srf->base_size = *srf->sizes;
srf->autogen_filter = SVGA3D_TEX_FILTER_NONE;
srf->multisample_count = 0;
-- 
2.10.0

[PATCH 1/3] drm/vmwgfx: Use kmalloc_array() in vmw_surface_define_ioctl()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 22 Sep 2016 21:54:33 +0200

Multiplications for the size determination of memory allocations
indicated that array data structures 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/vmwgfx/vmwgfx_surface.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index c2a721a..f557549 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -763,14 +763,16 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
memcpy(srf->mip_levels, req->mip_levels, sizeof(srf->mip_levels));
srf->num_sizes = num_sizes;
user_srf->size = size;
-
-   srf->sizes = kmalloc(srf->num_sizes * sizeof(*srf->sizes), GFP_KERNEL);
+   srf->sizes = kmalloc_array(srf->num_sizes,
+  sizeof(*srf->sizes),
+  GFP_KERNEL);
if (unlikely(srf->sizes == NULL)) {
ret = -ENOMEM;
goto out_no_sizes;
}
-   srf->offsets = kmalloc(srf->num_sizes * sizeof(*srf->offsets),
-  GFP_KERNEL);
+   srf->offsets = kmalloc_array(srf->num_sizes,
+sizeof(*srf->offsets),
+GFP_KERNEL);
if (unlikely(srf->offsets == NULL)) {
ret = -ENOMEM;
goto out_no_offsets;
-- 
2.10.0

[PATCH 0/3] drm/vmwgfx: Fine-tuning for 13 function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 23 Sep 2016 18:24:32 +0200

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

Markus Elfring (3):
  Use kmalloc_array() in vmw_surface_define_ioctl()
  Use memdup_user() rather than duplicating its implementation
  Adjust checks for null pointers in 13 functions

 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 56 ++---
 1 file changed, 23 insertions(+), 33 deletions(-)

-- 
2.10.0

GPU-DRM-QXL: Move three assignments in qxl_device_init()

[Dave Airlie]

On 23 September 2016 at 17:25, Sean Paul  wrote:
> On Thu, Sep 22, 2016 at 1:24 PM, Dan Carpenter  
> wrote:
>> On Thu, Sep 22, 2016 at 03:11:25PM +0200, SF Markus Elfring wrote:
>>> > If you restricted yourself to fixing bugs only then you would maybe fix 
>>> > more
>>> > bugs than you introduce but as it you are making the kernel worse.
>>>
>>> Would you like to discuss the statistics for my failure (or success) rate
>>> a bit more so that involved issues can be clarified in a constructive way?
>>
>> It should be that you target 20 bug fixes for each new regression that
>> you add.
>>
>> Since you are just sending clean ups, every bug you introduce sets us
>> further and further back.  There is no hope for improving the kernel
>> because you are not even trying to fix 20 bugs, only introducing them.
>>
>> Once you fix 20 bugs, then you will be even and you can start sending
>> cleanups again.  This is fair.
>>
>
> At the risk of piling on, but hopefully to benefit Markus going forward:
>
> I will refrain from merging any more style/checkpatch/"code cleanup"
> patches from Markus until we start getting real, tested, bug fixes.

I'd prefer if everyone on dri-devel just ignored Markus at this stage,

If you are going to pick up his patches, please spend time making sure they
are correct and tested, as he doesn't seem to.

Markus, please contact the list in advance in future before posting a bunch
of patches that don't fix any problems.

Dave.

[RFC][PATCH] arm: dts: qcom: apq8064-nexus7: Add pstore support to nexus7

[Stephen Boyd]

On 09/23/2016 05:13 PM, John Stultz wrote:
>  arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts 
> b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
> index 7637092..3005576 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
> +++ b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
> @@ -15,6 +15,20 @@
>   stdout-path = "serial0:115200n8";
>   };
>  
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + ramoops at 0x88d0{

s/0x//

and add a space between the number and the bracket please. Does this
match with the reservation in android kernel?

> + compatible = "ramoops";
> + reg = <0x88d0 0x10>;
> + record-size = <0x0002>;
> + console-size= <0x0002>;
> + ftrace-size = <0x0002>;
> + };
> + };
> +
>   ext_3p3v: regulator-fixed at 1 {
>   compatible = "regulator-fixed";
>   regulator-min-microvolt = <330>;


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

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

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #16 from Christian König  ---
(In reply to Roland Scheidegger from comment #15)
> Oh interesting - didn't know voltage was directly tied to clock frequency
> here. Makes sense then to not allow it (at least if that circuitry isn't
> shared with DP, as the DP link runs at much higher clock (540Mhz actually),
> but I suppose it's really different there).

The voltage is only indirectly related, but yes over clocking such parts is a
rather bad idea in the long term.

DP uses a fixed frequency which is way easier to generate than the variable
HDMI/DVI/VGA clock.

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

[RFC] drm/exynos: g2d: fix runtime PM

[Tobias Jakobi]

Tobias Jakobi wrote:
> The commit b05984e21a7e000bf5074ace00d7a574944b2c16 broke
> operation of the G2D. After this commit the following
> happens.
> - exynos_g2d_exec_ioctl() prepares a runqueue node and
>   calls g2d_exec_runqueue()
> - g2d_exec_runqueue() calls g2d_dma_start() which gets
>   runtime PM sync
> - runtime PM calls g2d_runtime_resume()
> - g2d_runtime_resume() calls g2d_exec_runqueue()
> 
> Luckily for us this doesn't really loop, but creates a
> mutex lockup, which the kernel even predicts.
> 
> Anyway, we fix this by reintroducing dedicated sleep
> operations again, and only letting runtime PM control
> the gate clock.
> 
> This is not enough to fix the issue though.
> - We switch runtime PM to autosuspend. Currently clocks get
>   disabled, and then re-enabled again in the runqueue worker
>   when a node is completed and the next is started.
>   With the upcoming introduction of IOMMU runtime PM this
>   situations gets worse, since now also the IOMMU goes
>   through a disable/enable cycle before the next node is
>   started.
> - We consolidate all runtime PM management to the runqueue
>   worker.
> - We introduce g2d_wait_finish() which waits until the currently
>   processed runqueue node is finished.
>   If this takes too long, the engine is forcibly reset. This
>   is necessary to properly close the driver in case the engine
>   should hang with read/write access to some area of memory.
>   In this situation we can't properly unmap GEM/userptr
>   objects, since this might create a pagefault.
> - Sleep suspend issues a suspend of the runqueue and then calls
>   g2d_wait_finished(), which returns the engine in the idle state.
This should read 'g2d_wait_finish()'.



>   The current runqueue node gets completed, all other queued
>   nodes stay in the queue. There is no hardware state that
>   needs to be retained.
> - Sleep resume just pokes the runqueue worker, which, should
>   something be in queue, continues its work, or just exits.
> 
> Signed-off-by: Tobias Jakobi 

[RFC][PATCH] arm: dts: qcom: apq8064-nexus7: Add pstore support to nexus7

[John Stultz]

Add pstore support for the nexus7. This was useful in debugging
a crash where the cpus were getting stuck with irqs off and
serial output wasn't reliably working.

Cc: Kees Cook 
Cc: Bjorn Andersson 
Cc: Rob Clark 
Cc: Stephen Boyd 
Cc: Andy Gross 
Cc: Vinay Simha 
Cc: Archit Taneja 
Cc: linux-arm-msm at vger.kernel.org
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: John Stultz 
---
 arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts 
b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
index 7637092..3005576 100644
--- a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
@@ -15,6 +15,20 @@
stdout-path = "serial0:115200n8";
};

+   reserved-memory {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   ramoops at 0x88d0{
+   compatible = "ramoops";
+   reg = <0x88d0 0x10>;
+   record-size = <0x0002>;
+   console-size= <0x0002>;
+   ftrace-size = <0x0002>;
+   };
+   };
+
ext_3p3v: regulator-fixed at 1 {
compatible = "regulator-fixed";
regulator-min-microvolt = <330>;
-- 
1.9.1

[RFC] drm/exynos: g2d: fix runtime PM

[Tobias Jakobi]

The commit b05984e21a7e000bf5074ace00d7a574944b2c16 broke
operation of the G2D. After this commit the following
happens.
- exynos_g2d_exec_ioctl() prepares a runqueue node and
  calls g2d_exec_runqueue()
- g2d_exec_runqueue() calls g2d_dma_start() which gets
  runtime PM sync
- runtime PM calls g2d_runtime_resume()
- g2d_runtime_resume() calls g2d_exec_runqueue()

Luckily for us this doesn't really loop, but creates a
mutex lockup, which the kernel even predicts.

Anyway, we fix this by reintroducing dedicated sleep
operations again, and only letting runtime PM control
the gate clock.

This is not enough to fix the issue though.
- We switch runtime PM to autosuspend. Currently clocks get
  disabled, and then re-enabled again in the runqueue worker
  when a node is completed and the next is started.
  With the upcoming introduction of IOMMU runtime PM this
  situations gets worse, since now also the IOMMU goes
  through a disable/enable cycle before the next node is
  started.
- We consolidate all runtime PM management to the runqueue
  worker.
- We introduce g2d_wait_finish() which waits until the currently
  processed runqueue node is finished.
  If this takes too long, the engine is forcibly reset. This
  is necessary to properly close the driver in case the engine
  should hang with read/write access to some area of memory.
  In this situation we can't properly unmap GEM/userptr
  objects, since this might create a pagefault.
- Sleep suspend issues a suspend of the runqueue and then calls
  g2d_wait_finished(), which returns the engine in the idle state.
  The current runqueue node gets completed, all other queued
  nodes stay in the queue. There is no hardware state that
  needs to be retained.
- Sleep resume just pokes the runqueue worker, which, should
  something be in queue, continues its work, or just exits.

Signed-off-by: Tobias Jakobi 
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 235 +---
 1 file changed, 186 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 6eca8bb..c4f7026 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -138,6 +138,18 @@ enum g2d_reg_type {
MAX_REG_TYPE_NR
 };

+enum g2d_flag_bits {
+   /*
+* If set, suspends the runqueue worker after the currently
+* processed node is finished.
+*/
+   G2D_BIT_SUSPEND_RUNQUEUE,
+   /*
+* If set, indicates that the engine is currently busy.
+*/
+   G2D_BIT_ENGINE_BUSY,
+};
+
 /* cmdlist data structure */
 struct g2d_cmdlist {
u32 head;
@@ -226,7 +238,7 @@ struct g2d_data {
struct workqueue_struct *g2d_workq;
struct work_struct  runqueue_work;
struct exynos_drm_subdrvsubdrv;
-   boolsuspended;
+   unsigned long   flags;

/* cmdlist */
struct g2d_cmdlist_node *cmdlist_node;
@@ -246,6 +258,12 @@ struct g2d_data {
unsigned long   max_pool;
 };

+static inline void g2d_hw_reset(struct g2d_data *g2d)
+{
+   writel(G2D_R | G2D_SFRCLEAR, g2d->regs + G2D_SOFT_RESET);
+   clear_bit(G2D_BIT_ENGINE_BUSY, &g2d->flags);
+}
+
 static int g2d_init_cmdlist(struct g2d_data *g2d)
 {
struct device *dev = g2d->dev;
@@ -803,12 +821,8 @@ static void g2d_dma_start(struct g2d_data *g2d,
struct g2d_cmdlist_node *node =
list_first_entry(&runqueue_node->run_cmdlist,
struct g2d_cmdlist_node, list);
-   int ret;
-
-   ret = pm_runtime_get_sync(g2d->dev);
-   if (ret < 0)
-   return;

+   set_bit(G2D_BIT_ENGINE_BUSY, &g2d->flags);
writel_relaxed(node->dma_addr, g2d->regs + G2D_DMA_SFR_BASE_ADDR);
writel_relaxed(G2D_DMA_START, g2d->regs + G2D_DMA_COMMAND);
 }
@@ -831,9 +845,6 @@ static void g2d_free_runqueue_node(struct g2d_data *g2d,
 {
struct g2d_cmdlist_node *node;

-   if (!runqueue_node)
-   return;
-
mutex_lock(&g2d->cmdlist_mutex);
/*
 * commands in run_cmdlist have been completed so unmap all gem
@@ -847,29 +858,63 @@ static void g2d_free_runqueue_node(struct g2d_data *g2d,
kmem_cache_free(g2d->runqueue_slab, runqueue_node);
 }

-static void g2d_exec_runqueue(struct g2d_data *g2d)
+/**
+ * g2d_remove_runqueue_nodes - remove items from the list of runqueue nodes
+ * @g2d: G2D state object
+ * @file: if not zero, only remove items with this DRM file
+ *
+ * Has to be called under runqueue lock.
+ */
+static void g2d_remove_runqueue_nodes(struct g2d_data *g2d, struct drm_file* 
file)
 {
-   g2d->runqueue_node = g2d_get_runqueue_node(g2d);
-   if (g2d->runqueue_node)
-   g2d_dma_start(g2d, g2d->runqueue_node);
+   struct g2d_runqueue_node *node, *n;

[RFC] drm/exynos: g2d: runpm fixing attempt

[Tobias Jakobi]

Hello,

I have already talked to Marek in private about this. The latest runpm patch 
(b05984e21a7e000bf5074ace00d7a574944b2c16) cripples G2D operation. I have tried 
to come up with a way to fix this and also to improve runpm behaviour while at 
it.

Marek pointed out that the current issue, i.e. the reason for the 
aforementioned patch, is that sleep ops (suspend/resume) are now called during 
runpm suspend time. I think that my approach should handle this situation.

In any case, feedback is much appreciated.

With best wishes,
Tobias

Tobias Jakobi (1):
  drm/exynos: g2d: fix runtime PM

 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 235 +---
 1 file changed, 186 insertions(+), 49 deletions(-)

-- 
2.7.3

[RFC][PATCH 2/2 v2] arm: dts: qcom: apq8064-nexus7: Add DSI and panel nodes

[John Stultz]

Add DSI and panel nodes to get graphics up and running
on the Nexus7.

This still depends on the panel driver being present
along with the rpmclk code.

Feedback would be greatly appreciated!

Cc: Archit Taneja 
Cc: vinay simha 
Cc: andy.gross at linaro.org
Cc: robdclark at gmail.com
Cc: linux-arm-msm at vger.kernel.org
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: John Stultz 
---
v2: Integrated cleanups from Vinay and Archit

 arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 63 +-
 1 file changed, 61 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts 
b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
index ff856c3..7637092 100644
--- a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
@@ -99,6 +99,7 @@
l2 {
regulator-min-microvolt = <120>;
regulator-max-microvolt = <120>;
+   regulator-always-on;
};

/* msm_otg-HSUSB_3p3 */
@@ -133,13 +134,14 @@
regulator-min-microvolt = <300>;
regulator-max-microvolt = <300>;
bias-pull-down;
+   regulator-always-on;
};

/* pwm_power for backlight */
l17 {
regulator-min-microvolt = <300>;
-   regulator-max-microvolt = <360>;
-   bias-pull-down;
+   regulator-max-microvolt = <300>;
+   regulator-always-on;
};

/* camera, qdsp6 */
@@ -184,6 +186,63 @@
};
};

+   mdp at 510 {
+   status = "okay";
+   ports {
+   port at 1 {
+   mdp_dsi1_out: endpoint {
+   remote-endpoint = <&dsi0_in>;
+   };
+   };
+   };
+   };
+
+   dsi0: mdss_dsi at 470 {
+   status = "okay";
+   vdda-supply = <&pm8921_l2>;/*VDD_MIPI1 to 4*/
+   vdd-supply = <&pm8921_l8>;
+   vddio-supply = <&pm8921_lvs7>;
+   avdd-supply = <&pm8921_l11>;
+   vcss-supply = <&ext_3p3v>;
+
+   panel at 0 {
+   reg = <0>;
+   compatible = "jdi,lt070me05000";
+
+   vddp-supply = <&pm8921_l17>;
+   iovcc-supply = <&pm8921_lvs7>;
+
+   enable-gpios = <&pm8921_gpio 36 
GPIO_ACTIVE_HIGH>;
+   reset-gpios = <&tlmm_pinmux 54 GPIO_ACTIVE_LOW>;
+   dcdc-en-gpios = <&pm8921_gpio 23 
GPIO_ACTIVE_HIGH>;
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <&dsi0_out>;
+   };
+   };
+   };
+   ports {
+   port at 0 {
+   dsi0_in: endpoint {
+   remote-endpoint = 
<&mdp_dsi1_out>;
+   };
+   };
+
+   port at 1 {
+   dsi0_out: endpoint {
+   remote-endpoint = <&panel_in>;
+   data-lanes = <0 1 2 3>;
+   };
+   };
+   };
+   };
+
+   dsi-phy at 4700200 {
+   status = "okay";
+   vddio-supply = <&pm8921_lvs7>;/*VDD_PLL2_1 to 7*/
+   };
+
gsbi at 1620 {
status = "okay";
qcom,mode = ;
-- 
1.9.1

[RFC][PATCH 1/2 v2] arm: dts: qcom: apq8064: Add dsi, gpu and iommu nodes

[John Stultz]

Adds the core gpu, and dsi nodes for the apq8064 needed
to get graphics working on the nexus7 and other devices.

These apply on top of Archit's patch set that enables HDMI for IFC6410

Feedback would be greatly appreciated!

Cc: Archit Taneja 
Cc: vinay simha 
Cc: andy.gross at linaro.org
Cc: robdclark at gmail.com
Cc: linux-arm-msm at vger.kernel.org
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: John Stultz 
---
v2:
* Integrated cleanups suggested by Vinay and Archit

 arch/arm/boot/dts/qcom-apq8064.dtsi | 230 
 1 file changed, 230 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi 
b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 35a5759..ee4244c 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -919,6 +919,231 @@
reg = <0x1a40 0x100>;
};

+   gpu: adreno-3xx at 430 {
+   compatible = "qcom,adreno-3xx";
+   reg = <0x0430 0x2>;
+   reg-names = "kgsl_3d0_reg_memory";
+   interrupts = ;
+   interrupt-names = "kgsl_3d0_irq";
+   clock-names =
+   "core_clk",
+   "iface_clk",
+   "mem_clk",
+   "mem_iface_clk";
+   clocks =
+   <&mmcc GFX3D_CLK>,
+   <&mmcc GFX3D_AHB_CLK>,
+   <&mmcc GFX3D_AXI_CLK>,
+   <&mmcc MMSS_IMEM_AHB_CLK>;
+   qcom,chipid = <0x03020002>;
+
+   iommus = <&gfx3d 0
+ &gfx3d 1
+ &gfx3d 2
+ &gfx3d 3
+ &gfx3d 4
+ &gfx3d 5
+ &gfx3d 6
+ &gfx3d 7
+ &gfx3d 8
+ &gfx3d 9
+ &gfx3d 10
+ &gfx3d 11
+ &gfx3d 12
+ &gfx3d 13
+ &gfx3d 14
+ &gfx3d 15
+ &gfx3d 16
+ &gfx3d 17
+ &gfx3d 18
+ &gfx3d 19
+ &gfx3d 20
+ &gfx3d 21
+ &gfx3d 22
+ &gfx3d 23
+ &gfx3d 24
+ &gfx3d 25
+ &gfx3d 26
+ &gfx3d 27
+ &gfx3d 28
+ &gfx3d 29
+ &gfx3d 30
+ &gfx3d 31
+ &gfx3d1 0
+ &gfx3d1 1
+ &gfx3d1 2
+ &gfx3d1 3
+ &gfx3d1 4
+ &gfx3d1 5
+ &gfx3d1 6
+ &gfx3d1 7
+ &gfx3d1 8
+ &gfx3d1 9
+ &gfx3d1 10
+ &gfx3d1 11
+ &gfx3d1 12
+ &gfx3d1 13
+ &gfx3d1 14
+ &gfx3d1 15
+ &gfx3d1 16
+ &gfx3d1 17
+ &gfx3d1 18
+ &gfx3d1 19
+ &gfx3d1 20
+ &gfx3d1 21
+ &gfx3d1 22
+ &gfx3d1 23
+ &gfx3d1 24
+ &gfx3d1 25
+ &gfx3d1 26
+ &gfx3d1 27
+ &gfx3d1 28
+ &gfx3d1 29
+ &gfx3d1 30
+ &gfx3d1 31>;
+
+   qcom,gpu-pwrlevels {
+   compatible = "qcom,gpu-pwrlevels";
+   qcom,gpu-pwrlevel at 0 {
+   qcom,gpu-freq = <45000>;
+   };
+   qcom,gpu-pwrlevel at 1 {
+   qcom,gpu-freq = <2700>;
+   };
+   };
+  

GPU-DRM-QXL: Move three assignments in qxl_device_init()

[SF Markus Elfring]

>> I am trying to improve various open issues also in Linux source files.
>>
> That the fact that you see issues (in these particular cases) while
> others do not

I guess that the discussed "story" affects more challenges in communication
and different opinions about where to invest software development attention.


> indicates that the commit summary could be explained better.

I imagine that there are a few improvement possibilities left over.


> A good commit summary should provide enough information to do that

This advice is generally fine.


> and make people _want_ the patch.

I guess that this expectation can become a research topic for some knowledge 
fields,
can't it?

There are update suggestion where the probability for acceptance is higher
than for others.

Some maintainers have got their own difficulties with changes when they 
categorise
them as "ordinary clean-up".


> From my limited experience through your patches (just skimmed a few)

Thanks for your general interest.


> you seems to be describing what the patch does

My collection of update suggestions is evolving over some source code search 
patterns.

I find that this approach fits to the recommended imperative wording style
according to document "SubmittingPatches", doesn't it?

I dared also some deviations or variations already.


> as opposed to why it does it and why should one find it interesting/wanted.

I am trying to express also this information to some degree.

Regards,
Markus

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

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=172421

--- Comment #15 from Roland Scheidegger  ---
(In reply to Christian König from comment #14)
> (In reply to Roland Scheidegger from comment #13)
> > Personally I've always thought the risk of damaging hardware with any kind
> > of overclocking is just about exactly zero as long as you don't increase
> > voltage levels
> 
> Unfortunately this is exactly what happens here. The clock is generated by a
> voltage controlled oscillator and for the desired resolution you need to
> over clock it by about 30-40%.
> 
> That in turn means you raise the voltage way over the nominal limit.

Oh interesting - didn't know voltage was directly tied to clock frequency here.
Makes sense then to not allow it (at least if that circuitry isn't shared with
DP, as the DP link runs at much higher clock (540Mhz actually), but I suppose
it's really different there).

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

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

[Chris Wilson]

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

Oh, it also means that

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

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

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

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

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

v3: switch to reservation_object_test_signaled_rcu

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

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

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/sun4i: rgb: Enable panel after controller

[Maxime Ripard]

On Thu, Sep 22, 2016 at 08:03:31AM +1000, Jonathan Liu wrote:
> Hi Maxime,
> 
> On Thursday, 22 September 2016, Maxime Ripard  free-electrons.
> com> wrote:
> 
> > On Wed, Sep 21, 2016 at 11:03:04PM +1000, Jonathan Liu wrote:
> > > The panel should be enabled after the controller so that the panel
> > > prepare/enable delays are properly taken into account. Similarly, the
> > > panel should be disabled before the controller so that the panel
> > > unprepare/disable delays are properly taken into account.
> > >
> > > This is useful for avoiding visual glitches.
> >
> > This is not really taking any delays into account, especially since
> > drm_panel_enable and prepare are suppose to block until their
> > operation is complete.
> 
> 
> drm_panel_prepare turns on power to the LCD using enable-gpios property of
> the panel and then blocks for prepare delay. The prepare delay for panel
> can be set to how long it takes between the time the panel is powered to
> when it is ready to receive images. If backlight property is specified the
> backlight will be off while the panel is powered on.
> 
> drm_panel_enable blocks for enable delay and then turns on the backlight.
> The enable delay can be set to how long it takes for panel to start making
> the image visible after receiving the first valid frame. For example if the
> panel starts off as white and the TFT takes some time to initialize to
> black before it shows the image being received.
> 
> Refer to drivers/gpu/drm/panel-panel.simple.c for details.

[GIT PULL] tilcdc 3rd set of changes for v4.9 (second attempt)

[Jyri Sarha]

Hi Dave,
Please pull these collected fixes and cleanups from various sources. The
request was rebased on top the previous tilcdc pull request tag, so it
contains all the accumulated tilcdc changes for v4.9 so far.

Thanks,
Jyri

The following changes since commit 2e0965b06d90b9dba76198d026c4c2ee04443aca:

  drm/tilcdc: WARN if CRTC is touched without CRTC lock (2016-09-07
15:54:43 +0300)

are available in the git repository at:

  https://github.com/jsarha/linux tags/tilcdc-4.9-3.1

for you to fetch changes up to 7b993855dfd5d87e07ac84285d3d9bb0c743dede:

  drm/tilcdc: fix wrong error handling (2016-09-23 15:12:57 +0300)


Second attempt for 3rd drm/tilcdc pull request for v4.9.


Baoyou Xie (2):
  drm/tilcdc: add missing header dependencies
  drm/tilcdc: mark symbols static where possible

Daniel Schultz (1):
  drm/tilcdc: fix wrong error handling

Jyri Sarha (1):
  drm/tilcdc: Remove "default" from blue-and-red-wiring property binding

Markus Elfring (1):
  drm/tilcdc: Return directly after a failed kfree_table_init() in
tilcdc_convert_slave_node()

Wei Yongjun (1):
  drm/tilcdc: Fix non static symbol warning

 Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt |  6 +++---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 10 +-
 drivers/gpu/drm/tilcdc/tilcdc_panel.c   |  1 +
 drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c|  8 
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c  |  1 +
 5 files changed, 14 insertions(+), 12 deletions(-)

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

[Chris Wilson]

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

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

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 2/2] drm/rockchip: analogix_dp: Refuse to enable PSR if panel doesn't support it

[Tomeu Vizoso]

There's no point in enabling PSR when the panel doesn't support it.

This also avoids a problem when PSR gets enabled when a CRTC is being
disabled, because sometimes in that situation the DSP_HOLD_VALID_INTR
interrupt on which we wait will never arrive. This was observed on
RK3288 with a panel without PSR (veyron-jaq Chromebook).

It's very easy to reproduce by running the kms_rmfb test in IGT a few
times.

Signed-off-by: Tomeu Vizoso 
Cc: Sean Paul 
Cc: Yakir Yang 
Cc: Archit Taneja 
---
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index e83be157cc2a..8548e8271639 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -85,6 +85,9 @@ static void analogix_dp_psr_set(struct drm_encoder *encoder, 
bool enabled)
struct rockchip_dp_device *dp = to_dp(encoder);
unsigned long flags;

+   if (!analogix_dp_psr_supported(dp->dev))
+   return;
+
dev_dbg(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");

spin_lock_irqsave(&dp->psr_lock, flags);
-- 
2.7.4

[PATCH 1/2] drm/bridge: analogix_dp: Add analogix_dp_psr_supported

[Tomeu Vizoso]

So users know whether PSR should be enabled or not.

Signed-off-by: Tomeu Vizoso 
Cc: Sean Paul 
Cc: Yakir Yang 
Cc: Archit Taneja 
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 8 
 include/drm/bridge/analogix_dp.h   | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index bf992460a6c7..91d8540ac8f0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -98,6 +98,14 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device 
*dp)
return 0;
 }

+int analogix_dp_psr_supported(struct device *dev)
+{
+   struct analogix_dp_device *dp = dev_get_drvdata(dev);
+
+   return dp->psr_support;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_psr_supported);
+
 int analogix_dp_enable_psr(struct device *dev)
 {
struct analogix_dp_device *dp = dev_get_drvdata(dev);
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 5f498ca07eea..c99d6eaef1ac 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -38,6 +38,7 @@ struct analogix_dp_plat_data {
 struct drm_connector *);
 };

+int analogix_dp_psr_supported(struct device *dev);
 int analogix_dp_enable_psr(struct device *dev);
 int analogix_dp_disable_psr(struct device *dev);

-- 
2.7.4

GPU-DRM-TTM: Fine-tuning for several function implementations

[Dan Carpenter]

On Fri, Sep 23, 2016 at 12:20:54PM +0200, SF Markus Elfring wrote:
> > if you call it "restart" or "lock_restart" doesn't make much difference.
> 
> Do other identifiers fit better to a specification from the document 
> "CodingStyle"
> like the following?
> 
> "…
> Choose label names which say what the goto does or why the goto exists.
> …"
>
> 
> Does this wording need any more adjustments?

No.  I wrote that and "restart" seems like a pretty clear name to me.  I
never wrote that you should harrass people with your nonsense patches.
In fact, I have asked you over and over again to stop.

regards,
dan carpenter

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

[Daniel Vetter]

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

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

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

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

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

[Daniel Vetter]

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

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

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

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

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

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

[Christian König]

Am 23.09.2016 um 13:30 schrieb Gustavo Padovan:
> 2016-09-22 Christian König :
>
>> Am 22.09.2016 um 13:16 schrieb Gustavo Padovan:
>>> 2016-09-22 Christian König :
>>>
 Dropping the rest of the patch, cause that really doesn't make sense any
 more.

 Am 22.09.2016 um 12:40 schrieb Gustavo Padovan:
>> E.g. for example it is illegal to do something like
>>> "while(!fence_is_signaled(f)) sleep();" without enabling signaling 
>>> before
>>> doing this.
>>>
>>> Could just be a misunderstanding, but the comments on your patch 
>>> actually
>>> sounds a bit like somebody is trying to do exactly that.
> I think the usecase in mind here is poll(fd, timeout=0)
 Exactly as I feared. Even if userspace polls with timeout=0 you still need
 to call enable_signaling().

 Otherwise you can run into a situation where userspace only uses timeout=0
 and so never activates the signaling check in the driver.

 This would in turn result in an endless loop on implementations where the
 driver never signals fences on their own.
>>> Polling is optional, userspace may never call it. And DRM/KMS or GPU
>>> drivers will be doing fence_wait() themselves so signaling is enabled at
>>> some point.
>> No they won't. We have an use case where we clearly want to avoid that as
>> much as possible because and so the driver never calls enable_signaling() on
>> it's own.
>>
>> Exposing this poll function to userspace without enabling signaling is a
>> clear NAK from my side.
> Okay. So you are NAK'ing the does_not_pool_wait change. Should we revert
> that one then? It is already broken.

Yeah, that would probably a good idea. The AMD driver changes which 
really rely on this aren't upstream yet, so if you point me to the 
commit hash I could revert that as well when we send that out.

On the other hand the idea behind fence_is_signaled() is really that you 
check the status multiple times after enabling signaling. So I would 
prefer if you would revert this change preliminary.

Double checking this patch (and thinking about it a bit more) reveals 
that it is most likely correct. So feel free to commit this one if it is 
still needed for something.

Regards,
Christian.

>
> Gustavo
>

GPU-DRM-TTM: Fine-tuning for several function implementations

[SF Markus Elfring]

>> Do other identifiers fit better to a specification from the document 
>> "CodingStyle"
>> like the following?
>>
>> "…
>> Choose label names which say what the goto does or why the goto exists.
>> …"
>>
>>
>> Does this wording need any more adjustments?
> 
> No.

I have got an other impression.

The terse description can trigger disagreements about the "what" and "why",
can't it?


> I wrote that and "restart" seems like a pretty clear name to me.

This identifier might be good enough to some degree.
I imagined that it would become better by the addition of a bit of information
from the jump target.


> I never wrote that you should harrass people with your nonsense patches.

This is true in principle.

But your adjustment for the document "CodingStyle" supported also a 
reconsideration
of the corresponding identifier selection.

Some developers disagreed with a proposed renaming while others reacted
in a positive way.


> In fact, I have asked you over and over again to stop.

This happened under different software update contexts occasionally.

Regards,
Markus

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

[Daniel Vetter]

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

Reviewed-by: Daniel Vetter 

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

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

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

[Brian Starkey]

On Fri, Sep 23, 2016 at 03:13:15PM +0200, Daniel Vetter wrote:
>On Fri, Sep 23, 2016 at 01:52:49PM +0100, Brian Starkey wrote:
>> On Fri, Sep 23, 2016 at 12:58:46PM +0200, Daniel Vetter wrote:
>> > On Fri, Sep 23, 2016 at 11:34 AM, Liviu Dudau  
>> > wrote:
>> > > rmmod-ing the hdlcd module generates a WARN() splat as the vsync is still
>> > > enabled, but we never got the call to turn off the CRTC. Brian is still
>> > > tracking through the fbdev emulation to figure out the cause for that.
>> >
>> > fbdev emulation doesn't do that for you. If you need/want to shut down
>> > all the crtcs on driver unload, you need to do that yourself. There's
>> > atomic helpers to do that for you that for you.
>>
>> The problem is a sort-of circular dependency between ->lastclose (at
>> least the common implementation of it), unregister and disabling
>> fbdev.
>>
>> I want to move drm_dev_unregister() to be the first thing we do at
>> rmmod-time. However we need to disable fbdev first, otherwise
>> ->lastclose restores the fbdev mode, guaranteeing that vsync is turned
>> on for drm_vsync_cleanup() to then WARN on.
>>
>> There's a slightly different (perceived) problem - the one that Liviu
>> mentions - that drm_fbdev_cma_fini() doesn't disable the CRTC anyway.
>> You say it's not the fbdev helpers' responsibility to teardown their
>> modeset, but regardless I have nowhere to disable the CRTC if I want
>> to do drm_dev_unregister() first; and if the CRTC isn't disabled
>> there's always a chance of hitting the same vsync WARN even without
>> fbdev.
>
>Just disable all crtc in a suitable place (after drm_dev_unregister,
>before you tear down fbdev).

I think this is predicated on first removing the drm_vblank_cleanup
call.

>>
>> We *could* add an ->unload and disable everything there, but as that's
>> deprecated I'm guessing there should be another way.
>> Perhaps we should track ->firstopen/->lastclose pairs so we can detect
>> that ->lastclose is being called from unregister and use it to
>> disable everything in that case.
>
>Hm, maybe we should simply not call ->lastclose for kms drivers. That is
>kinda only a hack for ums/dri1 drivers.
>

To be clear (and in response to Russell's question) - you mean
only the call to ->lastclose in drm_dev_unregister, not in general?

>But even with that gone you might still unload while fbdev is 
>enabled, so
>this won't fix it all.
>

Yeah it will be tidier, but I don't think it actually fixes anything.

>> drm_vblank_cleanup() seems to have been carried over to unregister
>> from drm_put_dev(), but drm_dev_register() doesn't call
>> drm_vblank_init() so it seems a little strange to have it there.
>> I can see other drivers I'd expect to hit the same WARN but I don't
>> have HW to test it on.
>
>Oops. That call to drm_vblank_cleanup() really shouldn't be in there. We
>should push it into all callers instead I think.

OK so two things to do - remove drm_vblank_cleanup() from
drm_dev_unregister(), and then do the teardown like so:

drm_dev_unregister();
drm_crtc_force_disable_all(); // or atomic equivalent
fbdev_teardown();
...

Seems good to me. Are there any ordering constraints you're aware of
for drm_vblank_cleanup()? Or you think just putting it after
drm_dev_unregister() should be OK?

Thanks,
-Brian

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

[PATCH v4 12/14] drm: vmwgfx: Replace drm_fb_get_bpp_depth() with drm_format_info()

[Laurent Pinchart]

Hi Daniel,

On Wednesday 21 Sep 2016 09:31:58 Daniel Vetter wrote:
> On Thu, Sep 08, 2016 at 05:44:26PM +0300, Laurent Pinchart wrote:
> > The driver is the last users of the drm_fb_get_bpp_depth() function. It
> > should ideally be converted to use struct drm_mode_fb_cmd2 instead of
> > the legacy struct drm_mode_fb_cmd internally, but that will require
> > broad changes across the code base. As a first step, replace
> > drm_fb_get_bpp_depth() with drm_format_info() in order to stop exporting
> > the function to drivers.
> > 
> > The new DRM_ERROR() message comes from the vmw_create_dmabuf_proxy(),
> > vmw_kms_new_framebuffer_surface() and vmw_kms_new_framebuffer_dmabuf()
> > functions that currently print an error if the pixel format is
> > unsupported.
> > 
> > Signed-off-by: Laurent Pinchart 
> > Reviewed-by: Sinclair Yeh 
> > ---
> > 
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > Cc: VMware Graphics 
> > Cc: Sinclair Yeh 
> > Cc: Thomas Hellstrom 
> > 
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index bf28ccc150df..c965514b82be
> > 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -980,14 +980,22 @@ static struct drm_framebuffer
> > *vmw_kms_fb_create(struct drm_device *dev,> 
> > struct vmw_dma_buffer *bo = NULL;
> > struct ttm_base_object *user_obj;
> > struct drm_mode_fb_cmd mode_cmd;
> > 
> > +   const struct drm_format_info *info;
> > 
> > int ret;
> > 
> > +   info = drm_format_info(mode_cmd2->pixel_format);
> > +   if (!info || !info->depth) {
> > +   DRM_ERROR("Unsupported framebuffer format %s\n",
> > + drm_get_format_name(mode_cmd2->pixel_format));
> > +   return ERR_PTR(-EINVAL);
> > +   }
> > +
> > 
> > mode_cmd.width = mode_cmd2->width;
> > mode_cmd.height = mode_cmd2->height;
> > mode_cmd.pitch = mode_cmd2->pitches[0];
> > mode_cmd.handle = mode_cmd2->handles[0];
> > 
> > -   drm_fb_get_bpp_depth(mode_cmd2->pixel_format, &mode_cmd.depth,
> > -   &mode_cmd.bpp);
> > +   mode_cmd.depth = info->depth;
> > +   mode_cmd.bpp = info->cpp[0] * 8;
> 
> I think this should use drm_helper_mode_fill_fb_struct instead.

I would do that if there was a struct drm_framebuffer to fill, but this piece 
of code converts from struct drm_mode_fb_cmd2 to drm_mode_fb_cmd that is then 
used all over the place internally. This should be fixed, but I think that's 
out of scope for this patch series.

> > /**
> > 
> >  * This code should be conditioned on Screen Objects not being used.

-- 
Regards,

Laurent Pinchart

[ADV7393] DRM Encoder Slave or DRM Bridge

[Vikas Patil]

Hi Tomi,

I added the missing check for "OMAP_DISPLAY_TYPE_VENC" in function
omap_connector_detect @ gpu/drm/omapdrm/omap_connector.c and now
modetest  seems to be showing correct status and connections.

But still I could not see kmscube on panel and can see some flicker is
going on display. I think I need to understand about what display
timing I could use as interlace doesn't seems to be supported as I
mentioned above.

Could you please comment on this?

Thanks & Regards,
Vikash


On Thu, Sep 22, 2016 at 6:52 PM, Vikas Patil  wrote:
> Hi Tomi,
>
>
> Now with the adv7393 driver in place, I was getting following error.
> After debugging found out that this is due to the “.interlace= true”
> in display timings
> “drivers\gpu\drm\omapdrm\displays\connector-analog-tv.c”.
>
>
> [   14.564872] [drm:drm_helper_probe_single_connector_modes_merge_bits]
> [CONNECTOR:32:Unknown-1]
> [   14.564882] [drm:omap_connector_get_modes] cvbs_out
> [   14.564898] -->adv7393_check_timings: start
> [   14.569646] [drm:omap_connector_mode_valid] connector: mode
> invalid: 45:"720x480i" 120 27000 720 739 801 858 480 490 493 527 0x48
> 0x2a15
> [   14.569659] [drm:drm_mode_debug_printmodeline] Modeline
> 45:"720x480i" 120 27000 720 739 801 858 480 490 493 527 0x48
> 0x2a15
> [   14.569668] [drm:drm_mode_prune_invalid] Not using 720x480i mode: BAD
>
> After setting “.interlace= false”  in display timings
> “\displays\connector-analog-tv.c” mode seems to be valid but still
> nothing on display probably because connector still doesn’t seem to be
> enabled from the below drm log.
>
> [   14.787200] [drm:drm_setup_crtcs]
> [   14.787211] [drm:drm_enable_connectors] connector 32 enabled? no
> [   14.787220] [drm:drm_enable_connectors] connector 36 enabled? Yes
>
> Could you help me to understand if I could use “interlace=false”?
> ADV7393 seems to be supporting non-interlaced mode. From datasheet:
> “The ADV7390/ADV7391/ADV7392/ADV7393 support an SD noninterlaced mode.
> Using this mode, progressive inputs at twice the frame rate of NTSC
> and PAL (240p/59.94 Hz and 288p/50 Hz, respectively) can be input into
> the ADV7390/ ADV7391/ADV7392/ADV7393. The SD noninterlaced mode can be
> enabled using Subaddress 0x88, Bit 1.”
>
> What/Where should I need to look for enabling the above connector and
> attached to the correct encoder/crtc?
>
> Also looking at function dispc_mgr_timings_ok () in
> drivers/gpu/drm/omapdrm/dss/dispc.c, it seems driver of DSS doesn’t
> support interlace out as comment suggests below. Would this be a
> problem for me for driving ADV7393? What does this means?
>
>
> if (dss_mgr_is_lcd(channel)) {
> /* TODO: OMAP4+ supports interlace for LCD outputs */
> if (timings->interlace)
> {
> DSSWARN("vikas->: interlace failed\n");
> return false;
> }
>
> if (!_dispc_lcd_timings_ok(timings->hsw, timings->hfp,
> timings->hbp, timings->vsw, timings->vfp,
>timings->vbp))
> {
> return false;
> }
> }
>
>
> Thanks & Regards,
> Vikash
>
>
>
> 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 = &adv7393_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 

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

[Markus Heiser]

Am 23.09.2016 um 14:59 schrieb Daniel Vetter :

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

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

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

-- Markus --

[PATCH 2/2] drm/exynos: fix pending update handling

[Gustavo Padovan]

2016-09-23 Andrzej Hajda :

> Exynos DRM devices update their registers at vblank time. Exynos-DRM uses
> custom mechanism to wait for vblank. This mechanism is error prone -
> variables are not updated atomically. As a result in certain circumstances
> user space can try to free buffers which are still in use by hardware,
> in such cases IOMMU can throw OOPS.
> The patch instead of fixing the mechanism replaces it with drm core helper.
> 
> Signed-off-by: Andrzej Hajda 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 16 +---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 44 
> +---
>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  2 --
>  3 files changed, 2 insertions(+), 60 deletions(-)

Reviewed-by: Gustavo Padovan 

Gustavo

[PATCH 1/2] drm/exynos/vidi: use timer for vblanks instead of sleeping worker

[Gustavo Padovan]

2016-09-23 Andrzej Hajda :

> VIDI driver uses fake vblank handler to generate vblank events.
> It was implemented using worker which slept for vblank time, additionally
> it did not work if there were no page flips. The patch replaces it with
> timer, uses drm_crtc_vblank_(on|off) helpers to manage it and fixes
> behavior for non-page-flip cases.
> This change allows further improvements of vblank in exynos-drm framework.
> 
> Signed-off-by: Andrzej Hajda 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c | 66 
> ++--
>  1 file changed, 20 insertions(+), 46 deletions(-)

Reviewed-by: Gustavo Padovan 

Gustavo

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

[Lucas Stach]

Am Freitag, den 23.09.2016, 12:58 +0200 schrieb Daniel Vetter:
> On Fri, Sep 23, 2016 at 11:34 AM, Liviu Dudau 
> wrote:
> > 
> > rmmod-ing the hdlcd module generates a WARN() splat as the vsync is
> > still
> > enabled, but we never got the call to turn off the CRTC. Brian is
> > still
> > tracking through the fbdev emulation to figure out the cause for
> > that.
> 
> fbdev emulation doesn't do that for you. If you need/want to shut
> down
> all the crtcs on driver unload, you need to do that yourself. There's
> atomic helpers to do that for you that for you.

Which reminds me that my patch (drm/atomic-helper: add unlocked disable
all outputs helper) to add such a helper wasn't applied. Probably my
own fault by being non-responsive to Seans question.

Regards,
Lucas

[PATCH] drm/exynos: use drm core to handle page-flip event

[Andrzej Hajda]

Exynos DRM framework handled page-flip event with custom code.
The patch replaces it with drm-core vblank queue.

Signed-off-by: Andrzej Hajda 
---
Hi Inki,

This patch is next step of vblank re-factoring in Exynos-DRM.
I have tested the code on fimd, vidi, decon5433, mixer.
I hope I have not introduced bugs.

Regards
Andrzej
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 11 ---
 drivers/gpu/drm/exynos/exynos7_drm_decon.c|  9 --
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 42 +--
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |  2 --
 drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 15 +-
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  1 -
 drivers/gpu/drm/exynos/exynos_drm_vidi.c  | 10 ---
 drivers/gpu/drm/exynos/exynos_mixer.c |  9 --
 8 files changed, 21 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c 
b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index ac21b40..6ca1f31 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -551,7 +551,6 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 {
struct decon_context *ctx = dev_id;
u32 val;
-   int win;

if (!test_bit(BIT_CLKS_ENABLED, &ctx->flags))
goto out;
@@ -560,16 +559,6 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
val &= VIDINTCON1_INTFRMDONEPEND | VIDINTCON1_INTFRMPEND;

if (val) {
-   for (win = ctx->first_win; win < WINDOWS_NR ; win++) {
-   struct exynos_drm_plane *plane = &ctx->planes[win];
-
-   if (!plane->pending_fb)
-   continue;
-
-   exynos_drm_crtc_finish_update(ctx->crtc, plane);
-   }
-
-   /* clear */
writel(val, ctx->addr + DECON_VIDINTCON1);
drm_crtc_handle_vblank(&ctx->crtc->base);
}
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c 
b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 7f9901b..f4d5a21 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -603,7 +603,6 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 {
struct decon_context *ctx = (struct decon_context *)dev_id;
u32 val, clear_bit;
-   int win;

val = readl(ctx->regs + VIDINTCON1);

@@ -617,14 +616,6 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)

if (!ctx->i80_if) {
drm_crtc_handle_vblank(&ctx->crtc->base);
-   for (win = 0 ; win < WINDOWS_NR ; win++) {
-   struct exynos_drm_plane *plane = &ctx->planes[win];
-
-   if (!plane->pending_fb)
-   continue;
-
-   exynos_drm_crtc_finish_update(ctx->crtc, plane);
-   }

/* set wait vsync event to zero and wake up queue. */
if (atomic_read(&ctx->wait_vsync_event)) {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 5b6845b..2530bf5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -69,8 +69,6 @@ static void exynos_crtc_atomic_begin(struct drm_crtc *crtc,
 {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);

-   exynos_crtc->event = crtc->state->event;
-
if (exynos_crtc->ops->atomic_begin)
exynos_crtc->ops->atomic_begin(exynos_crtc);
 }
@@ -79,9 +77,24 @@ static void exynos_crtc_atomic_flush(struct drm_crtc *crtc,
 struct drm_crtc_state *old_crtc_state)
 {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+   struct drm_pending_vblank_event *event;
+   unsigned long flags;

if (exynos_crtc->ops->atomic_flush)
exynos_crtc->ops->atomic_flush(exynos_crtc);
+
+   event = crtc->state->event;
+   if (event) {
+   crtc->state->event = NULL;
+
+   spin_lock_irqsave(&crtc->dev->event_lock, flags);
+   if (drm_crtc_vblank_get(crtc) == 0)
+   drm_crtc_arm_vblank_event(crtc, event);
+   else
+   drm_crtc_send_vblank_event(crtc, event);
+   spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
+   }
+
 }

 static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
@@ -173,22 +186,6 @@ void exynos_drm_crtc_disable_vblank(struct drm_device 
*dev, unsigned int pipe)
exynos_crtc->ops->disable_vblank(exynos_crtc);
 }

-void exynos_drm_crtc_finish_update(struct exynos_drm_crtc *exynos_crtc,
-   struct exynos_drm_plane *exynos_plane)
-{
-   struct drm_crtc *crtc = &exynos_crtc->base;
-   unsigned long flags;
-
-   exynos_plane->pe

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

[Daniel Vetter]

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

Reviewed-by: Daniel Vetter 

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

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

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

[Chris Wilson]

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

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

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

-- 
Chris Wilson, Intel Open Source Technology Centre

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

[Daniel Vetter]

On Fri, Sep 23, 2016 at 01:52:49PM +0100, Brian Starkey wrote:
> On Fri, Sep 23, 2016 at 12:58:46PM +0200, Daniel Vetter wrote:
> > On Fri, Sep 23, 2016 at 11:34 AM, Liviu Dudau  
> > wrote:
> > > rmmod-ing the hdlcd module generates a WARN() splat as the vsync is still
> > > enabled, but we never got the call to turn off the CRTC. Brian is still
> > > tracking through the fbdev emulation to figure out the cause for that.
> > 
> > fbdev emulation doesn't do that for you. If you need/want to shut down
> > all the crtcs on driver unload, you need to do that yourself. There's
> > atomic helpers to do that for you that for you.
> 
> The problem is a sort-of circular dependency between ->lastclose (at
> least the common implementation of it), unregister and disabling
> fbdev.
> 
> I want to move drm_dev_unregister() to be the first thing we do at
> rmmod-time. However we need to disable fbdev first, otherwise
> ->lastclose restores the fbdev mode, guaranteeing that vsync is turned
> on for drm_vsync_cleanup() to then WARN on.
> 
> There's a slightly different (perceived) problem - the one that Liviu
> mentions - that drm_fbdev_cma_fini() doesn't disable the CRTC anyway.
> You say it's not the fbdev helpers' responsibility to teardown their
> modeset, but regardless I have nowhere to disable the CRTC if I want
> to do drm_dev_unregister() first; and if the CRTC isn't disabled
> there's always a chance of hitting the same vsync WARN even without
> fbdev.

Just disable all crtc in a suitable place (after drm_dev_unregister,
before you tear down fbdev).
> 
> We *could* add an ->unload and disable everything there, but as that's
> deprecated I'm guessing there should be another way.
> Perhaps we should track ->firstopen/->lastclose pairs so we can detect
> that ->lastclose is being called from unregister and use it to
> disable everything in that case.

Hm, maybe we should simply not call ->lastclose for kms drivers. That is
kinda only a hack for ums/dri1 drivers.

But even with that gone you might still unload while fbdev is enabled, so
this won't fix it all.

> drm_vblank_cleanup() seems to have been carried over to unregister
> from drm_put_dev(), but drm_dev_register() doesn't call
> drm_vblank_init() so it seems a little strange to have it there.
> I can see other drivers I'd expect to hit the same WARN but I don't
> have HW to test it on.

Oops. That call to drm_vblank_cleanup() really shouldn't be in there. We
should push it into all callers instead I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH] drm/radeon: Fix negative cursor position

[Takashi Iwai]

radeon_cursor_move_unlock() contains a workaround for AVIVO chips that
are older than DCE6 when the cursor ends on 128 pixel boundary.  It
decreases the position when the calculated end position is on 128
pixel boundary.  However, it hits also the condition where x=-1 and
width=1 are passed, since cursor_end is 0 (which is aligned with 128,
too).  Then the value gets decreased and it hits WARN_ON_ONCE()
below, eventually screws up the GPU.

The fix is simply to skip the workaround when x is already zero.
Also, remove the superfluous WARN_ON_ON() for the negative value check
that wouldn't happen any longer after this change.

Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1000433
Cc: 
Signed-off-by: Takashi Iwai 
---
 drivers/gpu/drm/radeon/radeon_cursor.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c 
b/drivers/gpu/drm/radeon/radeon_cursor.c
index 2a10e24b34b1..4e436eb49a56 100644
--- a/drivers/gpu/drm/radeon/radeon_cursor.c
+++ b/drivers/gpu/drm/radeon/radeon_cursor.c
@@ -193,10 +193,8 @@ static int radeon_cursor_move_locked(struct drm_crtc 
*crtc, int x, int y)
if (w <= 0) {
w = 1;
cursor_end = x - xorigin + w;
-   if (!(cursor_end & 0x7f)) {
+   if (x > 0 && !(cursor_end & 0x7f))
x--;
-   WARN_ON_ONCE(x < 0);
-   }
}
}
}
-- 
2.10.0

GPU-DRM-TTM: Fine-tuning for several function implementations

[Christian König]

Am 23.09.2016 um 13:49 schrieb SF Markus Elfring:
>> Calling the label "unlock" instead of "out" is arguable a little better,
> Thanks that you can follow a renaming for this direction in principle.
>
>
>> but nothing I would call a major improvement either.
> This was not my intention for such an use case.
>
> I am proposing some small software updates according to such a design pattern.
>
>
>> So that is a clear NAK to all those patches.
> Do you reject also update steps like the following then?
>
> * drm/ttm: Use kmalloc_array() in two (or four?) functions"
>
> * drm/ttm: Less function calls in ttm_dma_pool_init() after error detection

The reason behind the advise to use kmalloc_array() is to avoid overruns 
when one of the parameters come from an IOCTL and so are controllable by 
user space.

Those overruns where the source of numerous security problems, but in 
this case the parameters don't come from an IOCTL and aren't user space 
controllable.

So this change actually doesn't make to much sense either, but I'm 
leaning towards accepting them for coding style consistency.

Regards,
Christian.

> * Would you like to improve the usage of the variables "n" and "t"
>in the function "ttm_dma_pool_init" any further as Joe Perches suggested 
> it?
>
>
>>> 2. How do you think about to add a single space character before any label?
>> Bad as well. Why would anybody want to do this?
> Do you find another software evolution interesting according to a recent 
> commit?
>
> "docs: Remove space-before-label guidance from CodingStyle"
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/Documentation/CodingStyle?id=79c70c304b0b443429b2a0019518532c5162817a
>
>
> Regards,
> Markus

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

[Russell King - ARM Linux]

On Fri, Sep 23, 2016 at 03:13:15PM +0200, Daniel Vetter wrote:
> Hm, maybe we should simply not call ->lastclose for kms drivers. That is
> kinda only a hack for ums/dri1 drivers.

Are you sure about that - isn't it needed so that the fbdev mode gets
restored when the last DRM user exits, so that the VT consoles becomes
functional again?

I ended up needing a call to drm_fb_helper_restore_fbdev_mode_unlocked()
in Armada's ->lastclose to ensure that VT consoles worked after Xorg was
shutdown.

-- 
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.

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

[Daniel Vetter]

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

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

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

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

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

[Chris Wilson]

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

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

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

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH v2 1/3] dt-bindings: display: display-timing: Add property to configure sync drive edge

[Rob Herring]

On Thu, Sep 22, 2016 at 01:35:24PM +0300, Peter Ujfalusi wrote:
> There are display panels which demands that the sync signal is driven on
> different edge than the pixel data.
> With the syncclk-active property we can specify the clk edge to be used to
> drive the sync signal. When the property is missing it indicates that the
> sync is driven on the same edge as the pixel data.
> 
> Signed-off-by: Peter Ujfalusi 
> CC: Rob Herring 
> CC: Mark Rutland 
> CC: devicetree at vger.kernel.org
> ---
>  .../devicetree/bindings/display/panel/display-timing.txt  | 8 
> 
>  1 file changed, 8 insertions(+)

Acked-by: Rob Herring 

[PATCH] drm/tilcdc: fix wrong error handling

[Jyri Sarha]

On 09/23/16 14:47, Sean Paul wrote:
> On Fri, Sep 23, 2016 at 3:52 AM, Daniel Schultz  
> wrote:
>> When 'component_bind_all' fails it should not try to unbind components
>> in the error handling. This will produce a null pointer kernel panic when
>> no component exist.
>>
>> This patch changes the order of the error handling. Now, it will only
>> unbind components if the are bound. Otherwise, the module will jump to
>> an error label below.
>>
>> Signed-off-by: Daniel Schultz 
> 
> Reviewed-by: Sean Paul 
> 

Thanks, for both. Should I pick this one :)?

BR,
Jyri

>> ---
>>  drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
>> b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> index d278093..d491610 100644
>> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> @@ -315,13 +315,13 @@ fail_irq_uninstall:
>>  fail_vblank_cleanup:
>> drm_vblank_cleanup(dev);
>>
>> -fail_mode_config_cleanup:
>> -   drm_mode_config_cleanup(dev);
>> -
>>  fail_component_cleanup:
>> if (priv->is_componentized)
>> component_unbind_all(dev->dev, dev);
>>
>> +fail_mode_config_cleanup:
>> +   drm_mode_config_cleanup(dev);
>> +
>>  fail_external_cleanup:
>> tilcdc_remove_external_encoders(dev);
>>
>> --
>> 1.9.1
>>

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

[Daniel Vetter]

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

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

Reviewed-by: Daniel Vetter 

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

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

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

[Daniel Vetter]

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

Reviewed-by: Daniel Vetter 

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

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

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

[Daniel Vetter]

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

Reviewed-by: Daniel Vetter 

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

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

[PATCH 03/11] drm/msm: Remove call to reservation_object_test_signaled_rcu before wait

[Daniel Vetter]

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

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

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

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

[Daniel Vetter]

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

Reviewed-by: Daniel Vetter 

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

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

[PATCH 2/2 v4] drm/bridge: adv7511: Enable the audio data and clock pads on adv7533

[Archit Taneja]

On 09/17/2016 04:47 AM, John Stultz wrote:
> From: Srinivas Kandagatla 
>
> This patch enables the Audio Data and Clock pads to the adv7533 bridge.
> Without this patch audio can not be played.
>
> Cc: David Airlie 
> Cc: Archit Taneja 
> Cc: Laurent Pinchart 
> Cc: Wolfram Sang 
> Cc: Srinivas Kandagatla 
> Cc: "Ville Syrjälä" 
> Cc: Boris Brezillon 
> Cc: Andy Green 
> Cc: Dave Long 
> Cc: Guodong Xu 
> Cc: Zhangfei Gao 
> Cc: Mark Brown 
> Cc: Lars-Peter Clausen 
> Cc: Jose Abreu 
> Cc: dri-devel at lists.freedesktop.org

Reviewed-by: Archit Taneja 

> Signed-off-by: Srinivas Kandagatla 
> Signed-off-by: John Stultz 
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7533.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c 
> b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index 5eebd15..6798ecf 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -29,6 +29,7 @@ static const struct reg_sequence 
> adv7533_cec_fixed_registers[] = {
>   { 0x17, 0xd0 },
>   { 0x24, 0x20 },
>   { 0x57, 0x11 },
> + { 0x05, 0xc8 },
>  };
>
>  static const struct regmap_config adv7533_cec_regmap_config = {
>

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

[PATCH 01/11] drm/amdgpu: Remove call to reservation_object_test_signaled_rcu before wait

[Daniel Vetter]

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

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

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

[PATCH 1/2 v4] drm/bridge: adv7511: Add Audio support.

[Archit Taneja]

On 09/17/2016 04:47 AM, John Stultz wrote:
> This patch adds support to Audio for both adv7511 and adv7533
> bridge chips.
>
> This patch was originally from [1] by Lars-Peter Clausen 
> and was adapted by Archit Taneja  and
> Srinivas Kandagatla .
>
> Then I heavily reworked it to use the hdmi-codec driver. And also
> folded in some audio packet initialization done by Andy Green
> . So credit to them, but blame to me.
>
> [1] 
> https://github.com/analogdevicesinc/linux/blob/xcomm_zynq/drivers/gpu/drm/i2c/adv7511_audio.c
>
> Cc: David Airlie 
> Cc: Archit Taneja 
> Cc: Laurent Pinchart 
> Cc: Wolfram Sang 
> Cc: Srinivas Kandagatla 
> Cc: "Ville Syrjälä" 
> Cc: Boris Brezillon 
> Cc: Andy Green 
> Cc: Dave Long 
> Cc: Guodong Xu 
> Cc: Zhangfei Gao 
> Cc: Mark Brown 
> Cc: Lars-Peter Clausen 
> Cc: Jose Abreu 
> Cc: dri-devel at lists.freedesktop.org
> Acked-by: Lars-Peter Clausen 
> Signed-off-by: John Stultz 

Reviewed-by: Archit Taneja 

> ---
> v4:
> * Kconfig tweaks suggested by Lars-Peter Clausen
> v3:
> * Allowed audio support to be configured in or out, as suggested by Laurent
> * Minor cleanups suggested by Laurent
> * Folded in Andy's audio packet initialization patch, as suggested by Archit
>
> drivers/gpu/drm/bridge/adv7511/Kconfig |   8 +
>  drivers/gpu/drm/bridge/adv7511/Makefile|   1 +
>  drivers/gpu/drm/bridge/adv7511/adv7511.h   |  16 ++
>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 213 
> +
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   4 +
>  5 files changed, 242 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig 
> b/drivers/gpu/drm/bridge/adv7511/Kconfig
> index d2b0499..2fed567 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig
> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
> @@ -6,6 +6,14 @@ config DRM_I2C_ADV7511
>   help
> Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
>
> +config DRM_I2C_ADV7511_AUDIO
> + bool "ADV7511 HDMI Audio driver"
> + depends on DRM_I2C_ADV7511 && SND_SOC
> + select SND_SOC_HDMI_CODEC
> + help
> +   Support the ADV7511 HDMI Audio interface. This is used in
> +   conjunction with the AV7511  HDMI driver.
> +
>  config DRM_I2C_ADV7533
>   bool "ADV7533 encoder"
>   depends on DRM_I2C_ADV7511
> diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile 
> b/drivers/gpu/drm/bridge/adv7511/Makefile
> index 9019327..5ba6755 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Makefile
> +++ b/drivers/gpu/drm/bridge/adv7511/Makefile
> @@ -1,3 +1,4 @@
>  adv7511-y := adv7511_drv.o
> +adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o
>  adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index 161c923..992d76c 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -309,6 +309,8 @@ struct adv7511 {
>   struct drm_display_mode curr_mode;
>
>   unsigned int f_tmds;
> + unsigned int f_audio;
> + unsigned int audio_source;
>
>   unsigned int current_edid_segment;
>   uint8_t edid_buf[256];
> @@ -334,6 +336,7 @@ struct adv7511 {
>   bool use_timing_gen;
>
>   enum adv7511_type type;
> + struct platform_device *audio_pdev;
>  };
>
>  #ifdef CONFIG_DRM_I2C_ADV7533
> @@ -389,4 +392,17 @@ static inline int adv7533_parse_dt(struct device_node 
> *np, struct adv7511 *adv)
>  }
>  #endif
>
> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
> +int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511);
> +void adv7511_audio_exit(struct adv7511 *adv7511);
> +#else /*CONFIG_DRM_I2C_ADV7511_AUDIO */
> +static inline int adv7511_audio_init(struct device *dev, struct adv7511 
> *adv7511)
> +{
> + return 0;
> +}
> +static inline void adv7511_audio_exit(struct adv7511 *adv7511)
> +{
> +}
> +#endif /* CONFIG_DRM_I2C_ADV7511_AUDIO */
> +
>  #endif /* __DRM_I2C_ADV7511_H__ */
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c 
> b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> new file mode 100644
> index 000..5ce29a5
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> @@ -0,0 +1,213 @@
> +/*
> + * Analog Devices ADV7511 HDMI transmitter driver
> + *
> + * Copyright 2012 Analog Devices Inc.
> + * Copyright (c) 2016, Linaro Limited
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "adv7511.h"
> +
> +static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned int fs,
> +unsigned int *cts, unsigned int *n)
> +{
> + switch (fs) {
> + case 32000:
> + *n = 4096;
> + break;
> + case 44100:
> + *n = 6272;
> + break;
> + case 48000:
> + *n = 6144;
> 

[PATCH v2 2/4] drm/bridge: adv7511: Initialize regulators

[Archit Taneja]

Maintain a table of regulator names expect by ADV7511 and ADV7533.
Use regulator_bulk_* api to configure these.

Initialize and enable the regulators during probe itself. Controlling
these dynamically is left for later.

Cc: dri-devel at lists.freedesktop.org
Cc: Laurent Pinchart 
Signed-off-by: Archit Taneja 
---
v2:
- Use regulator_bulk_* API to configure regulators as suggested by Laurent.
- Set up regulators for ADV7511 too.

 drivers/gpu/drm/bridge/adv7511/adv7511.h |  4 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 85 +---
 2 files changed, 80 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 161c923..83ebdaa 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -327,6 +328,9 @@ struct adv7511 {

struct gpio_desc *gpio_pd;

+   struct regulator_bulk_data *supplies;
+   int num_supplies;
+
/* ADV7533 DSI RX related params */
struct device_node *host_node;
struct mipi_dsi_device *dsi;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 8ed3906..f7e79ed 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -839,6 +839,57 @@ static struct drm_bridge_funcs adv7511_bridge_funcs = {
  * Probe & remove
  */

+static const char * const adv7511_supply_names[] = {
+   "avdd",
+   "dvdd",
+   "pvdd",
+   "v3p3",
+   "bgvdd",
+};
+
+static const char * const adv7533_supply_names[] = {
+   "avdd",
+   "dvdd",
+   "pvdd",
+   "a2vdd",
+   "v3p3",
+   "v1p2",
+};
+
+static int adv7511_init_regulators(struct adv7511 *adv)
+{
+   struct device *dev = &adv->i2c_main->dev;
+   const char * const *supply_names;
+   int i, ret;
+
+   if (adv->type == ADV7511) {
+   supply_names = adv7511_supply_names;
+   adv->num_supplies = ARRAY_SIZE(adv7511_supply_names);
+   } else {
+   supply_names = adv7533_supply_names;
+   adv->num_supplies = ARRAY_SIZE(adv7533_supply_names);
+   }
+
+   adv->supplies = devm_kcalloc(dev, adv->num_supplies,
+sizeof(*adv->supplies), GFP_KERNEL);
+   if (!adv->supplies)
+   return -ENOMEM;
+
+   for (i = 0; i < adv->num_supplies; i++)
+   adv->supplies[i].supply = supply_names[i];
+
+   ret = devm_regulator_bulk_get(dev, adv->num_supplies, adv->supplies);
+   if (ret)
+   return ret;
+
+   return regulator_bulk_enable(adv->num_supplies, adv->supplies);
+}
+
+static void adv7511_uninit_regulators(struct adv7511 *adv)
+{
+   regulator_bulk_disable(adv->num_supplies, adv->supplies);
+}
+
 static int adv7511_parse_dt(struct device_node *np,
struct adv7511_link_config *config)
 {
@@ -939,6 +990,7 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
if (!adv7511)
return -ENOMEM;

+   adv7511->i2c_main = i2c;
adv7511->powered = false;
adv7511->status = connector_status_disconnected;

@@ -956,13 +1008,21 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
if (ret)
return ret;

+   ret = adv7511_init_regulators(adv7511);
+   if (ret) {
+   dev_err(dev, "failed to init regulators\n");
+   return ret;
+   }
+
/*
 * The power down GPIO is optional. If present, toggle it from active to
 * inactive to wake up the encoder.
 */
adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
-   if (IS_ERR(adv7511->gpio_pd))
-   return PTR_ERR(adv7511->gpio_pd);
+   if (IS_ERR(adv7511->gpio_pd)) {
+   ret = PTR_ERR(adv7511->gpio_pd);
+   goto uninit_regulators;
+   }

if (adv7511->gpio_pd) {
mdelay(5);
@@ -970,12 +1030,14 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
}

adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
-   if (IS_ERR(adv7511->regmap))
-   return PTR_ERR(adv7511->regmap);
+   if (IS_ERR(adv7511->regmap)) {
+   ret = PTR_ERR(adv7511->regmap);
+   goto uninit_regulators;
+   }

ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
if (ret)
-   return ret;
+   goto uninit_regulators;
dev_dbg(dev, "Rev. %d\n", val);

if (adv7511->type == ADV7511)
@@ -985,7 +1047,7 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
else
ret = adv7533_

[PATCH v4 12/14] drm: vmwgfx: Replace drm_fb_get_bpp_depth() with drm_format_info()

[Daniel Vetter]

On Fri, Sep 23, 2016 at 03:40:17PM +0300, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Wednesday 21 Sep 2016 09:31:58 Daniel Vetter wrote:
> > On Thu, Sep 08, 2016 at 05:44:26PM +0300, Laurent Pinchart wrote:
> > > The driver is the last users of the drm_fb_get_bpp_depth() function. It
> > > should ideally be converted to use struct drm_mode_fb_cmd2 instead of
> > > the legacy struct drm_mode_fb_cmd internally, but that will require
> > > broad changes across the code base. As a first step, replace
> > > drm_fb_get_bpp_depth() with drm_format_info() in order to stop exporting
> > > the function to drivers.
> > > 
> > > The new DRM_ERROR() message comes from the vmw_create_dmabuf_proxy(),
> > > vmw_kms_new_framebuffer_surface() and vmw_kms_new_framebuffer_dmabuf()
> > > functions that currently print an error if the pixel format is
> > > unsupported.
> > > 
> > > Signed-off-by: Laurent Pinchart 
> > > Reviewed-by: Sinclair Yeh 
> > > ---
> > > 
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > Cc: VMware Graphics 
> > > Cc: Sinclair Yeh 
> > > Cc: Thomas Hellstrom 
> > > 
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index bf28ccc150df..c965514b82be
> > > 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > @@ -980,14 +980,22 @@ static struct drm_framebuffer
> > > *vmw_kms_fb_create(struct drm_device *dev,> 
> > >   struct vmw_dma_buffer *bo = NULL;
> > >   struct ttm_base_object *user_obj;
> > >   struct drm_mode_fb_cmd mode_cmd;
> > > 
> > > + const struct drm_format_info *info;
> > > 
> > >   int ret;
> > > 
> > > + info = drm_format_info(mode_cmd2->pixel_format);
> > > + if (!info || !info->depth) {
> > > + DRM_ERROR("Unsupported framebuffer format %s\n",
> > > +   drm_get_format_name(mode_cmd2->pixel_format));
> > > + return ERR_PTR(-EINVAL);
> > > + }
> > > +
> > > 
> > >   mode_cmd.width = mode_cmd2->width;
> > >   mode_cmd.height = mode_cmd2->height;
> > >   mode_cmd.pitch = mode_cmd2->pitches[0];
> > >   mode_cmd.handle = mode_cmd2->handles[0];
> > > 
> > > - drm_fb_get_bpp_depth(mode_cmd2->pixel_format, &mode_cmd.depth,
> > > - &mode_cmd.bpp);
> > > + mode_cmd.depth = info->depth;
> > > + mode_cmd.bpp = info->cpp[0] * 8;
> > 
> > I think this should use drm_helper_mode_fill_fb_struct instead.
> 
> I would do that if there was a struct drm_framebuffer to fill, but this piece 
> of code converts from struct drm_mode_fb_cmd2 to drm_mode_fb_cmd that is then 
> used all over the place internally. This should be fixed, but I think that's 
> out of scope for this patch series.

Oh right, I didn't realize that ...

Reviewed-by: Daniel Vetter 

> 
> > >   /**
> > >   
> > >* This code should be conditioned on Screen Objects not being used.
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

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

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

[Tomi Valkeinen]

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

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

 Tomi

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160923/ed1ad629/attachment.sig>

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

[Gustavo Padovan]

2016-09-22 Christian König :

> Am 22.09.2016 um 13:16 schrieb Gustavo Padovan:
> > 2016-09-22 Christian König :
> > 
> > > Dropping the rest of the patch, cause that really doesn't make sense any
> > > more.
> > > 
> > > Am 22.09.2016 um 12:40 schrieb Gustavo Padovan:
> > > > > E.g. for example it is illegal to do something like
> > > > > > "while(!fence_is_signaled(f)) sleep();" without enabling signaling 
> > > > > > before
> > > > > > doing this.
> > > > > > 
> > > > > > Could just be a misunderstanding, but the comments on your patch 
> > > > > > actually
> > > > > > sounds a bit like somebody is trying to do exactly that.
> > > > I think the usecase in mind here is poll(fd, timeout=0)
> > > Exactly as I feared. Even if userspace polls with timeout=0 you still need
> > > to call enable_signaling().
> > > 
> > > Otherwise you can run into a situation where userspace only uses timeout=0
> > > and so never activates the signaling check in the driver.
> > > 
> > > This would in turn result in an endless loop on implementations where the
> > > driver never signals fences on their own.
> > Polling is optional, userspace may never call it. And DRM/KMS or GPU
> > drivers will be doing fence_wait() themselves so signaling is enabled at
> > some point.
> 
> No they won't. We have an use case where we clearly want to avoid that as
> much as possible because and so the driver never calls enable_signaling() on
> it's own.
> 
> Exposing this poll function to userspace without enabling signaling is a
> clear NAK from my side.

Okay. So you are NAK'ing the does_not_pool_wait change. Should we revert
that one then? It is already broken.

Gustavo

GPU-DRM-QXL: Move three assignments in qxl_device_init()

[Emil Velikov]

On 23 September 2016 at 09:50, SF Markus Elfring
 wrote:
>> Markus, please contact the list in advance in future before posting a bunch
>> of patches that don't fix any problems.
>
> I am trying to improve various open issues also in Linux source files.
>
That the fact that you see issues (in these particular cases) while
others do not indicates that the commit summary could be explained
better.

A good commit summary should provide enough information to do that and
make people _want_ the patch. From my limited experience through your
patches (just skimmed a few) you seems to be describing what the patch
does as opposed to why it does it and why should one find it
interesting/wanted.

You might want to read through the following [1] [2] and many others
that exist out there.

-Emil

[1] http://chris.beams.io/posts/git-commit/
[2] http://who-t.blogspot.fi/2009/12/on-commit-messages.html

GPU-DRM-TILCDC: Less function calls in tilcdc_convert_slave_node() after error detection

[SF Markus Elfring]

>> I see a need to improve not only correctness there but also a bit of
>> software efficiency.
> 
> If you can measure any performance difference and present some results
> (esp. considering that this is something that just happens when the
> driver is loaded), then we'll talk.

Are you really interested to increase your software development attention
for a quicker exception handling implementation in this function?


> Until then, please don't send this sort of patch.  Thank you.

Will benchmark statistics change such a change rejection ever?

Regards,
Markus

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

[Daniel Vetter]

On Fri, Sep 23, 2016 at 07:00:25PM +0900, Michel Dänzer wrote:
> On 22/09/16 10:22 PM, Christian König wrote:
> > Am 22.09.2016 um 15:05 schrieb Daniel Vetter:
> >> On Thu, Sep 22, 2016 at 2:44 PM, Christian König
> >>  wrote:
>  - explicit fencing: Userspace passes around distinct fence objects for
>  any work going on on the gpu. The kernel doesn't insert any stall of
>  it's own (except for moving buffer objects around ofc). This is what
>  Android. This also seems to be what amdgpu is doing within one
>  process/owner.
> >>>
> >>> No, that is clearly not my understanding of explicit fencing.
> >>>
> >>> Userspace doesn't necessarily need to pass around distinct fence objects
> >>> with all of it's protocols and the kernel is still responsible for
> >>> inserting
> >>> stalls whenever an userspace protocol or application requires this
> >>> semantics.
> >>>
> >>> Otherwise you will never be able to use explicit fencing on the Linux
> >>> desktop with protocols like DRI2/DRI3.
> >> This is about mixing them. Explicit fencing still means userspace has
> >> an explicit piece, separate from buffers, (either sync_file fd, or a
> >> driver-specific cookie, or similar).
> >>
> >>> I would expect that every driver in the system waits for all fences of a
> >>> reservation object as long as it isn't told otherwise by providing a
> >>> distinct fence object with the IOCTL in question.
> >> Yup agreed. This way if your explicitly-fencing driver reads a shared
> >> buffer passed over a protocol that does implicit fencing (like
> >> DRI2/3), then it will work.
> >>
> >> The other interop direction is explicitly-fencing driver passes a
> >> buffer to a consumer which expects implicit fencing. In that case you
> >> must attach the right fence to the exclusive slot, but _only_ in that
> >> case.
> > 
> > Ok well sounds like you are close to understand why I can't do exactly
> > this: There simply is no right fence I could attach.
> > 
> > When amdgpu makes the command submissions it doesn't necessarily know
> > that the buffer will be exported and shared with another device later on.
> > 
> > So when the buffer is exported and given to the other device you might
> > have a whole bunch of fences which run concurrently and not in any
> > serial order.
> 
> I feel like you're thinking too much of buffers shared between GPUs as
> being short-lived and only shared late. In the use-cases I know about,
> shared buffers are created separately and shared ahead of time, the
> actual rendering work is done to non-shared buffers and then just copied
> to the shared buffers for transfer between GPUs. These copies are always
> performed by the same context in such a way that they should always be
> performed by the same HW engine and thus implicitly serialized.
> 
> Do you have any specific use-cases in mind where buffers are only shared
> between GPUs after the rendering operations creating the buffer contents
> to be shared have already been submitted?

Yeah, it should be known which buffer you use (at least in userspace,
maybe not in the kernel) for which you need implicit fencing. At least
with DRI2/3 it's really obvious which buffers are shared. Same holds for
external images and other imported buffers.

Yes that means you need to keep track of a few things in userspace, and
you need to add a special flag to CS to make sure the kernel does set the
exclusive fence.

> >> Otherwise you end up stalling your explicitly-fencing userspace,
> >> since implicit fencing doesn't allow more than 1 writer. For amdgpu
> >> one possible way to implement this might be to count how many users a
> >> dma-buf has, and if it's more than just the current context set the
> >> exclusive fence. Or do an uabi revision and let userspace decide (or
> >> at least overwrite it).
> > 
> > I mean I can pick one fence and wait for the rest to finish manually,
> > but that would certainly defeat the whole effort, doesn't it?
> 
> I'm afraid it's not clear to me why it would. Can you elaborate?
> 
> 
> >> But the current approach in amdgpu_sync.c of declaring a fence as
> >> exclusive after the fact (if owners don't match) just isn't how
> >> reservation_object works. You can of course change that, but that
> >> means you must change all drivers implementing support for implicit
> >> fencing of dma-buf. Fixing amdgpu will be easier ;-)
> > 
> > Well as far as I can see there is no way I can fix amdgpu in this case.
> > 
> > The handling clearly needs to be changed on the receiving side of the
> > reservation objects if I don't completely want to disable concurrent
> > access to BOs in amdgpu.
> 
> Anyway, we need a solution for this between radeon and amdgpu, and I
> don't think a solution which involves those drivers using reservation
> object semantics between them which are different from all other drivers
> is a good idea.

Afaik there's also amd+intel machines out there, so really the only option
is to either fix amdgpu to correctly 

[ADV7393] DRM Encoder Slave or DRM Bridge

[Tomi Valkeinen]

On 23/09/16 13:08, Vikas Patil wrote:
> Hi Tomi,
> 
> I added the missing check for "OMAP_DISPLAY_TYPE_VENC" in function
> omap_connector_detect @ gpu/drm/omapdrm/omap_connector.c and now
> modetest  seems to be showing correct status and connections.

Is there a cable detection support in the ADV hardware & driver? If not,
then the cable connection status is "unknown". It should still work if
the output is enabled manually. I think. I don't have any boards with
analog tv out..

But yes, your change is an easy hack to force the output on.

> But still I could not see kmscube on panel and can see some flicker is
> going on display. I think I need to understand about what display
> timing I could use as interlace doesn't seems to be supported as I
> mentioned above.

Yes, sounds like a video timings issue.

 Tomi

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160923/51340f52/attachment.sig>

[ADV7393] DRM Encoder Slave or DRM Bridge

[Tomi Valkeinen]

On 22/09/16 16:22, Vikas Patil wrote:

> Could you help me to understand if I could use “interlace=false”?
> ADV7393 seems to be supporting non-interlaced mode. From datasheet:
> “The ADV7390/ADV7391/ADV7392/ADV7393 support an SD noninterlaced mode.
> Using this mode, progressive inputs at twice the frame rate of NTSC
> and PAL (240p/59.94 Hz and 288p/50 Hz, respectively) can be input into
> the ADV7390/ ADV7391/ADV7392/ADV7393. The SD noninterlaced mode can be
> enabled using Subaddress 0x88, Bit 1.”

Difficult to say... So OMAP4+ DSS hardware does support interlace output
for DPI. The driver has never supported it, nor do I have any hardware
to test it. It might be quite easy to add, though.

If I read the above snippet right, to use progressive input, the DISPC
needs to output at double refresh rate. So probably what you would have
to do is in ADV driver, you have your set_timings function, where you
should double the pix clock before you pass the timings forward to the
DISPC's DPI driver.

 Tomi

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160923/268bba3d/attachment.sig>

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

[Brian Starkey]

On Fri, Sep 23, 2016 at 12:58:46PM +0200, Daniel Vetter wrote:
>On Fri, Sep 23, 2016 at 11:34 AM, Liviu Dudau  wrote:
>> rmmod-ing the hdlcd module generates a WARN() splat as the vsync is still
>> enabled, but we never got the call to turn off the CRTC. Brian is still
>> tracking through the fbdev emulation to figure out the cause for that.
>
>fbdev emulation doesn't do that for you. If you need/want to shut down
>all the crtcs on driver unload, you need to do that yourself. There's
>atomic helpers to do that for you that for you.

The problem is a sort-of circular dependency between ->lastclose (at
least the common implementation of it), unregister and disabling
fbdev.

I want to move drm_dev_unregister() to be the first thing we do at
rmmod-time. However we need to disable fbdev first, otherwise
->lastclose restores the fbdev mode, guaranteeing that vsync is turned
on for drm_vsync_cleanup() to then WARN on.

There's a slightly different (perceived) problem - the one that Liviu
mentions - that drm_fbdev_cma_fini() doesn't disable the CRTC anyway.
You say it's not the fbdev helpers' responsibility to teardown their
modeset, but regardless I have nowhere to disable the CRTC if I want
to do drm_dev_unregister() first; and if the CRTC isn't disabled
there's always a chance of hitting the same vsync WARN even without
fbdev.

We *could* add an ->unload and disable everything there, but as that's
deprecated I'm guessing there should be another way.
Perhaps we should track ->firstopen/->lastclose pairs so we can detect
that ->lastclose is being called from unregister and use it to
disable everything in that case.

drm_vblank_cleanup() seems to have been carried over to unregister
from drm_put_dev(), but drm_dev_register() doesn't call
drm_vblank_init() so it seems a little strange to have it there.
I can see other drivers I'd expect to hit the same WARN but I don't
have HW to test it on.

Thanks,
Brian

>-Daniel
>-- 
>Daniel Vetter
>Software Engineer, Intel Corporation
>+41 (0) 79 365 57 48 - http://blog.ffwll.ch
>

[PATCH] drm/udl: fix line iterator in damage handling

[Eric Engestrom]

On Fri, Sep 23, 2016 at 12:36:02PM +0200, David Herrmann wrote:
> The udl damage handler is supposed to render 'height' lines, but its
> iterator has an obvious typo that makes it miss most lines if the
> rectangle does not cover 0/0.
> 
> Fix the damage handler to correctly render all lines.
> 
> This is a fallout from:
> 
> commit e375882406d0cc24030746638592004755ed4ae0
> Author: Noralf Trønnes 
> Date:   Thu Apr 28 17:18:37 2016 +0200
> 
> drm/udl: Use drm_fb_helper deferred_io support
> 
> Tested-by: poma 
> Reviewed-by: Daniel Vetter 
> Signed-off-by: David Herrmann 

Reviewed-by: Eric Engestrom 

> ---
>  drivers/gpu/drm/udl/udl_fb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
> index 9688bfa..611b6b9 100644
> --- a/drivers/gpu/drm/udl/udl_fb.c
> +++ b/drivers/gpu/drm/udl/udl_fb.c
> @@ -122,7 +122,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, 
> int y,
>   return 0;
>   cmd = urb->transfer_buffer;
>  
> - for (i = y; i < height ; i++) {
> + for (i = y; i < y + height ; i++) {
>   const int line_offset = fb->base.pitches[0] * i;
>   const int byte_offset = line_offset + (x * bpp);
>   const int dev_byte_offset = (fb->base.width * bpp * i) + (x * 
> bpp);
> -- 
> 2.10.0

GPU-DRM-TTM: Fine-tuning for several function implementations

[SF Markus Elfring]

> Calling the label "unlock" instead of "out" is arguable a little better,

Thanks that you can follow a renaming for this direction in principle.


> but nothing I would call a major improvement either.

This was not my intention for such an use case.

I am proposing some small software updates according to such a design pattern.


> So that is a clear NAK to all those patches.

Do you reject also update steps like the following then?

* drm/ttm: Use kmalloc_array() in two (or four?) functions"

* drm/ttm: Less function calls in ttm_dma_pool_init() after error detection

* Would you like to improve the usage of the variables "n" and "t"
  in the function "ttm_dma_pool_init" any further as Joe Perches suggested it?


>> 2. How do you think about to add a single space character before any label?
> 
> Bad as well. Why would anybody want to do this?

Do you find another software evolution interesting according to a recent commit?

"docs: Remove space-before-label guidance from CodingStyle"
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/Documentation/CodingStyle?id=79c70c304b0b443429b2a0019518532c5162817a


Regards,
Markus

Please ignore this: [GIT PULL] tilcdc 3rd set of changes for v4.9

[Jyri Sarha]

Dave,
Please ignore this pull request. I'll send another soon.

The "drm/tilcdc: Add atomic and crtc headers to crtc.c" is already
coming trough drm-misc.

Best regards,
Jyri

On 09/23/16 00:37, Jyri Sarha wrote:
> Hi Dave,
> Please pull these collected fixes and cleanups from various sources. The
> request was rebased on top the previous pull tilcdc request tag, so it
> contains all the accumulated tilcdc changes for v4.9 so far.
> 
> Thanks,
> Jyri
> 
> The following changes since commit 2e0965b06d90b9dba76198d026c4c2ee04443aca:
> 
>   drm/tilcdc: WARN if CRTC is touched without CRTC lock (2016-09-07
> 15:54:43 +0300)
> 
> are available in the git repository at:
> 
>   https://github.com/jsarha/linux tags/tilcdc-4.9-3
> 
> for you to fetch changes up to 55fd61dfb8e37e27dcd44503f9ac3d676a5c3896:
> 
>   drm/tilcdc: Return directly after a failed kfree_table_init() in
> tilcdc_convert_slave_node() (2016-09-23 00:09:46 +0300)
> 
> 
> 3rd drm/tilcdc pull request for v4.9.
> 
> 
> Baoyou Xie (2):
>   drm/tilcdc: add missing header dependencies
>   drm/tilcdc: mark symbols static where possible
> 
> Jyri Sarha (1):
>   drm/tilcdc: Remove "default" from blue-and-red-wiring property binding
> 
> Markus Elfring (1):
>   drm/tilcdc: Return directly after a failed kfree_table_init() in
> tilcdc_convert_slave_node()
> 
> Sean Paul (1):
>   drm/tilcdc: Add atomic and crtc headers to crtc.c
> 
> Wei Yongjun (1):
>   drm/tilcdc: Fix non static symbol warning
> 
>  Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt | 6 +++---
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 6 --
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c | 4 ++--
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c   | 1 +
>  drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c| 8 
>  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c  | 1 +
>  6 files changed, 15 insertions(+), 11 deletions(-)
> 

[PATCH] drm/tilcdc: Add atomic and crtc headers to crtc.c

[Jyri Sarha]

On 09/23/16 09:23, Sean Paul wrote:
> On Thu, Sep 22, 2016 at 2:04 PM, Jyri Sarha  wrote:
>> On 09/22/16 09:18, Daniel Vetter wrote:
>>> On Wed, Sep 21, 2016 at 06:36:28AM -0700, Sean Paul wrote:
 Also reorder alphabetically and fix up drm_flip_work header.

 Signed-off-by: Sean Paul 
>>>
>>> Reviewed-by: Daniel Vetter 
>>>
>>
>> Picked this up. Thanks!
>>
> 
> Hi Jyri,
> I already picked this up through drm-misc to preface Daniel's clean-up
> patch series.
> 
> Could you drop this from your tree and we'll take it through -misc?
> 

Argh I sent the pull request already. However, Dave has not pulled it
yet, so I'll make another and ask him the ignore one with your patch.

BR,
Jyri

> Sean
> 
> 
>> Best regards,
>> Jyri
>>
 ---
  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
 b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
 index 2087689..cb9df10 100644
 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
 +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
 @@ -15,9 +15,11 @@
   * this program.  If not, see .
   */

 -#include "drm_flip_work.h"
 -#include 
 +#include 
  #include 
 +#include 
 +#include 
 +#include 

  #include "tilcdc_drv.h"
  #include "tilcdc_regs.h"
 --
 2.8.0.rc3.226.g39d4020

>>>
>>

GPU-DRM-TILCDC: Less function calls in tilcdc_convert_slave_node() after error detection

[Jyri Sarha]

On 09/23/16 10:36, SF Markus Elfring wrote:
>> I think the "if (node)" in the of_node_put() is there on purpose,
> 
> Yes, of course.
> 
> Does such an implementation detail correspond to a general software design 
> pattern?
> 

Yes it does. For instance standard malloc()/free() implementation [1].

> 
>> because it potentially saves the caller one extra if()-statement
> 
> This can occasionally happen.
> 
> 
>> and keeps the caller code simpler.
> 
> A special view on software simplicity can also lead to questionable 
> intermediate
> function implementation, can't it?
> 

I don't really follow. But in any case I do not see anything
questionable in the current tilcdc_convert_slave_node() implementation.

> 
>> Keeping the goto labels in right order needs precision
> 
> I can agree to this view.
> 
> 
>> and can lead to subtle errors.
> 
> The management of jump labels is just another software development challenge
> as usual, isn't it?
> 

Yes. But usually it pays of to avoid complexity when possible.

> 
>> Sometimes there is no way to avoid that,
> 
> How do you think about to clarify the constraints which you imagine a bit 
> more?
> 

If the the of_node_put() behaviour would not be specified with null
pointer as parameter, there would be such a constraint.

I am beginning to have a feeling that this discussion is not going anywhere.

> 
>> but here there is.
> 
> I disagree to this conclusion.
> 
> Would you like to care a bit more for efficiency and software correctness
> around the discussed exception handling?
> 

No, I would not. I think we have reached the bottom of this discussion.
For the moment I have more important tasks to do.

Best regards,
Jyri

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/free.html

GPU-DRM-TILCDC: Less function calls in tilcdc_convert_slave_node() after error detection

[SF Markus Elfring]

> iirc, there are Coccinelle rules that find code with unnecessary null
> checks and removes them.

This kind of software change is not needed here.

I find that a corresponding return value check happens one function call
too late.


> Although you probably made this complex enough that cocinelle would
> not find it.  That is not a complement.

I imagine that scripts for the semantic patch language can find more
source code places where questionable disjunctions are used so far.
Would you dare to split any more condition checks?


> One should not make error handling/cleanup more complex than needed.

I see a need to improve not only correctness there but also a bit of
software efficiency.

Regards,
Markus

GPU-DRM-TTM: Fine-tuning for several function implementations

[Christian König]

Am 23.09.2016 um 13:07 schrieb SF Markus Elfring:
>> It's just the names like "out" or "restart" perfectly explain why the labels 
>> exists.
> I have got an other impression.
>
>
>> So they fulfill this requirement from the coding style as far as I can see.
> Short identifiers might look more convenient in some cases because
> they are quicker to type.
>
>
>> So why do you want to change them?
> 1. I suggest to select identifiers also for jump labels which are more 
> meaningful
> and eventually unique for some function implementations.

I completely disagree. A longer identifier is not necessarily more 
meaningful than a shorter one.

The difference between calling a label "retry" and "lock_retry" is 
negligible, doesn't improve readability as far as I can see and is 
actually incorrect because the main meaning of the label is that we 
don't take the lock but rather that we restart the allocation operation.

Calling the label "unlock" instead of "out" is arguable a little better, 
but nothing I would call a major improvement either.

So that is a clear NAK to all those patches.

> 2. How do you think about to add a single space character before any label?

Bad as well. Why would anybody want to do this?

Christian.

>
> Regards,
> Markus

GPU-DRM-TTM: Fine-tuning for several function implementations

[SF Markus Elfring]

> It's just the names like "out" or "restart" perfectly explain why the labels 
> exists.

I have got an other impression.


> So they fulfill this requirement from the coding style as far as I can see.

Short identifiers might look more convenient in some cases because
they are quicker to type.


> So why do you want to change them?

1. I suggest to select identifiers also for jump labels which are more 
meaningful
   and eventually unique for some function implementations.

2. How do you think about to add a single space character before any label?

Regards,
Markus

[RFC PATCH v3 2/2] drm/panel: Add support for Chunghwa CLAA070WP03XG panel

[Rob Herring]

On Tue, Sep 20, 2016 at 03:02:51AM +0800, Randy Li wrote:
> The Chunghwa CLAA070WP03XG is a 7" 1280x800 panel, which can be
> supported by the simple panel driver.
> 
> Signed-off-by: Randy Li 
> ---
>  .../display/panel/chunghwa,claa070wp03xg.txt   |  7 ++
>  drivers/gpu/drm/panel/panel-simple.c   | 27 
> ++
>  2 files changed, 34 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/chunghwa,claa070wp03xg.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/chunghwa,claa070wp03xg.txt 
> b/Documentation/devicetree/bindings/display/panel/chunghwa,claa070wp03xg.txt
> new file mode 100644
> index 000..dd22685
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/display/panel/chunghwa,claa070wp03xg.txt
> @@ -0,0 +1,7 @@
> +Chunghwa Picture Tubes Ltd. 7" WXGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "chunghwa,claa070wp03xg"

What about VCC, VLED, LED_EN (PWM), LVBIT, LVFMT, DITHER signals?

> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.

[PATCH] drm/udl: fix udl_handle_damage

[Daniel Vetter]

Noralf accidentally made a small mistake for real subrectangles in his
patch to convert udl to the shared fb dirty handling code.

Fixes: e375882406d0 ("drm/udl: Use drm_fb_helper deferred_io support")
Cc:  # v4.7+
Cc: Noralf Trønnes 
Cc: Gerd Hoffmann 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/udl/udl_fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 9688bfa92ccd..611b6b9bb3cb 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -122,7 +122,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, 
int y,
return 0;
cmd = urb->transfer_buffer;

-   for (i = y; i < height ; i++) {
+   for (i = y; i < y + height ; i++) {
const int line_offset = fb->base.pitches[0] * i;
const int byte_offset = line_offset + (x * bpp);
const int dev_byte_offset = (fb->base.width * bpp * i) + (x * 
bpp);
-- 
2.7.4

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

[Daniel Vetter]

On Fri, Sep 23, 2016 at 11:34 AM, Liviu Dudau  wrote:
> rmmod-ing the hdlcd module generates a WARN() splat as the vsync is still
> enabled, but we never got the call to turn off the CRTC. Brian is still
> tracking through the fbdev emulation to figure out the cause for that.

fbdev emulation doesn't do that for you. If you need/want to shut down
all the crtcs on driver unload, you need to do that yourself. There's
atomic helpers to do that for you that for you.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

GPU-DRM-TILCDC: Less function calls in tilcdc_convert_slave_node() after error detection

[SF Markus Elfring]

>> A special view on software simplicity can also lead to questionable 
>> intermediate
>> function implementation, can't it?
> 
> I don't really follow. But in any case I do not see anything
> questionable in the current tilcdc_convert_slave_node() implementation.

I identified update candidates there like the following.

1. Delayed checking for null pointers
…
if (!slave || !of_device_is_available(lcdc))
…

2. Usage of a single jump label for (too many?) cases
…
goto out;
…

   Can the corresponding exception handling become also a bit more efficient?


>> Would you like to care a bit more for efficiency and software correctness
>> around the discussed exception handling?
> 
> No, I would not.

Thanks for this information.

I hope that the software situation can also be improved around
this design aspect somehow.


> For the moment I have more important tasks to do.

I know also that various open issues are competing for your software
development attention as usual.

Regards,
Markus

[PATCH] drm/tilcdc: fix wrong error handling

[Daniel Schultz]

When 'component_bind_all' fails it should not try to unbind components
in the error handling. This will produce a null pointer kernel panic when
no component exist.

This patch changes the order of the error handling. Now, it will only
unbind components if the are bound. Otherwise, the module will jump to
an error label below.

Signed-off-by: Daniel Schultz 
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index d278093..d491610 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -315,13 +315,13 @@ fail_irq_uninstall:
 fail_vblank_cleanup:
drm_vblank_cleanup(dev);

-fail_mode_config_cleanup:
-   drm_mode_config_cleanup(dev);
-
 fail_component_cleanup:
if (priv->is_componentized)
component_unbind_all(dev->dev, dev);

+fail_mode_config_cleanup:
+   drm_mode_config_cleanup(dev);
+
 fail_external_cleanup:
tilcdc_remove_external_encoders(dev);

-- 
1.9.1

[PATCH] drm/exynos/fimd: add clock rate checking

[Andrzej Hajda]

In case of some platforms fimd clocks can be configured to
very low values, as a result refresh rate can be very low and
driver/drm-core will timeout waiting for vblanks, it will result
in premature removal of framebuffers and will cause oopses.
The patch adds atomic_check callback to fimd to prevent setting
such modes.

Reported-by: Tobias Jakobi 
Signed-off-by: Andrzej Hajda 
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 41 ++--
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c 
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index d472164..0e74999 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -198,6 +198,7 @@ struct fimd_context {
atomic_twait_vsync_event;
atomic_twin_updated;
atomic_ttriggering;
+   int clkdiv;

const struct fimd_driver_data *driver_data;
struct drm_encoder *encoder;
@@ -389,15 +390,18 @@ static void fimd_clear_channels(struct exynos_drm_crtc 
*crtc)
pm_runtime_put(ctx->dev);
 }

-static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
-   const struct drm_display_mode *mode)
+
+static int fimd_atomic_check(struct exynos_drm_crtc *crtc,
+   struct drm_crtc_state *state)
 {
-   unsigned long ideal_clk;
-   u32 clkdiv;
+   struct drm_display_mode *mode = &state->adjusted_mode;
+   struct fimd_context *ctx = crtc->ctx;
+   unsigned long ideal_clk, lcd_rate;
+   int clkdiv;

if (mode->clock == 0) {
-   DRM_ERROR("Mode has zero clock value.\n");
-   return 0xff;
+   DRM_INFO("Mode has zero clock value.\n");
+   return -EINVAL;
}

ideal_clk = mode->clock * 1000;
@@ -410,10 +414,23 @@ static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
ideal_clk *= 2;
}

+   lcd_rate = clk_get_rate(ctx->lcd_clk);
+   if (2 * lcd_rate < ideal_clk) {
+   DRM_INFO("sclk_fimd clock too low(%lu) for requested pixel 
clock(%lu)\n",
+lcd_rate, ideal_clk);
+   return -EINVAL;
+   }
+
/* Find the clock divider value that gets us closest to ideal_clk */
-   clkdiv = DIV_ROUND_CLOSEST(clk_get_rate(ctx->lcd_clk), ideal_clk);
+   clkdiv = DIV_ROUND_CLOSEST(lcd_rate, ideal_clk);
+   if (clkdiv >= 0x200) {
+   DRM_INFO("requested pixel clock(%lu) too low\n", ideal_clk);
+   return -EINVAL;
+   }
+
+   ctx->clkdiv = (clkdiv < 0x100) ? clkdiv : 0xff;

-   return (clkdiv < 0x100) ? clkdiv : 0xff;
+   return 0;
 }

 static void fimd_setup_trigger(struct fimd_context *ctx)
@@ -442,7 +459,7 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
struct drm_display_mode *mode = &crtc->base.state->adjusted_mode;
const struct fimd_driver_data *driver_data = ctx->driver_data;
void *timing_base = ctx->regs + driver_data->timing_base;
-   u32 val, clkdiv;
+   u32 val;

if (ctx->suspended)
return;
@@ -543,9 +560,8 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
if (ctx->driver_data->has_clksel)
val |= VIDCON0_CLKSEL_LCD;

-   clkdiv = fimd_calc_clkdiv(ctx, mode);
-   if (clkdiv > 1)
-   val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
+   if (ctx->clkdiv > 1)
+   val |= VIDCON0_CLKVAL_F(ctx->clkdiv - 1) | VIDCON0_CLKDIR;

writel(val, ctx->regs + VIDCON0);
 }
@@ -939,6 +955,7 @@ static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
.update_plane = fimd_update_plane,
.disable_plane = fimd_disable_plane,
.atomic_flush = fimd_atomic_flush,
+   .atomic_check = fimd_atomic_check,
.te_handler = fimd_te_handler,
 };

-- 
2.7.4

[PATCH v2] drm/panel: simple: add support for Sharp LQ150X1LG11 panels

[Rob Herring]

On Sat, Sep 17, 2016 at 11:34:22AM +0200, Peter Rosin wrote:
> From: Gustaf Lindström 
> 
> The Sharp 15" LQ150X1LG11 panel is an XGA TFT LCD panel.
> 
> Signed-off-by: Gustaf Lindström 
> Signed-off-by: Peter Rosin 
> ---
>  .../bindings/display/panel/sharp,lq150x1lg11.txt   |  7 ++
>  drivers/gpu/drm/panel/panel-simple.c   | 27 
> ++
>  2 files changed, 34 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
> 
> v1->v2: correct author
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt 
> b/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
> new file mode 100644
> index ..014428c984c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt
> @@ -0,0 +1,7 @@
> +Sharp 15" LQ150X1LG11 XGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "sharp,lq150x1lg11"

Looking at the spec, what about 12V VDD, 3.3V VCC, XSTABY (backlight 
ctrl), VBR (PWM), RL/UD, SELLVDS signals?

Rob

GPU-DRM-TTM: Fine-tuning for several function implementations

[Christian König]

Am 23.09.2016 um 12:20 schrieb SF Markus Elfring:
>> Additional to that I don't really see the point in renaming some of the jump 
>> labels,
> I am suggesting changes for another collateral software evolution.
>
>
>> if you call it "restart" or "lock_restart" doesn't make much difference.
> Do other identifiers fit better to a specification from the document 
> "CodingStyle"
> like the following?

No, not really.

>
> "…
> Choose label names which say what the goto does or why the goto exists.
> …"
>
>
> Does this wording need any more adjustments?

Of hand I can't find any better wording.

It's just the names like "out" or "restart" perfectly explain why the 
labels exists. So they fulfill this requirement from the coding style as 
far as I can see.

So why do you want to change them?

Regards,
Christian.

>
> Regards,
> Markus

[PATCH] drm/udl: fix line iterator in damage handling

[David Herrmann]

The udl damage handler is supposed to render 'height' lines, but its
iterator has an obvious typo that makes it miss most lines if the
rectangle does not cover 0/0.

Fix the damage handler to correctly render all lines.

This is a fallout from:

commit e375882406d0cc24030746638592004755ed4ae0
Author: Noralf Trønnes 
Date:   Thu Apr 28 17:18:37 2016 +0200

drm/udl: Use drm_fb_helper deferred_io support

Tested-by: poma 
Reviewed-by: Daniel Vetter 
Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/udl/udl_fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 9688bfa..611b6b9 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -122,7 +122,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, 
int y,
return 0;
cmd = urb->transfer_buffer;

-   for (i = y; i < height ; i++) {
+   for (i = y; i < y + height ; i++) {
const int line_offset = fb->base.pitches[0] * i;
const int byte_offset = line_offset + (x * bpp);
const int dev_byte_offset = (fb->base.width * bpp * i) + (x * 
bpp);
-- 
2.10.0

[PATCH] drm_aux-dev: fix error handling in drm_dp_aux_dev_init()

[Alexey Khoroshilov]

Dear Daniel,

Sorry for long response.

> Applied to drm-misc, thanks for the patch. Out of curiosity, how
> exactly did you spot this bug?

We use static analysis tools that check various rules on correct usage
of kernel core API. An example of error trace for the rm_dp_aux_dev case
you can see here [1].

Some addition details are described in a paper [2].

[1]
http://linuxtesting.org/results/impl_reports_admin?action=preview&num=137
[2]
http://www.ispras.ru/en/publications/configurable_toolset_for_static_verification_of_operating_systems_kernel_modules.pdf


Best regards,
Alexey


On 12.07.2016 14:11, Daniel Vetter wrote:
> On Thu, Jun 30, 2016 at 12:52:15AM +0300, Alexey Khoroshilov wrote:
>> If class_create() fails, there is no need for class_destroy().
>>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> Signed-off-by: Alexey Khoroshilov 
> 
> Applied to drm-misc, thanks for the patch. Out of curiosity, how exactly
> did you spot this bug?
> -Daniel
> 
>> ---
>>  drivers/gpu/drm/drm_dp_aux_dev.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c 
>> b/drivers/gpu/drm/drm_dp_aux_dev.c
>> index 3334baacf43d..734f86a345f6 100644
>> --- a/drivers/gpu/drm/drm_dp_aux_dev.c
>> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
>> @@ -355,8 +355,7 @@ int drm_dp_aux_dev_init(void)
>>  
>>  drm_dp_aux_dev_class = class_create(THIS_MODULE, "drm_dp_aux_dev");
>>  if (IS_ERR(drm_dp_aux_dev_class)) {
>> -res = PTR_ERR(drm_dp_aux_dev_class);
>> -goto out;
>> +return PTR_ERR(drm_dp_aux_dev_class);
>>  }
>>  drm_dp_aux_dev_class->dev_groups = drm_dp_aux_groups;
>>  
>> -- 
>> 1.9.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

[PATCH] drm: Fix plane type uabi breakage

[Archit Taneja]

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

Reviewed-by: Archit Taneja 

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

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

GPU-DRM-TTM: Fine-tuning for several function implementations

[SF Markus Elfring]

> Additional to that I don't really see the point in renaming some of the jump 
> labels,

I am suggesting changes for another collateral software evolution.


> if you call it "restart" or "lock_restart" doesn't make much difference.

Do other identifiers fit better to a specification from the document 
"CodingStyle"
like the following?

"…
Choose label names which say what the goto does or why the goto exists.
…"


Does this wording need any more adjustments?

Regards,
Markus

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

[Christian König]

First of all please stop sending your patches as a reply to an earlier 
and completely unrelated series.

Second please prefix all TTM related patches with "drm/ttm:".

Additional to that I don't really see the point in renaming some of the 
jump labels, if you call it "restart" or "lock_restart" doesn't make 
much difference.

Regards,
Christian.

Am 22.09.2016 um 19:32 schrieb SF Markus Elfring:
> From: Markus Elfring 
> Date: Thu, 22 Sep 2016 19:00:01 +0200
>
> Several update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (14):
>Use kmalloc_array() in two functions
>Rename a jump label in ttm_alloc_new_pages()
>Rename jump labels in ttm_page_pool_free()
>Rename a jump label in ttm_page_pool_get_pages()
>Use kmalloc_array() in two more functions
>Rename a jump label in ttm_dma_pool_alloc_new_pages()
>Rename jump labels in ttm_dma_page_pool_free()
>Rename a jump label in ttm_dma_pool_shrink_scan()
>Return directly after a failed kzalloc() in ttm_dma_page_alloc_init()
>Return directly after a failed kobject_init_and_add() in 
> ttm_dma_page_alloc_init()
>Return an error code only as a constant in ttm_dma_pool_init()
>Less function calls in ttm_dma_pool_init() after error detection
>Delete unnecessary variable initialisations in ttm_dma_pool_init()
>Mark an array of text strings as "const" in ttm_dma_pool_init()
>
>   drivers/gpu/drm/ttm/ttm_page_alloc.c | 30 -
>   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 58 
> +++-
>   2 files changed, 42 insertions(+), 46 deletions(-)
>

linux-next: manual merge of the drm-misc tree with Linus' tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/drm_crtc.c

between commit:

  6f00975c6190 ("drm: Reject page_flip for !DRIVER_MODESET")

from Linus' tree and commit:

  43968d7b806d ("drm: Extract drm_plane.[hc]")

from the drm-misc tree.

I fixed it up (the latter incorporated the former, so I just used the
latter) and can carry the fix as necessary. This is now fixed as far as
linux-next is concerned, but any non trivial conflicts should be mentioned
to your upstream maintainer when your tree is submitted for merging.
You may also want to consider cooperating with the maintainer of the
conflicting tree to minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

drm/exynos: fimd: pagefault when restarting X

[Andrzej Hajda]

Hi Tobias,

On 06.05.2016 09:48, Tobias Jakobi wrote:
> Hello Andrzej,
>
>
> Andrzej Hajda wrote:
>> Hi Tobias,
>>
>> On 05/05/2016 07:27 PM, Tobias Jakobi wrote:
>>> Hello,
>>>
>>> here's another issue I experience when enabling FIMD on the ODROID-X2.
>>>
>>> I can trigger a IOMMU pagefault by starting X once, quitting, and
>>> restarting X again.
>>>

Finally I have tested RGB output on similar target - Odroid-U3. It seems
it has set very low fimd clocks, as a result refresh rate is very low
(5-10 fps)
and vblank waiting code timeouts, as a result framebuffers are prematurely
removed.
Adding assigned-clocks to fimd node should probably help.
Anyway I will try to add check to fimd driver preventing such situations.

Regards
Andrzej

GPU-DRM-QXL: Move three assignments in qxl_device_init()

[SF Markus Elfring]

> Markus, please contact the list in advance in future before posting a bunch
> of patches that don't fix any problems.

I am trying to improve various open issues also in Linux source files.

Unfortunately, some of the proposed changes might not fit to your software
development attention at the moment.
How are the chances that corresponding change acceptance will evolve a bit more
after a while?

Regards,
Markus

GPU-DRM-QXL: Move three assignments in qxl_device_init()

[SF Markus Elfring]

> I will refrain from merging any more style/checkpatch/"code cleanup"
> patches from Markus until we start getting real, tested, bug fixes.

Can such a kind of feedback be also interpreted in the way that you insist
to keep some weaknesses which I tried to point in the Linux source code out
for another while?

How do you think about to clarify your ranking for various update candidates
in this software?

Regards,
Markus