[PATCH] drm: allocate crtc mutex separately from crtc

[Ilija Hadzic]

(dropping stable at ... until we get the fix we can agree on)

While looking through that function (drm_crtc_helper_set_config) to
figure out what we really need to save and restore, I came across a
piece of code that you added in 25f397a4. The 'if (connector->dpms !=
DRM_MODE_DPMS_ON)'  (line 719 in the version that is on the top of
Dave's drm-next branch), comes right after the unconditional break, is
unreachable code (and removing it produces the same object code). Can
you explain what your intent was here? Also, the comment above the
break that reads "don't break so fail path works correct[sic]" doesn't
make much sense to me either.

-- Ilija

[PATCH 5/5] DRM: Armada: add support for drm tda19988 driver

[Rob Clark]

On Tue, Oct 8, 2013 at 11:34 AM, Jean-Francois Moine  wrote:
> On Tue, 8 Oct 2013 10:49:39 +0100
> Russell King - ARM Linux  wrote:
>
>> On Tue, Oct 08, 2013 at 11:19:13AM +0200, Jean-Francois Moine wrote:
>> > The Cubox is an open platform, and I use it just like a desktop PC.
>> > When its required drivers will be in the mainline, I will do the same
>> > as I do with PCs: I will not recompile a specific kernel each time
>> > there are kernel bugs or security issues. Instead, I will just upgrade
>> > my system from my distributor (Debian), and, in the packages, there
>> > will be a generic mvebu kernel as there is already one for Marvell
>> > Armada 370/xp, Freescale iMX5x/iMX6 (linux-image-3.10-3-armmp).
>> > But, for that, all the Cubox specific stuff must be described in a DT.
>>
>> Which scenario is better:
>>
>> 1. To have something in mainline which is capable of driving the hardware,
>>but may need some additional work to make it useful for DT based setups.
>>
>> or
>>
>> 2. To have nothing.
>>
>> Now, you may prefer to have nothing, but personally, I prefer there to be
>> forward progress.  Forward progress means getting some kind of DRM driver
>> into mainline.  Yes, it may not work with DT setups yet, but - as per
>> the discussions that have happened _endlessly_ on this topic, it's
>> something that can be resolved at a later date.
>>
>> We _still_ haven't worked out how to properly deal with the TDA998x
>> driver (or indeed any DRM based outputs) in a DT based setup, and all
>> the time that problem exists, it won't be possible to write a proper
>> stable DT binding for this.
>>
>> So please, get off your hobby horse about this and allow us to make some
>> modicum of progress.
>
> You forgot:
>
> 3. To have all patches ready for submission and have a working DT driven
>Cubox kernel.

No need to block this patch on DT support for all platforms that can
use this driver.  I don't see an issue with removing non-DT support
later if it is no longer needed, or adding DT support in a future
patch for platforms which can use it.  So this patch looks fine to me.

Signed-off-by: Rob Clark  I will submit a patch to add DT to the tda998x driver as soon as I have
> checked the new audio properties we talked about yesterday.
>
> Normally, this should have no impact on your Armada drm driver, and,
> yes, I will add DT to your driver as soon as it will be accepted
> (sorry to not ack it now: I had no time yet to have a look at it).
>
> --
> Ken ar c'henta? | ** Breizh ha Linux atav! **
> Jef |   http://moinejf.free.fr/

[Bug 70439] Mobility 4570: System freezes after ~5s of enabling audio via xrandr

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

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

--- Comment #17 from Alex Deucher  ---
Created attachment 87798
  --> https://bugs.freedesktop.org/attachment.cgi?id=87798=edit
possible fix

Does this patch fix the issue?

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

[PATCH 4/5] DRM: Armada: start of MMP2/MMP3 support

[Rob Clark]

On Sun, Oct 6, 2013 at 6:10 PM, Russell King
 wrote:
> Signed-off-by: Russell King 
> ---
>  drivers/gpu/drm/armada/Makefile  |1 +
>  drivers/gpu/drm/armada/armada_610.c  |   49 
> ++
>  drivers/gpu/drm/armada/armada_crtc.c |1 +
>  drivers/gpu/drm/armada/armada_drm.h  |1 +
>  drivers/gpu/drm/armada/armada_drv.c  |6 
>  drivers/gpu/drm/armada/armada_hw.h   |8 +
>  6 files changed, 66 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpu/drm/armada/armada_610.c
>
> diff --git a/drivers/gpu/drm/armada/Makefile b/drivers/gpu/drm/armada/Makefile
> index d6f43e0..088db4a 100644
> --- a/drivers/gpu/drm/armada/Makefile
> +++ b/drivers/gpu/drm/armada/Makefile
> @@ -2,6 +2,7 @@ armada-y:= armada_crtc.o armada_drv.o armada_fb.o 
> armada_fbdev.o \
>armada_gem.o armada_output.o armada_overlay.o \
>armada_slave.o
>  armada-y   += armada_510.o
> +armada-y   += armada_610.o
>  armada-$(CONFIG_DEBUG_FS) += armada_debugfs.o
>
>  obj-$(CONFIG_DRM_ARMADA) := armada.o
> diff --git a/drivers/gpu/drm/armada/armada_610.c 
> b/drivers/gpu/drm/armada/armada_610.c
> new file mode 100644
> index 000..36a10e3
> --- /dev/null
> +++ b/drivers/gpu/drm/armada/armada_610.c
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (C) 2012 Russell King
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Armada 610/MMP2/MMP3 variant support
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "armada_crtc.h"
> +#include "armada_drm.h"
> +#include "armada_hw.h"
> +
> +static int mmp23_init(struct armada_private *priv, struct device *dev)
> +{
> +   priv->extclk[0] = devm_clk_get(dev, NULL);
> +
> +   if (IS_ERR(priv->extclk[0]) && PTR_ERR(priv->extclk[0]) == -ENOENT)
> +   priv->extclk[0] = ERR_PTR(-EPROBE_DEFER);
> +
> +   return PTR_RET(priv->extclk[0]);
> +}
> +
> +/*
> + * This gets called with sclk = NULL to test whether the mode is
> + * supportable, and again with sclk != NULL to set the clocks up for
> + * that.  The former can return an error, but the latter is expected
> + * not to.
> + */
> +static int mmp23_crtc_compute_clock(struct armada_crtc *dcrtc,
> +   const struct drm_display_mode *mode, uint32_t *sclk)
> +{
> +   /*
> +* on MMP3 bits 31:29 select the clock, OLPC wants 0x1 here, LCD 
> clock 1
> +* on MMP2 bits 31:30 select the clock, OLPC wants 0x1 here, LCD 
> clock 1
> +*/
> +   *sclk = 0x20001100; // FIXME hardcoded mmp3 value
> +

maybe fix this?  Or comment out for now mmp2 below in armada_drm_platform_ids?

BR,
-R

> +   return 0;
> +}
> +
> +const struct armada_variant mmp23_ops = {
> +   .init = mmp23_init,
> +   .crtc_compute_clock = mmp23_crtc_compute_clock,
> +};
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c 
> b/drivers/gpu/drm/armada/armada_crtc.c
> index e8605bf..b5877ee 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.c
> +++ b/drivers/gpu/drm/armada/armada_crtc.c
> @@ -1054,6 +1054,7 @@ int armada_drm_crtc_create(struct drm_device *dev, 
> unsigned num,
> dcrtc->clk = ERR_PTR(-EINVAL);
> dcrtc->csc_yuv_mode = CSC_AUTO;
> dcrtc->csc_rgb_mode = CSC_AUTO;
> +   /* FIXME: MMP2/MMP3: OLPC panel is RGB666 */
> dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
> spin_lock_init(>irq_lock);
> diff --git a/drivers/gpu/drm/armada/armada_drm.h 
> b/drivers/gpu/drm/armada/armada_drm.h
> index eef09ec..f52fccb 100644
> --- a/drivers/gpu/drm/armada/armada_drm.h
> +++ b/drivers/gpu/drm/armada/armada_drm.h
> @@ -70,6 +70,7 @@ struct armada_variant {
>
>  /* Variant ops */
>  extern const struct armada_variant armada510_ops;
> +extern const struct armada_variant mmp23_ops;
>
>  struct armada_private {
> const struct armada_variant *variant;
> diff --git a/drivers/gpu/drm/armada/armada_drv.c 
> b/drivers/gpu/drm/armada/armada_drv.c
> index 7bfab9a..db62f1b 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -344,6 +344,12 @@ static const struct platform_device_id 
> armada_drm_platform_ids[] = {
> .name   = "armada-drm",
> .driver_data= (unsigned long)_ops,
> }, {
> +   .name   = "armada-610-drm", /* aka MMP2 */
> +   .driver_data= (unsigned long)_ops,
> +   }, {
> +   .name   = "mmp3-drm",
> +   .driver_data= (unsigned long)_ops,
> +   }, {
> .name   = "armada-510-drm",
> .driver_data= (unsigned long)_ops,
> },
> diff --git 

[PATCH 3/5] DRM: Armada: Add support for ARGB 32x64 or 64x32 hardware cursors

[Rob Clark]

On Mon, Oct 7, 2013 at 8:50 AM, Russell King - ARM Linux
 wrote:
> On Mon, Oct 07, 2013 at 03:29:15PM +0300, Siarhei Siamashka wrote:
>> On Mon, 7 Oct 2013 11:32:50 +0100
>> Russell King - ARM Linux  wrote:
>> > However, what you're suggesting is dangerous.  What do you do when you're
>> > presented with a cursor which is 64x64 ?  Do you:
>> >
>> > (a) not display it
>> > (b) crash the X server
>> >
>> > There isn't a fallback to using software cursors available in the X server
>> > because there's no error reporting for a failed hardware cursor update.
>>
>> Hmm, maybe I'm missing something, but doesn't returning FALSE from
>> UseHWCursorARGB just make the X server fallback to using a software
>> cursor?
>>
>> https://github.com/ssvb/xf86-video-fbturbo/blob/689c08256555/src/sunxi_disp_hwcursor.c#L72
>>
>> I'm just doing this. And also have hooks to notify the DRI2 code about
>> the status of the cursor. In the case if the hardware cursor is not
>> enabled, hardware overlays can't be safely used with the software
>> cursor and we need to fallback to CPU/GPU copy instead of zero-copy
>> buffers swapping.
>>
>> And I fully agree that it is the responsibility of the kernel to expose
>> as much of the hardware capabilities as possible (without compromising
>> reliability and/or security).
>
> If you wish to go in below the level presented by
> hw/xfree86/modes/xf86Cursors.c (as you are doing), then yes you can do
> this, and it's something which maybe should be looked into.  You have a
> good reason to do this (due to your hardware cursor being incompatible
> with overlays).
>
> We have no such restriction - the only issue here is the "odd" size of
> cursors.  Practically, choosing one or other of 64x32 or 32x64 works
> fine for the cases I've seen.  I haven't seen any cursors which are
> 64x64 yet.  I'm not saying they don't exist though!  I can imagine that
> some accessibility options might want to display a large cursor.
>
> Now, the way I handle this is that the kernel allows _either_ a
> (1 to 32)x(1 to 64) or (1 to 64)x(1 to 32) cursor.  Both width and
> height larger than 32 gets rejected with -EINVAL:
>
> +   /* maximum size is 64x32 or 32x64 */
> +   if (w > 64 || h > 64 || (w > 32 && h > 32))
> +   return -ENOMEM;
>
> which is quite reasonable for this hardware.  However, there is no way
> to export this from DRM - adding maximum cursor size properties wouldn't
> really represent this.

Until relatively recently, it had always been a device specific ddx
which would somehow know what w/h values to pass to
xf86_cursors_init().  I think for xf86-video-modesetting and wayland
compositors, we probably need to come up with something better.
Although off the top of my head I can't think of a good way to express
64x32 or 32x64.. that is a weird restriction.

Anyways, right now I don't think there is anything different that I'd
do in the kernel part.  I *suppose* you could try and figure out that
all the alpha values are either 0x00 or 0xff and support 64x64 in that
special case.  I'm not sure that is actually better.. it at least
makes the failure vs success cases more confusing.  And unless you are
rockin' the twm+xterm+xcalc setup, I doubt you will see many cursors
with only 0x00 or 0xff alpha.

So,

Signed-off-by: Rob Clark 


> The only question is whether DRM should export some capabilities to
> indicate what's possible with the cursor, so that X knows about this
> quirk of this hardware.  Overall, I suspect that it's just rare and
> not worth the effort.  I think it's just easier to pick one of these
> and stick with it, and if we happen to encounter a cursor larger than
> our chosen dimentions, XFree86 will already automatically fall back
> to s/w cursor.

[Patch v2][ 14/37] staging: imx-drm: parallel display: add regulator support.

[Dan Carpenter]

On Thu, Oct 17, 2013 at 05:02:12PM +0200, Denis Carikli wrote:
> + if (imxpd->lcd_reg)
> + if (regulator_enable(imxpd->lcd_reg))
> + dev_err(imxpd->dev, "Failed to enable lcd 
> regulator.\n");
> +

In staging the style is to use braces around multi-line indents for
readability.  Or you could do:

if (imxpd->lcd_reg && regulator_enable(imxpd->lcd_reg))
dev_err(imxpd->dev, "Failed to enable lcd regulator.\n");

These kind of tiny things aren't worth resending, but for next time.

regards,
dan carpenter

MmioTrace: Using the Instruction Decoder, etc.

[Pekka Paalanen]

On Mon, 14 Oct 2013 22:45:09 +0400
Eugene Shatokhin  wrote:

> Hi,
> 
> There is an interesting TODO item on MmioTraceDeveloper page:
> "kprobes has a generic instruction decoding facility, use that instead of
> homebrewn (or KVM), and use emulation instead of page faulting"
> 
> Actually, I have done something similar in one of my systems, KernelStrider
> (http://code.google.com/p/kernel-strider/). The system instruments a kernel
> module when that module is being loaded. The instrumented code executes
> instead of the original one and provides information about the memory
> accesses it makes and the functions it calls. These data are sent to user
> space for further analysis.
> 
> Currently, I use this system to detect data races in the Linux kernel (and
> have found some). I suppose, it could probably be useful to MmioTrace as
> well.
> 
> KernelStrider uses an enhanced version of the x86 instruction decoder that
> Kprobes use and relies on binary instrumentation rather than on page
> faults. So, it can track:
> - memory accesses (address and size of the accessed memory as well as the
> access type are recorded)
> - function calls (exported functions and callbacks, one can setup pre- and
> post- handlers for these)
> 
> Is there any interest in trying this approach to the task of MmioTrace?
> 
> If so, we can discuss it. When I have time, I could try to create a
> prototype based on KernelStrider's core that tracks the memory accesses
> Mmiotrace needs.
> What do you think?

Hi Eugene,

that is very interesting! I assume emulating the instructions is
not only cleaner, but also faster than page-faulting, right? Maybe
even more reliable, perhaps up to the point where we would not need
to disable all but one CPU.

Unfortunately, my job exhausts my coding energy, and I haven't even
touched mmiotrace in years.

However, let's see if there are interested people on the mailing
lists. I'm CC'ing nouveau, since that is where mmiotrace started,
and dri-devel in the hopes to catch other drivers' reverse
engineers.


Thanks,
pq

[PATCH 4/4] drm/i915: Make the debugfs structures const

[Damien Lespiau]

Signed-off-by: Damien Lespiau 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index ebbb50e..7dac943 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2693,7 +2693,7 @@ static int i915_debugfs_create(struct dentry *root,
return drm_add_fake_info_node(minor, ent, fops);
 }

-static struct drm_info_list i915_debugfs_list[] = {
+static const struct drm_info_list i915_debugfs_list[] = {
{"i915_capabilities", i915_capabilities, 0},
{"i915_gem_objects", i915_gem_object_info, 0},
{"i915_gem_gtt", i915_gem_gtt_info, 0},
@@ -2735,7 +2735,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)

-static struct i915_debugfs_files {
+static const struct i915_debugfs_files {
const char *name;
const struct file_operations *fops;
 } i915_debugfs_files[] = {
-- 
1.8.3.1

[PATCH 3/4] drm: Make drm_debugfs_list const

[Damien Lespiau]

Signed-off-by: Damien Lespiau 
---
 drivers/gpu/drm/drm_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 4813ff1..b4b51d4 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -42,7 +42,7 @@
  * Initialization, etc.
  **/

-static struct drm_info_list drm_debugfs_list[] = {
+static const struct drm_info_list drm_debugfs_list[] = {
{"name", drm_name_info, 0},
{"vm", drm_vm_info, 0},
{"clients", drm_clients_info, 0},
-- 
1.8.3.1

[PATCH 2/4] drm: Remove drm_debugfs_node and drm_debugfs_list

[Damien Lespiau]

Those structures are not used anywhere.

Signed-off-by: Damien Lespiau 
---
 include/drm/drmP.h | 21 -
 1 file changed, 21 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c3b659a..1c502ff 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -997,27 +997,6 @@ struct drm_driver {
 #define DRM_MINOR_CONTROL 2
 #define DRM_MINOR_RENDER 3

-
-/**
- * debugfs node list. This structure represents a debugfs file to
- * be created by the drm core
- */
-struct drm_debugfs_list {
-   const char *name; /** file name */
-   int (*show)(struct seq_file*, void*); /** show callback */
-   u32 driver_features; /**< Required driver features for this entry */
-};
-
-/**
- * debugfs node structure. This structure represents a debugfs file.
- */
-struct drm_debugfs_node {
-   struct list_head list;
-   struct drm_minor *minor;
-   struct drm_debugfs_list *debugfs_ent;
-   struct dentry *dent;
-};
-
 /**
  * Info file list entry. This structure represents a debugfs or proc file to
  * be created by the drm core
-- 
1.8.3.1

[PATCH 1/4] drm: Constify struct drm_info_list * arguments

[Damien Lespiau]

Those functions are just reading data from those pointers.

Signed-off-by: Damien Lespiau 
---
 drivers/gpu/drm/drm_debugfs.c | 4 ++--
 include/drm/drmP.h| 9 +
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index a05087c..4813ff1 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -84,7 +84,7 @@ static const struct file_operations drm_debugfs_fops = {
  * Create a given set of debugfs files represented by an array of
  * gdm_debugfs_lists in the given root directory.
  */
-int drm_debugfs_create_files(struct drm_info_list *files, int count,
+int drm_debugfs_create_files(const struct drm_info_list *files, int count,
 struct dentry *root, struct drm_minor *minor)
 {
struct drm_device *dev = minor->dev;
@@ -188,7 +188,7 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
  *
  * Remove all debugfs entries created by debugfs_init().
  */
-int drm_debugfs_remove_files(struct drm_info_list *files, int count,
+int drm_debugfs_remove_files(const struct drm_info_list *files, int count,
 struct drm_minor *minor)
 {
struct list_head *pos, *q;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2b954ad..c3b659a 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1470,10 +1470,11 @@ extern struct drm_local_map *drm_getsarea(struct 
drm_device *dev);
 #if defined(CONFIG_DEBUG_FS)
 extern int drm_debugfs_init(struct drm_minor *minor, int minor_id,
struct dentry *root);
-extern int drm_debugfs_create_files(struct drm_info_list *files, int count,
-   struct dentry *root, struct drm_minor 
*minor);
-extern int drm_debugfs_remove_files(struct drm_info_list *files, int count,
-struct drm_minor *minor);
+extern int drm_debugfs_create_files(const struct drm_info_list *files,
+   int count, struct dentry *root,
+   struct drm_minor *minor);
+extern int drm_debugfs_remove_files(const struct drm_info_list *files,
+   int count, struct drm_minor *minor);
 extern int drm_debugfs_cleanup(struct drm_minor *minor);
 #endif

-- 
1.8.3.1

[PATCH v2 12/26] drm/exynos: Split manager/display/subdrv

[Inki Dae]

> -Original Message-
> From: Sean Paul [mailto:seanpaul at chromium.org]
> Sent: Thursday, October 17, 2013 4:27 AM
> To: dri-devel at lists.freedesktop.org; inki.dae at samsung.com
> Cc: airlied at linux.ie; tomasz.figa at gmail.com; marcheu at chromium.org; 
> Sean
> Paul
> Subject: [PATCH v2 12/26] drm/exynos: Split manager/display/subdrv
> 
> This patch splits display and manager from subdrv. The result is that
> crtc functions can directly call into manager callbacks and encoder
> functions can directly call into display callbacks. This will allow
> us to remove the exynos_drm_hdmi shim and support mixer/hdmi & fimd/dp
> with common code.
> 
> Signed-off-by: Sean Paul 
> ---
> 
> Changes in v2:
>   - Pass display into display_ops instead of context

Sorry but it seems like more reasonable to pass device object into
display_ops and manager_ops.

I'm not sure but display_ops could be implemented in other framework based
driver such as CDF based lcd panel driver. So if you pass display - it's
specific to exynos drm framework - into display_ops, the other framework
based driver should include specific exynos drm header.

And another one, the patch 6 passes manager object to manager_ops, and for
this, you made the manager object to be set to driver data;
platform_set_drvdata(pdev, ). That isn't reasonable. Generally,
driver_data would point to device driver's context object.

Thanks,
Inki Dae

[Patch v2][ 14/37] staging: imx-drm: parallel display: add regulator support.

[Denis Carikli]

Cc: Rob Herring 
Cc: Pawel Moll 
Cc: Mark Rutland 
Cc: Stephen Warren 
Cc: Ian Campbell 
Cc: devicetree at vger.kernel.org
Cc: Greg Kroah-Hartman 
Cc: driverdev-devel at linuxdriverproject.org
Cc: David Airlie 
Cc: dri-devel at lists.freedesktop.org
Cc: Sascha Hauer 
Cc: linux-arm-kernel at lists.infradead.org
Cc: Eric B?nard 
Signed-off-by: Denis Carikli 
---
 drivers/staging/imx-drm/parallel-display.c |   19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/staging/imx-drm/parallel-display.c 
b/drivers/staging/imx-drm/parallel-display.c
index 1c8f63f..dfab7b5 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -37,6 +38,7 @@ struct imx_parallel_display {
struct drm_encoder encoder;
struct imx_drm_encoder *imx_drm_encoder;
struct device *dev;
+   struct regulator *lcd_reg;
void *edid;
int edid_len;
u32 interface_pix_fmt;
@@ -129,6 +131,10 @@ static void imx_pd_encoder_prepare(struct drm_encoder 
*encoder)
 {
struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);

+   if (imxpd->lcd_reg)
+   if (regulator_enable(imxpd->lcd_reg))
+   dev_err(imxpd->dev, "Failed to enable lcd 
regulator.\n");
+
imx_drm_crtc_panel_format(encoder->crtc, DRM_MODE_ENCODER_NONE,
imxpd->interface_pix_fmt);
 }
@@ -145,6 +151,11 @@ static void imx_pd_encoder_mode_set(struct drm_encoder 
*encoder,

 static void imx_pd_encoder_disable(struct drm_encoder *encoder)
 {
+   struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
+
+   if (imxpd->lcd_reg)
+   if (regulator_disable(imxpd->lcd_reg))
+   dev_err(imxpd->dev, "Failed to disable lcd 
regulator.\n");
 }

 static void imx_pd_encoder_destroy(struct drm_encoder *encoder)
@@ -248,6 +259,14 @@ static int imx_pd_probe(struct platform_device *pdev)
if (ret)
return ret;

+   imxpd->lcd_reg = devm_regulator_get(>dev, "lcd");
+   if (IS_ERR(imxpd->lcd_reg)) {
+   dev_dbg(>dev, "no lcd-supply given.\n");
+   imxpd->lcd_reg = NULL;
+   } else {
+   dev_info(>dev, "lcd-supply found.\n");
+   }
+
ret = imx_drm_encoder_add_possible_crtcs(imxpd->imx_drm_encoder, np);

platform_set_drvdata(pdev, imxpd);
-- 
1.7.9.5

[Patch v2][ 13/37] staging: imx-drm: Add RGB666 support for parallel display

[Denis Carikli]

Support the RGB666 format on the IPUv3 parallel display.

Cc: Rob Herring 
Cc: Pawel Moll 
Cc: Mark Rutland 
Cc: Stephen Warren 
Cc: Ian Campbell 
Cc: devicetree at vger.kernel.org
Cc: Greg Kroah-Hartman 
Cc: driverdev-devel at linuxdriverproject.org
Cc: David Airlie 
Cc: dri-devel at lists.freedesktop.org
Cc: Mauro Carvalho Chehab 
Cc: Laurent Pinchart 
Cc: linux-media at vger.kernel.org
Cc: Sascha Hauer 
Cc: linux-arm-kernel at lists.infradead.org
Cc: Eric B?nard 
Signed-off-by: Denis Carikli 
---
 .../bindings/staging/imx-drm/fsl-imx-drm.txt   |2 +-
 drivers/staging/imx-drm/ipu-v3/ipu-dc.c|9 +
 drivers/staging/imx-drm/parallel-display.c |2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt 
b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
index b876d49..2d24425 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
@@ -29,7 +29,7 @@ Required properties:
 - crtc: the crtc this display is connected to, see below
 Optional properties:
 - interface_pix_fmt: How this display is connected to the
-  crtc. Currently supported types: "rgb24", "rgb565", "bgr666"
+  crtc. Currently supported types: "rgb24", "rgb565", "bgr666", "rgb666"
 - edid: verbatim EDID data block describing attached display.
 - ddc: phandle describing the i2c bus handling the display data
   channel
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c 
b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
index 21bf1c8..c84ad22 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
@@ -91,6 +91,7 @@ enum ipu_dc_map {
IPU_DC_MAP_RGB565,
IPU_DC_MAP_GBR24, /* TVEv2 */
IPU_DC_MAP_BGR666,
+   IPU_DC_MAP_RGB666,
 };

 struct ipu_dc {
@@ -152,6 +153,8 @@ static int ipu_pixfmt_to_map(u32 fmt)
return IPU_DC_MAP_GBR24;
case V4L2_PIX_FMT_BGR666:
return IPU_DC_MAP_BGR666;
+   case V4L2_PIX_FMT_RGB666:
+   return IPU_DC_MAP_RGB666;
default:
return -EINVAL;
}
@@ -395,6 +398,12 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /* green */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */

+   /* rgb666 */
+   ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
+   ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
+   ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
+   ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */
+
return 0;
 }

diff --git a/drivers/staging/imx-drm/parallel-display.c 
b/drivers/staging/imx-drm/parallel-display.c
index c04b017..1c8f63f 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -238,6 +238,8 @@ static int imx_pd_probe(struct platform_device *pdev)
imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB565;
else if (!strcmp(fmt, "bgr666"))
imxpd->interface_pix_fmt = V4L2_PIX_FMT_BGR666;
+   else if (!strcmp(fmt, "rgb666"))
+   imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB666;
}

imxpd->dev = >dev;
-- 
1.7.9.5

[Patch v2][ 12/37] staging: imx-drm: ipuv3-crtc: don't harcode some mode flags.

[Denis Carikli]

This change is needed for making the eukrea-cpuimx51
  QVGA display work.

Greg Kroah-Hartman 
Cc: driverdev-devel at linuxdriverproject.org
Cc: Philipp Zabel 
Cc: Sascha Hauer 
Cc: linux-arm-kernel at lists.infradead.org
Cc: David Airlie 
Cc: dri-devel at lists.freedesktop.org
Cc: Eric B?nard 
Signed-off-by: Denis Carikli 
---
 drivers/staging/imx-drm/ipuv3-crtc.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c 
b/drivers/staging/imx-drm/ipuv3-crtc.c
index 6fd37a7..9279294 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -236,9 +236,11 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
sig_cfg.Hsync_pol = 1;
if (mode->flags & DRM_MODE_FLAG_PVSYNC)
sig_cfg.Vsync_pol = 1;
+   if (mode->flags & DRM_MODE_FLAG_PDATEN)
+   sig_cfg.enable_pol = 1;
+   if (mode->flags & DRM_MODE_FLAG_PPIXDATEDGE)
+   sig_cfg.clk_pol = 1;

-   sig_cfg.enable_pol = 1;
-   sig_cfg.clk_pol = 0;
sig_cfg.width = mode->hdisplay;
sig_cfg.height = mode->vdisplay;
sig_cfg.pixel_fmt = out_pixel_fmt;
-- 
1.7.9.5

[Patch v2][ 11/37] staging: imx-drm: use of_get_display_timings.

[Denis Carikli]

The comment on top of of_get_drm_display_mode says:
 * This function is expensive and should only be used, if only one mode is to be
 * read from DT. To get multiple modes start with of_get_display_timings and
 * work with that instead.

Cc: Greg Kroah-Hartman 
Cc: driverdev-devel at linuxdriverproject.org
Cc: David Airlie 
Cc: dri-devel at lists.freedesktop.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: Fabio Estevam 
Cc: Sascha Hauer 
Cc: linux-arm-kernel at lists.infradead.org
Cc: Eric B?nard 
Signed-off-by: Denis Carikli 
---
 drivers/staging/imx-drm/parallel-display.c |   26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/imx-drm/parallel-display.c 
b/drivers/staging/imx-drm/parallel-display.c
index 24aa9be..c04b017 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -23,6 +23,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 

 #include "imx-drm.h"

@@ -74,11 +76,25 @@ static int imx_pd_connector_get_modes(struct drm_connector 
*connector)

if (np) {
struct drm_display_mode *mode = drm_mode_create(connector->dev);
-   of_get_drm_display_mode(np, >mode, 0);
-   drm_mode_copy(mode, >mode);
-   mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
-   drm_mode_probed_add(connector, mode);
-   num_modes++;
+   struct display_timings *timings;
+   struct videomode vm;
+   int np_num_mode;
+
+   timings = of_get_display_timings(np);
+   if (!timings)
+   return num_modes;
+   for (np_num_mode = 0; np_num_mode < timings->num_timings;
+   np_num_mode++, num_modes++) {
+   if (videomode_from_timings(timings, , np_num_mode))
+   break;
+   drm_display_mode_from_videomode(, mode);
+   mode->type = DRM_MODE_TYPE_DRIVER;
+   if (timings->native_mode == np_num_mode)
+   mode->type |= DRM_MODE_TYPE_PREFERRED;
+
+   drm_mode_set_name(mode);
+   drm_mode_probed_add(connector, mode);
+   }
}

return num_modes;
-- 
1.7.9.5

[Patch v2][ 04/37] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.

[Denis Carikli]

That new macro is needed by the imx_drm staging driver
  for supporting the QVGA display of the eukrea-cpuimx51 board.

Cc: Rob Herring 
Cc: Pawel Moll 
Cc: Mark Rutland 
Cc: Stephen Warren 
Cc: Ian Campbell 
Cc: devicetree at vger.kernel.org
Cc: Greg Kroah-Hartman 
Cc: driverdev-devel at linuxdriverproject.org
Cc: David Airlie 
Cc: dri-devel at lists.freedesktop.org
Cc: Mauro Carvalho Chehab 
Cc: Laurent Pinchart 
Cc: linux-media at vger.kernel.org
Cc: Sascha Hauer 
Cc: linux-arm-kernel at lists.infradead.org
Cc: Eric B?nard 
Signed-off-by: Denis Carikli 
---
 include/uapi/linux/videodev2.h |1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 437f1b0..e8ff410 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -294,6 +294,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_RGB555X v4l2_fourcc('R', 'G', 'B', 'Q') /* 16  RGB-5-5-5 
BE  */
 #define V4L2_PIX_FMT_RGB565X v4l2_fourcc('R', 'G', 'B', 'R') /* 16  RGB-5-6-5 
BE  */
 #define V4L2_PIX_FMT_BGR666  v4l2_fourcc('B', 'G', 'R', 'H') /* 18  BGR-6-6-6  
  */
+#define V4L2_PIX_FMT_RGB666  v4l2_fourcc('R', 'G', 'B', 'H') /* 18  RGB-6-6-6  
  */
 #define V4L2_PIX_FMT_BGR24   v4l2_fourcc('B', 'G', 'R', '3') /* 24  BGR-8-8-8  
   */
 #define V4L2_PIX_FMT_RGB24   v4l2_fourcc('R', 'G', 'B', '3') /* 24  RGB-8-8-8  
   */
 #define V4L2_PIX_FMT_BGR32   v4l2_fourcc('B', 'G', 'R', '4') /* 32  
BGR-8-8-8-8   */
-- 
1.7.9.5

[Patch v2][ 03/37] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*

[Denis Carikli]

Without that fix, drivers using the drm_display_mode_from_videomode
  function will not be able to get certain information because
  some DISPLAY_FLAGS_* have no corresponding DRM_MODE_FLAG_*.

Cc: Greg Kroah-Hartman 
Cc: driverdev-devel at linuxdriverproject.org
Cc: David Airlie 
Cc: dri-devel at lists.freedesktop.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: Fabio Estevam 
Cc: Sascha Hauer 
Cc: linux-arm-kernel at lists.infradead.org
Cc: Eric B?nard 
Signed-off-by: Denis Carikli 
---
 drivers/gpu/drm/drm_modes.c |9 +
 include/uapi/drm/drm_mode.h |4 
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index b073315..353aaae 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -537,6 +537,15 @@ int drm_display_mode_from_videomode(const struct videomode 
*vm,
dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
dmode->flags |= DRM_MODE_FLAG_DBLCLK;
+   if (vm->flags & DISPLAY_FLAGS_DE_LOW)
+   dmode->flags |= DRM_MODE_FLAG_NDATEN;
+   if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+   dmode->flags |= DRM_MODE_FLAG_PDATEN;
+   if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+   dmode->flags |= DRM_MODE_FLAG_PPIXDATEDGE;
+   if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+   dmode->flags |= DRM_MODE_FLAG_NPIXDATEDGE;
+
drm_mode_set_name(dmode);

return 0;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index bafe612..13843c7 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -66,6 +66,10 @@
 #define DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH (1<<19)
 #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM(1<<20)
 #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (1<<21)
+#define DRM_MODE_FLAG_PDATEN   (1<<22)
+#define DRM_MODE_FLAG_NDATEN   (1<<23)
+#define DRM_MODE_FLAG_PPIXDATEDGE  (1<<24)
+#define DRM_MODE_FLAG_NPIXDATEDGE  (1<<25)

 /* DPMS flags */
 /* bit compatible with the xorg definitions. */
-- 
1.7.9.5

[PATCH 2/2] drm/radeon: rework audio option

[Alex Deucher]

In 3.12 I changed audio to be enabled by default,
but you still had to turn it on via xrandr.  This
was confusing to users so change it to minic the
previous behavior:

- audio option is set to -1 (auto) by default which is
  the current 3.12 behavior (audio is enabled but requires
  xrandr to turn it on.
- if audio = 1, the audio is enabled without needing
  to mess with xrandr (previous behavior)
- audio = 0 disables audio

It retains the new feature of allowing the user to enable
audio on the fly with xrandr, but turns audio on
automatically if radeon.audio=1 is set which is what
most users expect.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/radeon/atombios_encoders.c | 52 --
 drivers/gpu/drm/radeon/radeon_connectors.c | 33 ---
 drivers/gpu/drm/radeon/radeon_drv.c|  4 +--
 3 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c 
b/drivers/gpu/drm/radeon/atombios_encoders.c
index 2a761f2..5e891b2 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -707,24 +707,37 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
switch (connector->connector_type) {
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_HDMIB: /* HDMI-B is basically DL-DVI; analog 
works fine */
-   if ((radeon_connector->audio == RADEON_AUDIO_ENABLE) ||
-   (drm_detect_hdmi_monitor(radeon_connector->edid) &&
-(radeon_connector->audio == RADEON_AUDIO_AUTO)))
-   return ATOM_ENCODER_MODE_HDMI;
-   else if (radeon_connector->use_digital)
+   if (radeon_audio != 0) {
+   if (radeon_connector->use_digital &&
+   (radeon_connector->audio == RADEON_AUDIO_ENABLE))
+   return ATOM_ENCODER_MODE_HDMI;
+   else if 
(drm_detect_hdmi_monitor(radeon_connector->edid) &&
+(radeon_connector->audio == RADEON_AUDIO_AUTO))
+   return ATOM_ENCODER_MODE_HDMI;
+   else if (radeon_connector->use_digital)
+   return ATOM_ENCODER_MODE_DVI;
+   else
+   return ATOM_ENCODER_MODE_CRT;
+   } else if (radeon_connector->use_digital) {
return ATOM_ENCODER_MODE_DVI;
-   else
+   } else {
return ATOM_ENCODER_MODE_CRT;
+   }
break;
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_HDMIA:
default:
-   if ((radeon_connector->audio == RADEON_AUDIO_ENABLE) ||
-   (drm_detect_hdmi_monitor(radeon_connector->edid) &&
-(radeon_connector->audio == RADEON_AUDIO_AUTO)))
-   return ATOM_ENCODER_MODE_HDMI;
-   else
+   if (radeon_audio != 0) {
+   if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
+   return ATOM_ENCODER_MODE_HDMI;
+   else if 
(drm_detect_hdmi_monitor(radeon_connector->edid) &&
+(radeon_connector->audio == RADEON_AUDIO_AUTO))
+   return ATOM_ENCODER_MODE_HDMI;
+   else
+   return ATOM_ENCODER_MODE_DVI;
+   } else {
return ATOM_ENCODER_MODE_DVI;
+   }
break;
case DRM_MODE_CONNECTOR_LVDS:
return ATOM_ENCODER_MODE_LVDS;
@@ -732,14 +745,19 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
case DRM_MODE_CONNECTOR_DisplayPort:
dig_connector = radeon_connector->con_priv;
if ((dig_connector->dp_sink_type == 
CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
-   (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP))
+   (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP)) {
return ATOM_ENCODER_MODE_DP;
-   else if ((radeon_connector->audio == RADEON_AUDIO_ENABLE) ||
-(drm_detect_hdmi_monitor(radeon_connector->edid) &&
- (radeon_connector->audio == RADEON_AUDIO_AUTO)))
-   return ATOM_ENCODER_MODE_HDMI;
-   else
+   } else if (radeon_audio != 0) {
+   if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
+   return ATOM_ENCODER_MODE_HDMI;
+   else if 
(drm_detect_hdmi_monitor(radeon_connector->edid) &&
+(radeon_connector->audio == RADEON_AUDIO_AUTO))
+   return ATOM_ENCODER_MODE_HDMI;
+   else
+

[PATCH 1/2] drm/radeon/audio: don't set speaker allocation on DCE3.2

[Alex Deucher]

It causes hangs on some asics.

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

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/radeon/r600_hdmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c 
b/drivers/gpu/drm/radeon/r600_hdmi.c
index 5b72931..06022e3 100644
--- a/drivers/gpu/drm/radeon/r600_hdmi.c
+++ b/drivers/gpu/drm/radeon/r600_hdmi.c
@@ -309,6 +309,9 @@ static void dce3_2_afmt_write_speaker_allocation(struct 
drm_encoder *encoder)
u8 *sadb;
int sad_count;

+   /* XXX: setting this register causes hangs on some asics */
+   return;
+
list_for_each_entry(connector, 
>dev->mode_config.connector_list, head) {
if (connector->encoder == encoder)
radeon_connector = to_radeon_connector(connector);
-- 
1.8.3.1

[PATCH/RFC v3 00/19] Common Display Framework

[Tomi Valkeinen]

On 17/10/13 15:26, Andrzej Hajda wrote:

> I am not sure what exactly the encoder performs, if this is only image
> transport from dispc to panel CDF pipeline in both cases should look like:
> dispc > panel
> The only difference is that panels will be connected via different Linux bus
> adapters, but it will be irrelevant to CDF itself. In this case I would say
> this is DSI-master rather than encoder, or at least that the only
> function of the
> encoder is DSI.

Yes, as I said, it's up to the driver writer how he wants to use CDF. If
he doesn't see the point of representing the SoC's DSI encoder as a
separate CDF entity, nobody forces him to do that.

On OMAP, we have single DISPC with multiple parallel outputs, and a
bunch of encoder IPs (MIPI DPI, DSI, DBI, etc). Each encoder IP can be
connected to some of the DISPC's output. In this case, even if the DSI
encoder does nothing special, I see it much better to represent the DSI
encoder as a CDF entity so that the links between DISPC, DSI, and the
DSI peripherals are all there.

> If display_timings on input and output differs, I suppose it should be
> modeled
> as display_entity, as this is an additional functionality(not covered by
> DSI standard AFAIK).

Well, DSI standard is about the DSI output. Not about the encoder's
input, or the internal operation of the encoder.

>>> Of course there are some settings which are not panel dependent and those
>>> should reside in DSI node.
>> Exactly. And when the two panels require different non-panel-dependent
>> settings, how do you represent them in the DT data?
> 
> non-panel-dependent setting cannot depend on panel, by definition :)

With "non-panel-dependent" setting I meant something that is a property
of the DSI master device, but still needs to be configured differently
for each panel.

Say, pin configuration. When using panel A, the first pin of the DSI
block could be clock+. With panel B, the first pin could be clock-. This
configuration is about DSI master, but it is different for each panel.

If we have separate endpoint in the DSI master for each panel, this data
can be there. If we don't have the endpoint, as is the case with
separate control bus, where is that data?

>>> Could you describe such scenario?
>> If we have two independent APIs, ctrl and video, that affect the same
>> underlying hardware, the DSI bus, we could have a scenario like this:
>>
>> thread 1:
>>
>> ctrl->op_foo();
>> ctrl->op_bar();
>>
>> thread 2:
>>
>> video->op_baz();
>>
>> Even if all those ops do locking properly internally, the fact that
>> op_baz() can be called in between op_foo() and op_bar() may cause problems.
>>
>> To avoid that issue with two APIs we'd need something like:
>>
>> thread 1:
>>
>> ctrl->lock();
>> ctrl->op_foo();
>> ctrl->op_bar();
>> ctrl->unlock();
>>
>> thread 2:
>>
>> video->lock();
>> video->op_baz();
>> video->unlock();
> I should mention I was asking for real hw/drivers configuration.
> I do not know what do you mean with video->op_baz() ?
> DSI-master is not modeled in CDF, and only CDF provides video
> operations.

It was just an example of the additional complexity with regarding
locking when using two APIs.

The point is that if the panel driver has two pointers (i.e. API), one
for the control bus, one for the video bus, and ops on both buses affect
the same hardware, the locking is not easy.

If, on the other hand, the panel driver only has one API to use, it's
simple to require the caller to handle any locking.

> I guess one scenario, when two panels are connected to single DSI-master.
> In such case both can call DSI ops, but I do not know how do you want to
> prevent it in case of your CDF-T implementation.

No, that was not the case I was describing. This was about a single panel.

If we have two independent APIs, we need to define how locking is
managed for those APIs. Even if in practice both APIs are used by the
same driver, and the driver can manage the locking, that's not really a
valid requirement. It'd be almost the same as requiring that gpio API
cannot be called at the same time as i2c API.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/034cfbd6/attachment-0001.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Tomi Valkeinen]

On 17/10/13 15:17, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Thursday 17 October 2013 14:59:41 Tomi Valkeinen wrote:
>> On 17/10/13 14:51, Laurent Pinchart wrote:
>>>> I'm not sure if there's a specific need for the port or endpoint nodes
>>>> in cases like the above. Even if we have common properties describing
>>>> the endpoint, I guess they could just be in the parent node.
>>>>
>>>> panel {
>>>>remote = <>;
>>>>common-video-property = ;
>>>> };
>>>>
>>>> The above would imply one port and one endpoint. Would that work? If we
>>>> had a function like parse_endpoint(node), we could just point it to
>>>> either a real endpoint node, or to the device's node.
>>>
>>> You reference the display controller here, not a specific display
>>> controller output. Don't most display controllers have several outputs ?
>>
>> Sure. Then the display controller could have more verbose description.
>> But the panel could still have what I wrote above, except the 'remote'
>> property would point to a real endpoint node inside the dispc node, not
>> to the dispc node.
>>
>> This would, of course, need some extra code to handle the different
>> cases, but just from DT point of view, I think all the relevant
>> information is there.
> 
> There's many ways to describe the same information in DT. While we could have 
> DT bindings that use different descriptions for different devices and still 
> support all of them in our code, why should we opt for that option that will 
> make the implementation much more complex when we can describe connections in 
> a simple and generic way ?

My suggestion was simple and generic. I'm not proposing per-device
custom bindings.

My point was, if we can describe the connections as I described above,
which to me sounds possible, we can simplify the panel DT data for 99.9%
of the cases.

To me, the first of these looks much nicer:

panel {
remote = <>;
common-video-property = ;
};

panel {
port {
endpoint {
remote = <>;
common-video-property = ;
};
};
};

If that can be supported in the SW by adding complexity to a few
functions, and it covers practically all the panels, isn't it worth it?

Note that I have not tried this, so I don't know if there are issues.
It's just a thought. Even if there's need for a endpoint node, perhaps
the port node can be made optional.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/bed122fd/attachment-0001.pgp>

[Bug 68391] [radeonsi] Crash unigine-sanctuary

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

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

--- Comment #19 from Vladimir Ysikov  ---
Ok thx, after creating /etc/drirc heaven and sanctuary looks normal.
For me this bug may be closed. But for  Laurent carlier still not fixed.

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

[RFR 2/2] drm/panel: Add simple panel support

[Tomi Valkeinen]

On 17/10/13 14:51, Laurent Pinchart wrote:

>> I'm not sure if there's a specific need for the port or endpoint nodes
>> in cases like the above. Even if we have common properties describing
>> the endpoint, I guess they could just be in the parent node.
>>
>> panel {
>>  remote = <>;
>>  common-video-property = ;
>> };
>>
>> The above would imply one port and one endpoint. Would that work? If we
>> had a function like parse_endpoint(node), we could just point it to
>> either a real endpoint node, or to the device's node.
> 
> You reference the display controller here, not a specific display controller 
> output. Don't most display controllers have several outputs ?

Sure. Then the display controller could have more verbose description.
But the panel could still have what I wrote above, except the 'remote'
property would point to a real endpoint node inside the dispc node, not
to the dispc node.

This would, of course, need some extra code to handle the different
cases, but just from DT point of view, I think all the relevant
information is there.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/2ac17d35/attachment-0001.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Thierry Reding]

On Thu, Oct 17, 2013 at 02:23:32PM +0200, Laurent Pinchart wrote:
> Hi Thierry,
> 
> On Thursday 17 October 2013 14:06:47 Thierry Reding wrote:
> > On Thu, Oct 17, 2013 at 01:15:22PM +0200, Laurent Pinchart wrote:
> > > On Thursday 17 October 2013 13:05:18 Thierry Reding wrote:
> > > > On Thu, Oct 17, 2013 at 01:22:21PM +0300, Tomi Valkeinen wrote:
> > > > > On 17/10/13 11:53, Thierry Reding wrote:
> > > > > > I keep wondering why we absolutely must have compatibility between
> > > > > > CDF and this simple panel binding. DT content is supposed to concern
> > > > > > itself with the description of hardware only. What about the
> > > > > > following isn't an accurate description of the panel hardware?
> > > > > > 
> > > > > > panel: panel {
> > > > > > compatible = "cptt,claa101wb01";
> > > > > > 
> > > > > > power-supply = <_pnl_reg>;
> > > > > > enable-gpios = < 90 0>;
> > > > > > 
> > > > > > backlight = <>;
> > > > > > };
> > > > > > 
> > > > > > dsi-controller {
> > > > > > panel = <>;
> > > > > > };
> > > > > 
> > > > > That's quite similar to how my current out-of-mainline OMAP DSS DT
> > > > > bindings work. The difference is that I have a reference from the
> > > > > panel node to the video source (dsi-controller), instead of the other
> > > > > way around. I just find it more natural. It works the same way as,
> > > > > say, regulators, gpios, etc.
> > > > 
> > > > I suppose that depends on the way you look at it. In the above proposal
> > > > I consider the output (DSI controller) to use the panel as a resource
> > > > that provides a certain set of services (query mode timings, provide a
> > > > way to enable or disable the panel). Similarly the panel uses the
> > > > backlight as a resource that provides a certain set of services (such as
> > > > changing the brightness).
> > > > 
> > > > The above also follows the natural order of control. The panel has no
> > > > way to control the DSI output. However, it is the output that controls
> > > > when a panel is required and turn it on as appropriate.
> > > 
> > > I'm no DSI expert, but I know enough about it to be sure that Tomi will
> > > disagree. DSI panels can have complex power sequences that require the
> > > input stream to be finely controlled (for instance it might require a
> > > clock without video frames for some time, switch a GPIO or regulator,
> > > send a command to the panel, and then only get video frames). For that
> > > reason all developers I've talked to who have an in-depth knowledge of
> > > DSI and DSI panels told me that the panel needs to control the video bus,
> > > and request the video source to start/stop the video stream.
> > 
> > Oh, I'm very well aware of the various flavours of funkiness that DSI
> > panels come in. But it's wrong to say that the panel needs to control
> > the video bus. There's simply no way that a panel can actually do that.
> > It is true, however, that in order to make this work in a maintainable
> > fashion, the DSI panel *driver* may need to control the DSI bus. That's
> > an entirely different story.
> 
> Sure, but I don't think that's really related to the DT bindings. We don't 
> have to model every electrical signal in a detailed way that matches the 
> direction of the electrons flow :-) What we need to model is a connection 
> between a display controller and a panel (possibly with a direction). What 
> I'd 
> like to do is to express that link in a way that can also express more 
> complex 
> pipeline topologies. I don't want to make it overly complex, I had hoped that 
> my DT bindings proposal would be a good approach as it's both generic and 
> still pretty simple.

I get that, and for what it's worth I do think that your proposal looks
simple enough and if it can solve any of the problem you're facing with
CDF, then that's great.

But I don't think we should force inclusion of these properties on every
panel, even if it doesn't use any of the graph functionality. Is there
any problem with making them optional?

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/c1a7e062/attachment.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Tomi Valkeinen]

On 17/10/13 14:05, Thierry Reding wrote:

>> That's quite similar to how my current out-of-mainline OMAP DSS DT
>> bindings work. The difference is that I have a reference from the panel
>> node to the video source (dsi-controller), instead of the other way
>> around. I just find it more natural. It works the same way as, say,
>> regulators, gpios, etc.
> 
> I suppose that depends on the way you look at it. In the above proposal
> I consider the output (DSI controller) to use the panel as a resource
> that provides a certain set of services (query mode timings, provide a
> way to enable or disable the panel). Similarly the panel uses the
> backlight as a resource that provides a certain set of services (such as
> changing the brightness).
> 
> The above also follows the natural order of control. The panel has no
> way to control the DSI output. However, it is the output that controls
> when a panel is required and turn it on as appropriate.

I've discussed this issue to death with other people, and for some
reason I still see this the other way =).

I see the panel using the DSI controller as a resource. The panel driver
uses the DSI controller to send the video data to the panel device.
Compare this to, say, i2c client driver, which uses i2c master to send
data to the i2c device.

In the OMAP DSS driver model, the panel driver is in control. The panel
driver tells the DSI controller which kind of video timings to use, or
which bus speed to use, or whether to use low-power or high-speed mode,
when to enable the DSI clock or DSI video stream, etc.

With a simple panel the model you describe works usually fine. The
reason I went with the different model is that we had rather complex
display peripherals, that needed more specific control of the bus.

A simple example could be the enable sequence. I presume in your model
the DSI controller's enable would be maybe something like:

configure_dsi_timings(panel->get_timings());
enable_dsi_output();
panel->enable();

What if the DSI peripheral requires an enable sequence of, say:

- enable DSI clock
- use LP mode
- send some config messages
- re-configure DSI clock to higher speed
- send some more config messages
- enable HS mode
- send some more config messages
- enable DSI video stream

It gets a bit difficult to manage that kind of setup in a generic way in
the DSI controller driver. As I see it, the DSI peripheral driver has to
be in control of the nuances. The same goes for other video buses also,
but DSI is perhaps one of the most complex ones to support.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/d0388027/attachment-0001.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Thierry Reding]

 of view, there's no need to fix DRM to use the return 
> value before pushing this patch set to mainline, but I'd like a v2 with an 
> int 
> return value).

Why? What's the use of returning an error if you know up front that it
can't be used? This should be changed if or when we "fix" DRM to
propagate errors.

> > > > > Instead of hardcoding the modes in the driver, which would then
> > > > > require to be updated for every new simple panel model (and we know
> > > > > there are lots of them), why don't you specify the modes in the panel
> > > > > DT node ? The simple panel driver would then become much more generic.
> > > > > It would also allow board designers to tweak h/v sync timings
> > > > > depending on the system requirements.
> > > > 
> > > > Sigh... we keep second-guessing ourselves. Back at the time when power
> > > > sequences were designed (and NAKed at the last minute), we all decided
> > > > that the right thing to do would be to use specific compatible values
> > > > for individual panels, because that would allow us to encode the power
> > > > sequencing within the driver. And when we already have the panel model
> > > > encoded in the compatible value, we might just as well encode the mode
> > > > within the driver for that panel. Otherwise we'll have to keep adding
> > > > the same mode timings for every board that uses the same panel.
> > > > 
> > > > I do agree though that it might be useful to tweak the mode in case the
> > > > default one doesn't work. How about we provide a means to override the
> > > > mode encoded in the driver using one specified in the DT? That's
> > > > obviously a backwards-compatible change, so it could be added if or when
> > > > it becomes necessary.
> > > 
> > > I share Tomi's point of view here. Your three panels use the same power
> > > sequence, so I believe we should have a generic panel compatible string
> > > that would use modes described in DT for the common case. Only if a panel
> > > required something more complex which can't (or which could, but won't
> > > for any reason) accurately be described in DT would you need to extend
> > > the driver.
> >
> > I don't see the point quite frankly. You seem to be assuming that every
> > panel will only be used in a single board.
> 
> No, but in practice that's often the case, at least for boards with .dts 
> files 
> in the mainline kernel.
> 
> > However what you're proposing will require the mode timings to be repeated
> > in every board's DT file, if the same panel is ever used on more than a
> > single board.
> 
> Is that a problem ? You could #include a .dtsi for the panel that would 
> specify the video mode if you want to avoid multiple copies.

Yes, I don't think it's desirable to duplicate data needlessly in DT. It
also makes the binding more difficult to use. If I know that the panel
is one supported by the simple-panel binding, I can just go and add the
right compatible value without having to bother looking up the mode
timings and duplicating them. That way DT writers get to concern
themselves only with the variable data.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/2374dc01/attachment-0001.pgp>

[PATCH 2/8] [media] exynos4-is: Don't use i2c_client->driver

[Mauro Carvalho Chehab]

Em Sun, 29 Sep 2013 10:51:00 +0200
Lars-Peter Clausen  escreveu:

> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. The results of the expressions 'client->driver.driver->field' and
> 'client->dev.driver->field' are identical, so replace all occurrences of the
> former with the later.
> 
> Signed-off-by: Lars-Peter Clausen 
> Cc: Kyungmin Park 
> Cc: Sylwester Nawrocki 

Acked-by: Mauro Carvalho Chehab 

> ---
>  drivers/media/platform/exynos4-is/media-dev.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c 
> b/drivers/media/platform/exynos4-is/media-dev.c
> index a835112..7a4ee4c 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -411,8 +411,8 @@ static int fimc_md_of_add_sensor(struct fimc_md *fmd,
>  
>   device_lock(>dev);
>  
> - if (!client->driver ||
> - !try_module_get(client->driver->driver.owner)) {
> + if (!client->dev.driver ||
> + !try_module_get(client->dev.driver->owner)) {
>   ret = -EPROBE_DEFER;
>   v4l2_info(>v4l2_dev, "No driver found for %s\n",
>   node->full_name);
> @@ -442,7 +442,7 @@ static int fimc_md_of_add_sensor(struct fimc_md *fmd,
>   fmd->num_sensors++;
>  
>  mod_put:
> - module_put(client->driver->driver.owner);
> + module_put(client->dev.driver->owner);
>  dev_put:
>   device_unlock(>dev);
>   put_device(>dev);


-- 

Cheers,
Mauro

[PATCH 3/8] [media] core: Don't use i2c_client->driver

[Mauro Carvalho Chehab]

Em Sun, 29 Sep 2013 10:51:01 +0200
Lars-Peter Clausen  escreveu:

> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. The results of the expressions 'client->driver.driver->field' and
> 'client->dev.driver->field' are identical, so replace all occurrences of the
> former with the later.
> 
> Signed-off-by: Lars-Peter Clausen 

Acked-by: Mauro Carvalho Chehab 

> ---
>  drivers/media/v4l2-core/tuner-core.c  |  6 +++---
>  drivers/media/v4l2-core/v4l2-common.c | 10 +-
>  include/media/v4l2-common.h   |  2 +-
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/tuner-core.c 
> b/drivers/media/v4l2-core/tuner-core.c
> index ddc9379..4133af0 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -43,7 +43,7 @@
>  
>  #define UNSET (-1U)
>  
> -#define PREFIX (t->i2c->driver->driver.name)
> +#define PREFIX (t->i2c->dev.driver->name)
>  
>  /*
>   * Driver modprobe parameters
> @@ -452,7 +452,7 @@ static void set_type(struct i2c_client *c, unsigned int 
> type,
>   }
>  
>   tuner_dbg("%s %s I2C addr 0x%02x with type %d used for 0x%02x\n",
> -   c->adapter->name, c->driver->driver.name, c->addr << 1, type,
> +   c->adapter->name, c->dev.driver->name, c->addr << 1, type,
> t->mode_mask);
>   return;
>  
> @@ -556,7 +556,7 @@ static void tuner_lookup(struct i2c_adapter *adap,
>   int mode_mask;
>  
>   if (pos->i2c->adapter != adap ||
> - strcmp(pos->i2c->driver->driver.name, "tuner"))
> + strcmp(pos->i2c->dev.driver->name, "tuner"))
>   continue;
>  
>   mode_mask = pos->mode_mask;
> diff --git a/drivers/media/v4l2-core/v4l2-common.c 
> b/drivers/media/v4l2-core/v4l2-common.c
> index 037d7a5..433d6d7 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -236,14 +236,14 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, 
> struct i2c_client *client,
>   v4l2_subdev_init(sd, ops);
>   sd->flags |= V4L2_SUBDEV_FL_IS_I2C;
>   /* the owner is the same as the i2c_client's driver owner */
> - sd->owner = client->driver->driver.owner;
> + sd->owner = client->dev.driver->owner;
>   sd->dev = >dev;
>   /* i2c_client and v4l2_subdev point to one another */
>   v4l2_set_subdevdata(sd, client);
>   i2c_set_clientdata(client, sd);
>   /* initialize name */
>   snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
> - client->driver->driver.name, i2c_adapter_id(client->adapter),
> + client->dev.driver->name, i2c_adapter_id(client->adapter),
>   client->addr);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
> @@ -274,11 +274,11 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct 
> v4l2_device *v4l2_dev,
>  loaded. This delay-load mechanism doesn't work if other drivers
>  want to use the i2c device, so explicitly loading the module
>  is the best alternative. */
> - if (client == NULL || client->driver == NULL)
> + if (client == NULL || client->dev.driver == NULL)
>   goto error;
>  
>   /* Lock the module so we can safely get the v4l2_subdev pointer */
> - if (!try_module_get(client->driver->driver.owner))
> + if (!try_module_get(client->dev.driver->owner))
>   goto error;
>   sd = i2c_get_clientdata(client);
>  
> @@ -287,7 +287,7 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct 
> v4l2_device *v4l2_dev,
>   if (v4l2_device_register_subdev(v4l2_dev, sd))
>   sd = NULL;
>   /* Decrease the module use count to match the first try_module_get. */
> - module_put(client->driver->driver.owner);
> + module_put(client->dev.driver->owner);
>  
>  error:
>   /* If we have a client but no subdev, then something went wrong and
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 16550c4..a707529 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -35,7 +35,7 @@
>   printk(level "%s %d-%04x: " fmt, name, i2c_adapter_id(adapter), addr , 
> ## arg)
>  
>  #define v4l_client_printk(level, client, fmt, arg...)
> \
> - v4l_printk(level, (client)->driver->driver.name, (client)->adapter, \
> + v4l_printk(level, (client)->dev.driver->name, (client)->adapter, \
>  (client)->addr, fmt , ## arg)
>  
>  #define v4l_err(client, fmt, arg...) \


-- 

Cheers,
Mauro

[PATCH] drm: Pad drm_mode_get_connector to 64-bit boundary

[Ben Hutchings]

On Wed, 2013-10-16 at 13:09 +0300, Ville Syrj?l? wrote:
> On Wed, Oct 16, 2013 at 09:49:02AM +0100, Chris Wilson wrote:
> > Pavel Roskin reported that DRM_IOCTL_MODE_GETCONNECTOR was overwritting
> > the 4 bytes beyond the end of its structure with a 32-bit userspace
> > running on a 64-bit kernel. This is due to the padding gcc inserts as
> > the drm_mode_get_connector struct includes a u64 and its size is not a
> > natural multiple of u64s.
> > 
> > 64-bit kernel:
> > 
> > sizeof(drm_mode_get_connector)=80, alignof=8
> > sizeof(drm_mode_get_encoder)=20, alignof=4
> > sizeof(drm_mode_modeinfo)=68, alignof=4
> > 
> > 32-bit userspace:
> > 
> > sizeof(drm_mode_get_connector)=76, alignof=4
> > sizeof(drm_mode_get_encoder)=20, alignof=4
> > sizeof(drm_mode_modeinfo)=68, alignof=4
> > 
> > Fortuituously we can insert explicit padding to the tail of our
> > structures without breaking ABI.
> > 
> > Reported-by: Pavel Roskin 
> > Signed-off-by: Chris Wilson 
> > Cc: Dave Airlie 
> > Cc: dri-devel at lists.freedesktop.org
> > Cc: stable at vger.kernel.org
> 
> Hmm. But that only fixes things if you recompile the 32bit userland
> code.

Which is not a fix at all, but an even worse ABI break (now 32-bit
kernels will overrun userland buffers too).

> We could also fix old 32bit userland by adopting the same kind of size
> handling that we use for driver specific ioctls. The code is already
> there, we just need to set asize and usize appropriately.

Right, you have to do something like that.

Ben.

> > ---
> >  include/uapi/drm/drm_mode.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 550811712f78..28acbaf4a81e 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -223,6 +223,8 @@ struct drm_mode_get_connector {
> > __u32 connection;
> > __u32 mm_width, mm_height; /**< HxW in millimeters */
> > __u32 subpixel;
> > +
> > +   __u32 pad;
> >  };
> >  
> >  #define DRM_MODE_PROP_PENDING  (1<<0)
> > -- 
> > 1.8.4.rc3
> > 
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Ben Hutchings
Horngren's Observation:
   Among economists, the real world is often a special case.
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 828 bytes
Desc: This is a digitally signed message part
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/381036d4/attachment.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Tomi Valkeinen]

On 17/10/13 14:02, Laurent Pinchart wrote:

>> Okay, so if I understand correctly, translating those bindings to panel
>> nodes would look somewhat like this:
>>
>>  dc: display-controller {
>>  ports {
>>  port at 0 {
>>  remote-endpoint = <>;
>>  };
>>  };
>>  };
>>
>>  panel: panel {
>>  ports {
>>  port at 0 {
>>  remote-endpoint = <>;
>>  };
>>  };
>>  };
>>
>> The above leaves out any of the other, non-relevant properties. Does
>> that sound about right?
> 
> Yes it does.

It does?

Shouldn't it be something like:

panel {
ports {
port at 0 {
endpoint at 0 {
remote = <>;
};
};
};
};

And simplified:

panel {
port {
endpoint at 0 {
remote = <>;
};
};
};

You do need a node for the endpoint, a remote-endpoint property is not
enough.

> Please note that, when a device has as single port, the ports node can be 
> omitted, and the port doesn't need to be numbered. You would then end up with
> 
>   dc: display-controller {
>   port {
>   remote-endpoint = <>;
>   };
>   };
>  
>   panel: panel {
>   port {
>   remote-endpoint = <>;
>   };
>   };
> 
> I don't think there's a way to simplify it further.

I'm not sure if there's a specific need for the port or endpoint nodes
in cases like the above. Even if we have common properties describing
the endpoint, I guess they could just be in the parent node.

panel {
remote = <>;
common-video-property = ;
};

The above would imply one port and one endpoint. Would that work? If we
had a function like parse_endpoint(node), we could just point it to
either a real endpoint node, or to the device's node.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/deedefa3/attachment-0001.pgp>

[PATCH/RFC v3 00/19] Common Display Framework

[Andrzej Hajda]

On 10/17/2013 10:18 AM, Tomi Valkeinen wrote:
> On 17/10/13 10:48, Andrzej Hajda wrote:
>
>> The main function of DSI is to transport pixels from one IP to another IP
>> and this function IMO should not be modeled by display entity.
>> "Power, clocks, etc" will be performed via control bus according to
>> panel demands.
>> If 'DSI chip' has additional functions for video processing they can
>> be modeled by CDF entity if it makes sense.
> Now I don't follow. What do you mean with "display entity" and with "CDF
> entity"? Are they the same?
Yes, they are the same, sorry for confusion.
>
> Let me try to clarify my point:
>
> On OMAP SoC we have a DSI encoder, which takes input from the display
> controller in parallel RGB format, and outputs DSI.
>
> Then there are external encoders that take MIPI DPI as input, and output
> DSI.
>
> The only difference with the above two components is that the first one
> is embedded into the SoC. I see no reason to represent them in different
> ways (i.e. as you suggested, not representing the SoC's DSI at all).
>
> Also, if you use DSI burst mode, you will have to have different video
> timings in the DSI encoder's input and output. And depending on the
> buffering of the DSI encoder, you could have different timings in any case.
I am not sure what exactly the encoder performs, if this is only image
transport from dispc to panel CDF pipeline in both cases should look like:
dispc > panel
The only difference is that panels will be connected via different Linux bus
adapters, but it will be irrelevant to CDF itself. In this case I would say
this is DSI-master rather than encoder, or at least that the only
function of the
encoder is DSI.

If display_timings on input and output differs, I suppose it should be
modeled
as display_entity, as this is an additional functionality(not covered by
DSI standard AFAIK).
CDF in such case:
dispc ---> encoder ---> panel
In this case I would call it encoder with DSI master.

>
> Furthermore, both components could have extra processing. I know the
> external encoders sometimes do have features like scaling.
The same as above, ISP with embedded DSI.
>
>>> We still have two different endpoint configurations for the same
>>> DSI-master port. If that configuration is in the DSI-master's port node,
>>> not inside an endpoint data, then that can't be supported.
>> I am not sure if I understand it correctly. But it seems quite simple:
>> when panel starts/resumes it request DSI (via control bus) to fulfill
>> its configuration settings.
>> Of course there are some settings which are not panel dependent and those
>> should reside in DSI node.
> Exactly. And when the two panels require different non-panel-dependent
> settings, how do you represent them in the DT data?

non-panel-dependent setting cannot depend on panel, by definition :)
>
 We say then: callee handles locking :)
>>> Sure, but my point was that the caller handling the locking is much
>>> simpler than the callee handling locking. And the latter causes
>>> atomicity issues, as the other API could be invoked in between two calls
>>> for the first API.
>>>
>>> 
>> Could you describe such scenario?
> If we have two independent APIs, ctrl and video, that affect the same
> underlying hardware, the DSI bus, we could have a scenario like this:
>
> thread 1:
>
> ctrl->op_foo();
> ctrl->op_bar();
>
> thread 2:
>
> video->op_baz();
>
> Even if all those ops do locking properly internally, the fact that
> op_baz() can be called in between op_foo() and op_bar() may cause problems.
>
> To avoid that issue with two APIs we'd need something like:
>
> thread 1:
>
> ctrl->lock();
> ctrl->op_foo();
> ctrl->op_bar();
> ctrl->unlock();
>
> thread 2:
>
> video->lock();
> video->op_baz();
> video->unlock();
I should mention I was asking for real hw/drivers configuration.
I do not know what do you mean with video->op_baz() ?
DSI-master is not modeled in CDF, and only CDF provides video
operations.

I guess one scenario, when two panels are connected to single DSI-master.
In such case both can call DSI ops, but I do not know how do you want to
prevent it in case of your CDF-T implementation.

>
 Platform devices
 
 Platform devices are devices that typically appear as autonomous
 entities in the system. This includes legacy port-based devices and
 host bridges to peripheral buses, and most controllers integrated
 into system-on-chip platforms.  What they usually have in common
 is direct addressing from a CPU bus.  Rarely, a platform_device will
 be connected through a segment of some other kind of bus; but its
 registers will still be directly addressable.
>>> Yep, "typically" and "rarely" =). I agree, it's not clear. I think there
>>> are things with DBI/DSI that clearly point to a platform device, but
>>> also the other way.
>> Just to be sure, we are talking here about DSI-slaves, ie. for example
>> about panels,
>> where direct accessing from 

[RFR 2/2] drm/panel: Add simple panel support

[Laurent Pinchart]

 to have a DSI panel here. If the panel is truly not controlled in any
> > > > way, maybe having the panel as a normal platform device is ok. But if
> > > > the DSI panel is controlled with DSI messages, should it be a child of
> > > > the dsi-controller node, the same way i2c peripherals are children of
> > > > i2c master?
> > > 
> > > That's one possibility. In this particular case it's not even necessary
> > > to use any of DSIs control methods to talk to the panel. The panel only
> > > receives the data stream and "just works".
> > > 
> > > Even if the panel were to be controlled via DSI, I think keeping it as
> > > a separate device node is equally valid. It's really boils down to what
> > > I already said above. The output uses the panel as a resource, similar
> > > to how other devices might use a GPIO controller to use a GPIO pin, or
> > > an interrupt controller to request an IRQ.
> > 
> > Except that the display controller can also provides resources to the
> > panel. For DSI panels that support DSI commands, the control bus is
> > provided by the display controller. Much like an I2C device must be a
> > child of its I2C controller node, the DSI panel device should then be a
> > child of the display controller node.
> 
> I don't think resources is the right term here. The DSI controller
> provides services. That's traditionally been handled in DT by handing
> out phandles to the service providers.
> 
> That said I see why it would make sense to make a DSI panel the child of
> the DSI controller node, so we agree at least partially. I don't like it
> all that much because it makes handling of panels inconsistent across
> various types of output and therefore makes it more complicated to
> support it drivers, but I'm sure I can live with it if somebody wants to
> enforce that.
> 
> > > > > I do agree though that it might be useful to tweak the mode in case
> > > > > the default one doesn't work. How about we provide a means to
> > > > > override the mode encoded in the driver using one specified in the
> > > > > DT? That's obviously a backwards-compatible change, so it could be
> > > > > added if or when it becomes necessary.
> > > > 
> > > > This sounds good to me.
> > > > 
> > > > Although maybe it'd be good to have the driver compatible with
> > > > something like "generic-panel", so that you could have:
> > > > 
> > > > compatible = "foo,specific-panel", "generic-panel";
> > > > 
> > > > and if there's no need for any power/gpio setup for your board, you
> > > > may skip adding "foo,specific-panel" support to the panel driver.
> > > > Later, somebody else may need to implement fine grained power/gpio
> > > > support for "foo,specific-panel", and it would just work.
> > > > 
> > > > Maybe that would help us with the problem of adding support in the
> > > > driver for a hundred different simple panels each used only once on a
> > > > specific board.
> > > 
> > > Sure, that all sounds like reasonable additions. All of the suggestions
> > > are even backwards-compatible and hence can be added when needed without
> > > breaking compatibility with existing users of the binding.
> > 
> > What's wrong with moving the three hardcoded modes we already have in the
> > driver to DT before pushing this to mainline ?
> 
> I think I've answered that in a different subthread.

Then I might not have understood your answer :-)

-- 
Regards,

Laurent Pinchart
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/92a72d44/attachment.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Laurent Pinchart]

Hi Tomi,

On Thursday 17 October 2013 14:59:41 Tomi Valkeinen wrote:
> On 17/10/13 14:51, Laurent Pinchart wrote:
> >> I'm not sure if there's a specific need for the port or endpoint nodes
> >> in cases like the above. Even if we have common properties describing
> >> the endpoint, I guess they could just be in the parent node.
> >> 
> >> panel {
> >>remote = <>;
> >>common-video-property = ;
> >> };
> >> 
> >> The above would imply one port and one endpoint. Would that work? If we
> >> had a function like parse_endpoint(node), we could just point it to
> >> either a real endpoint node, or to the device's node.
> > 
> > You reference the display controller here, not a specific display
> > controller output. Don't most display controllers have several outputs ?
> 
> Sure. Then the display controller could have more verbose description.
> But the panel could still have what I wrote above, except the 'remote'
> property would point to a real endpoint node inside the dispc node, not
> to the dispc node.
> 
> This would, of course, need some extra code to handle the different
> cases, but just from DT point of view, I think all the relevant
> information is there.

There's many ways to describe the same information in DT. While we could have 
DT bindings that use different descriptions for different devices and still 
support all of them in our code, why should we opt for that option that will 
make the implementation much more complex when we can describe connections in 
a simple and generic way ?

-- 
Regards,

Laurent Pinchart
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/3a8c2808/attachment.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Laurent Pinchart]

T 
node ? It describe what's there: the panel has one input, why not make that 
explicit ?

> > > > > +static void panel_simple_enable(struct drm_panel *panel)
> > > > > +{
> > > > > + struct panel_simple *p = to_panel_simple(panel);
> > > > > + int err;
> > > > > +
> > > > > + if (p->enabled)
> > > > > + return;
> > > > > +
> > > > > + err = regulator_enable(p->supply);
> > > > > + if (err < 0)
> > > > > + dev_err(panel->dev, "failed to enable supply: %d\n", 
> > > > > err);
> > > > 
> > > > Is that really a non-fatal error ? Shouldn't the enable operation
> > > > return an int ?
> > > 
> > > There's no way to propagate this in DRM, so why go through the trouble
> > > of returning the error? Furthermore, there's nothing that the caller
> > > could do to remedy the situation anyway.
> > 
> > That's a DRM issue, which could be fixed. While the caller can't remedy
> > the situation, it should at least report the error to the application
> > instead of silently ignoring it.
> 
> Perhaps. It's not really relevant to the discussion and can always be
> fixed in a subsequent patch.

Most things can be fixed by a subsequent patch, that's not a good enough 
reason not to address the known problems before pushing the code to mainline 
(to clarify my point of view, there's no need to fix DRM to use the return 
value before pushing this patch set to mainline, but I'd like a v2 with an int 
return value).

> > > > > +static const struct drm_display_mode auo_b101aw03_mode = {
> > > > > + .clock = 51450,
> > > > > + .hdisplay = 1024,
> > > > > + .hsync_start = 1024 + 156,
> > > > > + .hsync_end = 1024 + 156 + 8,
> > > > > + .htotal = 1024 + 156 + 8 + 156,
> > > > > + .vdisplay = 600,
> > > > > + .vsync_start = 600 + 16,
> > > > > + .vsync_end = 600 + 16 + 6,
> > > > > + .vtotal = 600 + 16 + 6 + 16,
> > > > > + .vrefresh = 60,
> > > > > +};
> > > > 
> > > > Instead of hardcoding the modes in the driver, which would then
> > > > require to be updated for every new simple panel model (and we know
> > > > there are lots of them), why don't you specify the modes in the panel
> > > > DT node ? The simple panel driver would then become much more generic.
> > > > It would also allow board designers to tweak h/v sync timings
> > > > depending on the system requirements.
> > > 
> > > Sigh... we keep second-guessing ourselves. Back at the time when power
> > > sequences were designed (and NAKed at the last minute), we all decided
> > > that the right thing to do would be to use specific compatible values
> > > for individual panels, because that would allow us to encode the power
> > > sequencing within the driver. And when we already have the panel model
> > > encoded in the compatible value, we might just as well encode the mode
> > > within the driver for that panel. Otherwise we'll have to keep adding
> > > the same mode timings for every board that uses the same panel.
> > > 
> > > I do agree though that it might be useful to tweak the mode in case the
> > > default one doesn't work. How about we provide a means to override the
> > > mode encoded in the driver using one specified in the DT? That's
> > > obviously a backwards-compatible change, so it could be added if or when
> > > it becomes necessary.
> > 
> > I share Tomi's point of view here. Your three panels use the same power
> > sequence, so I believe we should have a generic panel compatible string
> > that would use modes described in DT for the common case. Only if a panel
> > required something more complex which can't (or which could, but won't
> > for any reason) accurately be described in DT would you need to extend
> > the driver.
>
> I don't see the point quite frankly. You seem to be assuming that every
> panel will only be used in a single board.

No, but in practice that's often the case, at least for boards with .dts files 
in the mainline kernel.

> However what you're proposing will require the mode timings to be repeated
> in every board's DT file, if the same panel is ever used on more than a
> single board.

Is that a problem ? You could #include a .dtsi for the panel that would 
specify the video mode if you want to avoid multiple copies.

-- 
Regards,

Laurent Pinchart
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/711c8eee/attachment-0001.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Thierry Reding]

On Thu, Oct 17, 2013 at 02:50:31PM +0300, Tomi Valkeinen wrote:
> On 17/10/13 14:05, Thierry Reding wrote:
> 
> >> That's quite similar to how my current out-of-mainline OMAP DSS DT
> >> bindings work. The difference is that I have a reference from the panel
> >> node to the video source (dsi-controller), instead of the other way
> >> around. I just find it more natural. It works the same way as, say,
> >> regulators, gpios, etc.
> > 
> > I suppose that depends on the way you look at it. In the above proposal
> > I consider the output (DSI controller) to use the panel as a resource
> > that provides a certain set of services (query mode timings, provide a
> > way to enable or disable the panel). Similarly the panel uses the
> > backlight as a resource that provides a certain set of services (such as
> > changing the brightness).
> > 
> > The above also follows the natural order of control. The panel has no
> > way to control the DSI output. However, it is the output that controls
> > when a panel is required and turn it on as appropriate.
> 
> I've discussed this issue to death with other people, and for some
> reason I still see this the other way =).
> 
> I see the panel using the DSI controller as a resource. The panel driver
> uses the DSI controller to send the video data to the panel device.
> Compare this to, say, i2c client driver, which uses i2c master to send
> data to the i2c device.
> 
> In the OMAP DSS driver model, the panel driver is in control. The panel
> driver tells the DSI controller which kind of video timings to use, or
> which bus speed to use, or whether to use low-power or high-speed mode,
> when to enable the DSI clock or DSI video stream, etc.
> 
> With a simple panel the model you describe works usually fine. The
> reason I went with the different model is that we had rather complex
> display peripherals, that needed more specific control of the bus.
> 
> A simple example could be the enable sequence. I presume in your model
> the DSI controller's enable would be maybe something like:
> 
> configure_dsi_timings(panel->get_timings());
> enable_dsi_output();
> panel->enable();
> 
> What if the DSI peripheral requires an enable sequence of, say:
> 
> - enable DSI clock
> - use LP mode
> - send some config messages
> - re-configure DSI clock to higher speed
> - send some more config messages
> - enable HS mode
> - send some more config messages
> - enable DSI video stream
> 
> It gets a bit difficult to manage that kind of setup in a generic way in
> the DSI controller driver. As I see it, the DSI peripheral driver has to
> be in control of the nuances. The same goes for other video buses also,
> but DSI is perhaps one of the most complex ones to support.

I've briefly gone over this in another subthread, but here it is again
for the sake of completeness.

The difference in perception I think comes from the fact that the DSI
panel *driver* may need to control the DSI bus. However, DT describes
the hardware in terms of devices. And from that point of view it is
still the DSI _control_ler that _control_s the panel. There's no
electrical way that the panel can take possession of the bus. Well,
there's BTA, but even that happens under supervision of the DSI
controller.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/395325d5/attachment-0001.pgp>

[PATCH v2 26/26] drm/exynos: Consolidate suspend/resume in drm_drv

[Inki Dae]

> -Original Message-
> From: Sean Paul [mailto:seanpaul at chromium.org]
> Sent: Thursday, October 17, 2013 5:40 AM
> To: dri-devel; InKi Dae
> Cc: Dave Airlie; Tomasz Figa; St?phane Marchesin; Sean Paul
> Subject: Re: [PATCH v2 26/26] drm/exynos: Consolidate suspend/resume in
> drm_drv
> 
> On Wed, Oct 16, 2013 at 3:26 PM, Sean Paul  wrote:
> > This patch removes all of the suspend/resume logic from the individual
> > drivers and consolidates it in drm_drv. This consolidation reduces the
> > number of functions which enable/disable the hardware to just one -- the
> > dpms callback. This ensures that we always power up/down in a consistent
> > manner.
> >
> > Signed-off-by: Sean Paul 
> > ---
> >
> > Changes in v2:
> > - Added to the patchset
> >
> >  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 97
> 
> >  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 91 +
> -
> >  drivers/gpu/drm/exynos/exynos_hdmi.c | 82
+-
> -
> >  drivers/gpu/drm/exynos/exynos_mixer.c| 75 +---
> >  4 files changed, 127 insertions(+), 218 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > index 03caa3a..91d6863 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > @@ -11,6 +11,7 @@
> >   * option) any later version.
> >   */
> >
> > +#include 
> >  #include 
> >  #include 
> >
> > @@ -51,6 +52,7 @@ static int exynos_drm_load(struct drm_device *dev,
> unsigned long flags)
> > return -ENOMEM;
> >
> > INIT_LIST_HEAD(>pageflip_event_list);
> > +   dev_set_drvdata(dev->dev, dev);
> > dev->dev_private = (void *)private;
> >
> > /*
> > @@ -155,6 +157,41 @@ static int exynos_drm_unload(struct drm_device
*dev)
> > return 0;
> >  }
> >
> > +static int exynos_drm_suspend(struct drm_device *dev, pm_message_t
> state)
> > +{
> > +   struct drm_connector *connector;
> > +
> > +   drm_modeset_lock_all(dev);
> > +   list_for_each_entry(connector, >mode_config.connector_list,
> head) {
> > +   int old_dpms = connector->dpms;
> > +
> > +   if (connector->funcs->dpms)
> > +   connector->funcs->dpms(connector,
DRM_MODE_DPMS_OFF);
> > +
> > +   /* Set the old mode back to the connector for resume */
> > +   connector->dpms = old_dpms;
> > +   }
> > +   drm_modeset_unlock_all(dev);
> > +
> > +   return 0;
> > +}
> > +
> > +static int exynos_drm_resume(struct drm_device *dev)
> > +{
> > +   struct drm_connector *connector;
> > +
> > +   drm_modeset_lock_all(dev);
> > +   list_for_each_entry(connector, >mode_config.connector_list,
> head) {
> > +   if (connector->funcs->dpms)
> > +   connector->funcs->dpms(connector,
connector->dpms);
> > +   }
> > +
> > +   drm_helper_resume_force_mode(dev);
> > +   drm_modeset_unlock_all(dev);
> > +
> > +   return 0;
> > +}
> > +
> >  static int exynos_drm_open(struct drm_device *dev, struct drm_file
> *file)
> >  {
> > struct drm_exynos_file_private *file_priv;
> > @@ -262,6 +299,8 @@ static struct drm_driver exynos_drm_driver = {
> > DRIVER_GEM | DRIVER_PRIME,
> > .load   = exynos_drm_load,
> > .unload = exynos_drm_unload,
> > +   .suspend= exynos_drm_suspend,
> > +   .resume = exynos_drm_resume,
> > .open   = exynos_drm_open,
> > .preclose   = exynos_drm_preclose,
> > .lastclose  = exynos_drm_lastclose,
> > @@ -293,6 +332,9 @@ static int exynos_drm_platform_probe(struct
> platform_device *pdev)
> >  {
> > pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> >
> > +   pm_runtime_enable(>dev);
> > +   pm_runtime_get_sync(>dev);
> > +
> > return drm_platform_init(_drm_driver, pdev);
> >  }
> >
> > @@ -303,12 +345,67 @@ static int exynos_drm_platform_remove(struct
> platform_device *pdev)
> > return 0;
> >  }
> >
> > +#ifdef CONFIG_PM_SLEEP
> > +static int exynos_drm_sys_suspend(struct device *dev)
> > +{
> > +   struct drm_device *drm_dev = dev_get_drvdata(dev);
> > +   pm_message_t message;
> > +
> > +   if (pm_runtime_suspended(dev))
> > +   return 0;
> > +
> > +   message.event = PM_EVENT_SUSPEND;
> > +   return exynos_drm_suspend(drm_dev, message);
> > +}
> > +
> > +static int exynos_drm_sys_resume(struct device *dev)
> > +{
> > +   struct drm_device *drm_dev = dev_get_drvdata(dev);
> > +
> > +   if (pm_runtime_suspended(dev))
> > +   return 0;
> > +
> > +   return exynos_drm_resume(drm_dev);
> > +}
> > +#endif
> > +
> > +#ifdef CONFIG_PM_RUNTIME
> > +static int 

[RFR 2/2] drm/panel: Add simple panel support

[Thierry Reding]

, or
> > an interrupt controller to request an IRQ.
> 
> Except that the display controller can also provides resources to the panel. 
> For DSI panels that support DSI commands, the control bus is provided by the 
> display controller. Much like an I2C device must be a child of its I2C 
> controller node, the DSI panel device should then be a child of the display 
> controller node.

I don't think resources is the right term here. The DSI controller
provides services. That's traditionally been handled in DT by handing
out phandles to the service providers.

That said I see why it would make sense to make a DSI panel the child of
the DSI controller node, so we agree at least partially. I don't like it
all that much because it makes handling of panels inconsistent across
various types of output and therefore makes it more complicated to
support it drivers, but I'm sure I can live with it if somebody wants to
enforce that.

> > > > I do agree though that it might be useful to tweak the mode in case the
> > > > default one doesn't work. How about we provide a means to override the
> > > > mode encoded in the driver using one specified in the DT? That's
> > > > obviously a backwards-compatible change, so it could be added if or when
> > > > it becomes necessary.
> > > 
> > > This sounds good to me.
> > > 
> > > Although maybe it'd be good to have the driver compatible with something
> > > like "generic-panel", so that you could have:
> > > 
> > > compatible = "foo,specific-panel", "generic-panel";
> > > 
> > > and if there's no need for any power/gpio setup for your board, you may
> > > skip adding "foo,specific-panel" support to the panel driver. Later,
> > > somebody else may need to implement fine grained power/gpio support for
> > > "foo,specific-panel", and it would just work.
> > > 
> > > Maybe that would help us with the problem of adding support in the
> > > driver for a hundred different simple panels each used only once on a
> > > specific board.
> > 
> > Sure, that all sounds like reasonable additions. All of the suggestions
> > are even backwards-compatible and hence can be added when needed without
> > breaking compatibility with existing users of the binding.
> 
> What's wrong with moving the three hardcoded modes we already have in the 
> driver to DT before pushing this to mainline ?

I think I've answered that in a different subthread.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/63a1008f/attachment.pgp>

[PATCH 0/7] drm: Return -ENOENT when objects can't be found

[Jakob Bornecrantz]

On Thu, Oct 17, 2013 at 12:34 PM,   wrote:
> We're rather inconsistent in which error values we return to userspace
> on failure. I want to unify the behaviour a bit and consistently return
> ENOENT when mode object lookups fail. We already do that in a few places
> but in most places we just return EINVAL.
>
> I made a separate patch for each affected driver just in case there's some
> magic meaning to the error values for certain drivers.
>
>
> Ville Syrj?l? (7):
>   drm: Consistently return -ENOENT when a mode object can't be found
>   drm: Return -ENOENT when a framebuffer can't be found
>   drm/gma500: Return -ENOENT when a mode object can't be found
>   drm/i915: Return -ENOENT when a mode object can't be found
>   drm/radeon: Return -ENOENT when a mode object can't be found
>   drm/vmwgfx: Return -ENOENT when a mode object can't be found
>   drm/vmwgfx: Return -ENOENT when a framebuffer can't be found

Sounds good to me, vmwgfx patches are:
Reviewed-By: Jakob Bornecrantz 

Cheers, Jakob.

[RFR 2/2] drm/panel: Add simple panel support

[Laurent Pinchart]

Hi Tomi,

On Thursday 17 October 2013 14:35:49 Tomi Valkeinen wrote:
> On 17/10/13 14:02, Laurent Pinchart wrote:
> >> Okay, so if I understand correctly, translating those bindings to panel
> >> 
> >> nodes would look somewhat like this:
> >>dc: display-controller {
> >>ports {
> >>port at 0 {
> >>remote-endpoint = <>;
> >>};
> >>};
> >>};
> >>
> >>panel: panel {
> >>ports {
> >>port at 0 {
> >>remote-endpoint = <>;
> >>};
> >>};
> >>};
> >> 
> >> The above leaves out any of the other, non-relevant properties. Does
> >> that sound about right?
> > 
> > Yes it does.
> 
> It does?
> 
> Shouldn't it be something like:
> 
> panel {
>   ports {
>   port at 0 {
>   endpoint at 0 {
>   remote = <>;
>   };
>   };
>   };
> };
> 
> And simplified:
> 
> panel {
>   port {
>   endpoint at 0 {
>   remote = <>;
>   };
>   };
> };
> 
> You do need a node for the endpoint, a remote-endpoint property is not
> enough.

My bad, you'r absolutely right. More sleep is needed.

(And while we're at it, the remote-endpoint properties must point to an 
endpoint, not the device DT node.

> > Please note that, when a device has as single port, the ports node can be
> > omitted, and the port doesn't need to be numbered. You would then end up
> > with> 
> > dc: display-controller {
> > port {
> > remote-endpoint = <>;
> > };
> > };
> > 
> > panel: panel {
> > port {
> > remote-endpoint = <>;
> > };
> > };
> > 
> > I don't think there's a way to simplify it further.
> 
> I'm not sure if there's a specific need for the port or endpoint nodes
> in cases like the above. Even if we have common properties describing
> the endpoint, I guess they could just be in the parent node.
> 
> panel {
>   remote = <>;
>   common-video-property = ;
> };
> 
> The above would imply one port and one endpoint. Would that work? If we
> had a function like parse_endpoint(node), we could just point it to
> either a real endpoint node, or to the device's node.

You reference the display controller here, not a specific display controller 
output. Don't most display controllers have several outputs ?

-- 
Regards,

Laurent Pinchart
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/2584b329/attachment.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Thierry Reding]

ht thing to do would be to use specific compatible values
> > for individual panels, because that would allow us to encode the power
> > sequencing within the driver. And when we already have the panel model
> > encoded in the compatible value, we might just as well encode the mode
> > within the driver for that panel. Otherwise we'll have to keep adding
> > the same mode timings for every board that uses the same panel.
> > 
> > I do agree though that it might be useful to tweak the mode in case the
> > default one doesn't work. How about we provide a means to override the
> > mode encoded in the driver using one specified in the DT? That's
> > obviously a backwards-compatible change, so it could be added if or when
> > it becomes necessary.
> 
> I share Tomi's point of view here. Your three panels use the same power 
> sequence, so I believe we should have a generic panel compatible string that 
> would use modes described in DT for the common case. Only if a panel required 
> something more complex which can't (or which could, but won't for any reason) 
> accurately be described in DT would you need to extend the driver.

I don't see the point quite frankly. You seem to be assuming that every
panel will only be used in a single board. However what you're proposing
will require the mode timings to be repeated in every board's DT file,
if the same panel is ever used on more than a single board.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/24f979e8/attachment.pgp>

[PATCH 7/7] drm/vmwgfx: Return -ENOENT when a framebuffer can't be found

[ville.syrj...@linux.intel.com]

From: Ville Syrj?l? 

Let's be a bit more consistent with our error values.

Signed-off-by: Ville Syrj?l? 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
index c509d40..a51f48e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
@@ -168,7 +168,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
fb = drm_framebuffer_lookup(dev, arg->fb_id);
if (!fb) {
DRM_ERROR("Invalid framebuffer id.\n");
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out_no_fb;
}
vfb = vmw_framebuffer_to_vfb(fb);
@@ -252,7 +252,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void 
*data,
fb = drm_framebuffer_lookup(dev, arg->fb_id);
if (!fb) {
DRM_ERROR("Invalid framebuffer id.\n");
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out_no_fb;
}

-- 
1.8.1.5

[PATCH 6/7] drm/vmwgfx: Return -ENOENT when a mode object can't be found

[ville.syrj...@linux.intel.com]

From: Ville Syrj?l? 

Let's be a bit more consistent with our error values.

Signed-off-by: Ville Syrj?l? 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index fc43c06..ecb3d86 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1508,7 +1508,7 @@ int vmw_kms_cursor_bypass_ioctl(struct drm_device *dev, 
void *data,

obj = drm_mode_object_find(dev, arg->crtc_id, DRM_MODE_OBJECT_CRTC);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out;
}

-- 
1.8.1.5

[PATCH 5/7] drm/radeon: Return -ENOENT when a mode object can't be found

[ville.syrj...@linux.intel.com]

From: Ville Syrj?l? 

Let's be a bit more consistent with our error values.

Signed-off-by: Ville Syrj?l? 
---
 drivers/gpu/drm/radeon/r100.c| 2 +-
 drivers/gpu/drm/radeon/r600_cs.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index d713330..784983d 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -1434,7 +1434,7 @@ int r100_cs_packet_parse_vline(struct radeon_cs_parser *p)
obj = drm_mode_object_find(p->rdev->ddev, crtc_id, 
DRM_MODE_OBJECT_CRTC);
if (!obj) {
DRM_ERROR("cannot find crtc %d\n", crtc_id);
-   return -EINVAL;
+   return -ENOENT;
}
crtc = obj_to_crtc(obj);
radeon_crtc = to_radeon_crtc(crtc);
diff --git a/drivers/gpu/drm/radeon/r600_cs.c b/drivers/gpu/drm/radeon/r600_cs.c
index 01a3ec8..44f9ed9 100644
--- a/drivers/gpu/drm/radeon/r600_cs.c
+++ b/drivers/gpu/drm/radeon/r600_cs.c
@@ -887,7 +887,7 @@ int r600_cs_common_vline_parse(struct radeon_cs_parser *p,
obj = drm_mode_object_find(p->rdev->ddev, crtc_id, 
DRM_MODE_OBJECT_CRTC);
if (!obj) {
DRM_ERROR("cannot find crtc %d\n", crtc_id);
-   return -EINVAL;
+   return -ENOENT;
}
crtc = obj_to_crtc(obj);
radeon_crtc = to_radeon_crtc(crtc);
-- 
1.8.1.5

[PATCH 4/7] drm/i915: Return -ENOENT when a mode object can't be found

[ville.syrj...@linux.intel.com]

From: Ville Syrj?l? 

Let's be a bit more consistent with our error values.

Signed-off-by: Ville Syrj?l? 
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 drivers/gpu/drm/i915/intel_sprite.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 4452a5b..b40492c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9777,7 +9777,7 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev, 
void *data,

if (!drmmode_obj) {
DRM_ERROR("no such CRTC id\n");
-   return -EINVAL;
+   return -ENOENT;
}

crtc = to_intel_crtc(obj_to_crtc(drmmode_obj));
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 8afaad6..07b13dc 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -955,7 +955,7 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void 
*data,

obj = drm_mode_object_find(dev, set->plane_id, DRM_MODE_OBJECT_PLANE);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out_unlock;
}

@@ -984,7 +984,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void 
*data,

obj = drm_mode_object_find(dev, get->plane_id, DRM_MODE_OBJECT_PLANE);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out_unlock;
}

-- 
1.8.1.5

[PATCH 3/7] drm/gma500: Return -ENOENT when a mode object can't be found

[ville.syrj...@linux.intel.com]

From: Ville Syrj?l? 

Let's be a bit more consistent with our error values.

Signed-off-by: Ville Syrj?l? 
---
 drivers/gpu/drm/gma500/psb_drv.c   | 4 ++--
 drivers/gpu/drm/gma500/psb_intel_display.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index dd607f8..679f953 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -449,7 +449,7 @@ static int psb_gamma_ioctl(struct drm_device *dev, void 
*data,
obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_CONNECTOR);
if (!obj) {
dev_dbg(dev->dev, "Invalid Connector object.\n");
-   return -EINVAL;
+   return -ENOENT;
}

connector = obj_to_connector(obj);
@@ -491,7 +491,7 @@ static int psb_mode_operation_ioctl(struct drm_device *dev, 
void *data,
obj = drm_mode_object_find(dev, obj_id,
DRM_MODE_OBJECT_CONNECTOR);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto mode_op_out;
}

diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c 
b/drivers/gpu/drm/gma500/psb_intel_display.c
index 97f8a03..c8841ac 100644
--- a/drivers/gpu/drm/gma500/psb_intel_display.c
+++ b/drivers/gpu/drm/gma500/psb_intel_display.c
@@ -572,7 +572,7 @@ int psb_intel_get_pipe_from_crtc_id(struct drm_device *dev, 
void *data,

if (!drmmode_obj) {
dev_err(dev->dev, "no such CRTC id\n");
-   return -EINVAL;
+   return -ENOENT;
}

crtc = to_gma_crtc(obj_to_crtc(drmmode_obj));
-- 
1.8.1.5

[PATCH 2/7] drm: Return -ENOENT when a framebuffer can't be found

[ville.syrj...@linux.intel.com]

From: Ville Syrj?l? 

Return -ENOENT for framebuffers like we do for other mode objects that
can't be found.

Signed-off-by: Ville Syrj?l? 
---
 drivers/gpu/drm/drm_crtc.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index bb5dedd..197956b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2164,7 +2164,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
if (!fb) {
DRM_DEBUG_KMS("Unknown FB ID%d\n",
crtc_req->fb_id);
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out;
}
}
@@ -2654,7 +2654,7 @@ fail_lookup:
mutex_unlock(>mode_config.fb_lock);
mutex_unlock(_priv->fbs_lock);

-   return -EINVAL;
+   return -ENOENT;
 }

 /**
@@ -2682,7 +2682,7 @@ int drm_mode_getfb(struct drm_device *dev,

fb = drm_framebuffer_lookup(dev, r->fb_id);
if (!fb)
-   return -EINVAL;
+   return -ENOENT;

r->height = fb->height;
r->width = fb->width;
@@ -2727,7 +2727,7 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,

fb = drm_framebuffer_lookup(dev, r->fb_id);
if (!fb)
-   return -EINVAL;
+   return -ENOENT;

num_clips = r->num_clips;
clips_ptr = (struct drm_clip_rect __user *)(unsigned long)r->clips_ptr;
@@ -3636,8 +3636,10 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
goto out;

fb = drm_framebuffer_lookup(dev, page_flip->fb_id);
-   if (!fb)
+   if (!fb) {
+   ret = -ENOENT;
goto out;
+   }

ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, >mode, fb);
if (ret)
-- 
1.8.1.5

[PATCH 1/7] drm: Consistently return -ENOENT when a mode object can't be found

[ville.syrj...@linux.intel.com]

From: Ville Syrj?l? 

We tend to return -EINVAL for everything. Let's try to help poor
userland developers a bit by at least returning -ENONET for missing
objects.

Signed-off-by: Ville Syrj?l? 
---
 drivers/gpu/drm/drm_crtc.c | 32 ++--
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d7a8370..bb5dedd 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1557,7 +1557,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
obj = drm_mode_object_find(dev, crtc_resp->crtc_id,
   DRM_MODE_OBJECT_CRTC);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out;
}
crtc = obj_to_crtc(obj);
@@ -1641,7 +1641,7 @@ int drm_mode_getconnector(struct drm_device *dev, void 
*data,
obj = drm_mode_object_find(dev, out_resp->connector_id,
   DRM_MODE_OBJECT_CONNECTOR);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out;
}
connector = obj_to_connector(obj);
@@ -1757,7 +1757,7 @@ int drm_mode_getencoder(struct drm_device *dev, void 
*data,
obj = drm_mode_object_find(dev, enc_resp->encoder_id,
   DRM_MODE_OBJECT_ENCODER);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out;
}
encoder = obj_to_encoder(obj);
@@ -2141,7 +2141,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
   DRM_MODE_OBJECT_CRTC);
if (!obj) {
DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_req->crtc_id);
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out;
}
crtc = obj_to_crtc(obj);
@@ -2232,7 +2232,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
if (!obj) {
DRM_DEBUG_KMS("Connector id %d unknown\n",
out_id);
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out;
}
connector = obj_to_connector(obj);
@@ -2280,7 +2280,7 @@ static int drm_mode_cursor_common(struct drm_device *dev,
obj = drm_mode_object_find(dev, req->crtc_id, DRM_MODE_OBJECT_CRTC);
if (!obj) {
DRM_DEBUG_KMS("Unknown CRTC ID %d\n", req->crtc_id);
-   return -EINVAL;
+   return -ENOENT;
}
crtc = obj_to_crtc(obj);

@@ -3059,7 +3059,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
drm_modeset_lock_all(dev);
obj = drm_mode_object_find(dev, out_resp->prop_id, 
DRM_MODE_OBJECT_PROPERTY);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto done;
}
property = obj_to_property(obj);
@@ -3188,7 +3188,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
drm_modeset_lock_all(dev);
obj = drm_mode_object_find(dev, out_resp->blob_id, 
DRM_MODE_OBJECT_BLOB);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto done;
}
blob = obj_to_blob(obj);
@@ -3349,7 +3349,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device 
*dev, void *data,

obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out;
}
if (!obj->properties) {
@@ -3402,8 +3402,10 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
*dev, void *data,
drm_modeset_lock_all(dev);

arg_obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type);
-   if (!arg_obj)
+   if (!arg_obj) {
+   ret = -ENOENT;
goto out;
+   }
if (!arg_obj->properties)
goto out;

@@ -3416,8 +3418,10 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
*dev, void *data,

prop_obj = drm_mode_object_find(dev, arg->prop_id,
DRM_MODE_OBJECT_PROPERTY);
-   if (!prop_obj)
+   if (!prop_obj) {
+   ret = -ENOENT;
goto out;
+   }
property = obj_to_property(prop_obj);

if (!drm_property_change_is_valid(property, arg->value))
@@ -3502,7 +3506,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
drm_modeset_lock_all(dev);
obj = drm_mode_object_find(dev, crtc_lut->crtc_id, 
DRM_MODE_OBJECT_CRTC);
if (!obj) {
-   ret = -EINVAL;
+   ret = -ENOENT;
goto out;
}
crtc = obj_to_crtc(obj);
@@ -3561,7 +3565,7 @@ int 

[PATCH 0/7] drm: Return -ENOENT when objects can't be found

[ville.syrj...@linux.intel.com]

We're rather inconsistent in which error values we return to userspace
on failure. I want to unify the behaviour a bit and consistently return 
ENOENT when mode object lookups fail. We already do that in a few places
but in most places we just return EINVAL.

I made a separate patch for each affected driver just in case there's some
magic meaning to the error values for certain drivers.


Ville Syrj?l? (7):
  drm: Consistently return -ENOENT when a mode object can't be found
  drm: Return -ENOENT when a framebuffer can't be found
  drm/gma500: Return -ENOENT when a mode object can't be found
  drm/i915: Return -ENOENT when a mode object can't be found
  drm/radeon: Return -ENOENT when a mode object can't be found
  drm/vmwgfx: Return -ENOENT when a mode object can't be found
  drm/vmwgfx: Return -ENOENT when a framebuffer can't be found

 drivers/gpu/drm/drm_crtc.c | 44 +-
 drivers/gpu/drm/gma500/psb_drv.c   |  4 +--
 drivers/gpu/drm/gma500/psb_intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c   |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c|  4 +--
 drivers/gpu/drm/radeon/r100.c  |  2 +-
 drivers/gpu/drm/radeon/r600_cs.c   |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c  |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c|  2 +-
 9 files changed, 36 insertions(+), 30 deletions(-)

[PATCH] drm: never write to the userspace more data than the caller wants

[Chris Wilson]

On Wed, Oct 16, 2013 at 08:12:35PM -0400, Pavel Roskin wrote:
> The amount of data wanted by the userspace caller is encoded in the
> ioctl number.  Generic drm ioctls were ignoring it.
> 
> As a result, Intel Xorg driver didn't work for i386 userspace on x86_64
> kernel on some systems.  sizeof(struct drm_mode_get_connector) is 76
> bytes on i686 and 80 bytes on x86_64 due to the tail alignment (the data
> positions match).  The userspace was using the 4 bytes after the
> structure to hold the result of the ioctl.  Since drm_ioctl() was
> copying 80 bytes instead of 76, it was clobbering that data.
> 
> A workaround has been committed to xf86-video-intel.
> 
> Signed-off-by: Pavel Roskin 
> Cc: stable at vger.kernel.org

Similar patch:
http://lists.freedesktop.org/archives/dri-devel/2013-October/047412.html
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[RFR 2/2] drm/panel: Add simple panel support

[Tomi Valkeinen]

On 17/10/13 11:53, Thierry Reding wrote:

> I keep wondering why we absolutely must have compatibility between CDF
> and this simple panel binding. DT content is supposed to concern itself
> with the description of hardware only. What about the following isn't an
> accurate description of the panel hardware?
> 
>   panel: panel {
>   compatible = "cptt,claa101wb01";
> 
>   power-supply = <_pnl_reg>;
>   enable-gpios = < 90 0>;
> 
>   backlight = <>;
>   };
> 
>   dsi-controller {
>   panel = <>;
>   };

That's quite similar to how my current out-of-mainline OMAP DSS DT
bindings work. The difference is that I have a reference from the panel
node to the video source (dsi-controller), instead of the other way
around. I just find it more natural. It works the same way as, say,
regulators, gpios, etc.

However, one thing unclear is where the panel node should be. You seem
to have a DSI panel here. If the panel is truly not controlled in any
way, maybe having the panel as a normal platform device is ok. But if
the DSI panel is controlled with DSI messages, should it be a child of
the dsi-controller node, the same way i2c peripherals are children of
i2c master?

>>> +static const struct drm_display_mode auo_b101aw03_mode = {
>>> +   .clock = 51450,
>>> +   .hdisplay = 1024,
>>> +   .hsync_start = 1024 + 156,
>>> +   .hsync_end = 1024 + 156 + 8,
>>> +   .htotal = 1024 + 156 + 8 + 156,
>>> +   .vdisplay = 600,
>>> +   .vsync_start = 600 + 16,
>>> +   .vsync_end = 600 + 16 + 6,
>>> +   .vtotal = 600 + 16 + 6 + 16,
>>> +   .vrefresh = 60,
>>> +};
>>
>> Instead of hardcoding the modes in the driver, which would then require to 
>> be 
>> updated for every new simple panel model (and we know there are lots of 
>> them), 
>> why don't you specify the modes in the panel DT node ? The simple panel 
>> driver 
>> would then become much more generic. It would also allow board designers to 
>> tweak h/v sync timings depending on the system requirements.
> 
> Sigh... we keep second-guessing ourselves. Back at the time when power
> sequences were designed (and NAKed at the last minute), we all decided
> that the right thing to do would be to use specific compatible values
> for individual panels, because that would allow us to encode the power
> sequencing within the driver. And when we already have the panel model
> encoded in the compatible value, we might just as well encode the mode
> within the driver for that panel. Otherwise we'll have to keep adding
> the same mode timings for every board that uses the same panel.

Oh, I didn't feel "we all decided that the right thing..." =).

> I do agree though that it might be useful to tweak the mode in case the
> default one doesn't work. How about we provide a means to override the
> mode encoded in the driver using one specified in the DT? That's
> obviously a backwards-compatible change, so it could be added if or when
> it becomes necessary.

This sounds good to me.

Although maybe it'd be good to have the driver compatible with something
like "generic-panel", so that you could have:

compatible = "foo,specific-panel", "generic-panel";

and if there's no need for any power/gpio setup for your board, you may
skip adding "foo,specific-panel" support to the panel driver. Later,
somebody else may need to implement fine grained power/gpio support for
"foo,specific-panel", and it would just work.

Maybe that would help us with the problem of adding support in the
driver for a hundred different simple panels each used only once on a
specific board.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/29c4dca6/attachment-0001.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Laurent Pinchart]

gt; >> system requirements.
> > > 
> > > Sigh... we keep second-guessing ourselves. Back at the time when power
> > > sequences were designed (and NAKed at the last minute), we all decided
> > > that the right thing to do would be to use specific compatible values
> > > for individual panels, because that would allow us to encode the power
> > > sequencing within the driver. And when we already have the panel model
> > > encoded in the compatible value, we might just as well encode the mode
> > > within the driver for that panel. Otherwise we'll have to keep adding
> > > the same mode timings for every board that uses the same panel.
> > 
> > Oh, I didn't feel "we all decided that the right thing..." =).
> > 
> > > I do agree though that it might be useful to tweak the mode in case the
> > > default one doesn't work. How about we provide a means to override the
> > > mode encoded in the driver using one specified in the DT? That's
> > > obviously a backwards-compatible change, so it could be added if or when
> > > it becomes necessary.
> > 
> > This sounds good to me.
> > 
> > Although maybe it'd be good to have the driver compatible with something
> > like "generic-panel", so that you could have:
> > 
> > compatible = "foo,specific-panel", "generic-panel";
> > 
> > and if there's no need for any power/gpio setup for your board, you may
> > skip adding "foo,specific-panel" support to the panel driver. Later,
> > somebody else may need to implement fine grained power/gpio support for
> > "foo,specific-panel", and it would just work.
> > 
> > Maybe that would help us with the problem of adding support in the
> > driver for a hundred different simple panels each used only once on a
> > specific board.
> 
> Sure, that all sounds like reasonable additions. All of the suggestions
> are even backwards-compatible and hence can be added when needed without
> breaking compatibility with existing users of the binding.

What's wrong with moving the three hardcoded modes we already have in the 
driver to DT before pushing this to mainline ?

-- 
Regards,

Laurent Pinchart
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/9868ff44/attachment.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Thierry Reding]

e = "foo,specific-panel", "generic-panel";
> 
> and if there's no need for any power/gpio setup for your board, you may
> skip adding "foo,specific-panel" support to the panel driver. Later,
> somebody else may need to implement fine grained power/gpio support for
> "foo,specific-panel", and it would just work.
> 
> Maybe that would help us with the problem of adding support in the
> driver for a hundred different simple panels each used only once on a
> specific board.

Sure, that all sounds like reasonable additions. All of the suggestions
are even backwards-compatible and hence can be added when needed without
breaking compatibility with existing users of the binding.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/c685f2e2/attachment-0001.pgp>

[RFR 2/2] drm/panel: Add simple panel support

[Laurent Pinchart]

hile the caller can't remedy the 
situation, it should at least report the error to the application instead of 
silently ignoring it.

> > > +static const struct drm_display_mode auo_b101aw03_mode = {
> > > + .clock = 51450,
> > > + .hdisplay = 1024,
> > > + .hsync_start = 1024 + 156,
> > > + .hsync_end = 1024 + 156 + 8,
> > > + .htotal = 1024 + 156 + 8 + 156,
> > > + .vdisplay = 600,
> > > + .vsync_start = 600 + 16,
> > > + .vsync_end = 600 + 16 + 6,
> > > + .vtotal = 600 + 16 + 6 + 16,
> > > + .vrefresh = 60,
> > > +};
> > 
> > Instead of hardcoding the modes in the driver, which would then require to
> > be updated for every new simple panel model (and we know there are lots
> > of them), why don't you specify the modes in the panel DT node ? The
> > simple panel driver would then become much more generic. It would also
> > allow board designers to tweak h/v sync timings depending on the system
> > requirements.
> 
> Sigh... we keep second-guessing ourselves. Back at the time when power
> sequences were designed (and NAKed at the last minute), we all decided
> that the right thing to do would be to use specific compatible values
> for individual panels, because that would allow us to encode the power
> sequencing within the driver. And when we already have the panel model
> encoded in the compatible value, we might just as well encode the mode
> within the driver for that panel. Otherwise we'll have to keep adding
> the same mode timings for every board that uses the same panel.
> 
> I do agree though that it might be useful to tweak the mode in case the
> default one doesn't work. How about we provide a means to override the
> mode encoded in the driver using one specified in the DT? That's
> obviously a backwards-compatible change, so it could be added if or when
> it becomes necessary.

I share Tomi's point of view here. Your three panels use the same power 
sequence, so I believe we should have a generic panel compatible string that 
would use modes described in DT for the common case. Only if a panel required 
something more complex which can't (or which could, but won't for any reason) 
accurately be described in DT would you need to extend the driver.

-- 
Regards,

Laurent Pinchart
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/ac2224f4/attachment.pgp>

[Bug 68391] [radeonsi] Crash unigine-sanctuary

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

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

--- Comment #18 from darkbasic  ---
Copy this into /etc/drirc:
http://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/drivers/dri/common/drirc

It it a list of per-executable workarounds for buggy apps.

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

[PATCH/RFC v3 00/19] Common Display Framework

[Tomi Valkeinen]

On 17/10/13 10:48, Andrzej Hajda wrote:

> The main function of DSI is to transport pixels from one IP to another IP
> and this function IMO should not be modeled by display entity.
> "Power, clocks, etc" will be performed via control bus according to
> panel demands.
> If 'DSI chip' has additional functions for video processing they can
> be modeled by CDF entity if it makes sense.

Now I don't follow. What do you mean with "display entity" and with "CDF
entity"? Are they the same?

Let me try to clarify my point:

On OMAP SoC we have a DSI encoder, which takes input from the display
controller in parallel RGB format, and outputs DSI.

Then there are external encoders that take MIPI DPI as input, and output
DSI.

The only difference with the above two components is that the first one
is embedded into the SoC. I see no reason to represent them in different
ways (i.e. as you suggested, not representing the SoC's DSI at all).

Also, if you use DSI burst mode, you will have to have different video
timings in the DSI encoder's input and output. And depending on the
buffering of the DSI encoder, you could have different timings in any case.

Furthermore, both components could have extra processing. I know the
external encoders sometimes do have features like scaling.

>> We still have two different endpoint configurations for the same
>> DSI-master port. If that configuration is in the DSI-master's port node,
>> not inside an endpoint data, then that can't be supported.
> I am not sure if I understand it correctly. But it seems quite simple:
> when panel starts/resumes it request DSI (via control bus) to fulfill
> its configuration settings.
> Of course there are some settings which are not panel dependent and those
> should reside in DSI node.

Exactly. And when the two panels require different non-panel-dependent
settings, how do you represent them in the DT data?

>>> We say then: callee handles locking :)
>> Sure, but my point was that the caller handling the locking is much
>> simpler than the callee handling locking. And the latter causes
>> atomicity issues, as the other API could be invoked in between two calls
>> for the first API.
>>
>> 
> Could you describe such scenario?

If we have two independent APIs, ctrl and video, that affect the same
underlying hardware, the DSI bus, we could have a scenario like this:

thread 1:

ctrl->op_foo();
ctrl->op_bar();

thread 2:

video->op_baz();

Even if all those ops do locking properly internally, the fact that
op_baz() can be called in between op_foo() and op_bar() may cause problems.

To avoid that issue with two APIs we'd need something like:

thread 1:

ctrl->lock();
ctrl->op_foo();
ctrl->op_bar();
ctrl->unlock();

thread 2:

video->lock();
video->op_baz();
video->unlock();

>>> Platform devices
>>> 
>>> Platform devices are devices that typically appear as autonomous
>>> entities in the system. This includes legacy port-based devices and
>>> host bridges to peripheral buses, and most controllers integrated
>>> into system-on-chip platforms.  What they usually have in common
>>> is direct addressing from a CPU bus.  Rarely, a platform_device will
>>> be connected through a segment of some other kind of bus; but its
>>> registers will still be directly addressable.
>> Yep, "typically" and "rarely" =). I agree, it's not clear. I think there
>> are things with DBI/DSI that clearly point to a platform device, but
>> also the other way.
> Just to be sure, we are talking here about DSI-slaves, ie. for example
> about panels,
> where direct accessing from CPU bus usually is not possible.

Yes. My point is that with DBI/DSI there's not much bus there (if a
normal bus would be PCI/USB/i2c etc), it's just a point to point link
without probing or a clearly specified setup sequence.

If DSI/DBI was used only for control, a linux bus would probably make
sense. But DSI/DBI is mainly a video transport channel, with the
control-part being "secondary".

And when considering that the video and control data are sent over the
same channel (i.e. there's no separate, independent ctrl channel), and
the strict timing restrictions with video, my gut feeling is just that
all the extra complexity brought with separating the control to a
separate bus is not worth it.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/578545c6/attachment-0001.pgp>

[PATCH] drm: never write to the userspace more data than the caller wants

[Pavel Roskin]

On Thu, 17 Oct 2013 13:26:47 +0100
Chris Wilson  wrote:

> On Wed, Oct 16, 2013 at 08:12:35PM -0400, Pavel Roskin wrote:
> > The amount of data wanted by the userspace caller is encoded in the
> > ioctl number.  Generic drm ioctls were ignoring it.
> > 
> > As a result, Intel Xorg driver didn't work for i386 userspace on
> > x86_64 kernel on some systems.  sizeof(struct
> > drm_mode_get_connector) is 76 bytes on i686 and 80 bytes on x86_64
> > due to the tail alignment (the data positions match).  The
> > userspace was using the 4 bytes after the structure to hold the
> > result of the ioctl.  Since drm_ioctl() was copying 80 bytes
> > instead of 76, it was clobbering that data.
> > 
> > A workaround has been committed to xf86-video-intel.
> > 
> > Signed-off-by: Pavel Roskin 
> > Cc: stable at vger.kernel.org
> 
> Similar patch:
> http://lists.freedesktop.org/archives/dri-devel/2013-October/047412.html
> -Chris

Wow, it's great that you also thought about it!

Your patch does almost the same thing.  There is one difference.  If
the userspace requests more data than the kernel needs, your patch would
trust the userspace and set usize to whatever the user wants.  It would
set asize to the same value, allocating more memory than the driver
wants, up to 16383 bytes.  I don't think it's a good idea for
performance reasons.  My patch would decrease usize rather than increase
asize.

The code for driver-specific ioctls could be fixed too, it's just not
so urgent as fixing a real bug.

That said, I have no format objection against your patch.  It would be
great to have that bug fixed.

-- 
Regards,
Pavel Roskin

[RFR 2/2] drm/panel: Add simple panel support

[Thierry Reding]

; > +
> > +   if (p->enabled)
> > +   return;
> > +
> > +   err = regulator_enable(p->supply);
> > +   if (err < 0)
> > +   dev_err(panel->dev, "failed to enable supply: %d\n", err);
> 
> Is that really a non-fatal error ? Shouldn't the enable operation return an 
> int ?

There's no way to propagate this in DRM, so why go through the trouble
of returning the error? Furthermore, there's nothing that the caller
could do to remedy the situation anyway.

> > +static const struct drm_display_mode auo_b101aw03_mode = {
> > +   .clock = 51450,
> > +   .hdisplay = 1024,
> > +   .hsync_start = 1024 + 156,
> > +   .hsync_end = 1024 + 156 + 8,
> > +   .htotal = 1024 + 156 + 8 + 156,
> > +   .vdisplay = 600,
> > +   .vsync_start = 600 + 16,
> > +   .vsync_end = 600 + 16 + 6,
> > +   .vtotal = 600 + 16 + 6 + 16,
> > +   .vrefresh = 60,
> > +};
> 
> Instead of hardcoding the modes in the driver, which would then require to be 
> updated for every new simple panel model (and we know there are lots of 
> them), 
> why don't you specify the modes in the panel DT node ? The simple panel 
> driver 
> would then become much more generic. It would also allow board designers to 
> tweak h/v sync timings depending on the system requirements.

Sigh... we keep second-guessing ourselves. Back at the time when power
sequences were designed (and NAKed at the last minute), we all decided
that the right thing to do would be to use specific compatible values
for individual panels, because that would allow us to encode the power
sequencing within the driver. And when we already have the panel model
encoded in the compatible value, we might just as well encode the mode
within the driver for that panel. Otherwise we'll have to keep adding
the same mode timings for every board that uses the same panel.

I do agree though that it might be useful to tweak the mode in case the
default one doesn't work. How about we provide a means to override the
mode encoded in the driver using one specified in the DT? That's
obviously a backwards-compatible change, so it could be added if or when
it becomes necessary.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/45b2d0c3/attachment-0001.pgp>

[PATCH v2 12/26] drm/exynos: Split manager/display/subdrv

[Sean Paul]

On Thu, Oct 17, 2013 at 4:21 AM, Inki Dae  wrote:
>
>
>> -Original Message-
>> From: Sean Paul [mailto:seanpaul at chromium.org]
>> Sent: Thursday, October 17, 2013 4:27 AM
>> To: dri-devel at lists.freedesktop.org; inki.dae at samsung.com
>> Cc: airlied at linux.ie; tomasz.figa at gmail.com; marcheu at chromium.org; 
>> Sean
>> Paul
>> Subject: [PATCH v2 12/26] drm/exynos: Split manager/display/subdrv
>>
>> This patch splits display and manager from subdrv. The result is that
>> crtc functions can directly call into manager callbacks and encoder
>> functions can directly call into display callbacks. This will allow
>> us to remove the exynos_drm_hdmi shim and support mixer/hdmi & fimd/dp
>> with common code.
>>
>> Signed-off-by: Sean Paul 
>> ---
>>
>> Changes in v2:
>>   - Pass display into display_ops instead of context
>
> Sorry but it seems like more reasonable to pass device object into
> display_ops and manager_ops.
>


So you've changed your mind from when you said the following?

>>> manager->ops->xxx(manager, ...);
>>> display->ops->xxx(display, ...);
>>>
>>> Agree.

It would have been nice if you had changed your mind *before* I
reworked everything. At any rate, I think it's still the right thing
to do.


> I'm not sure but display_ops could be implemented in other framework based
> driver such as CDF based lcd panel driver. So if you pass display - it's
> specific to exynos drm framework - into display_ops, the other framework
> based driver should include specific exynos drm header.
>

AFAIK, CDF will not land in its current separate-from-drm form, we
don't need to worry about this. Furthermore, these ops should just go
away and become drm_crtc/drm_encoder/drm_connector funcs anyways.


> And another one, the patch 6 passes manager object to manager_ops, and for
> this, you made the manager object to be set to driver data;
> platform_set_drvdata(pdev, ). That isn't reasonable. Generally,
> driver_data would point to device driver's context object.
>

I'm not sure why this isn't reasonable, but it's a moot point. The
driver data is only used up until we get rid of the pm ops, it needn't
be set at all once things go through dpms.

Sean


> Thanks,
> Inki Dae
>

[PATCH 1/7] drm: Consistently return -ENOENT when a mode object can't be found

[Alex Deucher]

On Thu, Oct 17, 2013 at 6:35 AM,   wrote:
> From: Ville Syrj?l? 
>
> We tend to return -EINVAL for everything. Let's try to help poor
> userland developers a bit by at least returning -ENONET for missing
> objects.
>
> Signed-off-by: Ville Syrj?l? 

For the series:

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/drm_crtc.c | 32 ++--
>  1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d7a8370..bb5dedd 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1557,7 +1557,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
> obj = drm_mode_object_find(dev, crtc_resp->crtc_id,
>DRM_MODE_OBJECT_CRTC);
> if (!obj) {
> -   ret = -EINVAL;
> +   ret = -ENOENT;
> goto out;
> }
> crtc = obj_to_crtc(obj);
> @@ -1641,7 +1641,7 @@ int drm_mode_getconnector(struct drm_device *dev, void 
> *data,
> obj = drm_mode_object_find(dev, out_resp->connector_id,
>DRM_MODE_OBJECT_CONNECTOR);
> if (!obj) {
> -   ret = -EINVAL;
> +   ret = -ENOENT;
> goto out;
> }
> connector = obj_to_connector(obj);
> @@ -1757,7 +1757,7 @@ int drm_mode_getencoder(struct drm_device *dev, void 
> *data,
> obj = drm_mode_object_find(dev, enc_resp->encoder_id,
>DRM_MODE_OBJECT_ENCODER);
> if (!obj) {
> -   ret = -EINVAL;
> +   ret = -ENOENT;
> goto out;
> }
> encoder = obj_to_encoder(obj);
> @@ -2141,7 +2141,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>DRM_MODE_OBJECT_CRTC);
> if (!obj) {
> DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_req->crtc_id);
> -   ret = -EINVAL;
> +   ret = -ENOENT;
> goto out;
> }
> crtc = obj_to_crtc(obj);
> @@ -2232,7 +2232,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
> if (!obj) {
> DRM_DEBUG_KMS("Connector id %d unknown\n",
> out_id);
> -   ret = -EINVAL;
> +   ret = -ENOENT;
> goto out;
> }
> connector = obj_to_connector(obj);
> @@ -2280,7 +2280,7 @@ static int drm_mode_cursor_common(struct drm_device 
> *dev,
> obj = drm_mode_object_find(dev, req->crtc_id, DRM_MODE_OBJECT_CRTC);
> if (!obj) {
> DRM_DEBUG_KMS("Unknown CRTC ID %d\n", req->crtc_id);
> -   return -EINVAL;
> +   return -ENOENT;
> }
> crtc = obj_to_crtc(obj);
>
> @@ -3059,7 +3059,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
> drm_modeset_lock_all(dev);
> obj = drm_mode_object_find(dev, out_resp->prop_id, 
> DRM_MODE_OBJECT_PROPERTY);
> if (!obj) {
> -   ret = -EINVAL;
> +   ret = -ENOENT;
> goto done;
> }
> property = obj_to_property(obj);
> @@ -3188,7 +3188,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
> drm_modeset_lock_all(dev);
> obj = drm_mode_object_find(dev, out_resp->blob_id, 
> DRM_MODE_OBJECT_BLOB);
> if (!obj) {
> -   ret = -EINVAL;
> +   ret = -ENOENT;
> goto done;
> }
> blob = obj_to_blob(obj);
> @@ -3349,7 +3349,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device 
> *dev, void *data,
>
> obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type);
> if (!obj) {
> -   ret = -EINVAL;
> +   ret = -ENOENT;
> goto out;
> }
> if (!obj->properties) {
> @@ -3402,8 +3402,10 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
> *dev, void *data,
> drm_modeset_lock_all(dev);
>
> arg_obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type);
> -   if (!arg_obj)
> +   if (!arg_obj) {
> +   ret = -ENOENT;
> goto out;
> +   }
> if (!arg_obj->properties)
> goto out;
>
> @@ -3416,8 +3418,10 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
> *dev, void *data,
>
> prop_obj = drm_mode_object_find(dev, arg->prop_id,
> DRM_MODE_OBJECT_PROPERTY);
> -   if (!prop_obj)
> +   if (!prop_obj) {
> +   ret = -ENOENT;
> goto out;
> +   }
> property = obj_to_property(prop_obj);
>
> if (!drm_property_change_is_valid(property, arg->value))
> @@ -3502,7 +3506,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,

[Bug 68391] [radeonsi] Crash unigine-sanctuary

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

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

--- Comment #17 from Arek Ru?niak  ---
it seem's they known the problem. DriConf applies it to any unigine-stuff by
default (arch).

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

[Bug 70569] DOTA2 background and other missing elements on HD4850 GPU

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

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

Roc Vall?s Dom?nech  changed:

   What|Removed |Added

  Attachment #87786|text/plain  |image/png
  mime type||

--- Comment #1 from Roc Vall?s Dom?nech  ---
Comment on attachment 87786
  --> https://bugs.freedesktop.org/attachment.cgi?id=87786
Screenshot of the fountain on a newly started game

Made it png image mime. (Or trying to)

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

[Bug 70569] New: DOTA2 background and other missing elements on HD4850 GPU

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

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

  Priority: medium
Bug ID: 70569
  Assignee: dri-devel at lists.freedesktop.org
   Summary: DOTA2 background and other missing elements on HD4850
GPU
  Severity: normal
Classification: Unclassified
OS: Linux (All)
  Reporter: vallesroc at gmail.com
  Hardware: x86-64 (AMD64)
Status: NEW
   Version: git
 Component: Drivers/Gallium/r600
   Product: Mesa

Created attachment 87786
  --> https://bugs.freedesktop.org/attachment.cgi?id=87786=edit
Screenshot of the fountain on a newly started game

On a HD4850, DOTA2 in-game background and some other objects aren't being
rendered. Attached screenshot shows the problem well.

DOTA2 graphics settings are at the lowest (didn't help). This is using git
mesa3d on a current (~amd64) Gentoo, built both with and without LLVM shader
compiler backend. Kernel is 3.12-rc5.

It has, to my knowledge, never worked... I kept trying it once in a while for
half a year.

Since nobody else has complained yet, I suspect it's a problem that affects
only the HD4xxx line of cards.

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

[PATCH/RFC v3 00/19] Common Display Framework

[Andrzej Hajda]

Hi Tomi,

Sorry for delayed response.


On 10/11/2013 04:45 PM, Tomi Valkeinen wrote:
> On 11/10/13 17:16, Andrzej Hajda wrote:
>
>> Picture size, content and format is the same on input and on output of DSI.
>> The same bits which enters DSI appears on the output. Internally bits
>> order can
>> be different but practically you are configuring DSI master and slave
>> with the same format.
>>
>> If you create DSI entity you will have to always set the same format and
>> size on DSI input, DSI output and encoder input.
>> If you skip creating DSI entity you loose nothing, and you do not need
>> to take care of it.
> Well, this is really a different question from the bus problem. But
> nothing says the DSI master cannot change the format or even size. For
> sure it can change the video timings. The DSI master could even take two
> parallel inputs, and combine them into one DSI output. You don't can't
> what all the possible pieces of hardware do =)
> If you have a bigger IP block that internally contains the DISPC and the
> DSI, then, yes, you can combine them into one display entity. I don't
> think that's correct, though. And if the DISPC and DSI are independent
> blocks, then especially I think there must be an entity for the DSI
> block, which will enable the powers, clocks, etc, when needed.
The main function of DSI is to transport pixels from one IP to another IP
and this function IMO should not be modeled by display entity.
"Power, clocks, etc" will be performed via control bus according to
panel demands.
If 'DSI chip' has additional functions for video processing they can
be modeled by CDF entity if it makes sense.
>>> Well, one point of the endpoints is also to allow "switching" of video
>>> devices.
>>>
>>> For example, I could have a board with a SoC's DSI output, connected to
>>> two DSI panels. There would be some kind of mux between, so that I can
>>> select which of the panels is actually connected to the SoC.
>>>
>>> Here the first panel could use 2 datalanes, the second one 4. Thus, the
>>> DSI master would have two endpoints, the other one using 2 and the other
>>> 4 datalanes.
>>>
>>> If we decide that kind of support is not needed, well, is there even
>>> need for the V4L2 endpoints in the DT data at all?
>> Hmm, both panels connected to one endpoint of dispc ?
>> The problem I see is which driver should handle panel switching,
>> but this is question about hardware design as well. If this is realized
>> by dispc I have told already the solution. If this is realized by other
>> device I do not see a problem to create corresponding CDF entity,
>> or maybe it can be handled by "Pipeline Controller" ???
> Well the switching could be automatic, when the panel power is enabled,
> the DSI mux is switched for that panel. It's not relevant.
>
> We still have two different endpoint configurations for the same
> DSI-master port. If that configuration is in the DSI-master's port node,
> not inside an endpoint data, then that can't be supported.
I am not sure if I understand it correctly. But it seems quite simple:
when panel starts/resumes it request DSI (via control bus) to fulfill
its configuration settings.
Of course there are some settings which are not panel dependent and those
should reside in DSI node.
> I agree that having DSI/DBI control and video separated would be
> elegant. But I'd like to hear what is the technical benefit of that? At
> least to me it's clearly more complex to separate them than to keep them
> together (to the extent that I don't yet see how it is even possible),
> so there must be a good reason for the separation. I don't understand
> that reason. What is it?
 Roughly speaking it is a question where is the more convenient place to
 put bunch
 of opses, technically both solutions can be somehow implemented.
>>> Well, it's also about dividing a single physical bus into two separate
>>> interfaces to it. It sounds to me that it would be much more complex
>>> with locking. With a single API, we can just say "the caller handles
>>> locking". With two separate interfaces, there must be locking at the
>>> lower level.
>> We say then: callee handles locking :)
> Sure, but my point was that the caller handling the locking is much
> simpler than the callee handling locking. And the latter causes
> atomicity issues, as the other API could be invoked in between two calls
> for the first API.
>
> 
Could you describe such scenario?
> But note that I'm not saying we should not implement bus model just
> because it's more complex. We should go for bus model if it's better. I
> just want to bring up these complexities, which I feel are quite more
> difficult than with the simpler model.
>
 Pros of mipi bus:
 - no fake entity in CDF, with fake opses, I have to use similar entities
 in MIPI-CSI
 camera pipelines and it complicates life without any benefit(at least
 from user side),
>>> You mean the DSI-master? I don't see how 

[PATCH] drm: allocate crtc mutex separately from crtc

[Daniel Vetter]

On Thu, Oct 17, 2013 at 1:35 AM, Ilija Hadzic  wrote:
> Embedding a mutex inside drm_crtc structure evokes a
> subtle and rare corruption in drm_crtc_helper_set_config
> failure-recovery path.
>
> Namely, if drm_crtc_helper_set_config takes the
> path under 'fail:' label *and* some other process
> has attempted to grab the crtc mutex (and got blocked),
> restoring the CRTC structure (which is done by bit-copying
> the entire structure from saved_crtcs array) will overwrite
> the mutex state and the waiters list pointer within the mutex
> structure. Consequently the blocked process will never
> be scheduled.
>
> This patch fixes the issue by un-embeding the mutex structure
> and allocating it separately from the drm_crtc structure
> when the CRTC is initialized. The bit-copying in
> drm_crtc_helper_set_config helper will now overwrite the pointer
> which is never modified during the CRTC's lifetime, thus
> avoiding corruption.
>
> Signed-off-by: Ilija Hadzic 
> Cc: stable at vger.kernel.org

Can't we instead just copy the few things we need to restore back?
This wholesale structure copying has rather tricky semantics and is
bound to trick up someone else in the future. And indeed we seem to
have a similar (but less catastrophic) thing going on with crtc->fb I
think.

I've looked a bit through the code and I think we don't actually need
to restore anything for crtcs. We pass the mode, fb and offsets in
explicitly, and everything else in drm_crtc is derived state. This is
also the same that i915 does, we only restore the connector/encoder
links even.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm: allocate crtc mutex separately from crtc

[Ilija Hadzic]

> Can't we instead just copy the few things we need to restore back?
> This wholesale structure copying has rather tricky semantics and is
> bound to trick up someone else in the future. And indeed we seem to
> have a similar (but less catastrophic) thing going on with crtc->fb I
> think.

Sure, it would be possible. If that's a preferred solution, I'll cut
the alternative patch.

I.

[Bug 70528] RadeonSI : Specviewperf10/proe-04 cause system hang

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

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

--- Comment #4 from samit vats  ---
The issue is still reproduced after updating the firmware.
Attached is the updated dmesg log with message "[drm] UVD initialized
sucessfully".

Additional Info : The issue is not observed with latest Closed source "fglrx"
driver

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

[Bug 70528] RadeonSI : Specviewperf10/proe-04 cause system hang

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

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

--- Comment #3 from samit vats  ---
Created attachment 87780
  --> https://bugs.freedesktop.org/attachment.cgi?id=87780=edit
dmesg-updated

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

[Bug 64475] Slow work and no brightness adjustment in Euro Track Simulator II game with HD6850

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

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

commiethebeastie at gmail.com changed:

   What|Removed |Added

Summary|Slow work and no HDR|Slow work and no brightness
   |effects in Euro Track   |adjustment in Euro Track
   |Simulator II game with  |Simulator II game with
   |HD6850  |HD6850

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

[Bug 67367] No HDR effects on the Radeon HD6850 in all applications

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

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

commiethebeastie at gmail.com changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |INVALID

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

[Bug 70088] Glamor on r600g crashes Xserver

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

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

Vadim Girlin  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #13 from Vadim Girlin  ---
Fix pushed with commit 62c8149472903a2f3fc4d319c3799b9e729419d5

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

[Bug 70528] RadeonSI : Specviewperf10/proe-04 cause system hang

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

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

--- Comment #2 from Michel D?nzer  ---
I notice in dmesg that UVD fails to initialize. Is the RLC microcode up to
date?

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

[Bug 67982] After boot, the APU is powered with its maximum voltage (trinity/ARUBA)

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

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

Kertesz Laszlo  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |WORKSFORME

--- Comment #8 from Kertesz Laszlo  ---
As this bug isnt present in the  newer kernels, i mark it as resolved.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 

[Bug 70514] Unresponsive system on boot with radeon + FireGL v7700

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

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

--- Comment #2 from Luke  ---
Kernel of the Arch Linux x64 mentioned previously:
$ uname -r
3.11.5-1-ARCH



As far as I know, this is not a regression. Some old live cds have the same
problem:

- Booting from an old Arch live-cd (2.6.33-ARCH) freezes shortly after modules
are loaded. Halfway down the screen, in the native resolution, it displays the
next line of booting up, and nothing else.

- When using a Ubuntu 12.04 live cd or a Linux Mint 10 live cd, there is a
black screen and the system locks up while booting. These cds, if using
'nomodeset', can get to a desktop. Switching to a VT and using 'rmmod radeon;
modprobe radeon modeset=1' makes it lock up.



I probably should have mentioned this in the initial comment, but the card
works fine in Windows, as well as using Linux + Catalyst.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 

[v3.10][v3.11][v3.12][Regression][PATCH 1/1] Revert "Revert "drm/i915: revert eDP bpp clamping code changes""

[Daniel Vetter]

On Wed, Oct 16, 2013 at 04:34:57PM -0400, Joseph Salisbury wrote:
> BugLink: http://bugs.launchpad.net/bugs/1195483
> 
> This reverts commit 657445fe8660100ad174600ebfa61536392b7624.
> 
> Signed-off-by: Joseph Salisbury 
> Cc: Daniel Vetter 
> Cc: Paulo Zanoni 
> Cc: David Airlie 
> Cc: stable at vger.kernel.org

It's by far not that simple. Jani is working on both the underlying bug
and a better w/a. See

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

for the full story in its entire glory.

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