likely signedness bug in drm and nvidia drivers

[Rasmus Villemoes]

Hi,

The files

  drivers/gpu/drm/nouveau/nv50_fbcon.c
  drivers/gpu/drm/nouveau/nvc0_fbcon.c
  drivers/video/fbdev/nvidia/nv_accel.c

all contain a right-shift of ~0 (aka -1) - just grep for '~0 >>'. gcc
always does arithmetic right shift of signed types, which means that the
result is always -1 again [type promotion/conversion doesn't kick in
until after the shift subexpression has been evaluated], independent of
the second operand. I can hardly believe that is intended (the result is
used as a mask, which thus consists of all 1s). If these are indeed
bugs, the patch is obvious (just make the literal an unsigned 0).

Rasmus

likely signedness bug in drm and nvidia drivers

[Ilia Mirkin]

I think you're right. The intent is to mask off the bits above
bits_per_pixel. So if bits_per_pixel is 24, the mask would be
0xff00. If it's 16, then the mask would be 0x. If it's 32,
then the mask is 0.

In reality, bits_per_pixel is almost exclusively 32, which will end up
with a mask of 0 (note that the shift result is inverted at the end).
So for the majority case, there's not bug... just a useless operation.

I took a look at linux/bitops.h, and there's nothing particularly
great there. GENMASK, I guess, but it's not quite right. Just
switching to 0U should be fine there.

Now that I think about it, I believe these patches have already been
sent in the past. I also just did the grep that I did before:

$ git grep -- '~0 >>'
arch/m32r/include/asm/thread_info.h:ti->flags = (ti->flags & (~0
>> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
arch/sh/include/asm/thread_info.h:  ti->flags = (ti->flags & (~0
>> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
drivers/gpu/drm/nouveau/nv50_fbcon.c:   uint32_t mask = ~(~0 >> (32 -
info->var.bits_per_pixel));
drivers/gpu/drm/nouveau/nvc0_fbcon.c:   uint32_t mask = ~(~0 >> (32 -
info->var.bits_per_pixel));
drivers/video/fbdev/nvidia/nv_accel.c:  u32 fg, bg, mask = ~(~0 >> (32
- info->var.bits_per_pixel));

which shows that these are the only ones. See email with subject
"[PATCH] nvidia/noveau: Fix color mask" from June 17, 2015.

  -ilia

On Mon, Jul 20, 2015 at 4:46 PM, Rasmus Villemoes
 wrote:
> Hi,
>
> The files
>
>   drivers/gpu/drm/nouveau/nv50_fbcon.c
>   drivers/gpu/drm/nouveau/nvc0_fbcon.c
>   drivers/video/fbdev/nvidia/nv_accel.c
>
> all contain a right-shift of ~0 (aka -1) - just grep for '~0 >>'. gcc
> always does arithmetic right shift of signed types, which means that the
> result is always -1 again [type promotion/conversion doesn't kick in
> until after the shift subexpression has been evaluated], independent of
> the second operand. I can hardly believe that is intended (the result is
> used as a mask, which thus consists of all 1s). If these are indeed
> bugs, the patch is obvious (just make the literal an unsigned 0).
>
> Rasmus
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/8] drm/bochs: phase 1 - use the transitional helpers

[John Hunter]

From: Zhao Junwang 

-register driver's own primary plane
-use drm_crtc_init_with_planes instead of drm_crtc_init

-split ->mode_set into:
1. set the new hw mode
2. update the primary plane (This is done by ->set_base)
-move what ->set_base do into ->atomic_update

-the new atomic infrastructure needs the ->mode_set_nofb callback
 to update CRTC timings before setting any plane

-since the ->cleanup_fb can't fail, set the interruptible argument
 of the ttm_bo_reserve to false, this make sure the ttm_bo_reserve
 can't fail

v2: -add a few checks to plane's ->atomic_check, using
 drm_plane_helper_check_update

v3: -polish the atomic_check, it does too much in v2, remove the
 ->disable_plane and ->set_config

v4: -use plane->state instead of old_state in
 bochs_plane_atomic_update

v5: -just return 0 in plane ->atomic_check, i.e. remove what we
 do in v2-v4

v6: -remove gpu_addr and look up the bo offset same as
 bochs_bo_gpu_offset
-add atomic_plane_disable hook, this will guarantee when it
 calls atomic_plane_update state->fb and state->crtc are both
 non-NULL

Cc: Gerd Hoffmann 
Cc: Maarten Lankhorst 
Cc: Daniel Vetter 
Signed-off-by: Zhao Junwang 
---
 drivers/gpu/drm/bochs/bochs.h |2 +
 drivers/gpu/drm/bochs/bochs_kms.c |  169 +
 2 files changed, 118 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index 71f2687..2f10480 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -5,6 +5,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include 
@@ -72,6 +73,7 @@ struct bochs_device {

/* drm */
struct drm_device  *dev;
+   struct drm_plane primary;
struct drm_crtc crtc;
struct drm_encoder encoder;
struct drm_connector connector;
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index 26bcd03..0eda7fe 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -11,6 +11,10 @@
 static int defx = 1024;
 static int defy = 768;

+static const uint32_t bochs_primary_formats[] = {
+   DRM_FORMAT_XRGB,
+};
+
 module_param(defx, int, 0444);
 module_param(defy, int, 0444);
 MODULE_PARM_DESC(defx, "default x resolution");
@@ -37,59 +41,12 @@ static bool bochs_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
 }

-static int bochs_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-   struct drm_framebuffer *old_fb)
-{
-   struct bochs_device *bochs =
-   container_of(crtc, struct bochs_device, crtc);
-   struct bochs_framebuffer *bochs_fb;
-   struct bochs_bo *bo;
-   u64 gpu_addr = 0;
-   int ret;
-
-   if (old_fb) {
-   bochs_fb = to_bochs_framebuffer(old_fb);
-   bo = gem_to_bochs_bo(bochs_fb->obj);
-   ret = ttm_bo_reserve(>bo, true, false, false, NULL);
-   if (ret) {
-   DRM_ERROR("failed to reserve old_fb bo\n");
-   } else {
-   bochs_bo_unpin(bo);
-   ttm_bo_unreserve(>bo);
-   }
-   }
-
-   if (WARN_ON(crtc->primary->fb == NULL))
-   return -EINVAL;
-
-   bochs_fb = to_bochs_framebuffer(crtc->primary->fb);
-   bo = gem_to_bochs_bo(bochs_fb->obj);
-   ret = ttm_bo_reserve(>bo, true, false, false, NULL);
-   if (ret)
-   return ret;
-
-   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM, _addr);
-   if (ret) {
-   ttm_bo_unreserve(>bo);
-   return ret;
-   }
-
-   ttm_bo_unreserve(>bo);
-   bochs_hw_setbase(bochs, x, y, gpu_addr);
-   return 0;
-}
-
-static int bochs_crtc_mode_set(struct drm_crtc *crtc,
-  struct drm_display_mode *mode,
-  struct drm_display_mode *adjusted_mode,
-  int x, int y, struct drm_framebuffer *old_fb)
+static void bochs_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
struct bochs_device *bochs =
container_of(crtc, struct bochs_device, crtc);

-   bochs_hw_setmode(bochs, mode);
-   bochs_crtc_mode_set_base(crtc, x, y, old_fb);
-   return 0;
+   bochs_hw_setmode(bochs, >mode);
 }

 static void bochs_crtc_prepare(struct drm_crtc *crtc)
@@ -116,7 +73,7 @@ static int bochs_crtc_page_flip(struct drm_crtc *crtc,
unsigned long irqflags;

crtc->primary->fb = fb;
-   bochs_crtc_mode_set_base(crtc, 0, 0, old_fb);
+   drm_helper_crtc_mode_set_base(crtc, 0, 0, old_fb);
if (event) {
spin_lock_irqsave(>dev->event_lock, irqflags);
drm_send_vblank_event(bochs->dev, -1, event);
@@ -136,8 +93,9 @@ static const struct drm_crtc_funcs bochs_crtc_funcs = {
 static const struct drm_crtc_helper_funcs 

[Bug 46711] Monitor not turning on after DisplayPort re-plug in Xorg

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

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

--- Comment #22 from Marcus  ---
I have the problem each time I turn off my monitor and then later turn it back
on. The desktop does not come back.

(In reply to Wolfgang Rupprecht from comment #4)
> xset dpms force off
> sleep 1
> xset dpms force on

This also gets the display back for me.

The monitor is a Samsung U24E850 (24 inch, 3840x2160)

The OS is Ubuntu 15.04 with 4.1.1 kernel installed and olibaf xork packages.

here is the relevant package information:

linux-image-4.1.1-040101-generic  4.1.1-040101.201506291635
xserver-xorg-video-radeon
1:7.5.99+git1507100730.b6d871~gd~v
xserver-xorg-core 2:1.17.1-0ubuntu3

# xrandr 
Screen 0: minimum 320 x 200, current 3840 x 2160, maximum 16384 x 16384
DisplayPort-0 connected primary 3840x2160+0+0 (normal left inverted right x
axis y axis) 521mm x 293mm
   3840x2160  60.0*+
   2560x1440  60.0  
   1920x1080  60.0 59.9  
   1680x1050  60.0  
   1600x900   59.9  
   1280x1024  60.0  
   1440x900   59.9  
   1280x800   59.8  
   1280x720   59.9  
   1024x768   60.0  
   800x60060.3 56.2  
   640x48060.0 59.9  
HDMI-0 disconnected (normal left inverted right x axis y axis)
DVI-0 disconnected (normal left inverted right x axis y axis)
DVI-1 disconnected (normal left inverted right x axis y axis)

-- 
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/20150720/3e9563f0/attachment.html>

[PATCH v2] drm/i915: remove unnecessary null test

[Sudip Mukherjee]

While creating the debugfs file we are setting the inode->i_private to
dev. That same dev is passed to these functions as private of struct
seq_file via single_open(). So at this point it can never be NULL.

Signed-off-by: Sudip Mukherjee 
---

v1 was drm/i915: fix possible null pointer dereference

 drivers/gpu/drm/i915/i915_debugfs.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index bc817da..63cb886 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4103,9 +4103,6 @@ static int i915_displayport_test_active_show(struct 
seq_file *m, void *data)
struct list_head *connector_list = >mode_config.connector_list;
struct intel_dp *intel_dp;

-   if (!dev)
-   return -ENODEV;
-
list_for_each_entry(connector, connector_list, head) {

if (connector->connector_type !=
@@ -4150,9 +4147,6 @@ static int i915_displayport_test_data_show(struct 
seq_file *m, void *data)
struct list_head *connector_list = >mode_config.connector_list;
struct intel_dp *intel_dp;

-   if (!dev)
-   return -ENODEV;
-
list_for_each_entry(connector, connector_list, head) {

if (connector->connector_type !=
@@ -4192,9 +4186,6 @@ static int i915_displayport_test_type_show(struct 
seq_file *m, void *data)
struct list_head *connector_list = >mode_config.connector_list;
struct intel_dp *intel_dp;

-   if (!dev)
-   return -ENODEV;
-
list_for_each_entry(connector, connector_list, head) {

if (connector->connector_type !=
-- 
1.8.1.2

[Bug 91375] [radeonsi] [drm:si_dpm_set_power_state [radeon]] *ERROR* si_restrict_performance_levels_before_switch failed

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

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

--- Comment #2 from Alexandre Demers  ---
(In reply to Alex Deucher from comment #1)
> please attach your xorg log and dmesg output.  In most cases this can be
> ignored if things are working properly.

Hi Alex. I'll be attaching them later today. Sorry, that was my intention but I
got distracted by another bug in Xserver (it was the bug in X that allowed me
to discover this error in dmesg). However, there was nothing interesting in
both files from what I remember.

I'm pretty sure it was not appearing in a previous combination of mesa, ddx,
drm and kernel. I could try with a previous combination. I'll begin with an
older kernel.

If this error can be ignored when things are working properly, we probably
shouldn't output an error. It should be at most a warning and be displayed as
an error only if it fails seriously, don't you think?

-- 
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/20150720/30e615f5/attachment.html>

[PATCH] tests: modetest: Accept connector names in addition to connector IDs

[Laurent Pinchart]

From: Thierry Reding 

Allow connector names to be used in the specification of the -s option.
This requires storing the string passed on the command-line so that it
can later be resolved to a connector ID (after the DRM device has been
opened).

Connector names are constructed from the connector type name and
connector type ID using the same format as used internally in the
Linux kernel.

Signed-off-by: Thierry Reding 
Signed-off-by: Laurent Pinchart 
---
 tests/modetest/modetest.c | 94 +--
 1 file changed, 82 insertions(+), 12 deletions(-)

I've carried this patch in my tree for too long and I'm now tired of rebasing
it. It's thus time to get it in mainline.

Compared to the original version developed by Thierry, I've

- rebased the patch on top of latest master
- dropped dependencies on other WIP patches in Thierry's tree
- fixed printf alignment
- used asprintf to simplify name string allocation
- constructed the connector name using the same format as the Linux kernel

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index c6e3cffdf89b..81ae923cd53a 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -77,6 +77,7 @@ struct connector {
drmModeConnector *connector;
drmModeObjectProperties *props;
drmModePropertyRes **props_info;
+   char *name;
 };

 struct fb {
@@ -372,18 +373,18 @@ static void dump_connectors(struct device *dev)
int i, j;

printf("Connectors:\n");
-   printf("id\tencoder\tstatus\t\ttype\tsize (mm)\tmodes\tencoders\n");
+   printf("id\tencoder\tstatus\t\tname\t\tsize (mm)\tmodes\tencoders\n");
for (i = 0; i < dev->resources->res->count_connectors; i++) {
struct connector *_connector = >resources->connectors[i];
drmModeConnector *connector = _connector->connector;
if (!connector)
continue;

-   printf("%d\t%d\t%s\t%s\t%dx%d\t\t%d\t",
+   printf("%d\t%d\t%s\t%-15s\t%dx%d\t\t%d\t",
   connector->connector_id,
   connector->encoder_id,
   connector_status_str(connector->connection),
-  connector_type_str(connector->connector_type),
+  _connector->name,
   connector->mmWidth, connector->mmHeight,
   connector->count_modes);

@@ -509,12 +510,13 @@ static void dump_planes(struct device *dev)

 static void free_resources(struct resources *res)
 {
+   int i;
+
if (!res)
return;

 #define free_resource(_res, __res, type, Type) 
\
do {
\
-   int i;  
\
if (!(_res)->type##s)   
\
break;  
\
for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) { 
\
@@ -527,7 +529,6 @@ static void free_resources(struct resources *res)

 #define free_properties(_res, __res, type) 
\
do {
\
-   int i;  
\
for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) { 
\
drmModeFreeObjectProperties(res->type##s[i].props); 
\
free(res->type##s[i].props_info);   
\
@@ -539,6 +540,10 @@ static void free_resources(struct resources *res)

free_resource(res, res, crtc, Crtc);
free_resource(res, res, encoder, Encoder);
+
+   for (i = 0; i < res->res->count_connectors; i++)
+   free(res->connectors[i].name);
+
free_resource(res, res, connector, Connector);
free_resource(res, res, fb, FB);

@@ -600,6 +605,15 @@ static struct resources *get_resources(struct device *dev)
get_resource(res, res, connector, Connector);
get_resource(res, res, fb, FB);

+   /* Set the name of all connectors based on the type name and the 
per-type ID. */
+   for (i = 0; i < res->res->count_connectors; i++) {
+   struct connector *connector = >connectors[i];
+
+   asprintf(>name, "%s-%u",
+
connector_type_str(connector->connector->connector_type),
+connector->connector->connector_type_id);
+   }
+
 #define get_properties(_res, __res, type, Type)
\
do {
\
int i;  
\
@@ 

[Bug 91386] Tonga HDMI Audio needs CPU load

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

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

--- Comment #1 from Alex Deucher  ---
All the gpu driver does is enable the audio stream in the video stream.  It's
not clear to me whether this is a problem with the audio driver or the gpu
driver.  Does audio work ok with the proprietary Linux 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/20150720/0252a98d/attachment.html>

[PATCH] drm/i915: fix possible null pointer dereference

[Sudip Mukherjee]

On Mon, Jul 20, 2015 at 01:38:46PM +0100, Chris Wilson wrote:
> On Mon, Jul 20, 2015 at 05:59:29PM +0530, Sudip Mukherjee wrote:
> > We were dereferencing dev first and then checking if it is NULL. Lets
> > check for NULL first and then dereference.
> 
> The code is bonkers. Testing for a lack of a correctly constructed
> debugfs seq_file inside the debugfs seq_file callback is inane.
I missed seeing before sending this patch that there are some more
places where this has been done.
Then are you suggesting to remove the test?

regards
sudip

[Bug 90681] Radeon Kernel Oops & Lockup when switching on DisplayPort attached monitor

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

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

Alex Deucher  changed:

   What|Removed |Added

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

--- Comment #12 from Alex Deucher  ---
problematic commit reverted:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2d1c18bba15daf89d75ce475ecd2068f483aa12f

Fix committed:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=fbfd3bc7dfd7efcad2d2e52bf634f84c80a77a35

-- 
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/20150720/699c4cea/attachment.html>

[PATCH v2 3/4] of: Add NVIDIA Tegra VIC binding

[Emil Velikov]

Hi Mikko,

On 20 July 2015 at 08:54, Mikko Perttunen  wrote:
> This adds device tree binding documentation for the Video Image
> Compositor (VIC) present on Tegra124 and newer SoC's.
>
> Signed-off-by: Mikko Perttunen 
> ---
>  .../devicetree/bindings/gpu/nvidia,tegra20-host1x.txt | 15 
> +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt 
> b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> index 009f4bf..1328f3f 100644
> --- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> +++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> @@ -240,6 +240,21 @@ of the following host1x client modules:
>  - dpaux
>- vdd-supply: phandle of a supply that powers the DisplayPort link
>
> +- vic: Video Image Compositor
> +  - compatible: For Tegra124, must contain "nvidia,tegra124-vic".  Otherwise,
> +must contain '"nvidia,-vic", "nvidia,tegra124-vic"', where
> + is tegra132.
Did you make a typo here, or is "tegra124" string really used to
identify the tegra132 chips ?

Thanks
Emil

[PATCH] drm/i915: fix possible null pointer dereference

[Sudip Mukherjee]

We were dereferencing dev first and then checking if it is NULL. Lets
check for NULL first and then dereference.

Signed-off-by: Sudip Mukherjee 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index bc817da..f316e49 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4100,12 +4100,13 @@ static int i915_displayport_test_active_show(struct 
seq_file *m, void *data)
 {
struct drm_device *dev = m->private;
struct drm_connector *connector;
-   struct list_head *connector_list = >mode_config.connector_list;
+   struct list_head *connector_list;
struct intel_dp *intel_dp;

if (!dev)
return -ENODEV;

+   connector_list = >mode_config.connector_list;
list_for_each_entry(connector, connector_list, head) {

if (connector->connector_type !=
-- 
1.8.1.2

[PATCH v10 1/5] drm/layerscape: Add Freescale DCU DRM driver

[jianwei wang]

Hi Alexander,

Thank you for your review.

On Mon, Jul 20, 2015 at 3:18 PM, Alexander Stein
 wrote:
> On Friday 17 July 2015 18:38:59, Jianwei Wang wrote:
>> [...]
>> +static const struct regmap_config fsl_dcu_regmap_config = {
>> + .reg_bits = 32,
>> + .reg_stride = 4,
>> + .val_bits = 32,
>> +};
>
> This defaults to REGCACHE_NONE which in the end sets regmap.cache_only = true.
>
>> [...]
>> +#ifdef CONFIG_PM_SLEEP
>> +static int fsl_dcu_drm_pm_suspend(struct device *dev)
>> +{
>> + struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
>> +
>> + if (!fsl_dev)
>> + return 0;
>> +
>> + drm_kms_helper_poll_disable(fsl_dev->drm);
>> + regcache_cache_only(fsl_dev->regmap, true);
>
> This should raise a warning (see 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/base/regmap/regcache.c#n472)
>  as map->cache_bypass is set because of REGCACHE_NONE.
> I think you set the cache_type to REGCACHE_FLAT, but neither _LZO or _RBTREE 
> (see https://lkml.org/lkml/2015/7/16/552 for that)
>

okay

BR.
Jianwei


> Best regards,
> Alexander
> --
> Dipl.-Inf. Alexander Stein
> SYS TEC electronic GmbH
> alexander.stein at systec-electronic.com
>
> Legal and Commercial Address:
> Am Windrad 2
> 08468 Heinsdorfergrund
> Germany
>
> Office: +49 (0) 3765 38600-11xx
> Fax:+49 (0) 0) 3765 38600-41xx
>
> Managing Directors:
> Director Technology/CEO: Dipl.-Phys. Siegmar Schmidt;
> Director Commercial Affairs/COO: Dipl. Ing. (FH) Armin von Collrepp
> Commercial Registry:
> Amtsgericht Chemnitz, HRB 28082; USt.-Id Nr. DE150534010
>

[Bug 90681] Radeon Kernel Oops & Lockup when switching on DisplayPort attached monitor

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

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

--- Comment #11 from Andreas Tunek  ---
Is this fixed? I am asking because 91253 is probably due to the same committ.

-- 
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/20150720/4515ad2c/attachment.html>

[PATCH v11 5/5] arm/dts/ls1021a: Add a TFT LCD panel dts node

[Jianwei Wang]

Add a TFT LCD panel. the TFT LCD panel is WQVGA "480x272",
and the bpp is 24.

Signed-off-by: Alison Wang 
Signed-off-by: Xiubo Li 
Signed-off-by: Jianwei Wang 
Reviewed-by: Alison Wang 
---
 arch/arm/boot/dts/ls1021a-twr.dts | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/ls1021a-twr.dts 
b/arch/arm/boot/dts/ls1021a-twr.dts
index a2c591e..2443329 100644
--- a/arch/arm/boot/dts/ls1021a-twr.dts
+++ b/arch/arm/boot/dts/ls1021a-twr.dts
@@ -56,6 +56,17 @@
enet0_sgmii_phy = _phy2;
enet1_sgmii_phy = _phy0;
};
+
+   panel: panel {
+   compatible = "nec,nl4827hc19_05b";
+   };
+
+};
+
+ {
+   fsl,panel = <>;
+   status = "okay";
+
 };

  {
-- 
2.1.0.27.g96db324

[PATCH v11 4/5] arm/dts/ls1021a: Add DCU dts node

[Jianwei Wang]

Add DCU node, DCU is a display controller of Freescale
named 2D-ACE.

Signed-off-by: Alison Wang 
Signed-off-by: Xiubo Li 
Signed-off-by: Jianwei Wang 
Reviewed-by: Alison Wang 
---
 arch/arm/boot/dts/ls1021a.dtsi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index c70bb27..6d6e3e2 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -383,6 +383,16 @@
 <_clk 1>;
};

+   dcu: dcu at 2ce {
+   compatible = "fsl,ls1021a-dcu";
+   reg = <0x0 0x2ce 0x0 0x1>;
+   interrupts = ;
+   clocks = <_clk 0>;
+   clock-names = "dcu";
+   big-endian;
+   status = "disabled";
+   };
+
mdio0: mdio at 2d24000 {
compatible = "gianfar";
device_type = "mdio";
-- 
2.1.0.27.g96db324

[PATCH v11 3/5] drm/panel: simple: Add support for NEC NL4827HC19-05B 480x272 panel

[Jianwei Wang]

This adds support for the NEC NL4827HC19-05B 480x272 panel to the DRM
simple panel driver.

Signed-off-by: Alison Wang 
Signed-off-by: Xiubo Li 
Signed-off-by: Jianwei Wang 
Acked-by: Daniel Vetter 
---
 .../bindings/panel/nec,nl4827hc19_05b.txt  |  7 ++
 MAINTAINERS|  1 +
 drivers/gpu/drm/panel/panel-simple.c   | 26 ++
 3 files changed, 34 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/panel/nec,nl4827hc19_05b.txt

diff --git a/Documentation/devicetree/bindings/panel/nec,nl4827hc19_05b.txt 
b/Documentation/devicetree/bindings/panel/nec,nl4827hc19_05b.txt
new file mode 100644
index 000..20e9473
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/nec,nl4827hc19_05b.txt
@@ -0,0 +1,7 @@
+NEC LCD Technologies,Ltd. WQVGA TFT LCD panel
+
+Required properties:
+- compatible: should be "nec,nl4827hc19_05b"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/MAINTAINERS b/MAINTAINERS
index dc9d371..5a97a6a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3557,6 +3557,7 @@ L:dri-devel at lists.freedesktop.org
 S: Supported
 F: drivers/gpu/drm/fsl-dcu/
 F: Documentation/devicetree/bindings/video/fsl,dcu.txt
+F: Documentation/devicetree/bindings/panel/nec,nl4827hc19_05b.txt

 DRM DRIVERS FOR FREESCALE IMX
 M: Philipp Zabel 
diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index f94201b..db61dd1 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -943,6 +943,29 @@ static const struct panel_desc lg_lp129qe = {
},
 };

+static const struct drm_display_mode nec_nl4827hc19_05b_mode = {
+   .clock = 10870,
+   .hdisplay = 480,
+   .hsync_start = 480 + 2,
+   .hsync_end = 480 + 2 + 41,
+   .htotal = 480 + 2 + 41 + 2,
+   .vdisplay = 272,
+   .vsync_start = 272 + 2,
+   .vsync_end = 272 + 2 + 4,
+   .vtotal = 272 + 2 + 4 + 2,
+   .vrefresh = 74,
+};
+
+static const struct panel_desc nec_nl4827hc19_05b = {
+   .modes = _nl4827hc19_05b_mode,
+   .num_modes = 1,
+   .size = {
+   .width = 95,
+   .height = 54,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB888_1X24
+};
+
 static const struct drm_display_mode ortustech_com43h4m85ulc_mode  = {
.clock = 25000,
.hdisplay = 480,
@@ -1113,6 +1136,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "lg,lp129qe",
.data = _lp129qe,
}, {
+   .compatible = "nec,nl4827hc19_05b",
+   .data = _nl4827hc19_05b,
+   }, {
.compatible = "ortustech,com43h4m85ulc",
.data = _com43h4m85ulc,
}, {
-- 
2.1.0.27.g96db324

[PATCH v11 2/5] devicetree: Add NEC to the vendor-prefix list

[Jianwei Wang]

NEC represent NEC LCD Technologies, Ltd.

Signed-off-by: Alison Wang 
Signed-off-by: Xiubo Li 
Signed-off-by: Jianwei Wang 
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d444757..42ca6bc 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -140,6 +140,7 @@ mundoreader Mundo Reader S.L.
 murata Murata Manufacturing Co., Ltd.
 mxicy  Macronix International Co., Ltd.
 national   National Semiconductor
+necNEC LCD Technologies, Ltd.
 neonodeNeonode Inc.
 netgearNETGEAR
 netlogic   Broadcom Corporation (formerly NetLogic Microsystems)
-- 
2.1.0.27.g96db324

[PATCH v11 1/5] drm/layerscape: Add Freescale DCU DRM driver

[Jianwei Wang]

This patch add support for Two Dimensional Animation and Compositing
Engine (2D-ACE) on the Freescale SoCs.

2D-ACE is a Freescale display controller. 2D-ACE describes
the functionality of the module extremely well its name is a value
that cannot be used as a token in programming languages.
Instead the valid token "DCU" is used to tag the register names and
function names.

The Display Controller Unit (DCU) module is a system master that
fetches graphics stored in internal or external memory and displays
them on a TFT LCD panel. A wide range of panel sizes is supported
and the timing of the interface signals is highly configurable.
Graphics are read directly from memory and then blended in real-time,
which allows for dynamic content creation with minimal CPU
intervention.

The features:
(1) Full RGB888 output to TFT LCD panel.
(2) Blending of each pixel using up to 4 source layers
dependent
on size of panel.
(3) Each graphic layer can be placed with one pixel resolution
in either axis.
(4) Each graphic layer support RGB565 and RGB888 direct colors
without alpha channel and BGRA BGRA ARGB1555 direct
colors
with an alpha channel and YUV422 format.
(5) Each graphic layer support alpha blending with 8-bit
resolution.

This is a simplified version, only one primary plane, one
framebuffer, one crtc, one connector and one encoder for TFT
LCD panel.

Signed-off-by: Alison Wang 
Signed-off-by: Xiubo Li 
Signed-off-by: Jianwei Wang 
Acked-by: Daniel Vetter 
Reviewed-by: Alison Wang 
---


Changed in V11
-set regmap_config.cache_type to REGCACHE_FLAT
Advanced by Alexander Stein

Changed in V10
-adjust commit log, remove meaningless statement
-cleanup code for it's format and style.
-remove platform related code out, including of tcon(vf610) and scfg(ls1021a)
-remove useless sentences: encoder->crtc = crtc; and connector->encoder = 
encoder; and so on
-add vendor prefix for panel pandle
-make a DCU_CTRLDESCLN(x, y)  to avoid high repetition
-introduce per-SoC capability structure to avoid check on the OF node's 
compatible string
-Implement some functions: crtc enable and disable, enable and disable VBLANK, 
plane atomic_disable and atomic_enable -atomic_check and so on
-move DCU config sentence to the right place
-move get resources functions to  ->probe()
-move fsl,dcu.txt to video/ folder
-add big-endian describe
All advaced by Thierry Reding

Changed in V9

-put node after calling of_drm_find_panel
-split clk_prepare_enable() to clk_prepare() and clk_enable(), just call 
clk_prepare once, and check return value
-check regmap_write/regmap_read return return value
-remove useless ".owner= THIS_MODULE,"
All advanced by Mark Yao

Changed in V8

- Remove useless code
 #define DRIVER_NAME "fsl-dcu-drm"
 MODULE_ALIAS("platform:fsl-dcu-drm");
Adviced by Paul Bolle

Changed in V7

- Remove redundant functions and replace deprecated hooker
Adviced by Daniel Vetter
- Replace drm_platform_init with drm_dev_alloc/register
Adviced by Daniel Vetter

Changed in V6

- Add NEC nl4827hc19_05b panel to panel-simple.c
Adviced by Mark Yao
- Add DRIVER_ATOMIC for driver_features
Adviced by Mark Yao
- check fsl_dev if it's NULL at PM suspend/resume
Adviced by Mark Yao

Changed in V5

- Update commit message
- Add layer registers initialization
- Remove unused functions
- Rename driver folder
Adviced by Stefan Agner
- Move pixel clock control functions to fsl_dcu_drm_drv.c
- remove redundant enable the clock implicitly using regmap
- Add maintainer message

Changed in V4:

-This version doesn't have functionality changed
Just a minor adjustment.

Changed in V3:

- Test driver on Vybrid board and add compatible string
- Remove unused functions
- set default crtc for encoder
- replace legacy functions with atomic help functions
Adviced by Daniel Vetter
- Set the unique name of the DRM device
- Implement irq handle function for vblank interrupt

Changed in v2:
- Add atomic support
Adviced by Daniel Vetter
- Modify bindings file
- Rename node for compatibility
- Move platform related code out for compatibility
Adviced by Stefan Agner


 .../devicetree/bindings/video/fsl,dcu.txt  |  22 ++
 MAINTAINERS|   8 +
 drivers/gpu/drm/Kconfig|   2 +
 drivers/gpu/drm/Makefile   |   1 +
 drivers/gpu/drm/fsl-dcu/Kconfig|  18 +
 drivers/gpu/drm/fsl-dcu/Makefile   |   7 +
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 208 +++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.h |  19 +
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c  | 403 +
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h  | 197 ++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_fbdev.c|  23 ++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c  |  43 +++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_output.h   |  33 ++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c| 261 +
 

[Intel-gfx] [PATCH v2] drm/i915: remove unnecessary null test

[Daniel Vetter]

On Mon, Jul 20, 2015 at 08:36:01PM +0530, Sudip Mukherjee wrote:
> While creating the debugfs file we are setting the inode->i_private to
> dev. That same dev is passed to these functions as private of struct
> seq_file via single_open(). So at this point it can never be NULL.
> 
> Signed-off-by: Sudip Mukherjee 
> ---
> 
> v1 was drm/i915: fix possible null pointer dereference

There's still one left in i915_displayport_test_active_write. Also please
mention the commit that introduced these and Cc: the author. Also please
Cc: Chris since he's commented on v1 of this patch.

Thanks, Daniel

> 
>  drivers/gpu/drm/i915/i915_debugfs.c | 9 -
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index bc817da..63cb886 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4103,9 +4103,6 @@ static int i915_displayport_test_active_show(struct 
> seq_file *m, void *data)
>   struct list_head *connector_list = >mode_config.connector_list;
>   struct intel_dp *intel_dp;
>  
> - if (!dev)
> - return -ENODEV;
> -
>   list_for_each_entry(connector, connector_list, head) {
>  
>   if (connector->connector_type !=
> @@ -4150,9 +4147,6 @@ static int i915_displayport_test_data_show(struct 
> seq_file *m, void *data)
>   struct list_head *connector_list = >mode_config.connector_list;
>   struct intel_dp *intel_dp;
>  
> - if (!dev)
> - return -ENODEV;
> -
>   list_for_each_entry(connector, connector_list, head) {
>  
>   if (connector->connector_type !=
> @@ -4192,9 +4186,6 @@ static int i915_displayport_test_type_show(struct 
> seq_file *m, void *data)
>   struct list_head *connector_list = >mode_config.connector_list;
>   struct intel_dp *intel_dp;
>  
> - if (!dev)
> - return -ENODEV;
> -
>   list_for_each_entry(connector, connector_list, head) {
>  
>   if (connector->connector_type !=
> -- 
> 1.8.1.2
> 
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH libdrm] xf86drm: correct the OpenBSD DRM_MAJOR define

[Emil Velikov]

On 20 July 2015 at 17:15, Jonathan Gray  wrote:
> On Mon, Jul 20, 2015 at 05:06:09PM +0100, Emil Velikov wrote:
>> On 18 July 2015 at 22:20, Jonathan Gray  wrote:
>> > As far as I can tell no OpenBSD platform ever used 81
>> > for a drm major.  While the value was added to libdrm in 2003
>> > or earlier drm didn't appear in OpenBSD till 2007.
>> >
>> > Of the OpenBSD platforms that support drm amd64/macppc/sparc64
>> > use a major of 87, i386 uses 88.
>> >
>> > Signed-off-by: Jonathan Gray 
>> > ---
>> >  xf86drm.c | 8 +---
>> >  1 file changed, 5 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/xf86drm.c b/xf86drm.c
>> > index 7d7f9c7..a2c549c 100644
>> > --- a/xf86drm.c
>> > +++ b/xf86drm.c
>> > @@ -72,9 +72,11 @@
>> >  #define DRM_MAJOR 34
>> >  #endif
>> >
>> > -# ifdef __OpenBSD__
>> > -#  define DRM_MAJOR 81
>> > -# endif
>> > +#if defined(__OpenBSD__) && defined(__i386__)
>> > +#define DRM_MAJOR 88
>> > +#elif defined(__OpenBSD__)
>> > +#define DRM_MAJOR 87
>> > +#endif
>> >
>> Bikeshed: any objections if we cascade the ifdef statements ?
>
> Do you mean indent the defines or have the entire block
> undef ifdef __OpenBSD__?  Either way would be fine with me.

Cascade seems like the wrong term here - here is what I meant:

 #ifdef __OpenBSD__
+#ifdef __i386__
+#define DRM_MAJOR 88
+#else
+#define DRM_MAJOR 87
 #endif
+#endif /* __OpenBSD__ */

-Emil

[PATCH libdrm] remove if HAVE_LIBUDEV for vbltest

[Emil Velikov]

On 20 July 2015 at 06:36, Joonyoung Shim  wrote:
> The vbltest doesn't have any dependency of LIBUDEV.
>
> Signed-off-by: Joonyoung Shim 
Nicely spotted. I'll push this in a couple of hours.

Reviewed-by: Emil Velikov 

-Emil

[PATCH libdrm] xf86drmMode: Implement drmCheckModesettingSupported() for OpenBSD

[Emil Velikov]

On 18 July 2015 at 22:20, Jonathan Gray  wrote:
> This is implemented with kms ioctls so it could also be used as a
> generic fallback.
>
> Signed-off-by: Jonathan Gray 
> ---
>  xf86drmMode.c | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/xf86drmMode.c b/xf86drmMode.c
> index 529429e..1a199fa 100644
> --- a/xf86drmMode.c
> +++ b/xf86drmMode.c
> @@ -822,8 +822,25 @@ int drmCheckModesettingSupported(const char *busid)
>  #elif defined(__DragonFly__)
> return 0;
>  #endif
> -   return -ENOSYS;
> +#ifdef __OpenBSD__
> +   int fd;
> +   struct drm_mode_card_res res;
> +   drmModeResPtr r = 0;
> +
> +   if ((fd = drmOpen(NULL, busid)) < 0)
> +   return -EINVAL;
> +
> +   memset(, 0, sizeof(struct drm_mode_card_res));
>
> +   if (drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, )) {
> +   drmClose(fd);
> +   return -errno;
> +   }
> +
> +   drmClose(fd);
> +   return 0;
> +#endif
> +   return -ENOSYS;
>  }
>
Not 100% on it yet, but might end up deprecated (there'll be time for
everyone can object, of course). Until/if that happens we can use
this.

With the minor nitpicks on the other two patches, that whole 3 "set" is
Reviewed-by: Emil Velikov 

-Emil

[PATCH libdrm] xf86drm: use the correct device minor names on OpenBSD

[Emil Velikov]

Hi Jonathan,

On 18 July 2015 at 22:20, Jonathan Gray  wrote:
> Add defines for the device minor names and make use of them
> in drmGetMinorName() so the correct paths will be used on OpenBSD.
>
> Signed-off-by: Jonathan Gray 
> ---
>  xf86drm.c |  6 +++---
>  xf86drm.h | 15 +++
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 4de5210..7d7f9c7 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -548,11 +548,11 @@ static const char *drmGetMinorName(int type)
>  {
>  switch (type) {
>  case DRM_NODE_PRIMARY:
> -return "card";
> +return DRM_PRIMARY_MINOR_NAME;
>  case DRM_NODE_CONTROL:
> -return "controlD";
> +return DRM_CONTROL_MINOR_NAME;
>  case DRM_NODE_RENDER:
> -return "renderD";
> +return DRM_RENDER_MINOR_NAME;
>  default:
>  return NULL;
>  }
> diff --git a/xf86drm.h b/xf86drm.h
> index 40c55c9..d54ba51 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -76,12 +76,27 @@ extern "C" {
> (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH)
>  #define DRM_DEV_MODE(S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP)
>
> +#ifdef __OpenBSD__
> +#define DRM_PRIMARY_MINOR_NAME "drm"
> +#define DRM_CONTROL_MINOR_NAME "drmC"
> +#define DRM_RENDER_MINOR_NAME  "drmR"
> +
Can we please not expose these via the public API ? I'm pondering
about doing some API cleanup/deprecation (as I get some feedback and
an extra function or two). Having these as is, makes it easier to
abuse.

Thanks
Emil

[PATCH libdrm] xf86drm: correct the OpenBSD DRM_MAJOR define

[Emil Velikov]

On 18 July 2015 at 22:20, Jonathan Gray  wrote:
> As far as I can tell no OpenBSD platform ever used 81
> for a drm major.  While the value was added to libdrm in 2003
> or earlier drm didn't appear in OpenBSD till 2007.
>
> Of the OpenBSD platforms that support drm amd64/macppc/sparc64
> use a major of 87, i386 uses 88.
>
> Signed-off-by: Jonathan Gray 
> ---
>  xf86drm.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 7d7f9c7..a2c549c 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -72,9 +72,11 @@
>  #define DRM_MAJOR 34
>  #endif
>
> -# ifdef __OpenBSD__
> -#  define DRM_MAJOR 81
> -# endif
> +#if defined(__OpenBSD__) && defined(__i386__)
> +#define DRM_MAJOR 88
> +#elif defined(__OpenBSD__)
> +#define DRM_MAJOR 87
> +#endif
>
Bikeshed: any objections if we cascade the ifdef statements ?

Thanks
Emil

[PATCH 11/16] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:35PM +0530, Kausal Malladi wrote:
> CHV/BSW platform supports various Gamma correction modes, which are:
> 1. Legacy 8-bit mode
> 2. 10-bit CGM (Color Gamut Mapping) mode
> 
> This patch does the following:
> 1. Adds the core function to program Gamma correction values for CHV/BSW
>platform
> 2. Adds Gamma correction macros/defines
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 
> ---
>  drivers/gpu/drm/i915/i915_reg.h|  12 +++
>  drivers/gpu/drm/i915/intel_color_manager.c | 149 
> +
>  drivers/gpu/drm/i915/intel_color_manager.h |  20 
>  drivers/gpu/drm/i915/intel_display.c   |   3 +
>  drivers/gpu/drm/i915/intel_drv.h   |   2 +
>  5 files changed, 186 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 059de0f..4ec2e4f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7906,4 +7906,16 @@ enum skl_disp_power_wells {
>  #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
>  #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
>  
> +/* Color Management */
> +#define PIPEA_CGM_CONTROL(VLV_DISPLAY_BASE + 0x67A00)
> +#define PIPEB_CGM_CONTROL(VLV_DISPLAY_BASE + 0x69A00)
> +#define PIPEC_CGM_CONTROL(VLV_DISPLAY_BASE + 0x6BA00)
> +#define PIPEA_CGM_GAMMA  (VLV_DISPLAY_BASE + 
> 0x67000)
> +#define PIPEB_CGM_GAMMA  (VLV_DISPLAY_BASE + 
> 0x69000)
> +#define PIPEC_CGM_GAMMA  (VLV_DISPLAY_BASE + 
> 0x6B000)
> +#define _PIPE_CGM_CONTROL(pipe) \
> + (_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))
> +#define _PIPE_GAMMA_BASE(pipe) \
> + (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
> +
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index 70c0d42..7e9e934 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,155 @@
>  
>  #include "intel_color_manager.h"
>  
> +int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob,
> +   struct drm_crtc *crtc)
> +{
> + struct drm_palette *gamma_data;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_mode_config *config = >mode_config;
> + u32 cgm_control_reg = 0;
> + u32 cgm_gamma_reg = 0;
> + enum pipe pipe;
> + u32 red, green, blue;
> + u8 red_int, blue_int, green_int;
> + u16 red_fract, green_fract, blue_fract;
> + u32 count = 0;
> + struct drm_r32g32b32 *correction_values = NULL;
> + u32 num_samples;
> + u32 word;
> + int ret = 0, length, flag = 0;
> +
> + if (!blob) {
> + DRM_ERROR("NULL Blob\n");
> + return -EINVAL;
> + }
> +
> + gamma_data = (struct drm_palette *)blob->data;
> +
> + if (gamma_data->version != CHV_GAMMA_DATA_STRUCT_VERSION) {
> + DRM_ERROR("Invalid Gamma Data struct version\n");
> + return -EINVAL;
> + }
> +
> + pipe = to_intel_crtc(crtc)->pipe;
> + num_samples = gamma_data->num_samples;
> + length = num_samples * sizeof(struct drm_r32g32b32);
> +
> + if (num_samples == 0) {
> +
> + /* Disable Gamma functionality on Pipe - CGM Block */
> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
> + cgm_control_reg &= ~CGM_GAMMA_EN;
> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
> +
> + DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
> + pipe_name(pipe));
> + ret = 0;
> + } else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS ||
> + num_samples == CHV_8BIT_GAMMA_MAX_VALS) {
> + cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);
> +
> + count = 0;
> + correction_values = (struct drm_r32g32b32 *)_data->lut;
> + while (count < num_samples) {
> + blue = correction_values[count].b32;
> + green = correction_values[count].g32;
> + red = correction_values[count].r32;
> +
> + blue_int = _GAMMA_INT_PART(blue);
> + if (blue_int > GAMMA_INT_MAX)
> + blue = CHV_MAX_GAMMA;
> + green_int = _GAMMA_INT_PART(green);
> + if (green_int > GAMMA_INT_MAX)
> + green = CHV_MAX_GAMMA;
> + red_int = _GAMMA_INT_PART(red);
> + if (red_int > GAMMA_INT_MAX)
> + red = CHV_MAX_GAMMA;
> +
> + blue_fract = _GAMMA_FRACT_PART(blue);
> + green_fract = 

[PATCH 10/16] drm/i915: Add set_property handler for pipe Gamma correction on CHV/BSW

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:34PM +0530, Kausal Malladi wrote:
> This patch adds set_property handler for Gamma color correction and
> enhancement capability at Pipe level on CHV/BSW platform. The set
> function just attaches the Gamma blob to CRTC state, that later gets
> committed using atomic path.
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 
> ---
>  drivers/gpu/drm/i915/intel_atomic.c|  7 +++
>  drivers/gpu/drm/i915/intel_color_manager.c | 19 +++
>  drivers/gpu/drm/i915/intel_drv.h   |  3 +++
>  3 files changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index d873bda..72d6b37 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -473,6 +473,13 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
>  struct drm_property *property,
>  uint64_t val)
>  {
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = >mode_config;
> +
> + if (property == config->prop_palette_after_ctm)
> + return intel_color_manager_set_pipe_gamma(dev, state,
> + >base, val);
> +
>   DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>   return -EINVAL;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index def20d0f..70c0d42 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,25 @@
>  
>  #include "intel_color_manager.h"
>  
> +int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
> + struct drm_crtc_state *crtc_state,
> + struct drm_mode_object *obj, uint32_t blob_id)
> +{
> + struct drm_property_blob *blob;
> +
> + blob = drm_property_lookup_blob(dev, blob_id);
> + if (!blob) {
> + DRM_ERROR("Invalid Blob ID\n");
> + return -EINVAL;
> + }

I'm not terribly familiar the semantics of the color correction
stuff...this prevents userspace from removing the gamma setting by
passing a 0; is that expected that gamma can't be disabled once set?
Same question for your degamma patch later in the series.


Matt

> +
> + if (crtc_state->palette_after_ctm_blob)
> + 
> drm_property_unreference_blob(crtc_state->palette_after_ctm_blob);
> +
> + crtc_state->palette_after_ctm_blob = blob;
> + return 0;
> +}
> +
>  int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
>   struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1e61036..45c42f0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1453,5 +1453,8 @@ extern const struct drm_plane_helper_funcs 
> intel_plane_helper_funcs;
>  /* intel_color_manager.c */
>  void intel_attach_color_properties_to_crtc(struct drm_device *dev,
>   struct drm_mode_object *mode_obj);
> +int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
> + struct drm_crtc_state *crtc_state,
> + struct drm_mode_object *obj, uint32_t blob_id);
>  
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 2.4.5
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[PATCH 07/16] drm/i915: Add atomic set property interface for CRTC

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:31PM +0530, Kausal Malladi wrote:
> This patch adds atomic set property interface for Intel CRTC. This
> interface will be used to set color correction DRM properties.
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 

I think we also need .get_property / .atomic_get_property entrypoints as
well.  When userspace tries to query properties, the DRM core will want
to call into i915 for any non-core property that it doesn't recognize.

We don't really have any examples of .atomic_get_property in i915 today,
so you might want to look at omap's omap_plane_atomic_get_property() for
a very basic example (but rather than returning a raw value for color
correction properties, you'll return blob->base.id for the relevant
property).


Matt

> ---
>  drivers/gpu/drm/i915/intel_atomic.c  | 11 +++
>  drivers/gpu/drm/i915/intel_display.c |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h |  4 
>  3 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 6ce6c3d..d873bda 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include "intel_drv.h"
> +#include "intel_color_manager.h"
>  
>  
>  /**
> @@ -465,3 +466,13 @@ void intel_atomic_state_clear(struct drm_atomic_state *s)
>   drm_atomic_state_default_clear(>base);
>   state->dpll_set = false;
>  }
> +
> +
> +int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
> +struct drm_crtc_state *state,
> +struct drm_property *property,
> +uint64_t val)
> +{
> + DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
> + return -EINVAL;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index b6e1dc5..11d491e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13477,6 +13477,8 @@ static const struct drm_crtc_funcs intel_crtc_funcs = 
> {
>   .set_config = intel_crtc_set_config,
>   .destroy = intel_crtc_destroy,
>   .page_flip = intel_crtc_page_flip,
> + .set_property = drm_atomic_helper_crtc_set_property,
> + .atomic_set_property = intel_crtc_atomic_set_property,
>   .atomic_duplicate_state = intel_crtc_duplicate_state,
>   .atomic_destroy_state = intel_crtc_destroy_state,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 05c809b..1e61036 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1438,6 +1438,10 @@ intel_atomic_get_crtc_state(struct drm_atomic_state 
> *state,
>  int intel_atomic_setup_scalers(struct drm_device *dev,
>   struct intel_crtc *intel_crtc,
>   struct intel_crtc_state *crtc_state);
> +int intel_crtc_atomic_set_property(struct drm_crtc *plane,
> +struct drm_crtc_state *state,
> +struct drm_property *property,
> +uint64_t val);
>  
>  /* intel_atomic_plane.c */
>  struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
> -- 
> 2.4.5
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[PATCH 06/16] drm/i915: Load gamma color capabilities for CHV CRTC

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:30PM +0530, Kausal Malladi wrote:
> As per Color Manager design, each driver is responsible to load its
> palette color correction and enhancement capabilities in the form of
> a DRM blob property, so that user space can query and read.
> 
> This patch loads all CHV platform specific gamma color capabilities
> for CRTC into a blob that can be accessible by user space to
> query capabilities via DRM property interface.
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 
> ---
>  drivers/gpu/drm/i915/intel_color_manager.c | 48 
> ++
>  drivers/gpu/drm/i915/intel_color_manager.h |  4 +++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index baa4536..def20d0f 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,15 +27,63 @@
>  
>  #include "intel_color_manager.h"
>  
> +int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
> + struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
> +{
> + struct drm_property_blob *blob = NULL;
> + struct drm_mode_config *config = >mode_config;
> + int ret;
> +
> + /*
> +  * This function exposes best capability for DeGamma and Gamma
> +  * For CHV, the DeGamma LUT has 65 entries
> +  * and the best Gamma capability has 257 entries (CGM unit)
> +  */
> + palette_caps->version = CHV_PALETTE_STRUCT_VERSION;
> + palette_caps->num_samples_before_ctm =
> + CHV_DEGAMMA_MAX_VALS;
> + palette_caps->num_samples_after_ctm =
> + CHV_10BIT_GAMMA_MAX_VALS;
> +
> + ret = drm_property_replace_global_blob(dev, ,

We're only calling this once at startup and never actually replacing the
capabilities, right?  Is there any difference between this and just
manually doing a drm_property_create_blob() and passing that blob's ID
as the value when we attach the property to the CRTC?  It feels a bit
strange to be calling 'replace' on a local variable that isn't
initialized and is never used/saved.


> + sizeof(struct drm_palette_caps),
> + (const void *)palette_caps,
> + >base, config->prop_color_capabilities);
> + if (ret) {
> + DRM_ERROR("Error updating Gamma blob\n");
> + return -EFAULT;
> + }
> +
> + return 0;
> +}
> +
> +int get_pipe_gamma_capabilities(struct drm_device *dev,
> + struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
> +{
> + if (IS_CHERRYVIEW(dev))
> + return get_chv_pipe_gamma_capabilities(dev, palette_caps, crtc);
> + return -EINVAL;
> +}
> +
>  void intel_attach_color_properties_to_crtc(struct drm_device *dev,
>   struct drm_mode_object *mode_obj)
>  {
>   struct drm_mode_config *config = >mode_config;
> + struct drm_palette_caps *palette_caps;
> + struct drm_crtc *crtc;
> + int ret;
>  
>   if (IS_CHERRYVIEW(dev)) {
> + crtc = obj_to_crtc(mode_obj);
>   if (config->prop_color_capabilities)
>   drm_object_attach_property(mode_obj,
>   config->prop_color_capabilities, 0);
> + palette_caps = kzalloc(sizeof(struct drm_palette_caps),
> + GFP_KERNEL);
> + ret = get_pipe_gamma_capabilities(dev, palette_caps, crtc);

Is palette_caps ever freed?  I believe the data is copied when a blob is
created, so I think you should be free to kfree() this allocation right
away.


Matt

> + if (ret)
> + DRM_ERROR("Error getting gamma capability for CHV\n");
> +
>   if (config->prop_palette_before_ctm)
>   drm_object_attach_property(mode_obj,
>   config->prop_palette_before_ctm, 0);
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h 
> b/drivers/gpu/drm/i915/intel_color_manager.h
> index 04c921d..51aeb91 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.h
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -27,3 +27,7 @@
>  #include 
>  #include 
>  #include "i915_drv.h"
> +
> +#define CHV_PALETTE_STRUCT_VERSION   1
> +#define CHV_DEGAMMA_MAX_VALS 65
> +#define CHV_10BIT_GAMMA_MAX_VALS 257
> -- 
> 2.4.5
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[PATCH 03/16] drm/i915: Attach color properties to CRTC

[Matt Roper]

On Wed, Jul 15, 2015 at 06:39:27PM +0530, Kausal Malladi wrote:
> This patch does the following:
> 1. Adds new files intel_color_manager(.c/.h)
> 2. Attaches color properties to CRTC while initialization
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 

I'd suggest moving this patch to the end of the series.  If someone is
bisecting an issue and they land somewhere in the middle of this
patchset, you'll be advertising properties to userspace that aren't
fully functional which may cause unexpected test failures (and make
bisect decisions harder).

Generally we like to "flip the switch" to make new functionality visible
only at the very end when all the internal plumbing is complete.


Matt

> ---
>  drivers/gpu/drm/i915/Makefile  |  3 +-
>  drivers/gpu/drm/i915/intel_color_manager.c | 49 
> ++
>  drivers/gpu/drm/i915/intel_color_manager.h | 29 ++
>  drivers/gpu/drm/i915/intel_display.c   |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h   |  4 +++
>  5 files changed, 86 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
>  create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index de21965..ad928d8 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -56,7 +56,8 @@ i915-y += intel_audio.o \
> intel_overlay.o \
> intel_psr.o \
> intel_sideband.o \
> -   intel_sprite.o
> +   intel_sprite.o \
> +   intel_color_manager.o
>  i915-$(CONFIG_ACPI)  += intel_acpi.o intel_opregion.o
>  i915-$(CONFIG_DRM_I915_FBDEV)+= intel_fbdev.o
>  
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
> b/drivers/gpu/drm/i915/intel_color_manager.c
> new file mode 100644
> index 000..baa4536
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma 
> + * Kausal Malladi 
> + */
> +
> +#include "intel_color_manager.h"
> +
> +void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> + struct drm_mode_object *mode_obj)
> +{
> + struct drm_mode_config *config = >mode_config;
> +
> + if (IS_CHERRYVIEW(dev)) {
> + if (config->prop_color_capabilities)
> + drm_object_attach_property(mode_obj,
> + config->prop_color_capabilities, 0);
> + if (config->prop_palette_before_ctm)
> + drm_object_attach_property(mode_obj,
> + config->prop_palette_before_ctm, 0);
> + if (config->prop_palette_after_ctm)
> + drm_object_attach_property(mode_obj,
> + config->prop_palette_after_ctm, 0);
> + if (config->prop_ctm)
> + drm_object_attach_property(mode_obj,
> + config->prop_ctm, 0);
> + }
> +}
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h 
> b/drivers/gpu/drm/i915/intel_color_manager.h
> new file mode 100644
> index 000..04c921d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is 

[Intel-gfx] [PATCH] drm/i915: fix possible null pointer dereference

[Daniel Vetter]

On Mon, Jul 20, 2015 at 06:58:32PM +0530, Sudip Mukherjee wrote:
> On Mon, Jul 20, 2015 at 01:38:46PM +0100, Chris Wilson wrote:
> > On Mon, Jul 20, 2015 at 05:59:29PM +0530, Sudip Mukherjee wrote:
> > > We were dereferencing dev first and then checking if it is NULL. Lets
> > > check for NULL first and then dereference.
> > 
> > The code is bonkers. Testing for a lack of a correctly constructed
> > debugfs seq_file inside the debugfs seq_file callback is inane.
> I missed seeing before sending this patch that there are some more
> places where this has been done.
> Then are you suggesting to remove the test?

It's all been added for the tp validation support. And yes it should be
removed everywhere.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

KMS timings (Re: [PATCH 6/8] drm/bochs: phase 3: provide a custom ->atomic_commit implementation)

[Daniel Vetter]

On Mon, Jul 20, 2015 at 12:35:48PM +0300, Pekka Paalanen wrote:
> On Mon, 20 Jul 2015 01:58:33 -0700
> Stéphane Marchesin  wrote:
> 
> > On Mon, Jul 20, 2015 at 12:46 AM, Pekka Paalanen  
> > wrote:
> > > On Sun, 19 Jul 2015 17:20:32 -0700
> > > Stéphane Marchesin  wrote:
> > >
> > >> On Thu, Jul 16, 2015 at 11:08 PM, Pekka Paalanen  > >> gmail.com> wrote:
> > >> >
> > >> > On Thu, 16 Jul 2015 20:20:39 +0800
> > >> > John Hunter  wrote:
> > >> >
> > >> > > From: Zhao Junwang 
> > >> > >
> > >> > > This supports the asynchronous commits, required for page-flipping
> > >> > > Since it's virtual hw it's ok to commit async stuff right away, we
> > >> > > never have to wait for vblank.
> > >> >
> > >> > Hi,
> > >> >
> > >> > in theory, yes. This is what a patch to bochs implemented not too long
> > >> > ago, so AFAIK you are only replicating the existing behaviour.
> > >> >
> > >> > However, if userspace doing an async commit (or sync, I suppose) does
> > >> > not incur any waits in the kernel in e.g. sending the page flip event,
> > >> > then flip driven programs (e.g. a Wayland compositor, say, Weston)
> > >> > will be running its rendering loop as a busy-loop, because the kernel
> > >> > does not throttle it to the (virtual) display refresh rate.
> > >> >
> > >> > This will cause maximal CPU usage and poor user experience as
> > >> > everything else needs to fight for CPU time and event dispatch to get
> > >> > through, like input.
> > >> >
> > >> > I would hope someone could do a follow-up to implement a refresh cycle
> > >> > emulation based on a clock. Userspace expects page flips to happen at
> > >> > most at refresh rate when asking for vblank-synced flips. It's only
> > >> > natural for userspace to drive its rendering loop based on the vblank
> > >> > cycle.
> > >>
> > >>
> > >> I've been asking myself the same question (for the UDL driver) and I'm
> > >> not sure if this policy should go in the kernel. After all, there
> > >> could be legitimate reasons for user space to render lots of frames
> > >> per second. It seems to me that if user space doesn't want too many
> > >> fps, it should just throttle itself.
> > >
> > > If userspace wants to render lots of frames per second, IMO it should
> > > not be using vblank-synced operations in a way that may throttle it.
> > > The lots of frames use case is already non-working for the majority of
> > > the drivers without DRM_MODE_PAGE_FLIP_ASYNC, right?
> > >
> > > The problem here I see is that one DRM driver decides to work different
> > > to other DRM drivers. All real-hardware DRM drivers, when asked to do
> > > vblank-synced update, actually do throttle to the vblank AFAIK.
> > 
> > udl is an exception here. It is (arguably) real hardware but doesn't 
> > throttle.
> > 
> > > Is it
> > > too much to assume, that the video mode set in a driver (refresh rate)
> > > corresponds to the vblank rate which implicitly delays the completion
> > > of vblank-sync'd operations to at least the next vblank boundary?
> > 
> > I think it's wrong to make user space think that a vsynced display
> > always matches the refresh rate in a world where:
> > 
> > - some displays have variable refresh rates (not just the fancy new
> > stuff like g-sync, look for lvds_downclock in the intel driver for
> > example, also consider DSI displays)
> > 
> > - some displays have no refresh rate (the ones we are talking about
> > here: udl, bochs...)
> 
> That means that refresh rate in a video mode is bogus. Can userspace
> know when the refresh rate is meaningless? I suppose there are two
> different cases of meaningless, too: when the driver ignores it as
> input argument, and when it is used but has no guarantees for timings.
> 
> Assuming it's always meaningless wrt. timings is pretty harsh. E.g. the
> Wayland Presentation extension's implementation in Weston uses the
> refresh rate to predict the next flip time and hands it out to clients
> for scheduling/interpolation purposes.

We removed lvds_downclock in i915 (was never enabled by default). The new
code is currently edp only, but enabled by default. And it tries _really_
hard to keep up the illusion that the requested vrefresh is the one you
actually have - it upclocks every time userspace changes the screen.

The only exception is when you only ask for vblank events, and the delay
to the next pageflip is longer than the timeout. That can't be fixed right
now because drm_irq.c is the last midlayer needed by modern drivers, but
I'd like to fix it eventually.

There's future plans to allow userspace to explicitly ask for a reduced
vrefresh (e.g. video playback), but then obviously they'll match again
too.

We also have a big pile of testcases in kms_flip which check that the
timestamps are evenly spaced and agree with vrefresh. There's some oddball
bugs around tv-out that we never bothered to fix (since the requested mode
is a fake one and rescaled by the TV port, which also has it's own clock).

Imo aiming for vrefresh to be 

[Bug 91375] [radeonsi] [drm:si_dpm_set_power_state [radeon]] *ERROR* si_restrict_performance_levels_before_switch failed

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

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

--- Comment #1 from Alex Deucher  ---
please attach your xorg log and dmesg output.  In most cases this can be
ignored if things are working properly.

-- 
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/20150720/da41875e/attachment.html>

[PATCH libdrm] remove if HAVE_LIBUDEV for vbltest

[Joonyoung Shim]

The vbltest doesn't have any dependency of LIBUDEV.

Signed-off-by: Joonyoung Shim 
---
 tests/Makefile.am | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 069285f..c5edec8 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,4 +1,4 @@
-SUBDIRS = modeprint proptest modetest
+SUBDIRS = modeprint proptest modetest vbltest

 if HAVE_LIBKMS
 SUBDIRS += kmstest
@@ -16,10 +16,6 @@ if HAVE_TEGRA
 SUBDIRS += tegra
 endif

-if HAVE_LIBUDEV
-SUBDIRS += vbltest
-endif
-
 AM_CFLAGS = \
$(WARN_CFLAGS)\
-I $(top_srcdir)/include/drm \
-- 
1.9.1

[Bug 61529] [r600g][kms][ATI RV710] r600_pcie_gart_tlb_flush+0xf5/0x110

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

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

Thaddaeus Tintenfisch  changed:

   What|Removed |Added

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

--- Comment #13 from Thaddaeus Tintenfisch  ---
Oh, 4.2-rc3 includes the patch. Thanks.

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=233709d2cd6bbaaeda0aeb8d11f6ca7f98563b39

-- 
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/20150720/0e4387b0/attachment.html>

[PATCH] drm/i915: fix possible null pointer dereference

[Chris Wilson]

On Mon, Jul 20, 2015 at 05:59:29PM +0530, Sudip Mukherjee wrote:
> We were dereferencing dev first and then checking if it is NULL. Lets
> check for NULL first and then dereference.

The code is bonkers. Testing for a lack of a correctly constructed
debugfs seq_file inside the debugfs seq_file callback is inane.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

KMS timings (Re: [PATCH 6/8] drm/bochs: phase 3: provide a custom ->atomic_commit implementation)

[Pekka Paalanen]

On Mon, 20 Jul 2015 01:58:33 -0700
Stéphane Marchesin  wrote:

> On Mon, Jul 20, 2015 at 12:46 AM, Pekka Paalanen  
> wrote:
> > On Sun, 19 Jul 2015 17:20:32 -0700
> > Stéphane Marchesin  wrote:
> >
> >> On Thu, Jul 16, 2015 at 11:08 PM, Pekka Paalanen  
> >> wrote:
> >> >
> >> > On Thu, 16 Jul 2015 20:20:39 +0800
> >> > John Hunter  wrote:
> >> >
> >> > > From: Zhao Junwang 
> >> > >
> >> > > This supports the asynchronous commits, required for page-flipping
> >> > > Since it's virtual hw it's ok to commit async stuff right away, we
> >> > > never have to wait for vblank.
> >> >
> >> > Hi,
> >> >
> >> > in theory, yes. This is what a patch to bochs implemented not too long
> >> > ago, so AFAIK you are only replicating the existing behaviour.
> >> >
> >> > However, if userspace doing an async commit (or sync, I suppose) does
> >> > not incur any waits in the kernel in e.g. sending the page flip event,
> >> > then flip driven programs (e.g. a Wayland compositor, say, Weston)
> >> > will be running its rendering loop as a busy-loop, because the kernel
> >> > does not throttle it to the (virtual) display refresh rate.
> >> >
> >> > This will cause maximal CPU usage and poor user experience as
> >> > everything else needs to fight for CPU time and event dispatch to get
> >> > through, like input.
> >> >
> >> > I would hope someone could do a follow-up to implement a refresh cycle
> >> > emulation based on a clock. Userspace expects page flips to happen at
> >> > most at refresh rate when asking for vblank-synced flips. It's only
> >> > natural for userspace to drive its rendering loop based on the vblank
> >> > cycle.
> >>
> >>
> >> I've been asking myself the same question (for the UDL driver) and I'm
> >> not sure if this policy should go in the kernel. After all, there
> >> could be legitimate reasons for user space to render lots of frames
> >> per second. It seems to me that if user space doesn't want too many
> >> fps, it should just throttle itself.
> >
> > If userspace wants to render lots of frames per second, IMO it should
> > not be using vblank-synced operations in a way that may throttle it.
> > The lots of frames use case is already non-working for the majority of
> > the drivers without DRM_MODE_PAGE_FLIP_ASYNC, right?
> >
> > The problem here I see is that one DRM driver decides to work different
> > to other DRM drivers. All real-hardware DRM drivers, when asked to do
> > vblank-synced update, actually do throttle to the vblank AFAIK.
> 
> udl is an exception here. It is (arguably) real hardware but doesn't throttle.
> 
> > Is it
> > too much to assume, that the video mode set in a driver (refresh rate)
> > corresponds to the vblank rate which implicitly delays the completion
> > of vblank-sync'd operations to at least the next vblank boundary?
> 
> I think it's wrong to make user space think that a vsynced display
> always matches the refresh rate in a world where:
> 
> - some displays have variable refresh rates (not just the fancy new
> stuff like g-sync, look for lvds_downclock in the intel driver for
> example, also consider DSI displays)
> 
> - some displays have no refresh rate (the ones we are talking about
> here: udl, bochs...)

That means that refresh rate in a video mode is bogus. Can userspace
know when the refresh rate is meaningless? I suppose there are two
different cases of meaningless, too: when the driver ignores it as
input argument, and when it is used but has no guarantees for timings.

Assuming it's always meaningless wrt. timings is pretty harsh. E.g. the
Wayland Presentation extension's implementation in Weston uses the
refresh rate to predict the next flip time and hands it out to clients
for scheduling/interpolation purposes.

> - you can do partial vsynced updates by just waiting for a specific
> scanline range which again breaks the assumption that "vsynced" ==
> "refreshes at the monitor rate". In this case there is no visible
> tearing (in that sense it is vsynced) but the flip time is not
> predictable using the refresh rate.

Okay. That also invalidates the design (well, at least the
implementation, and sounds like DRM does not give any tools to allow
implementing it) the Wayland Presentation extension even on "good"
hardware, so nice to realize. I was already suggesting we should
stabilize it since it looks good, but this puts it all back to the
drawing board.

I think it also mostly invalidates the whole scheduling implementation
in Weston.

> So I don't think we should perpetuate that problem. And I would like
> user space to "see" the actual flip times to enable some kind of
> scheduling where possible.
> 
> >
> > I think, if the driver cannot implement proper semantics (which IMO
> > includes the throttling) for vblank-sync'd operations and it does not
> > want to fake them with a clock, it should just refuse vblank-synced
> > operations.
> 
> Yes refusing vsynced flips for these drivers sounds reasonable. But
> please let's not bake in another 

[PATCH v2 2/4] drm/tegra: Add VIC support

[Thierry Reding]

On Mon, Jul 20, 2015 at 10:56:00AM +0100, Jon Hunter wrote:
> 
> On 20/07/15 09:51, Mikko Perttunen wrote:
> > On 07/20/2015 11:28 AM, Jon Hunter wrote:
> >> Hi Mikko,
> > 
> > Hi!
> > 
> >> ...
> >>> +static int vic_runtime_resume(struct device *dev)
> >>> +{
> >>> + struct vic *vic = dev_get_drvdata(dev);
> >>> + int err;
> >>> +
> >>> + err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
> >>> + vic->clk, vic->rst);
> >>
> >> I would like to deprecate use of the above function [1]. I have been
> >> trying to migrate driver to use a new API instead of the above.
> > 
> > Yes, we will need to coordinate the API change here. I kept the old way
> > here to not depend on your series for now.
> > 
> >>
> >>> + if (err < 0)
> >>> + dev_err(dev, "failed to power up device\n");
> >>> +
> >>> + return err;
> >>> +}
> >>> +
> >>> ...
> >>> +
> >>> + pm_runtime_enable(>dev);
> >>> + if (!pm_runtime_enabled(>dev)) {
> >>> + err = vic_runtime_resume(>dev);
> >>> + if (err < 0)
> >>> + goto unregister_client;
> >>> + }
> >>
> >> I don't see why pm_runtime_enable() should ever fail to enable
> >> pm_runtime here. Hence, the if-statement appears to be redundant. If you
> >> are trying to determine whether rpm is supported for the power-domains
> >> then you should simply check to see if the DT node for the device has
> >> the "power-domains" property. See my series [1].
> > 
> > The intention is that if runtime PM is not enabled, the power domain is
> > still brought up. On a cursory look, it seems like with your patch, this
> > should indeed work fine without this check if CONFIG_PM is enabled. Now,
> > that might always be enabled? If so, then everything would be fine; if
> > this lands after your series, we could even just make the power-domains
> > prop mandatory and not implement the legacy mode at all. If not, then
> > AFAIU we must still keep this path.
> 
> Ok, I see you just wish to test it is enabled in the kernel config. Then
> yes that would make sense and the above is fine.

Do we really need to bother about this? If runtime PM isn't enabled we
don't care very much about power management anyway. So in my opinion if
a driver supports runtime PM it should support it all the way and not
pretend to. Having this mix of runtime PM vs. no runtime PM is beside
the point of using something like runtime PM in the first place.

So if this is about supporting legacy vs. runtime PM, then I think it
should be based on some more explicit check, like if (!dev->pm_domain),
but otherwise we should assume that pm_runtime_enable() succeeds.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150720/314568c1/attachment.sig>

[PATCH v2 2/4] drm/tegra: Add VIC support

[Mikko Perttunen]

On 07/20/2015 11:28 AM, Jon Hunter wrote:
> Hi Mikko,

Hi!

> ...
>> +static int vic_runtime_resume(struct device *dev)
>> +{
>> +struct vic *vic = dev_get_drvdata(dev);
>> +int err;
>> +
>> +err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
>> +vic->clk, vic->rst);
> 
> I would like to deprecate use of the above function [1]. I have been
> trying to migrate driver to use a new API instead of the above.

Yes, we will need to coordinate the API change here. I kept the old way
here to not depend on your series for now.

> 
>> +if (err < 0)
>> +dev_err(dev, "failed to power up device\n");
>> +
>> +return err;
>> +}
>> +
>> ...
>> +
>> +pm_runtime_enable(>dev);
>> +if (!pm_runtime_enabled(>dev)) {
>> +err = vic_runtime_resume(>dev);
>> +if (err < 0)
>> +goto unregister_client;
>> +}
> 
> I don't see why pm_runtime_enable() should ever fail to enable
> pm_runtime here. Hence, the if-statement appears to be redundant. If you
> are trying to determine whether rpm is supported for the power-domains
> then you should simply check to see if the DT node for the device has
> the "power-domains" property. See my series [1].

The intention is that if runtime PM is not enabled, the power domain is
still brought up. On a cursory look, it seems like with your patch, this
should indeed work fine without this check if CONFIG_PM is enabled. Now,
that might always be enabled? If so, then everything would be fine; if
this lands after your series, we could even just make the power-domains
prop mandatory and not implement the legacy mode at all. If not, then
AFAIU we must still keep this path.

> 
> Cheers
> Jon
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg431051.html
> 

Cheers,
Mikko.

[PATCH] drm/tegra: dpaux: Fix transfers larger than 4 bytes

[Thierry Reding]

On Mon, Jul 20, 2015 at 02:39:36AM -0500, Steev Klimaszewski wrote:
> On Mon, Jul 20, 2015 at 1:49 AM, Thierry Reding 
> wrote:
> 
> > From: Thierry Reding 
> >
> > The DPAUX read/write FIFO registers aren't sequential in the register
> > space, causing transfers larger than 4 bytes to cause accesses to non-
> > existing FIFO registers.
> >
> > Fixes: 6b6b604215c6 ("drm/tegra: Add eDP support")
> > Signed-off-by: Thierry Reding 
> > ---
> >  drivers/gpu/drm/tegra/dpaux.c | 18 --
> >  1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> > index d6b55e3e3716..a43a836e6f88 100644
> > --- a/drivers/gpu/drm/tegra/dpaux.c
> > +++ b/drivers/gpu/drm/tegra/dpaux.c
> > @@ -72,34 +72,32 @@ static inline void tegra_dpaux_writel(struct
> > tegra_dpaux *dpaux,
> >  static void tegra_dpaux_write_fifo(struct tegra_dpaux *dpaux, const u8
> > *buffer,
> >size_t size)
> >  {
> > -   unsigned long offset = DPAUX_DP_AUXDATA_WRITE(0);
> > size_t i, j;
> >
> > -   for (i = 0; i < size; i += 4) {
> > -   size_t num = min_t(size_t, size - i, 4);
> > +   for (i = 0; i < DIV_ROUND_UP(size, 4); i++) {
> > +   size_t num = min_t(size_t, size - i * 4, 4);
> > unsigned long value = 0;
> >
> > for (j = 0; j < num; j++)
> > -   value |= buffer[i + j] << (j * 8);
> > +   value |= buffer[i * 4 + j] << (j * 8);
> >
> > -   tegra_dpaux_writel(dpaux, value, offset++);
> > +   tegra_dpaux_writel(dpaux, value,
> > DPAUX_DP_AUXDATA_WRITE(i));
> > }
> >  }
> >
> >  static void tegra_dpaux_read_fifo(struct tegra_dpaux *dpaux, u8 *buffer,
> >   size_t size)
> >  {
> > -   unsigned long offset = DPAUX_DP_AUXDATA_READ(0);
> > size_t i, j;
> >
> > -   for (i = 0; i < size; i += 4) {
> > -   size_t num = min_t(size_t, size - i, 4);
> > +   for (i = 0; i < DIV_ROUND_UP(size, 4); i++) {
> > +   size_t num = min_t(size_t, size - i * 4, 4);
> > unsigned long value;
> >
> > -   value = tegra_dpaux_readl(dpaux, offset++);
> > +   value = tegra_dpaux_readl(dpaux, DPAUX_DP_AUXDATA_READ(i));
> >
> > for (j = 0; j < num; j++)
> > -   buffer[i + j] = value >> (j * 8);
> > +   buffer[i * 4 + j] = value >> (j * 8);
> >     }
> >  }
> >
> > --
> > 2.4.5
> >
> >
> 
> This fixes the issue that I reported earlier, so feel free to add my
> 
> Tested-by: Steev Klimaszewski 

Great, thanks.

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

[PATCH v2 2/4] drm/tegra: Add VIC support

[Thierry Reding]

_reg;
> > +
> > +   /* Method call number store. */
> > +   if (offset == FALCON_UCLASS_METHOD_OFFSET >> 2) {
> > +   u32 method = val << 2;
> > +
> > +   if ((method >= VIC_SET_SURFACE0_SLOT0_LUMA_OFFSET &&
> > +method <= VIC_SET_SURFACE7_SLOT4_CHROMAV_OFFSET) ||
> > +   (method >= VIC_SET_CONFIG_STRUCT_OFFSET &&
> > +method <= VIC_SET_OUTPUT_SURFACE_CHROMAV_OFFSET))
> > +   vic->method_data_is_addr_reg = true;
> > +   else
> > +   vic->method_data_is_addr_reg = false;
> > +   }
> > +
> > +   return false;
> > +}
> > +
> > +static const struct tegra_drm_client_ops vic_ops = {
> > +   .open_channel = vic_open_channel,
> > +   .close_channel = vic_close_channel,
> > +   .is_addr_reg = vic_is_addr_reg,
> > +   .submit = tegra_drm_submit,
> > +};
> > +
> > +static const struct vic_config vic_t124_config = {
> > +   .ucode_name = "vic03_ucode.bin",
> > +};
> > +
> > +static const struct of_device_id vic_match[] = {
> > +   { .compatible = "nvidia,tegra124-vic", .data = _t124_config },
> > +   { },
> > +};
> > +
> > +static int vic_probe(struct platform_device *pdev)
> > +{
> > +   struct vic_config *vic_config = NULL;
> > +   struct device *dev = >dev;
> > +   struct host1x_syncpt **syncpts;
> > +   struct resource *regs;
> > +   const struct of_device_id *match;
> > +   struct vic *vic;
> > +   int err;
> > +
> > +   match = of_match_device(vic_match, dev);
> > +   vic_config = (struct vic_config *)match->data;
> > +
> > +   vic = devm_kzalloc(dev, sizeof(*vic), GFP_KERNEL);
> > +   if (!vic)
> > +   return -ENOMEM;
> > +
> > +   syncpts = devm_kzalloc(dev, sizeof(*syncpts), GFP_KERNEL);
> > +   if (!syncpts)
> > +   return -ENOMEM;
> > +
> > +   regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   if (!regs) {
> > +   dev_err(>dev, "failed to get registers\n");
> > +   return -ENXIO;
> > +   }
> > +
> > +   vic->regs = devm_ioremap_resource(dev, regs);
> > +   if (IS_ERR(vic->regs))
> > +   return PTR_ERR(vic->regs);
> > +
> > +   vic->clk = devm_clk_get(dev, NULL);
> > +   if (IS_ERR(vic->clk)) {
> > +   dev_err(>dev, "failed to get clock\n");
> > +   return PTR_ERR(vic->clk);
> > +   }
> > +
> > +   vic->rst = devm_reset_control_get(dev, "vic");
> > +   if (IS_ERR(vic->rst)) {
> > +   dev_err(>dev, "cannot get reset\n");
> > +   return PTR_ERR(vic->rst);
> > +   }
> > +
> > +   platform_set_drvdata(pdev, vic);
> > +
> > +   INIT_LIST_HEAD(>client.base.list);
> > +   vic->client.base.ops = _client_ops;
> > +   vic->client.base.dev = dev;
> > +   vic->client.base.class = HOST1X_CLASS_VIC;
> > +   vic->client.base.syncpts = syncpts;
> > +   vic->client.base.num_syncpts = 1;
> > +   vic->dev = dev;
> > +   vic->config = vic_config;
> > +
> > +   INIT_LIST_HEAD(>client.list);
> > +   vic->client.ops = _ops;
> > +
> > +   err = host1x_client_register(>client.base);
> > +   if (err < 0) {
> > +   dev_err(dev, "failed to register host1x client: %d\n", err);
> > +   platform_set_drvdata(pdev, NULL);
> > +   return err;
> > +   }
> > +
> > +   pm_runtime_enable(>dev);
> > +   if (!pm_runtime_enabled(>dev)) {
> > +   err = vic_runtime_resume(>dev);
> > +   if (err < 0)
> > +   goto unregister_client;
> > +   }
> 
> I don't see why pm_runtime_enable() should ever fail to enable
> pm_runtime here. Hence, the if-statement appears to be redundant. If you
> are trying to determine whether rpm is supported for the power-domains
> then you should simply check to see if the DT node for the device has
> the "power-domains" property. See my series [1].

Merely checking for a device tree property won't tell you anything.
There are no guarantees that some driver will make the power domains
available, even if they are defined in the DT.

Generally checking device tree properties is a bad idea. You should only
ever rely on whatever mechanism the operating system exposed as a result
of such properties instead.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150720/02d8fcc7/attachment-0001.sig>

[PULL] drm-amdkfd-next

[Oded Gabbay]

Hi Dave,

This is amdkfd's pull request for kernel 4.3.

drm-amdkfd-next-2015-07-20:

- Add Carrizo support for amdkfd, using the new amdgpu driver as the relevant
  kgd. The support includes interfaces with amdgpu both for gfx7 (Kaveri) and
  gfx8 (Carrizo). However, gfx7 interface is used for debugging purposes only,
  so amdkfd defaults to using radeon when Kaveri is installed.

I would like to note that no new IOCTLs are being introduced, and there is no 
change in the current IOCTLs, as they are suited both for gfx7 and gfx8.

Thanks,

Oded

The following changes since commit 52721d9d3334c1cb1f76219a161084094ec634dc:

  Linux 4.2-rc3 (2015-07-19 14:45:02 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~gabbayo/linux tags/drm-amdkfd-next-2015-07-20

for you to fetch changes up to 7639a8c420f04ca9be87974416efb2848b0962d9:

  drm/amdkfd: Set correct doorbell packet type for Carrizo (2015-07-20 09:16:49 
+0300)


Ben Goz (8):
  drm/amdgpu: Add amdgpu <--> amdkfd gfx8 interface
  drm/amdkfd: add supported CZ devices PCI IDs to amdkfd
  drm/amdkfd: add CP HWS packet headers for VI
  drm/amdkfd: add support for VI in MQD manager
  drm/amdkfd: Add support for VI in DQM
  drm/amdkfd: fix runlist length calculation
  drm/amdkfd: Implement create_map_queues() for Carrizo
  drm/amdkfd: Set correct doorbell packet type for Carrizo

Oded Gabbay (5):
  drm/radeon: Modify kgd_engine_type enum to match CZ
  drm/amdgpu: Add H/W agnostic amdgpu <--> amdkfd interface
  drm/amdgpu: add amdgpu <--> amdkfd gfx7 interface
  drm/amdkfd: Add dependency of DRM_AMDGPU to Kconfig
  drm/amdkfd: Use generic defines in new amd headers

 MAINTAINERS|   5 +
 drivers/gpu/drm/amd/amdgpu/Makefile|   6 +
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 267 
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h |  65 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c  | 670 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c  | 543 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|   5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c |   7 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|   7 +
 drivers/gpu/drm/amd/amdgpu/cik.c   |  11 +-
 drivers/gpu/drm/amd/amdgpu/cikd.h  |   6 +
 drivers/gpu/drm/amd/amdgpu/vid.h   |   5 +
 drivers/gpu/drm/amd/amdkfd/Kconfig |   2 +-
 drivers/gpu/drm/amd/amdkfd/Makefile|   3 +-
 drivers/gpu/drm/amd/amdkfd/cik_regs.h  |  11 -
 drivers/gpu/drm/amd/amdkfd/kfd_device.c|   7 +-
 .../drm/amd/amdkfd/kfd_device_queue_manager_cik.c  |  12 +-
 .../drm/amd/amdkfd/kfd_device_queue_manager_vi.c   | 103 +++-
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c   |  20 +-
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c| 249 +++-
 drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c|  99 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_pm4_headers_vi.h| 398 
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c  |   5 +
 drivers/gpu/drm/amd/amdkfd/kfd_topology.h  |   1 +
 drivers/gpu/drm/amd/include/kgd_kfd_interface.h|   3 +-
 drivers/gpu/drm/amd/include/vi_structs.h   | 417 +
 drivers/gpu/drm/radeon/radeon_kfd.c|   3 +-
 28 files changed, 2893 insertions(+), 40 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
 create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_pm4_headers_vi.h
 create mode 100644 drivers/gpu/drm/amd/include/vi_structs.h

[PATCH v2 2/4] drm/tegra: Add VIC support

[Jon Hunter]

On 20/07/15 09:51, Mikko Perttunen wrote:
> On 07/20/2015 11:28 AM, Jon Hunter wrote:
>> Hi Mikko,
> 
> Hi!
> 
>> ...
>>> +static int vic_runtime_resume(struct device *dev)
>>> +{
>>> +   struct vic *vic = dev_get_drvdata(dev);
>>> +   int err;
>>> +
>>> +   err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
>>> +   vic->clk, vic->rst);
>>
>> I would like to deprecate use of the above function [1]. I have been
>> trying to migrate driver to use a new API instead of the above.
> 
> Yes, we will need to coordinate the API change here. I kept the old way
> here to not depend on your series for now.
> 
>>
>>> +   if (err < 0)
>>> +   dev_err(dev, "failed to power up device\n");
>>> +
>>> +   return err;
>>> +}
>>> +
>>> ...
>>> +
>>> +   pm_runtime_enable(>dev);
>>> +   if (!pm_runtime_enabled(>dev)) {
>>> +   err = vic_runtime_resume(>dev);
>>> +   if (err < 0)
>>> +   goto unregister_client;
>>> +   }
>>
>> I don't see why pm_runtime_enable() should ever fail to enable
>> pm_runtime here. Hence, the if-statement appears to be redundant. If you
>> are trying to determine whether rpm is supported for the power-domains
>> then you should simply check to see if the DT node for the device has
>> the "power-domains" property. See my series [1].
> 
> The intention is that if runtime PM is not enabled, the power domain is
> still brought up. On a cursory look, it seems like with your patch, this
> should indeed work fine without this check if CONFIG_PM is enabled. Now,
> that might always be enabled? If so, then everything would be fine; if
> this lands after your series, we could even just make the power-domains
> prop mandatory and not implement the legacy mode at all. If not, then
> AFAIU we must still keep this path.

Ok, I see you just wish to test it is enabled in the kernel config. Then
yes that would make sense and the above is fine.

Cheers
Jon

[PATCH v2 4/4] ARM: tegra: Add VIC for Tegra124

[Mikko Perttunen]

From: Arto Merilainen 

This patch adds VIC device tree node for Video Image Compositor.

Signed-off-by: Arto Merilainen 
Signed-off-by: Mikko Perttunen 
---
 arch/arm/boot/dts/tegra124.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 87318a7..1233f4a 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -137,6 +137,18 @@
status = "disabled";
};

+   vic at 0,5434 {
+   compatible = "nvidia,tegra124-vic";
+   reg = <0x0 0x5434 0x0 0x0004>;
+   interrupts = ;
+   clocks = <_car TEGRA124_CLK_VIC03>;
+   clock-names = "vic";
+   resets = <_car TEGRA124_CLK_VIC03>;
+   reset-names = "vic";
+
+   iommus = < TEGRA_SWGROUP_VIC>;
+   };
+
sor at 0,5454 {
compatible = "nvidia,tegra124-sor";
reg = <0x0 0x5454 0x0 0x0004>;
-- 
2.1.4

[PATCH v2 3/4] of: Add NVIDIA Tegra VIC binding

[Mikko Perttunen]

This adds device tree binding documentation for the Video Image
Compositor (VIC) present on Tegra124 and newer SoC's.

Signed-off-by: Mikko Perttunen 
---
 .../devicetree/bindings/gpu/nvidia,tegra20-host1x.txt | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt 
b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
index 009f4bf..1328f3f 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
@@ -240,6 +240,21 @@ of the following host1x client modules:
 - dpaux
   - vdd-supply: phandle of a supply that powers the DisplayPort link

+- vic: Video Image Compositor
+  - compatible: For Tegra124, must contain "nvidia,tegra124-vic".  Otherwise,
+must contain '"nvidia,-vic", "nvidia,tegra124-vic"', where
+ is tegra132.
+  - reg: Physical base address and length of the controller's registers.
+  - interrupts: The interrupt outputs from the controller.
+  - clocks: Must contain an entry for each entry in clock-names.
+See ../clocks/clock-bindings.txt for details.
+  - clock-names: Must include the following entries:
+- vic: clock input for the VIC hardware
+  - resets: Must contain an entry for each entry in reset-names.
+See ../reset/reset.txt for details.
+  - reset-names: Must include the following entries:
+- vic
+
 Example:

 / {
-- 
2.1.4

[PATCH v2 2/4] drm/tegra: Add VIC support

[Mikko Perttunen]

From: Arto Merilainen 

This patch adds support for Video Image Compositor engine which
can be used for 2d operations.

Signed-off-by: Andrew Chew 
Signed-off-by: Arto Merilainen 
Signed-off-by: Mikko Perttunen 
---
 drivers/gpu/drm/tegra/Makefile |   3 +-
 drivers/gpu/drm/tegra/drm.c|   7 +
 drivers/gpu/drm/tegra/drm.h|   1 +
 drivers/gpu/drm/tegra/vic.c| 456 +
 drivers/gpu/drm/tegra/vic.h|  35 
 include/linux/host1x.h |   1 +
 6 files changed, 502 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tegra/vic.c
 create mode 100644 drivers/gpu/drm/tegra/vic.h

diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
index 93e9a4a..6af3a9a 100644
--- a/drivers/gpu/drm/tegra/Makefile
+++ b/drivers/gpu/drm/tegra/Makefile
@@ -14,6 +14,7 @@ tegra-drm-y := \
dpaux.o \
gr2d.o \
gr3d.o \
-   falcon.o
+   falcon.o \
+   vic.o

 obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index af4ff86..bc8cc2a 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1145,6 +1145,7 @@ static const struct of_device_id host1x_drm_subdevs[] = {
{ .compatible = "nvidia,tegra124-dc", },
{ .compatible = "nvidia,tegra124-sor", },
{ .compatible = "nvidia,tegra124-hdmi", },
+   { .compatible = "nvidia,tegra124-vic", },
{ /* sentinel */ }
 };

@@ -1194,8 +1195,14 @@ static int __init host1x_drm_init(void)
if (err < 0)
goto unregister_gr2d;

+   err = platform_driver_register(_vic_driver);
+   if (err < 0)
+   goto unregister_gr3d;
+
return 0;

+unregister_gr3d:
+   platform_driver_unregister(_gr3d_driver);
 unregister_gr2d:
platform_driver_unregister(_gr2d_driver);
 unregister_dpaux:
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 58c83b11..2fc7e42 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -284,5 +284,6 @@ extern struct platform_driver tegra_hdmi_driver;
 extern struct platform_driver tegra_dpaux_driver;
 extern struct platform_driver tegra_gr2d_driver;
 extern struct platform_driver tegra_gr3d_driver;
+extern struct platform_driver tegra_vic_driver;

 #endif /* HOST1X_DRM_H */
diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
new file mode 100644
index 000..fce7c04
--- /dev/null
+++ b/drivers/gpu/drm/tegra/vic.c
@@ -0,0 +1,456 @@
+/*
+ * Copyright (c) 2015, NVIDIA Corporation.
+ *
+ * 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "drm.h"
+#include "falcon.h"
+#include "vic.h"
+
+struct vic_config {
+   /* Firmware name */
+   const char *ucode_name;
+};
+
+struct vic {
+   struct falcon falcon;
+   bool booted;
+
+   void __iomem *regs;
+   struct tegra_drm_client client;
+   struct host1x_channel *channel;
+   struct iommu_domain *domain;
+   struct device *dev;
+   struct clk *clk;
+   struct reset_control *rst;
+
+   /* Platform configuration */
+   const struct vic_config *config;
+
+   /* for firewall - this determines if method 1 should be regarded
+* as an address register */
+   bool method_data_is_addr_reg;
+};
+
+static inline struct vic *to_vic(struct tegra_drm_client *client)
+{
+   return container_of(client, struct vic, client);
+}
+
+static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
+{
+   writel(value, vic->regs + offset);
+}
+
+static int vic_runtime_resume(struct device *dev)
+{
+   struct vic *vic = dev_get_drvdata(dev);
+   int err;
+
+   err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
+   vic->clk, vic->rst);
+   if (err < 0)
+   dev_err(dev, "failed to power up device\n");
+
+   return err;
+}
+
+static int vic_runtime_suspend(struct device *dev)
+{
+   struct vic *vic = dev_get_drvdata(dev);
+
+   clk_disable_unprepare(vic->clk);
+   reset_control_assert(vic->rst);
+   tegra_powergate_power_off(TEGRA_POWERGATE_VIC);
+
+   vic->booted = false;
+
+   return 0;
+}
+
+static int vic_boot(struct vic *vic)
+{
+   u32 fce_ucode_size, fce_bin_data_offset;
+   void *hdr;
+   int err = 0;
+
+   if (vic->booted)
+   return 0;
+
+   if (!vic->falcon.firmware.valid) {
+   err = falcon_read_firmware(>falcon,
+  vic->config->ucode_name);
+   if (err < 0)
+   return err;
+   }
+
+   /* ensure that 

[PATCH v2 1/4] drm/tegra: Add falcon helper library

[Mikko Perttunen]

From: Arto Merilainen 

Add a set of falcon helper routines for use by the tegradrm client drivers
of the various falcon-based engines.

The falcon is a microcontroller that acts as a frontend for the rest of a
particular Tegra engine.  In order to properly utilize these engines, the
frontend must be booted before pushing any commands.

Based on work by Andrew Chew 

Signed-off-by: Andrew Chew 
Signed-off-by: Arto Merilainen 
Signed-off-by: Mikko Perttunen 
---
 drivers/gpu/drm/tegra/Makefile |   3 +-
 drivers/gpu/drm/tegra/falcon.c | 256 +
 drivers/gpu/drm/tegra/falcon.h | 130 +
 3 files changed, 388 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tegra/falcon.c
 create mode 100644 drivers/gpu/drm/tegra/falcon.h

diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
index 2c66a8d..93e9a4a 100644
--- a/drivers/gpu/drm/tegra/Makefile
+++ b/drivers/gpu/drm/tegra/Makefile
@@ -13,6 +13,7 @@ tegra-drm-y := \
sor.o \
dpaux.o \
gr2d.o \
-   gr3d.o
+   gr3d.o \
+   falcon.o

 obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
diff --git a/drivers/gpu/drm/tegra/falcon.c b/drivers/gpu/drm/tegra/falcon.c
new file mode 100644
index 000..180b2fd
--- /dev/null
+++ b/drivers/gpu/drm/tegra/falcon.c
@@ -0,0 +1,256 @@
+/*
+ * Copyright (c) 2015, NVIDIA Corporation.
+ *
+ * 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "falcon.h"
+#include "drm.h"
+
+#define FALCON_IDLE_TIMEOUT_US 10
+#define FALCON_IDLE_CHECK_PERIOD_US10
+
+enum falcon_memory {
+   FALCON_MEMORY_IMEM,
+   FALCON_MEMORY_DATA,
+};
+
+static void falcon_writel(struct falcon *falcon, u32 value, u32 offset)
+{
+   writel(value, falcon->regs + offset);
+}
+
+int falcon_wait_idle(struct falcon *falcon)
+{
+   u32 idlestate;
+
+   return readl_poll_timeout(falcon->regs + FALCON_IDLESTATE, idlestate,
+ (!idlestate),
+ FALCON_IDLE_CHECK_PERIOD_US,
+ FALCON_IDLE_TIMEOUT_US);
+}
+
+static int falcon_dma_wait_idle(struct falcon *falcon)
+{
+   u32 dmatrfcmd;
+
+   return readl_poll_timeout(falcon->regs + FALCON_DMATRFCMD, dmatrfcmd,
+ (dmatrfcmd & FALCON_DMATRFCMD_IDLE),
+ FALCON_IDLE_CHECK_PERIOD_US,
+ FALCON_IDLE_TIMEOUT_US);
+}
+
+static int falcon_copy_chunk(struct falcon *falcon,
+phys_addr_t base,
+unsigned long offset,
+enum falcon_memory target)
+{
+   u32 cmd = FALCON_DMATRFCMD_SIZE_256B;
+
+   if (target == FALCON_MEMORY_IMEM)
+   cmd |= FALCON_DMATRFCMD_IMEM;
+
+   falcon_writel(falcon, offset, FALCON_DMATRFMOFFS);
+   falcon_writel(falcon, base, FALCON_DMATRFFBOFFS);
+   falcon_writel(falcon, cmd, FALCON_DMATRFCMD);
+
+   return falcon_dma_wait_idle(falcon);
+}
+
+static void falcon_copy_firmware_image(struct falcon *falcon,
+  const struct firmware *firmware)
+{
+   u32 *firmware_vaddr = falcon->firmware.vaddr;
+   size_t i;
+
+   /* copy the whole thing taking into account endianness */
+   for (i = 0; i < firmware->size / sizeof(u32); i++)
+   firmware_vaddr[i] = le32_to_cpu(((u32 *)firmware->data)[i]);
+
+   /* ensure that caches are flushed and falcon can see the firmware */
+   dma_sync_single_for_device(falcon->dev, virt_to_phys(firmware_vaddr),
+  falcon->firmware.size, DMA_TO_DEVICE);
+}
+
+static int falcon_parse_firmware_image(struct falcon *falcon)
+{
+   struct falcon_firmware_bin_header_v1 *bin_header =
+   (void *)falcon->firmware.vaddr;
+   struct falcon_firmware_os_header_v1 *os_header;
+
+   /* endian problems would show up right here */
+   if (bin_header->bin_magic != PCI_VENDOR_ID_NVIDIA) {
+   dev_err(falcon->dev, "failed to get firmware magic");
+   return -EINVAL;
+   }
+
+   /* currently only version 1 is supported */
+   if (bin_header->bin_ver != 1) {
+   dev_err(falcon->dev, "unsupported firmware version");
+   return -EINVAL;
+   }
+
+   /* check that the firmware size is consistent */
+   if (bin_header->bin_size > falcon->firmware.size) {
+   dev_err(falcon->dev, "firmware image size inconsistency");
+   return -EINVAL;
+   }
+
+   os_header = (falcon->firmware.vaddr +
+bin_header->os_bin_header_offset);
+
+   falcon->firmware.bin_data.size = bin_header->os_bin_size;

[PATCH v2 0/4] Add VIC support for Tegra124

[Mikko Perttunen]

>From Arto's original cover letter:

This series adds Video-Image-Compositor (VIC) support for Tegra124. The unit
replaced gr2d engine on T124 and it is effectively used for similar
operations: making simple surface copy and fill operations.

[..]

The series has been tested on Jetson TK1 by first disabling IOMMU (*),
enabling CMA and running a VIC clear test case that is posted to dri-devel
and linux-tegra mailing lists. The firmware image for VIC is publicly
available as part of Linux For Tegra driver package [0].

[0] https://developer.nvidia.com/linux-tegra

(*) Currently Tegra DRM does not support mapping the host1x command buffers
into kernel address space in case IOMMU is enabled.

End of original cover letter.

The aforementioned VIC clear test can be found at
https://patchwork.kernel.org/patch/6454821/.

This series is now composed of a commit that adds a general Falcon helper
library to be used by drivers whose hardware contains a Falcon, which
includes many Host1x clients; and then separately of the VIC driver that
uses that Falcon library. The fixes to host1x this series used to include
are in the 'Host1x/TegraDRM fixes/improvements' series I posted earlier;
that series is a dependency for this series.

Thanks,
Mikko.

Arto Merilainen (3):
  drm/tegra: Add falcon helper library
  drm/tegra: Add VIC support
  ARM: tegra: Add VIC for Tegra124

Mikko Perttunen (1):
  of: Add NVIDIA Tegra VIC binding

 .../bindings/gpu/nvidia,tegra20-host1x.txt |  15 +
 arch/arm/boot/dts/tegra124.dtsi|  12 +
 drivers/gpu/drm/tegra/Makefile |   4 +-
 drivers/gpu/drm/tegra/drm.c|   7 +
 drivers/gpu/drm/tegra/drm.h|   1 +
 drivers/gpu/drm/tegra/falcon.c | 256 
 drivers/gpu/drm/tegra/falcon.h | 130 ++
 drivers/gpu/drm/tegra/vic.c| 456 +
 drivers/gpu/drm/tegra/vic.h|  35 ++
 include/linux/host1x.h |   1 +
 10 files changed, 916 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tegra/falcon.c
 create mode 100644 drivers/gpu/drm/tegra/falcon.h
 create mode 100644 drivers/gpu/drm/tegra/vic.c
 create mode 100644 drivers/gpu/drm/tegra/vic.h

-- 
2.1.4

[PATCH 6/8] drm/bochs: phase 3: provide a custom ->atomic_commit implementation

[Pekka Paalanen]

On Sun, 19 Jul 2015 17:20:32 -0700
Stéphane Marchesin  wrote:

> On Thu, Jul 16, 2015 at 11:08 PM, Pekka Paalanen  
> wrote:
> >
> > On Thu, 16 Jul 2015 20:20:39 +0800
> > John Hunter  wrote:
> >
> > > From: Zhao Junwang 
> > >
> > > This supports the asynchronous commits, required for page-flipping
> > > Since it's virtual hw it's ok to commit async stuff right away, we
> > > never have to wait for vblank.
> >
> > Hi,
> >
> > in theory, yes. This is what a patch to bochs implemented not too long
> > ago, so AFAIK you are only replicating the existing behaviour.
> >
> > However, if userspace doing an async commit (or sync, I suppose) does
> > not incur any waits in the kernel in e.g. sending the page flip event,
> > then flip driven programs (e.g. a Wayland compositor, say, Weston)
> > will be running its rendering loop as a busy-loop, because the kernel
> > does not throttle it to the (virtual) display refresh rate.
> >
> > This will cause maximal CPU usage and poor user experience as
> > everything else needs to fight for CPU time and event dispatch to get
> > through, like input.
> >
> > I would hope someone could do a follow-up to implement a refresh cycle
> > emulation based on a clock. Userspace expects page flips to happen at
> > most at refresh rate when asking for vblank-synced flips. It's only
> > natural for userspace to drive its rendering loop based on the vblank
> > cycle.
> 
> 
> I've been asking myself the same question (for the UDL driver) and I'm
> not sure if this policy should go in the kernel. After all, there
> could be legitimate reasons for user space to render lots of frames
> per second. It seems to me that if user space doesn't want too many
> fps, it should just throttle itself.

If userspace wants to render lots of frames per second, IMO it should
not be using vblank-synced operations in a way that may throttle it.
The lots of frames use case is already non-working for the majority of
the drivers without DRM_MODE_PAGE_FLIP_ASYNC, right?

The problem here I see is that one DRM driver decides to work different
to other DRM drivers. All real-hardware DRM drivers, when asked to do
vblank-synced update, actually do throttle to the vblank AFAIK. Is it
too much to assume, that the video mode set in a driver (refresh rate)
corresponds to the vblank rate which implicitly delays the completion
of vblank-sync'd operations to at least the next vblank boundary?

I think, if the driver cannot implement proper semantics (which IMO
includes the throttling) for vblank-sync'd operations and it does not
want to fake them with a clock, it should just refuse vblank-synced
operations. That would push the problem to userspace, and it would be
obvious what's going wrong. Naturally, it would break some userspace
programs that expect vblank-synced operations to work, but is that
so much different to the current unfixed situation?


Thanks,
pq

KMS timings (Re: [PATCH 6/8] drm/bochs: phase 3: provide a custom ->atomic_commit implementation)

[Stéphane Marchesin]

On Mon, Jul 20, 2015 at 7:21 AM, Daniel Vetter  wrote:
> On Mon, Jul 20, 2015 at 12:35:48PM +0300, Pekka Paalanen wrote:
>> On Mon, 20 Jul 2015 01:58:33 -0700
>> Stéphane Marchesin  wrote:
>>
>> > On Mon, Jul 20, 2015 at 12:46 AM, Pekka Paalanen  
>> > wrote:
>> > > On Sun, 19 Jul 2015 17:20:32 -0700
>> > > Stéphane Marchesin  wrote:
>> > >
>> > >> On Thu, Jul 16, 2015 at 11:08 PM, Pekka Paalanen > > >> gmail.com> wrote:
>> > >> >
>> > >> > On Thu, 16 Jul 2015 20:20:39 +0800
>> > >> > John Hunter  wrote:
>> > >> >
>> > >> > > From: Zhao Junwang 
>> > >> > >
>> > >> > > This supports the asynchronous commits, required for page-flipping
>> > >> > > Since it's virtual hw it's ok to commit async stuff right away, we
>> > >> > > never have to wait for vblank.
>> > >> >
>> > >> > Hi,
>> > >> >
>> > >> > in theory, yes. This is what a patch to bochs implemented not too long
>> > >> > ago, so AFAIK you are only replicating the existing behaviour.
>> > >> >
>> > >> > However, if userspace doing an async commit (or sync, I suppose) does
>> > >> > not incur any waits in the kernel in e.g. sending the page flip event,
>> > >> > then flip driven programs (e.g. a Wayland compositor, say, Weston)
>> > >> > will be running its rendering loop as a busy-loop, because the kernel
>> > >> > does not throttle it to the (virtual) display refresh rate.
>> > >> >
>> > >> > This will cause maximal CPU usage and poor user experience as
>> > >> > everything else needs to fight for CPU time and event dispatch to get
>> > >> > through, like input.
>> > >> >
>> > >> > I would hope someone could do a follow-up to implement a refresh cycle
>> > >> > emulation based on a clock. Userspace expects page flips to happen at
>> > >> > most at refresh rate when asking for vblank-synced flips. It's only
>> > >> > natural for userspace to drive its rendering loop based on the vblank
>> > >> > cycle.
>> > >>
>> > >>
>> > >> I've been asking myself the same question (for the UDL driver) and I'm
>> > >> not sure if this policy should go in the kernel. After all, there
>> > >> could be legitimate reasons for user space to render lots of frames
>> > >> per second. It seems to me that if user space doesn't want too many
>> > >> fps, it should just throttle itself.
>> > >
>> > > If userspace wants to render lots of frames per second, IMO it should
>> > > not be using vblank-synced operations in a way that may throttle it.
>> > > The lots of frames use case is already non-working for the majority of
>> > > the drivers without DRM_MODE_PAGE_FLIP_ASYNC, right?
>> > >
>> > > The problem here I see is that one DRM driver decides to work different
>> > > to other DRM drivers. All real-hardware DRM drivers, when asked to do
>> > > vblank-synced update, actually do throttle to the vblank AFAIK.
>> >
>> > udl is an exception here. It is (arguably) real hardware but doesn't 
>> > throttle.
>> >
>> > > Is it
>> > > too much to assume, that the video mode set in a driver (refresh rate)
>> > > corresponds to the vblank rate which implicitly delays the completion
>> > > of vblank-sync'd operations to at least the next vblank boundary?
>> >
>> > I think it's wrong to make user space think that a vsynced display
>> > always matches the refresh rate in a world where:
>> >
>> > - some displays have variable refresh rates (not just the fancy new
>> > stuff like g-sync, look for lvds_downclock in the intel driver for
>> > example, also consider DSI displays)
>> >
>> > - some displays have no refresh rate (the ones we are talking about
>> > here: udl, bochs...)
>>
>> That means that refresh rate in a video mode is bogus. Can userspace
>> know when the refresh rate is meaningless? I suppose there are two
>> different cases of meaningless, too: when the driver ignores it as
>> input argument, and when it is used but has no guarantees for timings.
>>
>> Assuming it's always meaningless wrt. timings is pretty harsh. E.g. the
>> Wayland Presentation extension's implementation in Weston uses the
>> refresh rate to predict the next flip time and hands it out to clients
>> for scheduling/interpolation purposes.
>
> We removed lvds_downclock in i915 (was never enabled by default).

We use/ship it, so there is a value I would say.

> The new
> code is currently edp only, but enabled by default. And it tries _really_
> hard to keep up the illusion that the requested vrefresh is the one you
> actually have - it upclocks every time userspace changes the screen.

Except that there is latency in doing so (it can never be done
instantly), and user space definitely sees it, at least when it queues
the first flip. So the illusion doesn't exist.

>
> The only exception is when you only ask for vblank events, and the delay
> to the next pageflip is longer than the timeout. That can't be fixed right
> now because drm_irq.c is the last midlayer needed by modern drivers, but
> I'd like to fix it eventually.
>
> There's future plans to allow userspace to explicitly ask for a reduced
> vrefresh 

[PATCH v2 2/4] drm/tegra: Add VIC support

[Jon Hunter]

Hi Mikko,

On 20/07/15 08:54, Mikko Perttunen wrote:
> From: Arto Merilainen 
> 
> This patch adds support for Video Image Compositor engine which
> can be used for 2d operations.
> 
> Signed-off-by: Andrew Chew 
> Signed-off-by: Arto Merilainen 
> Signed-off-by: Mikko Perttunen 
> ---
>  drivers/gpu/drm/tegra/Makefile |   3 +-
>  drivers/gpu/drm/tegra/drm.c|   7 +
>  drivers/gpu/drm/tegra/drm.h|   1 +
>  drivers/gpu/drm/tegra/vic.c| 456 
> +
>  drivers/gpu/drm/tegra/vic.h|  35 
>  include/linux/host1x.h |   1 +
>  6 files changed, 502 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/tegra/vic.c
>  create mode 100644 drivers/gpu/drm/tegra/vic.h
> 
> diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
> index 93e9a4a..6af3a9a 100644
> --- a/drivers/gpu/drm/tegra/Makefile
> +++ b/drivers/gpu/drm/tegra/Makefile
> @@ -14,6 +14,7 @@ tegra-drm-y := \
>   dpaux.o \
>   gr2d.o \
>   gr3d.o \
> - falcon.o
> + falcon.o \
> + vic.o
>  
>  obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index af4ff86..bc8cc2a 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -1145,6 +1145,7 @@ static const struct of_device_id host1x_drm_subdevs[] = 
> {
>   { .compatible = "nvidia,tegra124-dc", },
>   { .compatible = "nvidia,tegra124-sor", },
>   { .compatible = "nvidia,tegra124-hdmi", },
> + { .compatible = "nvidia,tegra124-vic", },
>   { /* sentinel */ }
>  };
>  
> @@ -1194,8 +1195,14 @@ static int __init host1x_drm_init(void)
>   if (err < 0)
>   goto unregister_gr2d;
>  
> + err = platform_driver_register(_vic_driver);
> + if (err < 0)
> + goto unregister_gr3d;
> +
>   return 0;
>  
> +unregister_gr3d:
> + platform_driver_unregister(_gr3d_driver);
>  unregister_gr2d:
>   platform_driver_unregister(_gr2d_driver);
>  unregister_dpaux:
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 58c83b11..2fc7e42 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -284,5 +284,6 @@ extern struct platform_driver tegra_hdmi_driver;
>  extern struct platform_driver tegra_dpaux_driver;
>  extern struct platform_driver tegra_gr2d_driver;
>  extern struct platform_driver tegra_gr3d_driver;
> +extern struct platform_driver tegra_vic_driver;
>  
>  #endif /* HOST1X_DRM_H */
> diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> new file mode 100644
> index 000..fce7c04
> --- /dev/null
> +++ b/drivers/gpu/drm/tegra/vic.c
> @@ -0,0 +1,456 @@
> +/*
> + * Copyright (c) 2015, NVIDIA Corporation.
> + *
> + * 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.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include "drm.h"
> +#include "falcon.h"
> +#include "vic.h"
> +
> +struct vic_config {
> + /* Firmware name */
> + const char *ucode_name;
> +};
> +
> +struct vic {
> + struct falcon falcon;
> + bool booted;
> +
> + void __iomem *regs;
> + struct tegra_drm_client client;
> + struct host1x_channel *channel;
> + struct iommu_domain *domain;
> + struct device *dev;
> + struct clk *clk;
> + struct reset_control *rst;
> +
> + /* Platform configuration */
> + const struct vic_config *config;
> +
> + /* for firewall - this determines if method 1 should be regarded
> +  * as an address register */
> + bool method_data_is_addr_reg;
> +};
> +
> +static inline struct vic *to_vic(struct tegra_drm_client *client)
> +{
> + return container_of(client, struct vic, client);
> +}
> +
> +static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
> +{
> + writel(value, vic->regs + offset);
> +}
> +
> +static int vic_runtime_resume(struct device *dev)
> +{
> + struct vic *vic = dev_get_drvdata(dev);
> + int err;
> +
> + err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
> + vic->clk, vic->rst);

I would like to deprecate use of the above function [1]. I have been
trying to migrate driver to use a new API instead of the above.

> + if (err < 0)
> + dev_err(dev, "failed to power up device\n");
> +
> + return err;
> +}
> +
> +static int vic_runtime_suspend(struct device *dev)
> +{
> + struct vic *vic = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(vic->clk);
> + reset_control_assert(vic->rst);
> + tegra_powergate_power_off(TEGRA_POWERGATE_VIC);

Similarly here. I would like to get rid of the above API in favour of a
different API to help migrate to generic 

[PATCH v10 1/5] drm/layerscape: Add Freescale DCU DRM driver

[Alexander Stein]

On Friday 17 July 2015 18:38:59, Jianwei Wang wrote:
> [...]
> +static const struct regmap_config fsl_dcu_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> +};

This defaults to REGCACHE_NONE which in the end sets regmap.cache_only = true.

> [...]
> +#ifdef CONFIG_PM_SLEEP
> +static int fsl_dcu_drm_pm_suspend(struct device *dev)
> +{
> + struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
> +
> + if (!fsl_dev)
> + return 0;
> +
> + drm_kms_helper_poll_disable(fsl_dev->drm);
> + regcache_cache_only(fsl_dev->regmap, true);

This should raise a warning (see 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/base/regmap/regcache.c#n472)
 as map->cache_bypass is set because of REGCACHE_NONE.
I think you set the cache_type to REGCACHE_FLAT, but neither _LZO or _RBTREE 
(see https://lkml.org/lkml/2015/7/16/552 for that)

Best regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein
SYS TEC electronic GmbH
alexander.stein at systec-electronic.com

Legal and Commercial Address:
Am Windrad 2
08468 Heinsdorfergrund
Germany

Office: +49 (0) 3765 38600-11xx
Fax:+49 (0) 0) 3765 38600-41xx

Managing Directors:
Director Technology/CEO: Dipl.-Phys. Siegmar Schmidt;
Director Commercial Affairs/COO: Dipl. Ing. (FH) Armin von Collrepp
Commercial Registry:
Amtsgericht Chemnitz, HRB 28082; USt.-Id Nr. DE150534010

[Bug 61529] [r600g][kms][ATI RV710] r600_pcie_gart_tlb_flush+0xf5/0x110

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

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

--- Comment #12 from Thaddaeus Tintenfisch  ---
Is anything missing? Does the patch need more testing?

-- 
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/20150720/b274734a/attachment.html>

[PATCH] drm/tegra: dpaux: Fix transfers larger than 4 bytes

[Thierry Reding]

From: Thierry Reding 

The DPAUX read/write FIFO registers aren't sequential in the register
space, causing transfers larger than 4 bytes to cause accesses to non-
existing FIFO registers.

Fixes: 6b6b604215c6 ("drm/tegra: Add eDP support")
Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/dpaux.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index d6b55e3e3716..a43a836e6f88 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -72,34 +72,32 @@ static inline void tegra_dpaux_writel(struct tegra_dpaux 
*dpaux,
 static void tegra_dpaux_write_fifo(struct tegra_dpaux *dpaux, const u8 *buffer,
   size_t size)
 {
-   unsigned long offset = DPAUX_DP_AUXDATA_WRITE(0);
size_t i, j;

-   for (i = 0; i < size; i += 4) {
-   size_t num = min_t(size_t, size - i, 4);
+   for (i = 0; i < DIV_ROUND_UP(size, 4); i++) {
+   size_t num = min_t(size_t, size - i * 4, 4);
unsigned long value = 0;

for (j = 0; j < num; j++)
-   value |= buffer[i + j] << (j * 8);
+   value |= buffer[i * 4 + j] << (j * 8);

-   tegra_dpaux_writel(dpaux, value, offset++);
+   tegra_dpaux_writel(dpaux, value, DPAUX_DP_AUXDATA_WRITE(i));
}
 }

 static void tegra_dpaux_read_fifo(struct tegra_dpaux *dpaux, u8 *buffer,
  size_t size)
 {
-   unsigned long offset = DPAUX_DP_AUXDATA_READ(0);
size_t i, j;

-   for (i = 0; i < size; i += 4) {
-   size_t num = min_t(size_t, size - i, 4);
+   for (i = 0; i < DIV_ROUND_UP(size, 4); i++) {
+   size_t num = min_t(size_t, size - i * 4, 4);
unsigned long value;

-   value = tegra_dpaux_readl(dpaux, offset++);
+   value = tegra_dpaux_readl(dpaux, DPAUX_DP_AUXDATA_READ(i));

for (j = 0; j < num; j++)
-   buffer[i + j] = value >> (j * 8);
+   buffer[i * 4 + j] = value >> (j * 8);
}
 }

-- 
2.4.5

[PATCH 6/8] drm/bochs: phase 3: provide a custom ->atomic_commit implementation

[John Hunter]

Hi Pekka,

Thanks for the information, I will talk to my mentor Daniel and try to find
out
what I can do about this.

Cheers,
Zhao Junwang

On Fri, Jul 17, 2015 at 2:08 PM, Pekka Paalanen  wrote:

> On Thu, 16 Jul 2015 20:20:39 +0800
> John Hunter  wrote:
>
> > From: Zhao Junwang 
> >
> > This supports the asynchronous commits, required for page-flipping
> > Since it's virtual hw it's ok to commit async stuff right away, we
> > never have to wait for vblank.
>
> Hi,
>
> in theory, yes. This is what a patch to bochs implemented not too long
> ago, so AFAIK you are only replicating the existing behaviour.
>
> However, if userspace doing an async commit (or sync, I suppose) does
> not incur any waits in the kernel in e.g. sending the page flip event,
> then flip driven programs (e.g. a Wayland compositor, say, Weston)
> will be running its rendering loop as a busy-loop, because the kernel
> does not throttle it to the (virtual) display refresh rate.
>
> This will cause maximal CPU usage and poor user experience as
> everything else needs to fight for CPU time and event dispatch to get
> through, like input.
>
> I would hope someone could do a follow-up to implement a refresh cycle
> emulation based on a clock. Userspace expects page flips to happen at
> most at refresh rate when asking for vblank-synced flips. It's only
> natural for userspace to drive its rendering loop based on the vblank
> cycle.
>
>
> Thanks,
> pq
>
> > Cc: Maarten Lankhorst 
> > Cc: Daniel Vetter 
> > Signed-off-by: Zhao Junwang 
> > ---
> >  drivers/gpu/drm/bochs/bochs_mm.c |9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bochs/bochs_mm.c
> b/drivers/gpu/drm/bochs/bochs_mm.c
> > index c1d819c..37ac2ca 100644
> > --- a/drivers/gpu/drm/bochs/bochs_mm.c
> > +++ b/drivers/gpu/drm/bochs/bochs_mm.c
> > @@ -545,8 +545,15 @@ bochs_user_framebuffer_create(struct drm_device
> *dev,
> >   return _fb->base;
> >  }
> >
> > +static int bochs_atomic_commit(struct drm_device *dev,
> > +  struct drm_atomic_state *a,
> > +  bool async)
> > +{
> > + return drm_atomic_helper_commit(dev, a, false);
> > +}
> > +
> >  const struct drm_mode_config_funcs bochs_mode_funcs = {
> >   .fb_create = bochs_user_framebuffer_create,
> >   .atomic_check = drm_atomic_helper_check,
> > - .atomic_commit = drm_atomic_helper_commit,
> > + .atomic_commit = bochs_atomic_commit,
> >  };
>
>


-- 
Best regards
Junwang Zhao
Microprocessor Research and Develop Center
Department of Computer Science 
Peking University
Beijing, 100871, PRC
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150720/214528ef/attachment.html>

[PATCH] drm/tegra: dpaux: Fix transfers larger than 4 bytes

[Steev Klimaszewski]

On Mon, Jul 20, 2015 at 1:49 AM, Thierry Reding 
wrote:

> From: Thierry Reding 
>
> The DPAUX read/write FIFO registers aren't sequential in the register
> space, causing transfers larger than 4 bytes to cause accesses to non-
> existing FIFO registers.
>
> Fixes: 6b6b604215c6 ("drm/tegra: Add eDP support")
> Signed-off-by: Thierry Reding 
> ---
>  drivers/gpu/drm/tegra/dpaux.c | 18 --
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> index d6b55e3e3716..a43a836e6f88 100644
> --- a/drivers/gpu/drm/tegra/dpaux.c
> +++ b/drivers/gpu/drm/tegra/dpaux.c
> @@ -72,34 +72,32 @@ static inline void tegra_dpaux_writel(struct
> tegra_dpaux *dpaux,
>  static void tegra_dpaux_write_fifo(struct tegra_dpaux *dpaux, const u8
> *buffer,
>size_t size)
>  {
> -   unsigned long offset = DPAUX_DP_AUXDATA_WRITE(0);
> size_t i, j;
>
> -   for (i = 0; i < size; i += 4) {
> -   size_t num = min_t(size_t, size - i, 4);
> +   for (i = 0; i < DIV_ROUND_UP(size, 4); i++) {
> +   size_t num = min_t(size_t, size - i * 4, 4);
> unsigned long value = 0;
>
> for (j = 0; j < num; j++)
> -   value |= buffer[i + j] << (j * 8);
> +   value |= buffer[i * 4 + j] << (j * 8);
>
> -   tegra_dpaux_writel(dpaux, value, offset++);
> +   tegra_dpaux_writel(dpaux, value,
> DPAUX_DP_AUXDATA_WRITE(i));
> }
>  }
>
>  static void tegra_dpaux_read_fifo(struct tegra_dpaux *dpaux, u8 *buffer,
>   size_t size)
>  {
> -   unsigned long offset = DPAUX_DP_AUXDATA_READ(0);
> size_t i, j;
>
> -   for (i = 0; i < size; i += 4) {
> -   size_t num = min_t(size_t, size - i, 4);
> +   for (i = 0; i < DIV_ROUND_UP(size, 4); i++) {
> +   size_t num = min_t(size_t, size - i * 4, 4);
> unsigned long value;
>
> -   value = tegra_dpaux_readl(dpaux, offset++);
> +   value = tegra_dpaux_readl(dpaux, DPAUX_DP_AUXDATA_READ(i));
>
> for (j = 0; j < num; j++)
> -   buffer[i + j] = value >> (j * 8);
> +   buffer[i * 4 + j] = value >> (j * 8);
> }
>  }
>
> --
> 2.4.5
>
>

This fixes the issue that I reported earlier, so feel free to add my

Tested-by: Steev Klimaszewski 
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150720/be4f8a7b/attachment-0001.html>

[PATCH 6/8] drm/bochs: phase 3: provide a custom ->atomic_commit implementation

[Stéphane Marchesin]

On Mon, Jul 20, 2015 at 12:46 AM, Pekka Paalanen  wrote:
> On Sun, 19 Jul 2015 17:20:32 -0700
> Stéphane Marchesin  wrote:
>
>> On Thu, Jul 16, 2015 at 11:08 PM, Pekka Paalanen  
>> wrote:
>> >
>> > On Thu, 16 Jul 2015 20:20:39 +0800
>> > John Hunter  wrote:
>> >
>> > > From: Zhao Junwang 
>> > >
>> > > This supports the asynchronous commits, required for page-flipping
>> > > Since it's virtual hw it's ok to commit async stuff right away, we
>> > > never have to wait for vblank.
>> >
>> > Hi,
>> >
>> > in theory, yes. This is what a patch to bochs implemented not too long
>> > ago, so AFAIK you are only replicating the existing behaviour.
>> >
>> > However, if userspace doing an async commit (or sync, I suppose) does
>> > not incur any waits in the kernel in e.g. sending the page flip event,
>> > then flip driven programs (e.g. a Wayland compositor, say, Weston)
>> > will be running its rendering loop as a busy-loop, because the kernel
>> > does not throttle it to the (virtual) display refresh rate.
>> >
>> > This will cause maximal CPU usage and poor user experience as
>> > everything else needs to fight for CPU time and event dispatch to get
>> > through, like input.
>> >
>> > I would hope someone could do a follow-up to implement a refresh cycle
>> > emulation based on a clock. Userspace expects page flips to happen at
>> > most at refresh rate when asking for vblank-synced flips. It's only
>> > natural for userspace to drive its rendering loop based on the vblank
>> > cycle.
>>
>>
>> I've been asking myself the same question (for the UDL driver) and I'm
>> not sure if this policy should go in the kernel. After all, there
>> could be legitimate reasons for user space to render lots of frames
>> per second. It seems to me that if user space doesn't want too many
>> fps, it should just throttle itself.
>
> If userspace wants to render lots of frames per second, IMO it should
> not be using vblank-synced operations in a way that may throttle it.
> The lots of frames use case is already non-working for the majority of
> the drivers without DRM_MODE_PAGE_FLIP_ASYNC, right?
>
> The problem here I see is that one DRM driver decides to work different
> to other DRM drivers. All real-hardware DRM drivers, when asked to do
> vblank-synced update, actually do throttle to the vblank AFAIK.

udl is an exception here. It is (arguably) real hardware but doesn't throttle.

> Is it
> too much to assume, that the video mode set in a driver (refresh rate)
> corresponds to the vblank rate which implicitly delays the completion
> of vblank-sync'd operations to at least the next vblank boundary?

I think it's wrong to make user space think that a vsynced display
always matches the refresh rate in a world where:

- some displays have variable refresh rates (not just the fancy new
stuff like g-sync, look for lvds_downclock in the intel driver for
example, also consider DSI displays)

- some displays have no refresh rate (the ones we are talking about
here: udl, bochs...)

- you can do partial vsynced updates by just waiting for a specific
scanline range which again breaks the assumption that "vsynced" ==
"refreshes at the monitor rate". In this case there is no visible
tearing (in that sense it is vsynced) but the flip time is not
predictable using the refresh rate.

So I don't think we should perpetuate that problem. And I would like
user space to "see" the actual flip times to enable some kind of
scheduling where possible.

>
> I think, if the driver cannot implement proper semantics (which IMO
> includes the throttling) for vblank-sync'd operations and it does not
> want to fake them with a clock, it should just refuse vblank-synced
> operations.

Yes refusing vsynced flips for these drivers sounds reasonable. But
please let's not bake in another assumption in the API (or rather,
let's try to un-bake it).

Stéphane


> That would push the problem to userspace, and it would be
> obvious what's going wrong. Naturally, it would break some userspace
> programs that expect vblank-synced operations to work, but is that
> so much different to the current unfixed situation?
>
>
> Thanks,
> pq