[Bug 104064] (DC 4.15-rc2) WARNING: CPU: 4 PID: 75 at drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:601 dm_suspend+0x4e/0x60 [amdgpu]

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=104064

--- Comment #50 from Bjorn  ---
That did not help. However, looking closer at my dmesg output on the 4.17-wip
branch, I see that there's actually some error loading the intel graphics
firmware. In particular, there are these lines:

[1.806186] i915 :00:02.0: Direct firmware load for
i915/kbl_dmc_ver1_04.bin failed with error -2
[1.806188] i915 :00:02.0: Failed to load DMC firmware
i915/kbl_dmc_ver1_04.bin. Disabling runtime power management.

This definitely seems to imply the intel card is having some power management
issue. So perhaps my problem was never with the AMD GPU at all...

In any case, the original amdgpu crash is fixed for me too, whether it impacted
my suspend/resume or not.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[radeon-alex:drm-next-4.17-wip 31/42] drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:1847:8: warning: missing braces around initializer

2018-03-27 Thread kbuild test robot
tree:   git://people.freedesktop.org/~agd5f/linux.git drm-next-4.17-wip
head:   576e538e5fe6ac103cde6b269c6210985b026689
commit: 3c6dcef49f42b8843a589a73fa4d968cbe3bb689 [31/42] drm/amd/display: 
Rename encoder_info_packet to dc_info_packet
config: x86_64-randconfig-g0-03280443 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
git checkout 3c6dcef49f42b8843a589a73fa4d968cbe3bb689
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c: In function 
'set_avi_info_frame':
>> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:1847:8: warning: 
>> missing braces around initializer [-Wmissing-braces]
 union hdmi_info_packet hdmi_info = {0};
   ^
   drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:1847:8: warning: 
(near initialization for 'hdmi_info.bits') [-Wmissing-braces]
   Cyclomatic Complexity 3 include/linux/string.h:memset
   Cyclomatic Complexity 4 include/linux/string.h:memcmp
   Cyclomatic Complexity 1 include/linux/math64.h:div64_s64
   Cyclomatic Complexity 1 
drivers/gpu/drm/amd/amdgpu/../display/include/signal_types.h:dc_is_hdmi_signal
   Cyclomatic Complexity 3 
drivers/gpu/drm/amd/amdgpu/../display/include/signal_types.h:dc_is_dp_signal
   Cyclomatic Complexity 1 
drivers/gpu/drm/amd/amdgpu/../display/include/signal_types.h:dc_is_embedded_signal
   Cyclomatic Complexity 2 
drivers/gpu/drm/amd/amdgpu/../display/include/signal_types.h:dc_is_dvi_signal
   Cyclomatic Complexity 3 
drivers/gpu/drm/amd/amdgpu/../display/include/signal_types.h:dc_is_audio_capable_signal
   Cyclomatic Complexity 12 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:convert_pixel_format_to_dalsurface
   Cyclomatic Complexity 1 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:rect_swap_helper
   Cyclomatic Complexity 15 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:calculate_viewport
   Cyclomatic Complexity 15 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:calculate_recout
   Cyclomatic Complexity 5 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:acquire_first_split_pipe
   Cyclomatic Complexity 4 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:is_timing_changed
   Cyclomatic Complexity 4 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:are_stream_backends_same
   Cyclomatic Complexity 3 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:update_stream_engine_usage
   Cyclomatic Complexity 4 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:acquire_first_free_pipe
   Cyclomatic Complexity 7 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:find_first_free_match_stream_enc_for_link
   Cyclomatic Complexity 7 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:find_first_free_audio
   Cyclomatic Complexity 3 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:patch_gamut_packet_checksum
   Cyclomatic Complexity 31 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:set_avi_info_frame
   Cyclomatic Complexity 13 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:set_vendor_info_packet
   Cyclomatic Complexity 9 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:set_spd_info_packet
   Cyclomatic Complexity 8 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:set_hdr_static_info_packet
   Cyclomatic Complexity 5 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:set_vsc_info_packet
   Cyclomatic Complexity 8 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:get_norm_pix_clk
   Cyclomatic Complexity 2 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:calculate_phy_pix_clks
   Cyclomatic Complexity 6 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:update_num_audio
   Cyclomatic Complexity 5 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:calculate_scaling_ratios
   Cyclomatic Complexity 3 
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:dal_fixed31_32_from_int
   Cyclomatic Complexity 1 
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:dal_fixed31_32_div_int
   Cyclomatic Complexity 1 
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:dal_fixed31_32_add_int
   Cyclomatic Complexity 1 
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:dal_fixed31_32_mul_int
   Cyclomatic Complexity 32 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:calculate_inits_and_adj_vp
   Cyclomatic Complexity 10 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:resource_parse_asic_id
   Cyclomatic Complexity 12 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:dc_create_resource_pool
   Cyclomatic Complexity 3 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:dc_destroy_resource_pool
   Cyclomatic Complexity 16 

[PATCH 4/4] drm: drm_dev_set_unique private, again

2018-03-27 Thread Emil Velikov
From: Emil Velikov 

As of last commit we hide this from the drivers.

Effectively reverts commit a6b5fac59cb216ac906f02300d3630c24520d9ef.

Cc: Daniel Vetter 
Signed-off-by: Emil Velikov 
---
 drivers/gpu/drm/drm_drv.c | 38 ++
 include/drm/drm_drv.h |  3 ---
 2 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 88da984ff9eb..26e360a0e50c 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -266,10 +266,9 @@ void drm_minor_release(struct drm_minor *minor)
  * callbacks implemented by the driver. The driver then needs to initialize all
  * the various subsystems for the drm device like memory management, vblank
  * handling, modesetting support and intial output configuration plus obviously
- * initialize all the corresponding hardware bits. An important part of this is
- * also calling drm_dev_set_unique() to set the userspace-visible unique name 
of
- * this device instance. Finally when everything is up and running and ready 
for
- * userspace the device instance can be published using drm_dev_register().
+ * initialize all the corresponding hardware bits. Finally when everything is 
up
+ * and running and ready for userspace the device instance can be published
+ * using drm_dev_register().
  *
  * There is also deprecated support for initalizing device instances using
  * bus-specific helpers and the _driver.load callback. But due to
@@ -290,6 +289,17 @@ void drm_minor_release(struct drm_minor *minor)
  * structure, which is supported through drm_dev_init().
  */
 
+static int drm_dev_set_unique(struct drm_device *dev, const char *name)
+{
+   if (!name)
+   return -EINVAL;
+
+   kfree(dev->unique);
+   dev->unique = kstrdup(name, GFP_KERNEL);
+
+   return dev->unique ? 0 : -ENOMEM;
+}
+
 /**
  * drm_put_dev - Unregister and release a DRM device
  * @dev: DRM device
@@ -840,26 +850,6 @@ void drm_dev_unregister(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_dev_unregister);
 
-/**
- * drm_dev_set_unique - Set the unique name of a DRM device
- * @dev: device of which to set the unique name
- * @name: unique name
- *
- * Sets the unique name of a DRM device using the specified string. Drivers
- * can use this at driver probe time if the unique name of the devices they
- * drive is static.
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int drm_dev_set_unique(struct drm_device *dev, const char *name)
-{
-   kfree(dev->unique);
-   dev->unique = kstrdup(name, GFP_KERNEL);
-
-   return dev->unique ? 0 : -ENOMEM;
-}
-EXPORT_SYMBOL(drm_dev_set_unique);
-
 /*
  * DRM Core
  * The DRM core module initializes all global DRM objects and makes them
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index d32b688eb346..3e6671fc9ced 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -642,7 +642,4 @@ static inline int drm_dev_is_unplugged(struct drm_device 
*dev)
 }
 
 
-int drm_dev_set_unique(struct drm_device *dev, const char *name);
-
-
 #endif
-- 
2.16.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 2/4] drm: BUG_ON if passing NULL parent to drm_dev_init

2018-03-27 Thread Emil Velikov
From: Emil Velikov 

Previous commit removed the only reason why we were allowing NULL as
a parent device. With that resolved, we can enforce nobody else does
that mistake.

With that we can drop the ugly drm_dev_set_unique workaround.

Cc: Daniel Vetter 
Cc: Deepak Sharma 
Signed-off-by: Emil Velikov 
---
 drivers/gpu/drm/drm_drv.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index a1b9338736e3..88da984ff9eb 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -432,8 +432,6 @@ static void drm_fs_inode_free(struct inode *inode)
  * The initial ref-count of the object is 1. Use drm_dev_get() and
  * drm_dev_put() to take and drop further ref-counts.
  *
- * Note that for purely virtual devices @parent can be NULL.
- *
  * Drivers that do not want to allocate their own device struct
  * embedding  drm_device can call drm_dev_alloc() instead. For drivers
  * that do embed  drm_device it must be placed first in the overall
@@ -458,6 +456,8 @@ int drm_dev_init(struct drm_device *dev,
return -ENODEV;
}
 
+   BUG_ON(parent == NULL);
+
kref_init(>ref);
dev->dev = parent;
dev->driver = driver;
@@ -506,9 +506,7 @@ int drm_dev_init(struct drm_device *dev,
}
}
 
-   /* Use the parent device name as DRM device unique identifier, but fall
-* back to the driver name for virtual devices like vgem. */
-   ret = drm_dev_set_unique(dev, parent ? dev_name(parent) : driver->name);
+   ret = drm_dev_set_unique(dev, dev_name(parent));
if (ret)
goto err_setunique;
 
-- 
2.16.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/4] drm/vgem: Fix vgem_init to get drm device avaliable.

2018-03-27 Thread Emil Velikov
From: Deepak Sharma 

Modify vgem_init to take platform dev as parent in drm_dev_init.
This will make drm device available at "/sys/devices/platform/vgem"
in x86 chromebook.

Cc: Daniel Vetter 
Signed-off-by: Deepak Sharma 
Reviewed-by: Sean Paul 
Signed-off-by: Emil Velikov 
---
 drivers/gpu/drm/vgem/vgem_drv.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 2524ff116f00..636ce32fa945 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -472,31 +472,30 @@ static int __init vgem_init(void)
if (!vgem_device)
return -ENOMEM;
 
-   ret = drm_dev_init(_device->drm, _driver, NULL);
-   if (ret)
-   goto out_free;
-
vgem_device->platform =
platform_device_register_simple("vgem", -1, NULL, 0);
if (IS_ERR(vgem_device->platform)) {
ret = PTR_ERR(vgem_device->platform);
-   goto out_fini;
+   goto out_free;
}
 
dma_coerce_mask_and_coherent(_device->platform->dev,
 DMA_BIT_MASK(64));
+   ret = drm_dev_init(_device->drm, _driver, 
_device->platform->dev);
+   if (ret)
+   goto out_unregister;
 
/* Final step: expose the device/driver to userspace */
ret  = drm_dev_register(_device->drm, 0);
if (ret)
-   goto out_unregister;
+   goto out_fini;
 
return 0;
 
-out_unregister:
-   platform_device_unregister(vgem_device->platform);
 out_fini:
drm_dev_fini(_device->drm);
+out_unregister:
+   platform_device_unregister(vgem_device->platform);
 out_free:
kfree(vgem_device);
return ret;
-- 
2.16.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 3/4] drm/virtio: remove drm_dev_set_unique workaround

2018-03-27 Thread Emil Velikov
From: Emil Velikov 

Ealier commit a325725633c26aa66ab940f762a6b0778edf76c0 did not attribute
that virtio can be either PCI or a platform device and removed the
.set_busid hook. Whereas only the "platform" instance should have been
removed.

Since then, two things have happened:
 - the driver manually calls drm_dev_set_unique, addressing the Xserver
regression - see commit 9785b4321b0bd701f8d21d3d3c676a7739a5cf22
 - core itself calls drm_pci_set_busid, on drm_set_busid IOCTL setting
the busid, so we don't need to fallback to dev->unique - see commit
5c484cee7ef9c4fd29fa0ba09640d55960977145

With that in place we can remove the local workaround.

Cc: Daniel Vetter 
Cc: Gerd Hoffmann 
Cc: Hans de Goede 
Cc: Laszlo Ersek 
Signed-off-by: Emil Velikov 
---
Gents, can someone double-check this please? Just in case.
---
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c 
b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
index 7df8d0c9026a..19fd7c661b8a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
@@ -62,7 +62,6 @@ int drm_virtio_init(struct drm_driver *driver, struct 
virtio_device *vdev)
struct pci_dev *pdev = to_pci_dev(vdev->dev.parent);
const char *pname = dev_name(>dev);
bool vga = (pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA;
-   char unique[20];
 
DRM_INFO("pci: %s detected at %s\n",
 vga ? "virtio-vga" : "virtio-gpu-pci",
@@ -70,12 +69,6 @@ int drm_virtio_init(struct drm_driver *driver, struct 
virtio_device *vdev)
dev->pdev = pdev;
if (vga)
virtio_pci_kick_out_firmware_fb(pdev);
-
-   snprintf(unique, sizeof(unique), "pci:%s", pname);
-   ret = drm_dev_set_unique(dev, unique);
-   if (ret)
-   goto err_free;
-
}
 
ret = drm_dev_register(dev, 0);
-- 
2.16.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105515] hw_init of IP block failed

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105515

Edward Kigwana  changed:

   What|Removed |Added

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

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105515] hw_init of IP block failed

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105515

--- Comment #11 from Edward Kigwana  ---
Created attachment 138387
  --> https://bugs.freedesktop.org/attachment.cgi?id=138387=edit
dmesg

Shows successful load of amdgpu driver for polaris 12 card.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105515] hw_init of IP block failed

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105515

--- Comment #10 from Edward Kigwana  ---
Not sure what commit fixed it for me but with amd-staging-drm-next
525b7b1e13c3214f04c9ab4d72c88f55a7bd4288 I no longer have this issue.


[2.285918] [drm] initializing kernel modesetting (POLARIS12 0x1002:0x699F
0x1462:0x8A90 0xC7).
[2.285925] [drm] register mmio base: 0xDF30
[2.285925] [drm] register mmio size: 262144
[2.285929] [drm] probing gen 2 caps for device 8086:1901 = 261ad03/e
[2.285930] [drm] probing mlw for device 8086:1901 = 261ad03
[2.285931] [drm] add ip block number 0 
[2.285932] [drm] add ip block number 1 
[2.285933] [drm] add ip block number 2 
[2.285933] [drm] add ip block number 3 
[2.285934] [drm] add ip block number 4 
[2.285935] [drm] add ip block number 5 
[2.285935] [drm] add ip block number 6 
[2.285936] [drm] add ip block number 7 
[2.285936] [drm] add ip block number 8 
[2.285942] [drm] UVD is enabled in VM mode
[2.285943] [drm] UVD ENC is enabled in VM mode
[2.285944] [drm] VCE enabled in VM mode
...

As before enabling dpm results in an instant lockup that I can't even begin to
debug since the kernel locks up and I don't have a serial console.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105425] 3D & games produce periodic GPU crashes (Radeon R7 370)

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105425

--- Comment #8 from MirceaKitsune  ---
Testing is still heavily undergoing. There's still nothing conclusive yet, but
I should definitely share a piece of information early on.

To my surprise, it would appear the culprit may be either Anti-Aliasing or
Anisotropic Filtering. I decided to re-enable their cvars first in Xonotic
since I honestly suspected them the least... the moment I did that all hell
broke lose again: In 30 minutes I had two system lockups! Then I disabled them
once more, and could play a 40 minute match with no problem.

I have no idea which of the two it could be, but I should be getting there in
the following days. I'm slowly re-enabling the other cvars first to rule them
out, then I'll see whether AA or texture filtering is behind the crashes.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 198885] amdgpu.dc=1 on CIK (R4 Mullins APU) brightness impossible to change. by Fn or "echo".

2018-03-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=198885

Przemek (sop...@gmail.com) changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |CODE_FIX

--- Comment #7 from Przemek (sop...@gmail.com) ---
Sorry for the delay but I had to git clone whole agd5f branch repo once again
since I was moving my installation to another drive.

Moreover I had kernel panic with PSP platform module turned on
(CONFIG_CRYPTO_DEV_SP_PSP), but it is not the case of this bug report. (BTW as
from the specs this apu has one included).

Brightness keys are working as expected,
/sys/class/backlight/amdgpu_bl0/brightness is now editable by "echo", Display
Core is initialized with v3.1.40, so I am closing this bug report as
solved/code fix.

Thank you very much for fast and accurate action on this. Done to a turn.

Thanks once again,
Przemek.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 199229] New: choppy cursor on ryzen 5 2400g

2018-03-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=199229

Bug ID: 199229
   Summary: choppy cursor on ryzen 5 2400g
   Product: Drivers
   Version: 2.5
Kernel Version: 4.16rc5+
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: davidbecerrapor...@gmail.com
Regression: No

as said on https://bugzilla.kernel.org/show_bug.cgi?id=199123 since kernel
4.16rc5 cursor is super choppy 4.15 is fine

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/2] drm/dp: Move DPCD_REV_XX to drm_dp_helper

2018-03-27 Thread matthew . s . atwood
From: Matt Atwood 

As more differentation occurs between DP spec. Its useful to have these
as macros in a drm_dp_helper.

Signed-off-by: Matt Atwood 
---
 drivers/gpu/drm/amd/display/include/dpcd_defs.h | 8 
 include/drm/drm_dp_helper.h | 5 +
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/include/dpcd_defs.h 
b/drivers/gpu/drm/amd/display/include/dpcd_defs.h
index d8e52e3..d13e0f4 100644
--- a/drivers/gpu/drm/amd/display/include/dpcd_defs.h
+++ b/drivers/gpu/drm/amd/display/include/dpcd_defs.h
@@ -28,14 +28,6 @@
 
 #include 
 
-enum dpcd_revision {
-   DPCD_REV_10 = 0x10,
-   DPCD_REV_11 = 0x11,
-   DPCD_REV_12 = 0x12,
-   DPCD_REV_13 = 0x13,
-   DPCD_REV_14 = 0x14
-};
-
 /* these are the types stored at DOWNSTREAMPORT_PRESENT */
 enum dpcd_downstream_port_type {
DOWNSTREAM_DP = 0,
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 4de97e9..f77746e 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -64,6 +64,11 @@
 /* AUX CH addresses */
 /* DPCD */
 #define DP_DPCD_REV 0x000
+# define DPCD_REV_100x10
+# define DPCD_REV_110x11
+# define DPCD_REV_120x12
+# define DPCD_REV_130x13
+# define DPCD_REV_140x14
 
 #define DP_MAX_LINK_RATE0x001
 
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 2/2] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4

2018-03-27 Thread matthew . s . atwood
From: Matt Atwood 

DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheeme from 8
bits to 7 in DPCD 0x000e. The 8th bit is used to identify extended
receiver capabilities. For panels that use this new feature wait interval
would be increased by 512 ms, when spec is max 16 ms. This behavior is
described in table 2-158 of DP 1.4 spec address eh.

With the introduction of DP 1.4 spec main link clock recovery was
standardized to 100 us regardless of TRAINING_AUX_RD_INTERVAL value.

To avoid breaking panels that are not spec compiant we now warn on
invalid values.

V2: commit title/message, masking all 7 bits, warn on out of spec values.
V3: commit message, make link train clock recovery follow DP 1.4 spec.
V4: style changes
V5: typo
V6: print statement revisions, DP_REV to DPCD_REV, comment correction
V7: typo
V8: Style
V9: Strip out DPCD_REV_XX into seperate patch

Signed-off-by: Matt Atwood 
Reviewed-by: Rodrigo Vivi 
---
 drivers/gpu/drm/drm_dp_helper.c | 22 ++
 include/drm/drm_dp_helper.h |  1 +
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ffe14ec..f9a8bf9 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -119,18 +119,32 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 
link_status[DP_LINK_STATUS_SI
 EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
 
 void drm_dp_link_train_clock_recovery_delay(const u8 
dpcd[DP_RECEIVER_CAP_SIZE]) {
-   if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
+   int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
+ DP_TRAINING_AUX_RD_MASK;
+
+   if (rd_interval > 4)
+   DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n",
+ rd_interval);
+
+   if (rd_interval == 0 || dpcd[DP_DPCD_REV] >= DPCD_REV_14)
udelay(100);
else
-   mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
+   mdelay(rd_interval * 4);
 }
 EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
 
 void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) {
-   if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
+   int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
+ DP_TRAINING_AUX_RD_MASK;
+
+   if (rd_interval > 4)
+   DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n",
+ rd_interval);
+
+   if (rd_interval == 0)
udelay(400);
else
-   mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
+   mdelay(rd_interval * 4);
 }
 EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index f77746e..c1ba415 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -124,6 +124,7 @@
 # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher */
 
 #define DP_TRAINING_AUX_RD_INTERVAL 0x00e   /* XXX 1.2? */
+# define DP_TRAINING_AUX_RD_MASK0x7F/* XXX 1.2? */
 
 #define DP_ADAPTER_CAP 0x00f   /* 1.2 */
 # define DP_FORCE_LOAD_SENSE_CAP   (1 << 0)
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 199123] kernel 4.16rc5 doesnt boot on ryzen 5 2400g due to an amdgpu change

2018-03-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=199123

david becerra (davidbecerrapor...@gmail.com) changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |CODE_FIX

--- Comment #15 from david becerra (davidbecerrapor...@gmail.com) ---
(In reply to Harry Wentland from comment #14)
> Can we close this ticket if the original issue has been fixed and open a new
> one for the choppy cursor? It gets confusing when tickets evolve from one
> issue to another.

yes, thats a good idea closing

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 3/3] drm: make drm_core_check_feature() bool that it is

2018-03-27 Thread Chris Wilson
Quoting Jani Nikula (2018-03-27 21:47:22)
> Bool is the more appropriate return type here, use it.
> 
> Signed-off-by: Jani Nikula 

All 3,
Reviewed-by: Chris Wilson 
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 2/3] drm: remove old documentation comment cruft from drmP.h

2018-03-27 Thread Jani Nikula
Throw out the leftovers.

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

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 4bbef061c9c0..b5d52a3d7d19 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -95,14 +95,6 @@ struct dma_buf_attachment;
 struct pci_dev;
 struct pci_controller;
 
-/***/
-/** \name DRM template customization defaults */
-/*@{*/
-
-/***/
-/** \name Internal types and structures */
-/*@{*/
-
 #define DRM_IF_VERSION(maj, min) (maj << 16 | min)
 
 /**
@@ -128,19 +120,6 @@ static inline int drm_core_check_feature(struct drm_device 
*dev, int feature)
return ((dev->driver->driver_features & feature) ? 1 : 0);
 }
 
-/**/
-/** \name Internal function definitions */
-/*@{*/
-
-   /* Driver support (drm_drv.h) */
-
-/*
- * These are exported to drivers so that they can implement fencing using
- * DMA quiscent + idle. DMA quiescent usually requires the hardware lock.
- */
-
-/*@}*/
-
 /* returns true if currently okay to sleep */
 static inline bool drm_can_sleep(void)
 {
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 3/3] drm: make drm_core_check_feature() bool that it is

2018-03-27 Thread Jani Nikula
Bool is the more appropriate return type here, use it.

Signed-off-by: Jani Nikula 
---
 include/drm/drmP.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index b5d52a3d7d19..f5099c12c6a6 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -115,9 +115,9 @@ static inline bool drm_drv_uses_atomic_modeset(struct 
drm_device *dev)
 #define DRM_SWITCH_POWER_CHANGING 2
 #define DRM_SWITCH_POWER_DYNAMIC_OFF 3
 
-static inline int drm_core_check_feature(struct drm_device *dev, int feature)
+static inline bool drm_core_check_feature(struct drm_device *dev, int feature)
 {
-   return ((dev->driver->driver_features & feature) ? 1 : 0);
+   return dev->driver->driver_features & feature;
 }
 
 /* returns true if currently okay to sleep */
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/3] drm: prefer inline over __inline__

2018-03-27 Thread Jani Nikula
Remove last users of __inline__.

Signed-off-by: Jani Nikula 
---
 include/drm/drmP.h   | 5 ++---
 include/drm/drm_legacy.h | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index ccd09347..4bbef061c9c0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -123,8 +123,7 @@ static inline bool drm_drv_uses_atomic_modeset(struct 
drm_device *dev)
 #define DRM_SWITCH_POWER_CHANGING 2
 #define DRM_SWITCH_POWER_DYNAMIC_OFF 3
 
-static __inline__ int drm_core_check_feature(struct drm_device *dev,
-int feature)
+static inline int drm_core_check_feature(struct drm_device *dev, int feature)
 {
return ((dev->driver->driver_features & feature) ? 1 : 0);
 }
@@ -143,7 +142,7 @@ static __inline__ int drm_core_check_feature(struct 
drm_device *dev,
 /*@}*/
 
 /* returns true if currently okay to sleep */
-static __inline__ bool drm_can_sleep(void)
+static inline bool drm_can_sleep(void)
 {
if (in_atomic() || in_dbg_master() || irqs_disabled())
return false;
diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
index cf0e7d89bcdf..8fad66f88e4f 100644
--- a/include/drm/drm_legacy.h
+++ b/include/drm/drm_legacy.h
@@ -194,8 +194,8 @@ void drm_legacy_ioremap(struct drm_local_map *map, struct 
drm_device *dev);
 void drm_legacy_ioremap_wc(struct drm_local_map *map, struct drm_device *dev);
 void drm_legacy_ioremapfree(struct drm_local_map *map, struct drm_device *dev);
 
-static __inline__ struct drm_local_map *drm_legacy_findmap(struct drm_device 
*dev,
-  unsigned int token)
+static inline struct drm_local_map *drm_legacy_findmap(struct drm_device *dev,
+  unsigned int token)
 {
struct drm_map_list *_entry;
list_for_each_entry(_entry, >maplist, head)
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 104825] [amdgpu] [drm:gfx_v8_0_hw_fini] *ERROR* KCQ disabled failed (scratch(0xC040)=0x00000000) when unbinding

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=104825

--- Comment #14 from Andrey Grodzovsky  ---
(In reply to Harry Wentland from comment #13)
> This should be fixed in drm-next-4.17-wip and amd-staging-drm-next. Can
> someone test if this resolves this ticket satisfactorily?
> 
> Both branches can be found on https://cgit.freedesktop.org/~agd5f/linux

Yes, DAL issues are gone, also gone KIQ ring error on unbind. 
Still there is a KIQ ring error on rebind which I am investigating now + there
is SDMA ring IB test failure which I will get to after KIQ is resloved.

Andrey

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] dma-buf: use parameter structure for dma_buf_attach

2018-03-27 Thread kbuild test robot
Hi Christian,

I love your patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.16-rc7 next-20180327]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Christian-K-nig/dma-buf-use-parameter-structure-for-dma_buf_attach/20180326-044631
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: openrisc-allmodconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc 

All errors (new ones prefixed by >>):

   drivers/staging/media/tegra-vde/tegra-vde.c: In function 
'tegra_vde_attach_dmabuf':
>> drivers/staging/media/tegra-vde/tegra-vde.c:534:13: error: 'dmabuf' 
>> undeclared (first use in this function)
  .dmabuf = dmabuf
^~
   drivers/staging/media/tegra-vde/tegra-vde.c:534:13: note: each undeclared 
identifier is reported only once for each function it appears in

vim +/dmabuf +534 drivers/staging/media/tegra-vde/tegra-vde.c

   521  
   522  static int tegra_vde_attach_dmabuf(struct device *dev,
   523 int fd,
   524 unsigned long offset,
   525 unsigned int min_size,
   526 struct dma_buf_attachment **a,
   527 dma_addr_t *addr,
   528 struct sg_table **s,
   529 size_t *size,
   530 enum dma_data_direction dma_dir)
   531  {
   532  struct dma_buf_attach_info attach_info = {
   533  .dev = dev,
 > 534  .dmabuf = dmabuf
   535  };
   536  struct dma_buf_attachment *attachment;
   537  struct dma_buf *dmabuf;
   538  struct sg_table *sgt;
   539  int err;
   540  
   541  dmabuf = dma_buf_get(fd);
   542  if (IS_ERR(dmabuf)) {
   543  dev_err(dev, "Invalid dmabuf FD\n");
   544  return PTR_ERR(dmabuf);
   545  }
   546  
   547  if ((u64)offset + min_size > dmabuf->size) {
   548  dev_err(dev, "Too small dmabuf size %zu @0x%lX, "
   549   "should be at least %d\n",
   550  dmabuf->size, offset, min_size);
   551  return -EINVAL;
   552  }
   553  
   554  attachment = dma_buf_attach(_info);
   555  if (IS_ERR(attachment)) {
   556  dev_err(dev, "Failed to attach dmabuf\n");
   557  err = PTR_ERR(attachment);
   558  goto err_put;
   559  }
   560  
   561  sgt = dma_buf_map_attachment(attachment, dma_dir);
   562  if (IS_ERR(sgt)) {
   563  dev_err(dev, "Failed to get dmabufs sg_table\n");
   564  err = PTR_ERR(sgt);
   565  goto err_detach;
   566  }
   567  
   568  if (sgt->nents != 1) {
   569  dev_err(dev, "Sparse DMA region is unsupported\n");
   570  err = -EINVAL;
   571  goto err_unmap;
   572  }
   573  
   574  *addr = sg_dma_address(sgt->sgl) + offset;
   575  *a = attachment;
   576  *s = sgt;
   577  
   578  if (size)
   579  *size = dmabuf->size - offset;
   580  
   581  return 0;
   582  
   583  err_unmap:
   584  dma_buf_unmap_attachment(attachment, sgt, dma_dir);
   585  err_detach:
   586  dma_buf_detach(dmabuf, attachment);
   587  err_put:
   588  dma_buf_put(dmabuf);
   589  
   590  return err;
   591  }
   592  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105712] intel-gpu-overlay is showing insane power consumption amounts

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105712

Chris Wilson  changed:

   What|Removed |Added

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

--- Comment #11 from Chris Wilson  ---
Thank you for the bug report and the invaluable testing.

commit 3fa0b027304ec28cd24b314349d3731b55dfcc0a (HEAD, upstream/master)
Author: Chris Wilson 
Date:   Tue Mar 27 15:20:51 2018 +0100

overlay: Call setlocale around strtod

strtod() is locale-dependent. The decimal conversion depends on the radix
character ('.' for some of us like myself) varies by locale. As the
kernel reports its values using the "C" locale, we need to switch to
that when parsing; and switch back before reporting to the user.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105712
Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Reviewed-by: Tvrtko Ursulin 

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105725] WARNING: CPU: 0 PID: 487 at drivers/gpu/drm/amd/amdgpu/../display /dc/gpio/gpio_base.c:64 dal_gpio_open_ex+0xc/0x30 [amdgpu]

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105725

--- Comment #3 from Harry Wentland  ---
The warnings are likely not related to the hang in this case.

Installing or building the kernel might depend on your distribution. You should
be able to find a guide by googling "building a custom kernel ".

In general you might want to
 * mkdir mybuilddir
 * git clone git://people.freedesktop.org/~agd5f/linux
 * cd linux
 * cp /boot/config- .config
 * if you're on ubuntu/debian:
   * make deb-pkg
   * sudo dpkg -i 

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 104412] RX 460 HDMI 4k 60fps not working, DisplayPort is.

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=104412

--- Comment #14 from S.H.  ---
(In reply to Alfe from comment #12)
> (In reply to S.H. from comment #6)
> > Hello,
> > Last night I was able to get my hands on a Gigabyte RX460 and did some
> > testing.
> > Booting and normal startup with the Gigabyte also only used 4k@30Hz but I
> > was able to reach 4k@60Hz by adding a modeline via xrandr.
> > With the Saphhire I am not able to get 4k@60Hz even with a custom modeline
> > set via xrandr.
> > Something seems broken here and I don't think both vendors messed up their
> > cards.
> 
> Hi!  Since I'm also trying around with this, it would be really helpful if
> you shared your modeline with which you were able to achieve 4k@60Hz ;-)
> 
> Cheers, Alfe

Hi,
I use these three lines to achieve 60Hz output:
xrandr --newmode "mymode" 594  3840 4016 4104 4400  2160 2168 2178 2250 +hsync
+vsync
xrandr --addmode HDMI-A-0 mymode
xrandr --output HDMI-A-0 --mode mymode

Cheers,
Sven

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 104412] RX 460 HDMI 4k 60fps not working, DisplayPort is.

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=104412

--- Comment #13 from Harry Wentland  ---
The problem with 4k60 being blocked due to a BIOS bit should be fixed now. I
recommend trying the drm-next-4.17-wip from
https://cgit.freedesktop.org/~agd5f/linux

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105354] RIP: dm_update_crtcs_state+0x36b/0x3f0 [amdgpu]

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105354

--- Comment #2 from Harry Wentland  ---
Is this still an issue for you?

What do you do when this happens?

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105712] intel-gpu-overlay is showing insane power consumption amounts

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105712

--- Comment #10 from leozinho29...@hotmail.com ---
With this patch, the power consumption is shown correctly.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 104825] [amdgpu] [drm:gfx_v8_0_hw_fini] *ERROR* KCQ disabled failed (scratch(0xC040)=0x00000000) when unbinding

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=104825

--- Comment #13 from Harry Wentland  ---
This should be fixed in drm-next-4.17-wip and amd-staging-drm-next. Can someone
test if this resolves this ticket satisfactorily?

Both branches can be found on https://cgit.freedesktop.org/~agd5f/linux

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 104624] [regression, vega] Running DOOM causes *ERROR* amdgpu_dm_commit_planes: acrtc 2, already busy WARNING amdgpu_dm_atomic_commit_tail and prepare_flip_isr

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=104624

--- Comment #4 from Harry Wentland  ---
Have you ever gotten around to bisecting the kernel on this?

Are you still seeing the problem?

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 199123] kernel 4.16rc5 doesnt boot on ryzen 5 2400g due to an amdgpu change

2018-03-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=199123

--- Comment #14 from Harry Wentland (harry.wentl...@amd.com) ---
Can we close this ticket if the original issue has been fixed and open a new
one for the choppy cursor? It gets confusing when tickets evolve from one issue
to another.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/scheduler: fix param documentation

2018-03-27 Thread Nayan Deshmukh
On Tue, Mar 27, 2018 at 1:47 PM, Daniel Vetter  wrote:
> On Mon, Mar 26, 2018 at 08:51:14PM +0530, Nayan Deshmukh wrote:
>> Signed-off-by: Nayan Deshmukh 
>
> You might want to add a kerneldoc page in Documentation/gpu/scheduler.rst,
> which pulls in all the nice kerneldoc you have here + has a short intro
> text what this is all about.
>
Yeah Sure. I'll send a patch for this in a while.

Cheers,
Nayan
> Cheers, Daniel
>
>> ---
>>  drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c 
>> b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>> index 0d95888ccc3e..1d368bc66ac2 100644
>> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
>> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>> @@ -117,8 +117,9 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
>>   * @schedThe pointer to the scheduler
>>   * @entity   The pointer to a valid drm_sched_entity
>>   * @rq   The run queue this entity belongs
>> - * @kernel   If this is an entity for the kernel
>>   * @jobs The max number of jobs in the job queue
>> + * @guilty  atomic_t set to 1 when a job on this queue
>> + *  is found to be guilty causing a timeout
>>   *
>>   * return 0 if succeed. negative error code on failure
>>  */
>> --
>> 2.14.3
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105284] Every boot I get an error in dmesg "WARNING: CPU: 2 PID: 1380 at drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:132 generic_reg_update_ex+0x108/0x150 [amdgpu]"

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105284

--- Comment #3 from Harry Wentland  ---
Can you try the latest amd-staging-drm-next or drm-next-4.17-wip from
https://cgit.freedesktop.org/~agd5f/linux? It should be fixed now.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[gabbayo:amdkfd-next 14/18] drivers/gpu/drm/amd/amdkfd/kfd_chardev.c:1094:6: sparse: symbol 'kfd_dev_is_large_bar' was not declared. Should it be static?

2018-03-27 Thread kbuild test robot
tree:   git://people.freedesktop.org/~gabbayo/linux amdkfd-next
head:   1679ae8f8f4148766423066aeb3dbb0a985a373a
commit: 5ec7e02854b3b9b55936c3b44b8acfb85e333f49 [14/18] drm/amdkfd: Add ioctls 
for GPUVM memory management
reproduce:
# apt-get install sparse
git checkout 5ec7e02854b3b9b55936c3b44b8acfb85e333f49
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c:1094:6: sparse: symbol 
>> 'kfd_dev_is_large_bar' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC PATCH gabbayo] drm/amdkfd: kfd_dev_is_large_bar() can be static

2018-03-27 Thread kbuild test robot

Fixes: 5ec7e02854b3 ("drm/amdkfd: Add ioctls for GPUVM memory management")
Signed-off-by: Fengguang Wu 
---
 kfd_chardev.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index a563ff2..2a36592 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1091,7 +1091,7 @@ static int kfd_ioctl_acquire_vm(struct file *filep, 
struct kfd_process *p,
return ret;
 }
 
-bool kfd_dev_is_large_bar(struct kfd_dev *dev)
+static bool kfd_dev_is_large_bar(struct kfd_dev *dev)
 {
struct kfd_local_mem_info mem_info;
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105177] amdgpu wrong colors with rx460 connected via hdmi

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105177

--- Comment #19 from Reimar Imhof  ---
I will wait for suse providing a 4.17 kernel

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook

2018-03-27 Thread David Lechner

On 03/27/2018 05:07 AM, Ville Syrjälä wrote:

On Sat, Mar 24, 2018 at 12:26:32PM -0500, David Lechner wrote:

On 03/22/2018 03:27 PM, Ville Syrjala wrote:

From: Ville Syrjälä 

We'll need access to the plane state during .atomic_enable().



Some more details in the commit message would be useful. It is
not clear to me why this change is being made.


"tinydrm enable hook wants to play around with the new fb in
.atomic_enable(), thus we'll need access to the plane state."

Better? Worse?



Better.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 3/5] drm/udl: Get rid of dev->struct_mutex usage

2018-03-27 Thread Sean Paul
On Tue, Mar 27, 2018 at 10:23:54AM +0200, Daniel Vetter wrote:
> It's only used to protect our page list, and only when we know we have
> a full reference. This means none of these code paths can ever race
> with the final unref, and hence we do not need dev->struct_mutex
> serialization and can simply switch to our own locking.
> 
> For more context the only magic the locked gem_free_object provides is
> that it prevents concurrent final unref (and destruction) of gem
> objects while anyone is holding dev->struct_mutex. This was used by
> i915 (and other drivers) to implement eviction handling with less
> headaches.
> 
> Signed-off-by: Daniel Vetter 

Reviewed-by: Sean Paul 

> Cc: Dave Airlie 
> ---
>  drivers/gpu/drm/udl/udl_dmabuf.c | 5 +++--
>  drivers/gpu/drm/udl/udl_drv.c| 2 +-
>  drivers/gpu/drm/udl/udl_drv.h| 2 ++
>  drivers/gpu/drm/udl/udl_gem.c| 5 +++--
>  drivers/gpu/drm/udl/udl_main.c   | 2 ++
>  5 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c 
> b/drivers/gpu/drm/udl/udl_dmabuf.c
> index 2867ed155ff6..0a20695eb120 100644
> --- a/drivers/gpu/drm/udl/udl_dmabuf.c
> +++ b/drivers/gpu/drm/udl/udl_dmabuf.c
> @@ -76,6 +76,7 @@ static struct sg_table *udl_map_dma_buf(struct 
> dma_buf_attachment *attach,
>   struct udl_drm_dmabuf_attachment *udl_attach = attach->priv;
>   struct udl_gem_object *obj = to_udl_bo(attach->dmabuf->priv);
>   struct drm_device *dev = obj->base.dev;
> + struct udl_device *udl = dev->dev_private;
>   struct scatterlist *rd, *wr;
>   struct sg_table *sgt = NULL;
>   unsigned int i;
> @@ -112,7 +113,7 @@ static struct sg_table *udl_map_dma_buf(struct 
> dma_buf_attachment *attach,
>   return ERR_PTR(-ENOMEM);
>   }
>  
> - mutex_lock(>struct_mutex);
> + mutex_lock(>gem_lock);
>  
>   rd = obj->sg->sgl;
>   wr = sgt->sgl;
> @@ -137,7 +138,7 @@ static struct sg_table *udl_map_dma_buf(struct 
> dma_buf_attachment *attach,
>   attach->priv = udl_attach;
>  
>  err_unlock:
> - mutex_unlock(>struct_mutex);
> + mutex_unlock(>gem_lock);
>   return sgt;
>  }
>  
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 3c45a3064726..9ef515df724b 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -53,7 +53,7 @@ static struct drm_driver driver = {
>   .unload = udl_driver_unload,
>  
>   /* gem hooks */
> - .gem_free_object = udl_gem_free_object,
> + .gem_free_object_unlocked = udl_gem_free_object,
>   .gem_vm_ops = _gem_vm_ops,
>  
>   .dumb_create = udl_dumb_create,
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 2a75ab80527a..55c0cc309198 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -54,6 +54,8 @@ struct udl_device {
>   struct usb_device *udev;
>   struct drm_crtc *crtc;
>  
> + struct mutex gem_lock;
> +
>   int sku_pixel_limit;
>  
>   struct urb_list urbs;
> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
> index dee6bd9a3dd1..9a15cce22cce 100644
> --- a/drivers/gpu/drm/udl/udl_gem.c
> +++ b/drivers/gpu/drm/udl/udl_gem.c
> @@ -214,9 +214,10 @@ int udl_gem_mmap(struct drm_file *file, struct 
> drm_device *dev,
>  {
>   struct udl_gem_object *gobj;
>   struct drm_gem_object *obj;
> + struct udl_device *udl = dev->dev_private;
>   int ret = 0;
>  
> - mutex_lock(>struct_mutex);
> + mutex_lock(>gem_lock);
>   obj = drm_gem_object_lookup(file, handle);
>   if (obj == NULL) {
>   ret = -ENOENT;
> @@ -236,6 +237,6 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device 
> *dev,
>  out:
>   drm_gem_object_put(>base);
>  unlock:
> - mutex_unlock(>struct_mutex);
> + mutex_unlock(>gem_lock);
>   return ret;
>  }
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index f1ec4528a73e..d518de8f496b 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -324,6 +324,8 @@ int udl_driver_load(struct drm_device *dev, unsigned long 
> flags)
>   udl->ddev = dev;
>   dev->dev_private = udl;
>  
> + mutex_init(>gem_lock);
> +
>   if (!udl_parse_vendor_descriptor(dev, udl->udev)) {
>   ret = -ENODEV;
>   DRM_ERROR("firmware not recognized. Assume incompatible 
> device\n");
> -- 
> 2.16.2
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 3/4] drm/msm: Remove msm_commit/worker, use atomic helper commit

2018-03-27 Thread Sean Paul
Moving further towards switching fully to the the atomic helpers, this
patch removes the hand-rolled worker nonblock commit code and uses the
atomic helpers commit_work model.

Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/msm/msm_atomic.c | 145 +--
 drivers/gpu/drm/msm/msm_drv.c|   1 -
 drivers/gpu/drm/msm/msm_drv.h|   4 -
 3 files changed, 39 insertions(+), 111 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 9c6fa52a40b7..af828df6e060 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -20,66 +20,6 @@
 #include "msm_gem.h"
 #include "msm_fence.h"
 
-struct msm_commit {
-   struct drm_device *dev;
-   struct drm_atomic_state *state;
-   struct work_struct work;
-   uint32_t crtc_mask;
-};
-
-static void commit_worker(struct work_struct *work);
-
-/* block until specified crtcs are no longer pending update, and
- * atomically mark them as pending update
- */
-static int start_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
-   int ret;
-
-   spin_lock(>pending_crtcs_event.lock);
-   ret = wait_event_interruptible_locked(priv->pending_crtcs_event,
-   !(priv->pending_crtcs & crtc_mask));
-   if (ret == 0) {
-   DBG("start: %08x", crtc_mask);
-   priv->pending_crtcs |= crtc_mask;
-   }
-   spin_unlock(>pending_crtcs_event.lock);
-
-   return ret;
-}
-
-/* clear specified crtcs (no longer pending update)
- */
-static void end_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
-   spin_lock(>pending_crtcs_event.lock);
-   DBG("end: %08x", crtc_mask);
-   priv->pending_crtcs &= ~crtc_mask;
-   wake_up_all_locked(>pending_crtcs_event);
-   spin_unlock(>pending_crtcs_event.lock);
-}
-
-static struct msm_commit *commit_init(struct drm_atomic_state *state)
-{
-   struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL);
-
-   if (!c)
-   return NULL;
-
-   c->dev = state->dev;
-   c->state = state;
-
-   INIT_WORK(>work, commit_worker);
-
-   return c;
-}
-
-static void commit_destroy(struct msm_commit *c)
-{
-   end_atomic(c->dev->dev_private, c->crtc_mask);
-   kfree(c);
-}
-
 static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
struct drm_atomic_state *old_state)
 {
@@ -126,6 +66,10 @@ static void msm_atomic_commit_tail(struct drm_atomic_state 
*state)
 
msm_atomic_wait_for_commit_done(dev, state);
 
+   drm_atomic_helper_commit_hw_done(state);
+
+   drm_atomic_helper_wait_for_vblanks(dev, state);
+
drm_atomic_helper_cleanup_planes(dev, state);
 
kms->funcs->complete_commit(kms, state);
@@ -134,21 +78,25 @@ static void msm_atomic_commit_tail(struct drm_atomic_state 
*state)
 /* The (potentially) asynchronous part of the commit.  At this point
  * nothing can fail short of armageddon.
  */
-static void complete_commit(struct msm_commit *c)
+static void commit_tail(struct drm_atomic_state *state)
 {
-   struct drm_atomic_state *state = c->state;
-   struct drm_device *dev = state->dev;
+   drm_atomic_helper_wait_for_fences(state->dev, state, false);
 
-   drm_atomic_helper_wait_for_fences(dev, state, false);
+   drm_atomic_helper_wait_for_dependencies(state);
 
msm_atomic_commit_tail(state);
 
+   drm_atomic_helper_commit_cleanup_done(state);
+
drm_atomic_state_put(state);
 }
 
-static void commit_worker(struct work_struct *work)
+static void commit_work(struct work_struct *work)
 {
-   complete_commit(container_of(work, struct msm_commit, work), true);
+   struct drm_atomic_state *state = container_of(work,
+ struct drm_atomic_state,
+ commit_work);
+   commit_tail(state);
 }
 
 /**
@@ -167,17 +115,12 @@ int msm_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state, bool nonblock)
 {
struct msm_drm_private *priv = dev->dev_private;
-   struct msm_commit *c;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
int i, ret;
 
-   ret = drm_atomic_helper_prepare_planes(dev, state);
-   if (ret)
-   return ret;
-
/*
 * Note that plane->atomic_async_check() should fail if we need
 * to re-assign hwpipe or anything that touches global atomic
@@ -185,44 +128,38 @@ int msm_atomic_commit(struct drm_device *dev,
 * cases.
 */
if (state->async_update) {
+   ret = drm_atomic_helper_prepare_planes(dev, state);
+   if (ret)
+   return ret;
+
drm_atomic_helper_async_commit(dev, state);

[PATCH 2/4] drm/msm: Mark the crtc->state->event consumed

2018-03-27 Thread Sean Paul
Don't leave the event != NULL once it's consumed, this is used a signal
to the atomic helpers that the event will be handled by the driver.

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 +
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 6e5e1aa54ce1..b001699297c4 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -351,6 +351,7 @@ static void mdp4_crtc_atomic_flush(struct drm_crtc *crtc,
 
spin_lock_irqsave(>event_lock, flags);
mdp4_crtc->event = crtc->state->event;
+   crtc->state->event = NULL;
spin_unlock_irqrestore(>event_lock, flags);
 
blend_setup(crtc);
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 9893e43ba6c5..76b96081916f 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -708,6 +708,7 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc,
 
spin_lock_irqsave(>event_lock, flags);
mdp5_crtc->event = crtc->state->event;
+   crtc->state->event = NULL;
spin_unlock_irqrestore(>event_lock, flags);
 
/*
-- 
Sean Paul, Software Engineer, Google / Chromium OS

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/4] drm/msm: Refactor complete_commit() to look more the helpers

2018-03-27 Thread Sean Paul
Factor out the commit_tail() portions of complete_commit() into a
separate function to facilitate moving to the atomic helpers in future
patches.

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/msm/msm_atomic.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index e792158676aa..9c6fa52a40b7 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -97,18 +97,12 @@ static void msm_atomic_wait_for_commit_done(struct 
drm_device *dev,
}
 }
 
-/* The (potentially) asynchronous part of the commit.  At this point
- * nothing can fail short of armageddon.
- */
-static void complete_commit(struct msm_commit *c, bool async)
+static void msm_atomic_commit_tail(struct drm_atomic_state *state)
 {
-   struct drm_atomic_state *state = c->state;
struct drm_device *dev = state->dev;
struct msm_drm_private *priv = dev->dev_private;
struct msm_kms *kms = priv->kms;
 
-   drm_atomic_helper_wait_for_fences(dev, state, false);
-
kms->funcs->prepare_commit(kms, state);
 
drm_atomic_helper_commit_modeset_disables(dev, state);
@@ -135,10 +129,21 @@ static void complete_commit(struct msm_commit *c, bool 
async)
drm_atomic_helper_cleanup_planes(dev, state);
 
kms->funcs->complete_commit(kms, state);
+}
 
-   drm_atomic_state_put(state);
+/* The (potentially) asynchronous part of the commit.  At this point
+ * nothing can fail short of armageddon.
+ */
+static void complete_commit(struct msm_commit *c)
+{
+   struct drm_atomic_state *state = c->state;
+   struct drm_device *dev = state->dev;
 
-   commit_destroy(c);
+   drm_atomic_helper_wait_for_fences(dev, state, false);
+
+   msm_atomic_commit_tail(state);
+
+   drm_atomic_state_put(state);
 }
 
 static void commit_worker(struct work_struct *work)
-- 
Sean Paul, Software Engineer, Google / Chromium OS

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 0/4] drm/msm: Switch to atomic helpers

2018-03-27 Thread Sean Paul
I originally sent these patches targetted against the msm dpu code, but
I've rebased them on msm-next since they're _mostly_ the same. The set
is based on 'drm/msm: Use drm_private_obj/state instead of subclassing'
which I sent up earlier.

The set has been tested on mdp5 db410c.

Sean

Sean Paul (4):
  drm/msm: Refactor complete_commit() to look more the helpers
  drm/msm: Mark the crtc->state->event consumed
  drm/msm: Remove msm_commit/worker, use atomic helper commit
  drm/msm: Switch to atomic_helper_commit()

 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |   1 +
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c |   1 +
 drivers/gpu/drm/msm/msm_atomic.c  | 190 +-
 drivers/gpu/drm/msm/msm_drv.c |   8 +-
 drivers/gpu/drm/msm/msm_drv.h |   7 +-
 5 files changed, 14 insertions(+), 193 deletions(-)

-- 
Sean Paul, Software Engineer, Google / Chromium OS

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 4/4] drm/msm: Switch to atomic_helper_commit()

2018-03-27 Thread Sean Paul
Now that all of the msm-specific goo is tucked safely away we can switch
over to using the atomic helper commit directly. \o/

Cc: Abhinav Kumar 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/msm/msm_atomic.c | 120 +--
 drivers/gpu/drm/msm/msm_drv.c|   7 +-
 drivers/gpu/drm/msm/msm_drv.h|   3 +-
 3 files changed, 8 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index af828df6e060..0a63ce8f33a3 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -17,8 +17,6 @@
 
 #include "msm_drv.h"
 #include "msm_kms.h"
-#include "msm_gem.h"
-#include "msm_fence.h"
 
 static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
struct drm_atomic_state *old_state)
@@ -37,7 +35,7 @@ static void msm_atomic_wait_for_commit_done(struct drm_device 
*dev,
}
 }
 
-static void msm_atomic_commit_tail(struct drm_atomic_state *state)
+void msm_atomic_commit_tail(struct drm_atomic_state *state)
 {
struct drm_device *dev = state->dev;
struct msm_drm_private *priv = dev->dev_private;
@@ -74,119 +72,3 @@ static void msm_atomic_commit_tail(struct drm_atomic_state 
*state)
 
kms->funcs->complete_commit(kms, state);
 }
-
-/* The (potentially) asynchronous part of the commit.  At this point
- * nothing can fail short of armageddon.
- */
-static void commit_tail(struct drm_atomic_state *state)
-{
-   drm_atomic_helper_wait_for_fences(state->dev, state, false);
-
-   drm_atomic_helper_wait_for_dependencies(state);
-
-   msm_atomic_commit_tail(state);
-
-   drm_atomic_helper_commit_cleanup_done(state);
-
-   drm_atomic_state_put(state);
-}
-
-static void commit_work(struct work_struct *work)
-{
-   struct drm_atomic_state *state = container_of(work,
- struct drm_atomic_state,
- commit_work);
-   commit_tail(state);
-}
-
-/**
- * drm_atomic_helper_commit - commit validated state object
- * @dev: DRM device
- * @state: the driver state object
- * @nonblock: nonblocking commit
- *
- * This function commits a with drm_atomic_helper_check() pre-validated state
- * object. This can still fail when e.g. the framebuffer reservation fails.
- *
- * RETURNS
- * Zero for success or -errno.
- */
-int msm_atomic_commit(struct drm_device *dev,
-   struct drm_atomic_state *state, bool nonblock)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct drm_crtc *crtc;
-   struct drm_crtc_state *crtc_state;
-   struct drm_plane *plane;
-   struct drm_plane_state *old_plane_state, *new_plane_state;
-   int i, ret;
-
-   /*
-* Note that plane->atomic_async_check() should fail if we need
-* to re-assign hwpipe or anything that touches global atomic
-* state, so we'll never go down the async update path in those
-* cases.
-*/
-   if (state->async_update) {
-   ret = drm_atomic_helper_prepare_planes(dev, state);
-   if (ret)
-   return ret;
-
-   drm_atomic_helper_async_commit(dev, state);
-   drm_atomic_helper_cleanup_planes(dev, state);
-   return 0;
-   }
-
-   ret = drm_atomic_helper_setup_commit(state, nonblock);
-   if (ret)
-   return ret;
-
-   INIT_WORK(>commit_work, commit_work);
-
-   ret = drm_atomic_helper_prepare_planes(dev, state);
-   if (ret)
-   return ret;
-
-   if (!nonblock) {
-   ret = drm_atomic_helper_wait_for_fences(dev, state, true);
-   if (ret)
-   goto error;
-   }
-
-   /*
-* This is the point of no return - everything below never fails except
-* when the hw goes bonghits. Which means we can commit the new state on
-* the software side now.
-*
-* swap driver private state while still holding state_lock
-*/
-   BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
-
-   /*
-* Everything below can be run asynchronously without the need to grab
-* any modeset locks at all under one conditions: It must be guaranteed
-* that the asynchronous work has either been cancelled (if the driver
-* supports it, which at least requires that the framebuffers get
-* cleaned up with drm_atomic_helper_cleanup_planes()) or completed
-* before the new state gets committed on the software side with
-* drm_atomic_helper_swap_state().
-*
-* This scheme allows new atomic state updates to be prepared and
-* checked in parallel to the asynchronous completion of the previous
-* update. Which is important since compositors need to figure out the
-* composition of the next 

Re: [PATCH] drm: clarify adjusted_mode for a bridge connected to a crtc

2018-03-27 Thread Daniel Vetter
On Mon, Feb 26, 2018 at 01:16:04PM +0100, Philippe Cornu wrote:
> This patch clarifies the adjusted_mode documentation
> for a bridge directly connected to a crtc.
> 
> Signed-off-by: Philippe Cornu 
> ---
> This patch is linked to the discussion https://lkml.org/lkml/2018/1/25/367

Reviewed-by: Daniel Vetter 

Aside, and kinda why I noticed this patch to begin with: You have drm-misc
commit rights, but you seem to not use that. And with 18 patches in the
4.17 cycle you're the top contributor who's not pushing his own patches.

What's the hold-up? Tools don't work, or something else? I really think
regular contributors should just push their own stuff themselves (after
appropriate review ofc).
-Daniel

> 
>  include/drm/drm_bridge.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 3270fec46979..b5f3c070467c 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -177,7 +177,8 @@ struct drm_bridge_funcs {
>* pipeline has been called already. If the bridge is the first element
>* then this would be _encoder_helper_funcs.mode_set. The display
>* pipe (i.e.  clocks and timing signals) is off when this function is
> -  * called.
> +  * called. If the bridge is connected to the crtc, the adjusted_mode
> +  * parameter is the one defined in _crtc_state.adjusted_mode.
>*/
>   void (*mode_set)(struct drm_bridge *bridge,
>struct drm_display_mode *mode,
> -- 
> 2.15.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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


[Bug 198885] amdgpu.dc=1 on CIK (R4 Mullins APU) brightness impossible to change. by Fn or "echo".

2018-03-27 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=198885

--- Comment #6 from Harry Wentland (harry.wentl...@amd.com) ---
Can you try the drm-next-4.17-wip branch from
https://cgit.freedesktop.org/~agd5f/linux? This should be fixed there.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105177] amdgpu wrong colors with rx460 connected via hdmi

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105177

--- Comment #18 from Harry Wentland  ---
Can you try the drm-next-4.17-wip branch from
https://cgit.freedesktop.org/~agd5f/linux and see if that fixes the issue?

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 104090] Reduced colors on RX580 through eDP on Asus GL702ZC laptop

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=104090

Harry Wentland  changed:

   What|Removed |Added

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

--- Comment #17 from Harry Wentland  ---
Thanks for testing. I'll set this to resolved. If you still see issues feel
free to reopen.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/5] staging/vboxvideo: Use gem_free_object_unlocked

2018-03-27 Thread Greg Kroah-Hartman
On Tue, Mar 27, 2018 at 11:18:03AM +0200, Daniel Vetter wrote:
> On Tue, Mar 27, 2018 at 10:34:07AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Mar 27, 2018 at 10:23:52AM +0200, Daniel Vetter wrote:
> > > vboxvideo doesn't use dev->struct_mutex and therefore has no need to use
> > > gem_free_object.
> > > 
> > > Signed-off-by: Daniel Vetter 
> > > Cc: Greg Kroah-Hartman 
> > > Cc: Hans de Goede 
> > > Cc: Michael Thayer 
> > > Cc: Colin Ian King 
> > > Cc: Daniel Vetter 
> > > Cc: Stephen Rothwell 
> > > ---
> > >  drivers/staging/vboxvideo/vbox_drv.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > 
> > Reviewed-by: Greg Kroah-Hartman 
> 
> You'll pick this up or ack for stuffing into drm-misc (but only for 4.18)?

Feel free to take it through your tree if you want.

thanks,

greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/vmwgfx: Fix vmw_du_cursor_plane_atomic_check

2018-03-27 Thread Thomas Hellstrom

On 03/27/2018 05:08 PM, Ville Syrjälä wrote:

On Tue, Mar 27, 2018 at 04:26:17PM +0200, Thomas Hellstrom wrote:

Use the correct helper and also return early on helper
success rather than on helper failure.

Also explicitly return 0 in the case of no fb.

Signed-off-by: Thomas Hellstrom 
Reported-by: Dan Carpenter 
Reported-by: Daniel Vetter 
---
  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 3628a9fe705f..0f7dc9ea2657 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -494,23 +494,23 @@ int vmw_du_cursor_plane_atomic_check(struct drm_plane 
*plane,
 struct drm_plane_state *new_state)
  {
int ret = 0;
+   struct drm_crtc_state *crtc_state = NULL;
struct vmw_surface *surface = NULL;
struct drm_framebuffer *fb = new_state->fb;
  
-	struct drm_rect src = drm_plane_state_src(new_state);

-   struct drm_rect dest = drm_plane_state_dest(new_state);
-
/* Turning off */
if (!fb)
-   return ret;
+   return 0;

This should probably be checked after
drm_atomic_helper_check_plane_state() has been called. Otherwise
new_state->visible may be left with a stale value.



Thanks. I'll respin.

/Thomas

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/vmwgfx: Fix vmw_du_cursor_plane_atomic_check

2018-03-27 Thread Ville Syrjälä
On Tue, Mar 27, 2018 at 04:26:17PM +0200, Thomas Hellstrom wrote:
> Use the correct helper and also return early on helper
> success rather than on helper failure.
> 
> Also explicitly return 0 in the case of no fb.
> 
> Signed-off-by: Thomas Hellstrom 
> Reported-by: Dan Carpenter 
> Reported-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +++---
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 3628a9fe705f..0f7dc9ea2657 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -494,23 +494,23 @@ int vmw_du_cursor_plane_atomic_check(struct drm_plane 
> *plane,
>struct drm_plane_state *new_state)
>  {
>   int ret = 0;
> + struct drm_crtc_state *crtc_state = NULL;
>   struct vmw_surface *surface = NULL;
>   struct drm_framebuffer *fb = new_state->fb;
>  
> - struct drm_rect src = drm_plane_state_src(new_state);
> - struct drm_rect dest = drm_plane_state_dest(new_state);
> -
>   /* Turning off */
>   if (!fb)
> - return ret;
> + return 0;

This should probably be checked after
drm_atomic_helper_check_plane_state() has been called. Otherwise
new_state->visible may be left with a stale value.

>  
> - ret = drm_plane_helper_check_update(plane, new_state->crtc, fb,
> - , ,
> - DRM_MODE_ROTATE_0,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - true, true, _state->visible);
> - if (!ret)
> + if (new_state->crtc)
> + crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> +new_state->crtc);
> +
> + ret = drm_atomic_helper_check_plane_state(new_state, crtc_state,
> +   DRM_PLANE_HELPER_NO_SCALING,
> +   DRM_PLANE_HELPER_NO_SCALING,
> +   true, true);
> + if (ret)
>   return ret;
>  
>   /* A lot of the code assumes this */
> -- 
> 2.14.3
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2] gpu: drm: nouveau: Use list_for_each_entry_from_reverse

2018-03-27 Thread Arushi Singhal
It's better to use "list_for_each_entry_from_reverse" for iterating list
than "for loop" as it makes the code more clear to read.
This patch replace "for loop" with "list_for_each_entry_from_reverse"
and "start" variable with "cstate" which helps in refactoring
the code and also "cstate" variable is more commonly used in the other
functions.

Signed-off-by: Arushi Singhal 
---
changes in v2:
"start" variable is removed, before "cstate" variable was removed
but "cstate" is more common so preferred "cstate" over "start".

 drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
index 81c3567..ba6a868 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
@@ -109,18 +109,17 @@ nvkm_cstate_valid(struct nvkm_clk *clk, struct 
nvkm_cstate *cstate,
 
 static struct nvkm_cstate *
 nvkm_cstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *pstate,
- struct nvkm_cstate *start)
+ struct nvkm_cstate *cstate)
 {
struct nvkm_device *device = clk->subdev.device;
struct nvkm_volt *volt = device->volt;
-   struct nvkm_cstate *cstate;
int max_volt;
 
-   if (!pstate || !start)
+   if (!pstate || !cstate)
return NULL;
 
if (!volt)
-   return start;
+   return cstate;
 
max_volt = volt->max_uv;
if (volt->max0_id != 0xff)
@@ -133,8 +132,7 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct 
nvkm_pstate *pstate,
max_volt = min(max_volt,
   nvkm_volt_map(volt, volt->max2_id, clk->temp));
 
-   for (cstate = start; >head != >list;
-cstate = list_prev_entry(cstate, head)) {
+   list_for_each_entry_from_reverse(cstate, >list, head) {
if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
break;
}
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v4] drm/msm: Use drm_private_obj/state instead of subclassing

2018-03-27 Thread Sean Paul
Now that we have private state handled by the core, we can use those
instead of rolling our own swap_state for private data.

Originally posted here: https://patchwork.freedesktop.org/patch/211361/

Changes in v2:
 - Use state->state in disp duplicate_state callback (Jeykumar)
Changes in v3:
 - Update comment describing msm_kms_state (Jeykumar)
Changes in v4:
 - Rebased on msm-next
 - Don't always use private state from atomic state (Archit)
 - Renamed some of the state accessors 
 - Tested on mdp5 db410c

Cc: Jeykumar Sankaran 
Cc: Archit Taneja 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c   | 77 ++-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h   | 11 +--
 drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 12 ++-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c  | 28 ---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c   | 19 +++--
 drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h   |  4 +-
 drivers/gpu/drm/msm/msm_atomic.c   | 37 -
 drivers/gpu/drm/msm/msm_drv.c  | 87 +-
 drivers/gpu/drm/msm/msm_drv.h  |  3 -
 drivers/gpu/drm/msm/msm_kms.h  | 21 --
 10 files changed, 183 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 6d8e3a9a6fc0..366670043190 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -70,60 +70,62 @@ static int mdp5_hw_init(struct msm_kms *kms)
return 0;
 }
 
-struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
+struct mdp5_state *mdp5_state_from_atomic(struct drm_atomic_state *state)
 {
-   struct msm_drm_private *priv = s->dev->dev_private;
-   struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
-   struct msm_kms_state *state = to_kms_state(s);
-   struct mdp5_state *new_state;
-   int ret;
+   struct msm_kms_state *kms_state = msm_kms_state_from_atomic(state);
 
-   if (state->state)
-   return state->state;
+   if (IS_ERR_OR_NULL(kms_state))
+   return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
 
-   ret = drm_modeset_lock(_kms->state_lock, s->acquire_ctx);
-   if (ret)
-   return ERR_PTR(ret);
+   return kms_state->state;
+}
 
-   new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
-   if (!new_state)
-   return ERR_PTR(-ENOMEM);
+struct mdp5_state *mdp5_state_from_dev(struct drm_device *dev)
+{
+   struct msm_kms_state *kms_state = msm_kms_state_from_dev(dev);
 
-   /* Copy state: */
-   new_state->hwpipe = mdp5_kms->state->hwpipe;
-   new_state->hwmixer = mdp5_kms->state->hwmixer;
-   if (mdp5_kms->smp)
-   new_state->smp = mdp5_kms->state->smp;
+   if (IS_ERR_OR_NULL(kms_state))
+   return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
 
-   state->state = new_state;
+   return kms_state->state;
+}
+
+static void *mdp5_duplicate_state(void *state)
+{
+   if (!state)
+   return kzalloc(sizeof(struct mdp5_state), GFP_KERNEL);
 
-   return new_state;
+   return kmemdup(state, sizeof(struct mdp5_state), GFP_KERNEL);
 }
 
-static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state 
*state)
+static void mdp5_destroy_state(void *state)
 {
-   struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
-   swap(to_kms_state(state)->state, mdp5_kms->state);
+   struct mdp5_state *mdp_state = (struct mdp5_state *)state;
+   kfree(mdp_state);
 }
 
-static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state 
*state)
+static void mdp5_prepare_commit(struct msm_kms *kms,
+   struct drm_atomic_state *old_state)
 {
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+   struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
struct device *dev = _kms->pdev->dev;
 
pm_runtime_get_sync(dev);
 
if (mdp5_kms->smp)
-   mdp5_smp_prepare_commit(mdp5_kms->smp, _kms->state->smp);
+   mdp5_smp_prepare_commit(mdp5_kms->smp, _state->smp);
 }
 
-static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state 
*state)
+static void mdp5_complete_commit(struct msm_kms *kms,
+struct drm_atomic_state *old_state)
 {
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+   struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
struct device *dev = _kms->pdev->dev;
 
if (mdp5_kms->smp)
-   mdp5_smp_complete_commit(mdp5_kms->smp, _kms->state->smp);
+   mdp5_smp_complete_commit(mdp5_kms->smp, _state->smp);
 
pm_runtime_put_sync(dev);
 }
@@ -229,7 +231,8 @@ static const struct mdp_kms_funcs kms_funcs = {

[PULL] drm-intel-next-fixes

2018-03-27 Thread Joonas Lahtinen
Hi Dave,

Two human-reported bugs to close for display and a more rare fix
that could result in GPU hang.

There was some unclarity about the GVT pull, so I'm not including
it here.

Happy Easter holidays!

Regards, Joonas

drm-intel-next-fixes-2018-03-27:
- Display fixes for booting with MST hub lid closed and display
  freezing after hibernation (fd.o bugs 105470 & 105196)
- Fix for a very rare interrupt handling race resulting in GPU hang

The following changes since commit 33d009cd889490838c5db9b9339856c9e3d3facc:

  Merge branch 'drm-next-4.17' of git://people.freedesktop.org/~agd5f/linux 
into drm-next (2018-03-26 10:01:11 +1000)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-intel 
tags/drm-intel-next-fixes-2018-03-27

for you to fetch changes up to 300efa9eea451bdcf3b5a1eb29e06e85bb2c:

  drm/i915: Fix hibernation with ACPI S0 target state (2018-03-27 11:20:06 
+0300)


- Display fixes for booting with MST hub lid closed and display
  freezing after hibernation (fd.o bugs 105470 & 105196)
- Fix for a very rare interrupt handling race resulting in GPU hang


Chris Wilson (2):
  drm/i915: Specify which engines to reset following semaphore/event lockups
  drm/i915/execlists: Use a locked clear_bit() for synchronisation with 
interrupt

Dhinakaran Pandiyan (1):
  drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.

Imre Deak (1):
  drm/i915: Fix hibernation with ACPI S0 target state

 drivers/gpu/drm/i915/i915_drv.c| 22 ++
 drivers/gpu/drm/i915/i915_drv.h|  2 +-
 drivers/gpu/drm/i915/intel_ddi.c   |  7 ++-
 drivers/gpu/drm/i915/intel_hangcheck.c |  4 ++--
 drivers/gpu/drm/i915/intel_lrc.c   | 21 -
 5 files changed, 23 insertions(+), 33 deletions(-)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 09/10] drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers

2018-03-27 Thread Maxime Ripard
On Tue, Mar 27, 2018 at 10:41:38AM +0200, Paul Kocialkowski wrote:
> > > +int drm_sun4i_gem_create_tiled(struct drm_device *dev, void *data,
> > > +struct drm_file *file_priv);
> > 
> > Do you need it to be non-static, and part of the header as well?
> 
> Here as well, I just find that it looks more readable that way, below
> the drm driver structure definition instead of above it.

But it also creates a global symbol for no particular reason, while
we're doing the function-first-structure-later pattern pretty much
everywhere else in the kernel.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 06/10] drm/sun4i: Move and extend format-related helpers and tables

2018-03-27 Thread Maxime Ripard
On Tue, Mar 27, 2018 at 10:27:44AM +0200, Paul Kocialkowski wrote:
> > > +bool sun4i_format_is_rgb(uint32_t format);
> > > +bool sun4i_format_is_yuv(uint32_t format);
> > > +bool sun4i_format_is_yuv411(uint32_t format);
> > > +bool sun4i_format_is_yuv420(uint32_t format);
> > > +bool sun4i_format_is_yuv422(uint32_t format);
> > > +bool sun4i_format_is_yuv444(uint32_t format);
> > > +bool sun4i_format_is_packed(uint32_t format);
> > > +bool sun4i_format_is_semiplanar(uint32_t format);
> > > +bool sun4i_format_is_planar(uint32_t format);
> > > +bool sun4i_format_supports_tiling(uint32_t format);
> > 
> > If we're going to add so many of them, then we should really consider
> > to move them to drm_fourcc.c instead. Every one has some variation of
> > some of these functions, we don't really need to duplicate it all the
> > time.
> 
> Should I try to get that through in this patchset and have sun4i-drm
> be their first user? Also, does introducing such a change require
> identifying duplicates of these functions in each DRM driver's
> codebase?

I guess converting at least one of them would prove how usable it
would be to other drivers, but I won't ask you to convert all of them
as part of this serie :)

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105712] intel-gpu-overlay is showing insane power consumption amounts

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105712

--- Comment #9 from Chris Wilson  ---
Created attachment 138378
  --> https://bugs.freedesktop.org/attachment.cgi?id=138378=edit
Use setlocale("C") around strtod

Please try the attached patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[git pull] amdkfd next 4.17

2018-03-27 Thread Oded Gabbay
Hi Dave,

Last pull for 4.17. Highlights:

- GPUVM support for dGPUs
- KFD events support for dGPUs
- Fix live-lock situation when restoring multiple evicted processes
- Fix VM page table allocation on large-bar systems
- Fix for build failure on frv architecture

The following changes since commit 33d009cd889490838c5db9b9339856c9e3d3facc:

  Merge branch 'drm-next-4.17' of git://people.freedesktop.org/~agd5f/linux 
into drm-next (2018-03-26 10:01:11 +1000)

are available in the Git repository at:

  git://people.freedesktop.org/~gabbayo/linux tags/drm-amdkfd-next-2018-03-27

for you to fetch changes up to 1679ae8f8f4148766423066aeb3dbb0a985a373a:

  drm/amdkfd: Use ordered workqueue to restore processes (2018-03-23 15:30:36 
-0400)


Arnd Bergmann (1):
  drm/amdkfd: fix uninitialized variable use

Felix Kuehling (15):
  drm/amdgpu: Move KFD-specific fields into struct amdgpu_vm
  drm/amdgpu: Fix initial validation of PD BO for KFD VMs
  drm/amdgpu: Add helper to turn an existing VM into a compute VM
  drm/amdgpu: Add kfd2kgd interface to acquire an existing VM
  drm/amdkfd: Create KFD VMs on demand
  drm/amdkfd: Remove limit on number of GPUs
  drm/amdkfd: Aperture setup for dGPUs
  drm/amdkfd: Add per-process IDR for buffer handles
  drm/amdkfd: Allocate CWSR trap handler memory for dGPUs
  drm/amdkfd: Add TC flush on VMID deallocation for Hawaii
  drm/amdkfd: Add ioctls for GPUVM memory management
  drm/amdkfd: Kmap event page for dGPUs
  drm/amdkfd: Add module option for testing large-BAR functionality
  drm/amdgpu: Fix acquiring VM on large-BAR systems
  drm/amdkfd: Use ordered workqueue to restore processes

Oak Zeng (1):
  drm/amdkfd: Populate DRM render device minor

Oded Gabbay (1):
  drm/amdkfd: add missing include of mm.h

 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h |  28 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c  |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c  |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c   | 249 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |  73 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  10 +
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c   | 532 +
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c  |   5 +-
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  |  22 +-
 drivers/gpu/drm/amd/amdkfd/kfd_events.c|  31 +-
 drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c   |  59 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_module.c|  11 +-
 drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c|  37 ++
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h  |  39 +-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c   | 334 -
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c  |   4 +
 drivers/gpu/drm/amd/amdkfd/kfd_topology.h  |   1 +
 drivers/gpu/drm/amd/include/kgd_kfd_interface.h|   4 +
 include/uapi/linux/kfd_ioctl.h | 122 -
 19 files changed, 1398 insertions(+), 165 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/vmwgfx: Fix vmw_du_cursor_plane_atomic_check

2018-03-27 Thread Thomas Hellstrom
Use the correct helper and also return early on helper
success rather than on helper failure.

Also explicitly return 0 in the case of no fb.

Signed-off-by: Thomas Hellstrom 
Reported-by: Dan Carpenter 
Reported-by: Daniel Vetter 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 3628a9fe705f..0f7dc9ea2657 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -494,23 +494,23 @@ int vmw_du_cursor_plane_atomic_check(struct drm_plane 
*plane,
 struct drm_plane_state *new_state)
 {
int ret = 0;
+   struct drm_crtc_state *crtc_state = NULL;
struct vmw_surface *surface = NULL;
struct drm_framebuffer *fb = new_state->fb;
 
-   struct drm_rect src = drm_plane_state_src(new_state);
-   struct drm_rect dest = drm_plane_state_dest(new_state);
-
/* Turning off */
if (!fb)
-   return ret;
+   return 0;
 
-   ret = drm_plane_helper_check_update(plane, new_state->crtc, fb,
-   , ,
-   DRM_MODE_ROTATE_0,
-   DRM_PLANE_HELPER_NO_SCALING,
-   DRM_PLANE_HELPER_NO_SCALING,
-   true, true, _state->visible);
-   if (!ret)
+   if (new_state->crtc)
+   crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
+  new_state->crtc);
+
+   ret = drm_atomic_helper_check_plane_state(new_state, crtc_state,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
+   if (ret)
return ret;
 
/* A lot of the code assumes this */
-- 
2.14.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 100069] Dirt: Showdown bad performance with enabled advanced lightning

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100069

--- Comment #9 from Gregor Münch  ---
Found YouTube video on Nvidia showing same settings, including advanced
lightning working flawless and good speed. https://youtu.be/hP4V5W-IRJ0?t=4m49s


However in a phoronix article, Michael is showing ingame pictures showing cars
with obviously missing textures. (Yet he claims that the game is looking good
and the same for both vendors) it's the same kind of corruption I see with
activated advanced lightning btw. Though I additionally see over bright lights.
https://www.phoronix.com/scan.php?page=article=dirt-showdown-linux=4
The performance with catalyst was still way better than Mesa.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 100069] Dirt: Showdown bad performance with enabled advanced lightning

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100069

--- Comment #8 from Gregor Münch  ---
All graphic options are set to the maximum possible value including MSAA.

Its not only about low performance, once advanced lightning is set, the graphic
is also seriously bugged, including wrong textures.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 2/5] drm: bridge: add API to query the expected input formats of bridges

2018-03-27 Thread Peter Rosin
On 2018-03-27 11:47, jacopo mondi wrote:
>> + * RETURNS:
>> + * The number of bus input formats the bridge accepts. Zero means that
>> + * the chain of bridges are not converting the bus format and that the
>> + * format of the drm_connector should be used.
> 
> How do we get to the connector format from a bridge that has maybe
> other components in between in the pipeline?

Forgot to write something here in my previous reply, sorry...

The chain of bridges do not end with the connector in the in-kernel
representation, IIUC. So, I think it is up to the caller to find the
format desired by the connector. See patch [5/5] for an example.

Maybe the connector should be passed in as an argument?

Cheers,
Peter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] gpu: host1x: Fix compiler errors

2018-03-27 Thread Emil Goode
The compiler is complaining with the following errors:

drivers/gpu/host1x/cdma.c:94:48: error:
passing argument 3 of ‘dma_alloc_wc’ from incompatible pointer type
[-Werror=incompatible-pointer-types]

drivers/gpu/host1x/cdma.c:113:48: error:
passing argument 3 of ‘dma_alloc_wc’ from incompatible pointer type
[-Werror=incompatible-pointer-types]

The expected pointer type of the third argument to dma_alloc_wc() is
dma_addr_t but phys_addr_t is passed. Fix this by adding casts to the
expected pointer type.

Signed-off-by: Emil Goode 
---
 drivers/gpu/host1x/cdma.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
index 28541b280739..5e8b321a751e 100644
--- a/drivers/gpu/host1x/cdma.c
+++ b/drivers/gpu/host1x/cdma.c
@@ -91,8 +91,8 @@ static int host1x_pushbuffer_init(struct push_buffer *pb)
 
size = iova_align(>iova, size);
 
-   pb->mapped = dma_alloc_wc(host1x->dev, size, >phys,
- GFP_KERNEL);
+   pb->mapped = dma_alloc_wc(host1x->dev, size,
+ (dma_addr_t *)>phys, GFP_KERNEL);
if (!pb->mapped)
return -ENOMEM;
 
@@ -110,8 +110,8 @@ static int host1x_pushbuffer_init(struct push_buffer *pb)
if (err)
goto iommu_free_iova;
} else {
-   pb->mapped = dma_alloc_wc(host1x->dev, size, >phys,
- GFP_KERNEL);
+   pb->mapped = dma_alloc_wc(host1x->dev, size,
+ (dma_addr_t *)>phys, GFP_KERNEL);
if (!pb->mapped)
return -ENOMEM;
 
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 5/5] drm/atmel-hlcdc: take bridges into account when selecting output format

2018-03-27 Thread Peter Rosin
Bridges may affect the required bus output format of the encoder, in
which case it may be wrong to use the output format of the panel or
connector as is. So, examine if any of the intermediate bridges needs
specific bus formats (if there are intermediate bridges).

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 49 --
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index d73281095fac..920eb16c0213 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -226,6 +226,45 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc 
*c,
 #define ATMEL_HLCDC_RGB888_OUTPUT  BIT(3)
 #define ATMEL_HLCDC_OUTPUT_MODE_MASK   GENMASK(3, 0)
 
+/* We know that our connectors will only ever have one encoder. Find it. */
+static struct drm_encoder *atmel_hlcdc_encoder(struct drm_connector *connector)
+{
+   struct drm_encoder *encoder;
+   int i;
+
+   for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
+   if (!connector->encoder_ids[i])
+   break;
+
+   encoder = drm_encoder_find(connector->dev, NULL,
+  connector->encoder_ids[i]);
+   if (encoder)
+   return encoder;
+   }
+
+   return NULL;
+}
+
+static int atmel_hlcdc_output_formats(struct drm_connector *connector,
+ const u32 **bus_formats)
+{
+   struct drm_encoder *encoder = atmel_hlcdc_encoder(connector);
+   int num_bus_formats;
+
+   if (!encoder)
+   return 0;
+
+   if (encoder->bridge) {
+   num_bus_formats = drm_bridge_input_formats(encoder->bridge,
+  bus_formats);
+   if (num_bus_formats)
+   return num_bus_formats;
+   }
+
+   *bus_formats = connector->display_info.bus_formats;
+   return connector->display_info.num_bus_formats;
+}
+
 static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
 {
unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
@@ -238,15 +277,19 @@ static int atmel_hlcdc_crtc_select_output_mode(struct 
drm_crtc_state *state)
crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
 
for_each_new_connector_in_state(state->state, connector, cstate, i) {
-   struct drm_display_info *info = >display_info;
+   int num_bus_formats;
+   const u32 *bus_formats;
unsigned int supported_fmts = 0;
int j;
 
+   num_bus_formats = atmel_hlcdc_output_formats(connector,
+_formats);
+
if (!cstate->crtc)
continue;
 
-   for (j = 0; j < info->num_bus_formats; j++) {
-   switch (info->bus_formats[j]) {
+   for (j = 0; j < num_bus_formats; j++) {
+   switch (bus_formats[j]) {
case MEDIA_BUS_FMT_RGB444_1X12:
supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
break;
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH libdrm 1/2] intel: Do not use libpciaccess on Android

2018-03-27 Thread Tomasz Figa
On Wed, Mar 21, 2018 at 2:36 AM, Emil Velikov  wrote:
> From: Tomasz Figa 
>
> This patch makes the code not rely anymore on libpciaccess when compiled
> for Android to eliminate ioperm() and iopl() syscalls required by that
> library. As a side effect, the mappable aperture size is hardcoded to 64
> MiB on Android, however nothing seems to rely on this value anyway, as
> checked be grepping relevant code in drm_gralloc and Mesa.
>
> Cc: John Stultz 
> Cc: Rob Herring 
> Cc: John Stultz 
> Cc: Tomasz Figa 
> Signed-off-by: Tomasz Figa 
> [Emil Velikov: rebase against master]
> Signed-off-by: Emil Velikov 
> ---
> Tomasz, I've taken the liberty of pulling the patch from the Android
> tree. Hope you don't mind.

Thanks Emil for digging it up. I have no objections.

For reference, we used this as a quick hack before moving build of
graphics components to Chrome OS side. After that, for short time, we
had libpciaccess being built with autotools (and some small patch
disabling port IO related stuff). Eventually we got rid of it
completely, as Mesa stopped using libdrm_intel for i965 (and we don't
use i915).

Best regards,
Tomasz
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 3/5] drm: of: add display bus-format parser

2018-03-27 Thread Peter Rosin
Add a common API to parse display bus format strings into fourcc codes.

Signed-off-by: Peter Rosin 
---
 .../devicetree/bindings/display/bus-format.txt | 35 +
 drivers/gpu/drm/drm_of.c   | 59 ++
 include/drm/drm_of.h   |  9 
 3 files changed, 103 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bus-format.txt

diff --git a/Documentation/devicetree/bindings/display/bus-format.txt 
b/Documentation/devicetree/bindings/display/bus-format.txt
new file mode 100644
index ..590e6c73f3dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bus-format.txt
@@ -0,0 +1,35 @@
+Bus formats in the display pipe
+===
+
+Various encoders in display controllers output the pixels in different
+formats. Circuits handling display connectors and hardwired panels also
+expect pixel input in various formats. We call these formats bus formats.
+
+Some bus formats are:
+
+Parallel
+
+
+rgb888
+   8 parallel lines for red, green and blue respectively. There are
+   also lines for the pixel-clock, horizontal-sync, vertical-sync
+   and data-enable.
+
+rgb666
+   Same as rgb888, but with 6 lines per color.
+
+rgb565
+   Same as rgb888, but with 6 green lines and 5 red and blue lines.
+
+rgb444
+   Same as rgb888, but with 4 lines per color.
+
+
+LVDS
+
+
+jeida-18
+jeida-24
+vesa-24
+   These are LVDS bus formats, see the data-mapping property in
+   panel/panel-lvds.txt for a description of these bus formats.
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 4c191c050e7d..5f65471225bb 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -262,3 +262,62 @@ int drm_of_find_panel_or_bridge(const struct device_node 
*np,
return ret;
 }
 EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge);
+
+/*
+ * drm_of_bus_formats - parse list of bus format strings into drm fourcc
+ * @np: device tree node containing bus format property
+ * @propname: name of bus format property
+ * @bus_formats: array of parsed fourcc codes
+ *
+ * On success, @bus_formats points to a location where the actual drm
+ * fourcc codes are.
+ * WARNING: The caller is responsible for freeing this memory with kfree.
+ *
+ * Returns the number of parsed bus format entries, or one of the standard
+ * error codes on failure.
+ */
+int drm_of_bus_formats(const struct device_node *np, const char *propname,
+  u32 **bus_formats)
+{
+   int num_bus_formats = of_property_count_strings(np, propname);
+   const char *fmt;
+   int ret;
+   int i;
+
+   if (num_bus_formats <= 0)
+   return num_bus_formats;
+
+   *bus_formats = kmalloc(num_bus_formats * sizeof(**bus_formats),
+  GFP_KERNEL);
+   if (!*bus_formats)
+   return -ENOMEM;
+
+   for (i = 0; i < num_bus_formats; ++i) {
+   ret = of_property_read_string_index(np, propname, i, );
+   if (ret < 0)
+   return ret;
+
+   if (!strcmp(fmt, "rgb444")) {
+   *bus_formats[i] = MEDIA_BUS_FMT_RGB444_1X12;
+   } else if (!strcmp(fmt, "rgb565")) {
+   *bus_formats[i] = MEDIA_BUS_FMT_RGB565_1X16;
+   } else if (!strcmp(fmt, "rgb666")) {
+   *bus_formats[i] = MEDIA_BUS_FMT_RGB666_1X18;
+   } else if (!strcmp(fmt, "rgb888")) {
+   *bus_formats[i] = MEDIA_BUS_FMT_RGB888_1X24;
+   } else if (!strcmp(fmt, "jeida-18")) {
+   *bus_formats[i] = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG;
+   } else if (!strcmp(fmt, "jeida-24")) {
+   *bus_formats[i] = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
+   } else if (!strcmp(fmt, "vesa-24")) {
+   *bus_formats[i] = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
+   } else {
+   kfree(*bus_formats);
+   *bus_formats = NULL;
+   return -EINVAL;
+   }
+   }
+
+   return num_bus_formats;
+}
+EXPORT_SYMBOL_GPL(drm_of_bus_formats);
diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
index b93c239afb60..ccacddc03a2e 100644
--- a/include/drm/drm_of.h
+++ b/include/drm/drm_of.h
@@ -33,6 +33,8 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
int port, int endpoint,
struct drm_panel **panel,
struct drm_bridge **bridge);
+int drm_of_bus_formats(const struct device_node *np, const char *propname,
+  u32 **bus_formats);
 #else
 static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
  struct device_node *port)
@@ -69,6 +71,13 @@ static 

[PATCH v2 1/5] dt-bindings: display: bridge: lvds-transmitter: add ti, ds90c185

2018-03-27 Thread Peter Rosin
Start list of actual chips compatible with "lvds-encoder".

Signed-off-by: Peter Rosin 
---
 .../devicetree/bindings/display/bridge/lvds-transmitter.txt   | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git 
a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt 
b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index fd39ad34c383..50220190c203 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -22,7 +22,13 @@ among others.
 
 Required properties:
 
-- compatible: Must be "lvds-encoder"
+- compatible: Must be one or more of the following
+  - "ti,ds90c185" for the TI DS90C185 FPD-Link Serializer
+  - "lvds-encoder" for a generic LVDS encoder device
+
+  When compatible with the generic version, nodes must list the
+  device-specific version corresponding to the device first
+  followed by the generic version.
 
 Required nodes:
 
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 4/5] drm: bridge: lvds-encoder: allow specifying the input bus format

2018-03-27 Thread Peter Rosin
Hi Jacopo,

Thanks for you feedback!

On 2018-03-27 12:27, jacopo mondi wrote:
> Hi Peter, Laurent,
>thanks for the patches,
> 
> On Mon, Mar 26, 2018 at 11:24:46PM +0200, Peter Rosin wrote:
>> If the bridge changes the bus format, allow this to be described in
>> the bridge, instead of providing false information about the bus
>> format of the connector or panel.
>>
>> Signed-off-by: Peter Rosin 
>> ---
>>  .../bindings/display/bridge/lvds-transmitter.txt   |  6 ++
>>  drivers/gpu/drm/bridge/lvds-encoder.c  | 25 
>> ++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt 
>> b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>> index 50220190c203..8d40a2069252 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>> @@ -30,6 +30,12 @@ Required properties:
>>device-specific version corresponding to the device first
>>followed by the generic version.
>>
>> +Optional properties:
>> +
>> +- interface-pix-fmt:
>> +  List of valid input bus formats of the encoder. Recognized bus formats
>> +  are listed in ../bus-format.txt
>> +
> 
> This comments applies here and to [3/5] as well.

I'm not sure how the below comments apply to [3/5]? I guess you are
suggesting that [3/5] should be dropped?

> Are we sure we want the supported bridge input format defined in DT
> bindings?

Yes, I'm pretty sure. In my case, it has to be specified manually
*somewhere* (at least I think it has to, but what do I know?). The
bridge is the best position, if you ask me.

> Again, I may be biased, but as I see this, each bridge driver should
> list its supported formats with MEDIA_BUS_FMT_ fourcc codes and return
> the currently 'active' one when requested by the preceding bridge.

In my case this is not possible, the bridge supports maximum rgb888
input, but since it cannot know how much of that is actually wired,
the actual format needs to be provided manually somewhere. In my
case, I know that I want to send rgb565 to the bridge. Sure, the
wiring between the encoder and the bridge chip could be seen as
another "bridge" that goes from rgb565 to rgb888, but adding some
extra layer to the model for this step seems like over-engineering
to me. Especially when the binding for the generic lvds-transmitter
bridge needs to specify the input format anyway. At least, that's
the right thing to do if you ask me. How else should it know what
format to request?

> I understand for this driver, being compatible with both a generic lvds
> encoder and a specific chip, the supported input formats are
> different, but I would have used the 'of_device_id.data' field for
> this and leave this out from DT bindings.

No, the lvds-transmitter accepts parallel input and spits out lvds,
just like the specific chip I'm using. I do not *need* to specify the
specific chip, and the driver does nothing special for it. It is just
good practice to specify the details when they are readily available.

> This makes the translation routine here implemented not required if
> I'm not wrong...

I think you are.

Cheers,
Peter

> Thanks
>j
> 
>>  Required nodes:
>>
>>  This device has two video ports. Their connections are modeled using the OF
>> diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c 
>> b/drivers/gpu/drm/bridge/lvds-encoder.c
>> index 75b0d3f6e4de..b78619b5560a 100644
>> --- a/drivers/gpu/drm/bridge/lvds-encoder.c
>> +++ b/drivers/gpu/drm/bridge/lvds-encoder.c
>> @@ -9,6 +9,7 @@
>>
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>
>>  #include 
>> @@ -16,6 +17,8 @@
>>  struct lvds_encoder {
>>  struct drm_bridge bridge;
>>  struct drm_bridge *panel_bridge;
>> +int num_bus_formats;
>> +u32 *bus_formats;
>>  };
>>
>>  static int lvds_encoder_attach(struct drm_bridge *bridge)
>> @@ -28,8 +31,22 @@ static int lvds_encoder_attach(struct drm_bridge *bridge)
>>   bridge);
>>  }
>>
>> +static int lvds_encoder_input_formats(struct drm_bridge *bridge,
>> +  const u32 **bus_formats)
>> +{
>> +struct lvds_encoder *lvds_encoder = container_of(bridge,
>> + struct lvds_encoder,
>> + bridge);
>> +
>> +if (lvds_encoder->num_bus_formats)
>> +*bus_formats = lvds_encoder->bus_formats;
>> +
>> +return lvds_encoder->num_bus_formats;
>> +}
>> +
>>  static struct drm_bridge_funcs funcs = {
>>  .attach = lvds_encoder_attach,
>> +.input_formats = lvds_encoder_input_formats,
>>  };
>>
>>  static int lvds_encoder_probe(struct platform_device *pdev)
>> @@ -39,6 +56,7 @@ static int lvds_encoder_probe(struct platform_device *pdev)
>>  struct device_node *panel_node;
>>  struct drm_panel 

Re: [PATCH v2 2/5] drm: bridge: add API to query the expected input formats of bridges

2018-03-27 Thread Peter Rosin
Hi Jacopo,

Thanks for the feedback!

On 2018-03-27 11:47, jacopo mondi wrote:
> Hi Peter,
>thanks for the patches
> 
> On Mon, Mar 26, 2018 at 11:24:44PM +0200, Peter Rosin wrote:
>> Bridges may affect the required bus output format of the encoder, in
>> which case it may be wrong to use the output format of the panel or
>> connector as is. Add infrastructure to address this problem.
> 
> Bridges not only affect the format expected by the connector at the
> end of the display pipeline, they may perform encoding/decoding
> between protocols and their accepted input formats may be unrelated to
> the connector at the end of the pipeline as there may an arbitrary
> number of bridges in between.
> 
> Eg.
> 
> ENCODERCONNECTOR
>   | |
> |DU LVDS| ->lvds-> |THC63| ->rgb-> |ADV7511| ->hdmi-> |HDMI connector|
> 
> The fact that THC63 wants a specific LVDS input format is unrelated to
> the format required by the HDMI connector at the end of the pipeline.
> 
> I would just state here that bridges need a way to report their
> accepted media bus formats, and this patch extends the DRM Bridge APIs
> to implement that.

Yes, I can add some words, but I would like to clear up the naming
before re-spinning.

>>
>> Signed-off-by: Peter Rosin 
>> ---
>>  drivers/gpu/drm/drm_bridge.c | 32 
>>  include/drm/drm_bridge.h | 18 ++
>>  2 files changed, 50 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>> index 1638bfe9627c..f85e61b7416e 100644
>> --- a/drivers/gpu/drm/drm_bridge.c
>> +++ b/drivers/gpu/drm/drm_bridge.c
>> @@ -348,6 +348,38 @@ void drm_bridge_enable(struct drm_bridge *bridge)
>>  }
>>  EXPORT_SYMBOL(drm_bridge_enable);
>>
>> +/**
>> + * drm_bridge_input_formats - get the expected bus input format of the 
>> bridge
> 
> I may be biased by the V4L2 APIs, but this seems to me very much like
> similar to g_fmt/s_fmt callbacks we have there. Bridges have an input and

g_fmt/s_fmt says nothing to me. Graphics format? Source Format? I have
no idea if those guesses are even close? So, that seems like poor naming
to me. The only reason I see to go that way is for the sake of consistency.

> an output formats, and that calls for something that takes that into
> account, as well as they may have different input ports with different
> accepted formats but I would for now simplify this to just 'g_fmt()'

You mean rename the function to drm_bridge_g_fmt(), right?

As indicated above, I'm not that fond of it, but don't really care
either. Maintainers?

>> + * @bridge: bridge control structure
>> + * @bus_formats: where to store a pointer to the bus input formats
>> + *
>> + * Calls the _bridge_funcs.input_formats op for the frist bridge in the
>> + * chain that has registered this op.
> 
> I'm not sure passing the call down the pipeline is desirable. Each
> component should make sure its output format is accepted as the next
> bridge input format, passing the call to the next bridge is not
> different that getting to the connector at the end of the pipeline and
> return to the initial caller its supported format. Do you agree with this?

Not passing down the call does one of two things. Either all bridges have
to implement the call or all users have to walk the pipeline "manually".
I don't like either or those options, so I still think it is good to
pass the call down the pipeline.

*time passes*

Oh, you do not think it is useful for the bridge to have a callback but
still report "no format change", and that the call down to pipeline
should only happen for bridges that have not implemented the op. But I
do think that is useful, see below.

>> + *
>> + * Note that the bridge passed should normally be the bridge closest to the
>> + * encoder, but possibly the bridge closest to an intermediate bridge in
>> + * convoluted cases.
>> + *
> 
> As I see this, any bridge in the arbitrary long pipeline should call
> this operation on next bridge if it supports different output formats.
> Ie. I would not name here the encoder nor refer to the bridge position
> in the pipeline.

Ok, I can change that, it just seemed like a convoluted case to me.
I mean, if this was a real issue and complicated pipelines were
common, something along the lines of this patch series would be
available already. Right? I guess not, but how the 

Re: [PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

2018-03-27 Thread Simon Horman
On Fri, Mar 23, 2018 at 09:16:13PM +, Kieran Bingham wrote:
> Hi Simon,
> 
> On 23/03/18 08:51, Simon Horman wrote:
> > On Thu, Mar 22, 2018 at 09:30:40PM +, Kieran Bingham wrote:
> >> The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
> >> bus, however in low power mode the ADV7513 will reset it's slave maps to
> >> use the hardware defined default addresses.
> >>
> >> The ADV7511 driver was adapted to allow the two devices to be registered
> >> correctly - but it did not take into account the fault whereby the
> >> devices reset the addresses.
> >>
> >> This results in an address conflict between the device using the default
> >> addresses, and the other device if it is in low-power-mode.
> >>
> >> Repair this issue by moving both devices away from the default address
> >> definitions.
> > 
> > Hi Kierean,
> > 
> > as this is a fix
> > a) Does it warrant a fixes tag?
> >Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
> > b) Does it warrant being posted as a fix for v4.16;
> > c) or v4.17?
> 
> Tricky one, yes it could but this DTS fix, will only actually 'fix' the issue 
> if
> the corresponding driver updates to allow secondary addresses to be parsed are
> also backported.
> 
> It should be safe to back port the dts fix without the driver updates, but the
> addresses specified by this patch will simply be ignored.

In that case I think its safe to add the fixes tag and take the DTS patch
via the renesas tree. Perhaps applying it for v4.18 and allowing automatic
backporting to take its course is the cleanest option.

> Thus if this is marked with the fixes tag the corresponding patch "drm: 
> adv7511:
> Add support for i2c_new_secondary_device" should also be marked.
> 
> It looks like that patch has yet to be picked up by the DRM subsystem, so how
> about I bundle both of these two patches together in a repost along with the
> fixes tag.
> 
> In fact, I don't think the ADV7511 dt-bindings update has made any progress
> either. (dt-bindings: adv7511: Extend bindings to allow specifying slave map
> addresses). The media tree variants for the adv7604 have already been picked 
> up
> by Mauro I believe though.
> 
> I presume it would be acceptable for this dts patch (or rather all three 
> patches
> mentioned) to get integrated through the DRM tree ?

Unless there is a strong reason I would prefer the dts patch to go via
my tree. The reason is to avoid merge conflicts bubbling up to Linus,
which really is something best avoided.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 0/5] allow override of bus format in bridges

2018-03-27 Thread Peter Rosin
Hi!

[I got to v2 sooner than expected]

I have an Atmel sama5d31 hooked up to an lvds encoder and then
on to an lvds panel. Which seems like something that has been
done one or two times before...

The problem is that the bus_format of the SoC and the panel do
not agree. The SoC driver (atmel-hlcdc) can handle the
rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
wired for the rgb565 case. The lvds encoder supports rgb888 on
its input side with the LSB wires for each color simply pulled
down internally in the encoder in my case which means that the
rgb565 bus_format is the format that works best. And the panel
is expecting lvds (vesa-24), which is what the encoder outputs.

The reason I "blame" the bus_format of the drm_connector is that
with the below DT snippet, things do not work *exactly* due to
that. At least, it starts to work if I hack the panel-lvds driver
to report the rgb565 bus_format instead of vesa-24.

panel: panel {
compatible = "panel-lvds";

width-mm = <304>;
height-mm = <228;

data-mapping = "vesa-24";

panel-timing {
// 1024x768 @ 60Hz (typical)
clock-frequency = <5214 6500 7110>;
hactive = <1024>;
vactive = <768>;
hfront-porch = <48 88 88>;
hback-porch = <96 168 168>;
hsync-len = <32 64 64>;
vfront-porch = <8 13 14>;
vback-porch = <8 13 14>;
vsync-len = <8 12 14>;
};

port {
panel_input: endpoint {
remote-endpoint = <_encoder_output>;
};
};
};

lvds-encoder {
compatible = "ti,ds90c185", "lvds-encoder";

ports {
#address-cells = <1>;
#size-cells = <0>;

port@0 {
reg = <0>;

lvds_encoder_input: endpoint {
remote-endpoint = <_output>;
};
};

port@1 {
reg = <1>;

lvds_encoder_output: endpoint {
remote-endpoint = <_input>;
};
};
};
};

But, instead of perverting the panel-lvds driver with support
for a totally fake non-lvds bus_format, I intruduce an API that allows
display controller drivers to query the required bus_format of any
intermediate bridges, and match up with that instead of the formats
given by the drm_connector. I trigger this with this addition to the
lvds-encoder DT node:

interface-pix-fmt = "rgb565";

Naming is hard though, so I'm not sure if that's good?

I threw in the first patch, since that is the actual lvds encoder
I have in this case.

Suggestions welcome.

Cheers,
Peter

Changes since v1 https://lkml.org/lkml/2018/3/17/221
- Add a proper bridge API to query the bus_format instead of abusing
  the ->get_modes part of the code. This is cleaner but requires
  changes to all display controller drivers wishing to participate.
- Add patch to adjust the atmel-hlcdc driver according to the above.
- Hook the new info into the bridge local to the lvds-encoder instead
  of messing about with new interfaces for the panel-bridge driver.
- Add patch with a DT parsing function of bus_formats in a central place.
- Rephrase the addition of ti,ds90c185 to the lvds-transmitter binding.

Peter Rosin (5):
  dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185
  drm: bridge: add API to query the expected input formats of bridges
  drm: of: add display bus-format parser
  drm: bridge: lvds-encoder: allow specifying the input bus format
  drm/atmel-hlcdc: take bridges into account when selecting output
format

 .../bindings/display/bridge/lvds-transmitter.txt   | 14 -
 .../devicetree/bindings/display/bus-format.txt | 35 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 49 --
 drivers/gpu/drm/bridge/lvds-encoder.c  | 25 +
 drivers/gpu/drm/drm_bridge.c   | 32 
 drivers/gpu/drm/drm_of.c   | 59 ++
 include/drm/drm_bridge.h   | 18 +++
 include/drm/drm_of.h   |  9 
 8 files changed, 237 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bus-format.txt

-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org

Re: [PATCH] drm/mxsfb: add poll_changed event handler to set_par on startup

2018-03-27 Thread Michael Grzeschik
On Mon, Mar 26, 2018 at 06:17:44PM +0200, Michael Grzeschik wrote:
> We move drm_kms_helper_poll_init behind the drm_fbdev_cma_init so the
> set_par will be called and fb will be active.
> 

As this commit message is not very informative and digging deeper into
the stubs I came up with another Idea.

By default drm_kms_helper_hotplug_event could call
drm_fb_helper_output_poll_changed if no other output_poll_changed was
registered.

I will send another patch.

> Signed-off-by: Michael Grzeschik 
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c 
> b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 1207ffe362505..a047a729af6b8 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -94,6 +94,7 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
>  
>  static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
>   .fb_create  = drm_gem_fb_create,
> + .output_poll_changed= drm_fb_helper_output_poll_changed,
>   .atomic_check   = drm_atomic_helper_check,
>   .atomic_commit  = drm_atomic_helper_commit,
>  };
> @@ -221,8 +222,6 @@ static int mxsfb_load(struct drm_device *drm, unsigned 
> long flags)
>   goto err_irq;
>   }
>  
> - drm_kms_helper_poll_init(drm);
> -
>   mxsfb->fbdev = drm_fbdev_cma_init(drm, 32,
> drm->mode_config.num_connector);
>   if (IS_ERR(mxsfb->fbdev)) {
> @@ -232,6 +231,8 @@ static int mxsfb_load(struct drm_device *drm, unsigned 
> long flags)
>   goto err_cma;
>   }
>  
> + drm_kms_helper_poll_init(drm);
> +
>   platform_set_drvdata(pdev, drm);
>  
>   drm_helper_hpd_irq_event(drm);
> -- 
> 2.16.1
> 
> 
> 

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 2/5] drm: bridge: add API to query the expected input formats of bridges

2018-03-27 Thread Peter Rosin
Bridges may affect the required bus output format of the encoder, in
which case it may be wrong to use the output format of the panel or
connector as is. Add infrastructure to address this problem.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/drm_bridge.c | 32 
 include/drm/drm_bridge.h | 18 ++
 2 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 1638bfe9627c..f85e61b7416e 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -348,6 +348,38 @@ void drm_bridge_enable(struct drm_bridge *bridge)
 }
 EXPORT_SYMBOL(drm_bridge_enable);
 
+/**
+ * drm_bridge_input_formats - get the expected bus input format of the bridge
+ * @bridge: bridge control structure
+ * @bus_formats: where to store a pointer to the bus input formats
+ *
+ * Calls the _bridge_funcs.input_formats op for the frist bridge in the
+ * chain that has registered this op.
+ *
+ * Note that the bridge passed should normally be the bridge closest to the
+ * encoder, but possibly the bridge closest to an intermediate bridge in
+ * convoluted cases.
+ *
+ * RETURNS:
+ * The number of bus input formats the bridge accepts. Zero means that
+ * the chain of bridges are not converting the bus format and that the
+ * format of the drm_connector should be used.
+ */
+int drm_bridge_input_formats(struct drm_bridge *bridge,
+const u32 **bus_formats)
+{
+   int ret = 0;
+
+   if (!bridge)
+   return 0;
+
+   if (bridge->funcs->input_formats)
+   ret = bridge->funcs->input_formats(bridge, bus_formats);
+
+   return ret ?: drm_bridge_input_formats(bridge->next, bus_formats);
+}
+EXPORT_SYMBOL(drm_bridge_input_formats);
+
 #ifdef CONFIG_OF
 /**
  * of_drm_find_bridge - find the bridge corresponding to the device node in
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 682d01ba920c..ae8d3c4af0b8 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -220,6 +220,22 @@ struct drm_bridge_funcs {
 * The enable callback is optional.
 */
void (*enable)(struct drm_bridge *bridge);
+
+   /**
+* @input_formats:
+*
+* The callback reports the expected bus input formats of the bridge.
+*
+* The @input_formats callback is optional. The bridge is assumed to
+* not convert the bus format if the callback is not installed.
+*
+* RETURNS:
+*
+* Zero if the bridge does not convert the bus format, otherwise the
+* number of bus input formats returned in _formats.
+*/
+   int (*input_formats)(struct drm_bridge *bridge,
+const u32 **bus_formats);
 };
 
 /**
@@ -263,6 +279,8 @@ void drm_bridge_mode_set(struct drm_bridge *bridge,
struct drm_display_mode *adjusted_mode);
 void drm_bridge_pre_enable(struct drm_bridge *bridge);
 void drm_bridge_enable(struct drm_bridge *bridge);
+int drm_bridge_input_formats(struct drm_bridge *bridge,
+const u32 **bus_formats);
 
 #ifdef CONFIG_DRM_PANEL_BRIDGE
 struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/mxsfb: add poll_changed event handler to set_par on startup

2018-03-27 Thread Michael Grzeschik
We move drm_kms_helper_poll_init behind the drm_fbdev_cma_init so the
set_par will be called and fb will be active.

Signed-off-by: Michael Grzeschik 
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c 
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 1207ffe362505..a047a729af6b8 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -94,6 +94,7 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
 
 static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
.fb_create  = drm_gem_fb_create,
+   .output_poll_changed= drm_fb_helper_output_poll_changed,
.atomic_check   = drm_atomic_helper_check,
.atomic_commit  = drm_atomic_helper_commit,
 };
@@ -221,8 +222,6 @@ static int mxsfb_load(struct drm_device *drm, unsigned long 
flags)
goto err_irq;
}
 
-   drm_kms_helper_poll_init(drm);
-
mxsfb->fbdev = drm_fbdev_cma_init(drm, 32,
  drm->mode_config.num_connector);
if (IS_ERR(mxsfb->fbdev)) {
@@ -232,6 +231,8 @@ static int mxsfb_load(struct drm_device *drm, unsigned long 
flags)
goto err_cma;
}
 
+   drm_kms_helper_poll_init(drm);
+
platform_set_drvdata(pdev, drm);
 
drm_helper_hpd_irq_event(drm);
-- 
2.16.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] gpu: host1x: Fix compiler errors

2018-03-27 Thread Emil Goode
Hello,

On Mon, Mar 26, 2018 at 04:57:34PM +0200, Thierry Reding wrote:
> On Mon, Mar 26, 2018 at 04:44:14PM +0200, Emil Goode wrote:
> > The compiler is complaining with the following errors:
> > 
> > drivers/gpu/host1x/cdma.c:94:48: error:
> > passing argument 3 of ‘dma_alloc_wc’ from incompatible pointer type
> > [-Werror=incompatible-pointer-types]
> > 
> > drivers/gpu/host1x/cdma.c:113:48: error:
> > passing argument 3 of ‘dma_alloc_wc’ from incompatible pointer type
> > [-Werror=incompatible-pointer-types]
> > 
> > The expected pointer type of the third argument to dma_alloc_wc() is
> > dma_addr_t but phys_addr_t is passed. Fix this by adding casts to the
> > expected pointer type.
> > 
> > Signed-off-by: Emil Goode 
> > ---
> >  drivers/gpu/host1x/cdma.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> What compiler do you use? I do regular builds and check for warnings and
> errors, and this one is new to me.
> 
> Thierry
>

I use gcc version 6.3.0 cross compiler for armhf supplied with Debian Stretch.

Best regards,

Emil

> > 
> > diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
> > index 28541b280739..5e8b321a751e 100644
> > --- a/drivers/gpu/host1x/cdma.c
> > +++ b/drivers/gpu/host1x/cdma.c
> > @@ -91,8 +91,8 @@ static int host1x_pushbuffer_init(struct push_buffer *pb)
> >  
> > size = iova_align(>iova, size);
> >  
> > -   pb->mapped = dma_alloc_wc(host1x->dev, size, >phys,
> > - GFP_KERNEL);
> > +   pb->mapped = dma_alloc_wc(host1x->dev, size,
> > + (dma_addr_t *)>phys, GFP_KERNEL);
> > if (!pb->mapped)
> > return -ENOMEM;
> >  
> > @@ -110,8 +110,8 @@ static int host1x_pushbuffer_init(struct push_buffer 
> > *pb)
> > if (err)
> > goto iommu_free_iova;
> > } else {
> > -   pb->mapped = dma_alloc_wc(host1x->dev, size, >phys,
> > - GFP_KERNEL);
> > +   pb->mapped = dma_alloc_wc(host1x->dev, size,
> > + (dma_addr_t *)>phys, GFP_KERNEL);
> > if (!pb->mapped)
> > return -ENOMEM;
> >  
> > -- 
> > 2.11.0
> > 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

2018-03-27 Thread Simon Horman
On Mon, Mar 26, 2018 at 10:04:24AM +0100, Kieran Bingham wrote:
> Hi Simon,
> 
> On 26/03/18 09:31, Simon Horman wrote:
> > On Fri, Mar 23, 2018 at 09:16:13PM +, Kieran Bingham wrote:
> >> Hi Simon,
> >>
> >> On 23/03/18 08:51, Simon Horman wrote:
> >>> On Thu, Mar 22, 2018 at 09:30:40PM +, Kieran Bingham wrote:
>  The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
>  bus, however in low power mode the ADV7513 will reset it's slave maps to
>  use the hardware defined default addresses.
> 
>  The ADV7511 driver was adapted to allow the two devices to be registered
>  correctly - but it did not take into account the fault whereby the
>  devices reset the addresses.
> 
>  This results in an address conflict between the device using the default
>  addresses, and the other device if it is in low-power-mode.
> 
>  Repair this issue by moving both devices away from the default address
>  definitions.
> >>>
> >>> Hi Kierean,
> >>>
> >>> as this is a fix
> >>> a) Does it warrant a fixes tag?
> >>>Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
> >>> b) Does it warrant being posted as a fix for v4.16;
> >>> c) or v4.17?
> >>
> >> Tricky one, yes it could but this DTS fix, will only actually 'fix' the 
> >> issue if
> >> the corresponding driver updates to allow secondary addresses to be parsed 
> >> are
> >> also backported.
> >>
> >> It should be safe to back port the dts fix without the driver updates, but 
> >> the
> >> addresses specified by this patch will simply be ignored.
> > 
> > In that case I think its safe to add the fixes tag and take the DTS patch
> > via the renesas tree. Perhaps applying it for v4.18 and allowing automatic
> > backporting to take its course is the cleanest option.
> > 
> >> Thus if this is marked with the fixes tag the corresponding patch "drm: 
> >> adv7511:
> >> Add support for i2c_new_secondary_device" should also be marked.
> >>
> >> It looks like that patch has yet to be picked up by the DRM subsystem, so 
> >> how
> >> about I bundle both of these two patches together in a repost along with 
> >> the
> >> fixes tag.
> >>
> >> In fact, I don't think the ADV7511 dt-bindings update has made any progress
> >> either. (dt-bindings: adv7511: Extend bindings to allow specifying slave 
> >> map
> >> addresses). The media tree variants for the adv7604 have already been 
> >> picked up
> >> by Mauro I believe though.
> >>
> >> I presume it would be acceptable for this dts patch (or rather all three 
> >> patches
> >> mentioned) to get integrated through the DRM tree ?
> > 
> > Unless there is a strong reason I would prefer the dts patch to go via
> > my tree. The reason is to avoid merge conflicts bubbling up to Linus,
> > which really is something best avoided.
> 
> That's perfectly fine with me.
> 
> Feel free to add:
> 
> Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
> 
> as you suggested when you apply, or alternatively let me know if you need a 
> repost.

Thanks, applied for v4.18 with the fixes tag.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 4/5] drm: bridge: lvds-encoder: allow specifying the input bus format

2018-03-27 Thread Peter Rosin
If the bridge changes the bus format, allow this to be described in
the bridge, instead of providing false information about the bus
format of the connector or panel.

Signed-off-by: Peter Rosin 
---
 .../bindings/display/bridge/lvds-transmitter.txt   |  6 ++
 drivers/gpu/drm/bridge/lvds-encoder.c  | 25 ++
 2 files changed, 31 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt 
b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index 50220190c203..8d40a2069252 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -30,6 +30,12 @@ Required properties:
   device-specific version corresponding to the device first
   followed by the generic version.
 
+Optional properties:
+
+- interface-pix-fmt:
+  List of valid input bus formats of the encoder. Recognized bus formats
+  are listed in ../bus-format.txt
+
 Required nodes:
 
 This device has two video ports. Their connections are modeled using the OF
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c 
b/drivers/gpu/drm/bridge/lvds-encoder.c
index 75b0d3f6e4de..b78619b5560a 100644
--- a/drivers/gpu/drm/bridge/lvds-encoder.c
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -16,6 +17,8 @@
 struct lvds_encoder {
struct drm_bridge bridge;
struct drm_bridge *panel_bridge;
+   int num_bus_formats;
+   u32 *bus_formats;
 };
 
 static int lvds_encoder_attach(struct drm_bridge *bridge)
@@ -28,8 +31,22 @@ static int lvds_encoder_attach(struct drm_bridge *bridge)
 bridge);
 }
 
+static int lvds_encoder_input_formats(struct drm_bridge *bridge,
+ const u32 **bus_formats)
+{
+   struct lvds_encoder *lvds_encoder = container_of(bridge,
+struct lvds_encoder,
+bridge);
+
+   if (lvds_encoder->num_bus_formats)
+   *bus_formats = lvds_encoder->bus_formats;
+
+   return lvds_encoder->num_bus_formats;
+}
+
 static struct drm_bridge_funcs funcs = {
.attach = lvds_encoder_attach,
+   .input_formats = lvds_encoder_input_formats,
 };
 
 static int lvds_encoder_probe(struct platform_device *pdev)
@@ -39,6 +56,7 @@ static int lvds_encoder_probe(struct platform_device *pdev)
struct device_node *panel_node;
struct drm_panel *panel;
struct lvds_encoder *lvds_encoder;
+   int ret;
 
lvds_encoder = devm_kzalloc(>dev, sizeof(*lvds_encoder),
GFP_KERNEL);
@@ -79,6 +97,12 @@ static int lvds_encoder_probe(struct platform_device *pdev)
if (IS_ERR(lvds_encoder->panel_bridge))
return PTR_ERR(lvds_encoder->panel_bridge);
 
+   ret = drm_of_bus_formats(pdev->dev.of_node, "interface-pix-fmt",
+_encoder->bus_formats);
+   if (ret < 0)
+   return ret;
+   lvds_encoder->num_bus_formats = ret;
+
/* The panel_bridge bridge is attached to the panel's of_node,
 * but we need a bridge attached to our of_node for our user
 * to look up.
@@ -96,6 +120,7 @@ static int lvds_encoder_remove(struct platform_device *pdev)
 {
struct lvds_encoder *lvds_encoder = platform_get_drvdata(pdev);
 
+   kfree(lvds_encoder->bus_formats);
drm_bridge_remove(_encoder->bridge);
 
return 0;
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/atmel-hlcdc: add command line option to specify preferred depth

2018-03-27 Thread Peter Rosin
I have an sama5d31-based system with 64MB of memory and a 1920x1080
LVDS display wired for 16-bpp. When I enable legacy fbdev support,
the contiguous memory allocator invariably fails with the order-11
allocation for a 1920x1080@24-bpp buffer (~6MB). But this HW can never
make any good use of RGB888, so that is a wasted attempt anyway that
would also waste precious memory should it succeed.

Sure, I could rewrite user-space to go directly to KMS etc, and that
makes the (attempted) order-11 allocation go away, replacing it with
one order-10 allocation per application restart for a 1920x1080@16-bpp
buffer (<4MB). But after a few restarts, order-10 allocations start to
fail as well, which is only to be expected AFAIU.

So, I'd rather not change user-space (which was originally written
to target a smaller display) so that I at the same time get the
benefit of an early pre-allocated fbdev frame-buffer that can be
reused over and over. But to do that I need to tell the driver that
16-bpp is the preferred depth. Add a module parameter to do just that.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

I found some inspiration regarding naming and implementation here:
https://patchwork.kernel.org/patch/9848631/

I have found no feedback on that patch though, which makes me wonder if
I'm perhaps barking up the wronig tree?

Cheers,
Peter

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index c1ea5c36b006..f0148627c221 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -29,6 +29,11 @@
 
 #define ATMEL_HLCDC_LAYER_IRQS_OFFSET  8
 
+static int atmel_hlcdc_preferred_depth __read_mostly;
+
+MODULE_PARM_DESC(preferreddepth, "Set preferred bpp");
+module_param_named(preferreddepth, atmel_hlcdc_preferred_depth, int, 0400);
+
 static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9n12_layers[] = {
{
.name = "base",
@@ -590,6 +595,7 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device 
*dev)
dev->mode_config.min_height = dc->desc->min_height;
dev->mode_config.max_width = dc->desc->max_width;
dev->mode_config.max_height = dc->desc->max_height;
+   dev->mode_config.preferred_depth = 24;
dev->mode_config.funcs = _config_funcs;
 
return 0;
@@ -658,7 +664,7 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
 
platform_set_drvdata(pdev, dev);
 
-   drm_fb_cma_fbdev_init(dev, 24, 0);
+   drm_fb_cma_fbdev_init(dev, atmel_hlcdc_preferred_depth, 0);
 
drm_kms_helper_poll_init(dev);
 
@@ -756,6 +762,16 @@ static int atmel_hlcdc_dc_drm_probe(struct platform_device 
*pdev)
struct drm_device *ddev;
int ret;
 
+   switch (atmel_hlcdc_preferred_depth) {
+   case 0: /* driver default */
+   case 8:
+   case 16:
+   case 24:
+   break;
+   default:
+   return -EINVAL;
+   }
+
ddev = drm_dev_alloc(_hlcdc_dc_driver, >dev);
if (IS_ERR(ddev))
return PTR_ERR(ddev);
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Preferring cursor plane over overlay plane

2018-03-27 Thread Joonas Kylmälä
Hi DRM subsystem developers,

I ran into this patch where overlay plane was switched to cursor plane
because there was no proper cursor plane available on the display
hardware: . Can we discuss whether
to have a policy of using a normal plane for cursor plane in case a
dedicated HW cursor plane is missing?

Daniel Vetter suggests that it might be fine to use normal plane for
cursor plane because how to use the plane would be only "a hint to
userspace" (see the email linked).

My motivation for having this discussion is that the newer Allwinner
SoCs don't have dedicated HW cursor plane and the sun4i DRM driver
currently uses the extra planes as overlay planes which makes moving the
cursor on Xfce4 DE a terrible experience. To have better cursor moving
experience one overlay plane would need to be sacrificed.

Also, I probably missed some recipients for this email so please forward
it to the recipients you think might be interested about this.

Best regards,
Joonas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4

2018-03-27 Thread kbuild test robot
Hi Matt,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.16-rc7 next-20180326]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/matthew-s-atwood-intel-com/drm-dp-Correctly-mask-DP_TRAINING_AUX_RD_INTERVAL-values-for-DP-1-4/20180324-035824
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: x86_64-federa-25 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from 
drivers/gpu/drm/amd/amdgpu/../display/include/dpcd_defs.h:29:0,
from 
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:42:
>> include/drm/drm_dp_helper.h:67:45: error: expected identifier before numeric 
>> constant
# define DPCD_REV_100x10
^
   drivers/gpu/drm/amd/amdgpu/../display/include/dpcd_defs.h:32:2: note: in 
expansion of macro 'DPCD_REV_10'
 DPCD_REV_10 = 0x10,
 ^~~
--
   In file included from 
drivers/gpu//drm/amd/amdgpu/../display/include/dpcd_defs.h:29:0,
from 
drivers/gpu//drm/amd/amdgpu/../display/dc/core/dc_link.c:42:
>> include/drm/drm_dp_helper.h:67:45: error: expected identifier before numeric 
>> constant
# define DPCD_REV_100x10
^
   drivers/gpu//drm/amd/amdgpu/../display/include/dpcd_defs.h:32:2: note: in 
expansion of macro 'DPCD_REV_10'
 DPCD_REV_10 = 0x10,
 ^~~

vim +67 include/drm/drm_dp_helper.h

63  
64  /* AUX CH addresses */
65  /* DPCD */
66  #define DP_DPCD_REV 0x000
  > 67  # define DPCD_REV_100x10
68  # define DPCD_REV_110x11
69  # define DPCD_REV_120x12
70  # define DPCD_REV_130x13
71  # define DPCD_REV_140x14
72  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 2/5] drm: bridge: add API to query the expected input formats of bridges

2018-03-27 Thread jacopo mondi
Hi Peter,

On Tue, Mar 27, 2018 at 02:12:42PM +0200, Peter Rosin wrote:
> Hi Jacopo,
>
> Thanks for the feedback!
>
> On 2018-03-27 11:47, jacopo mondi wrote:
> > Hi Peter,
> >thanks for the patches
> >
> > On Mon, Mar 26, 2018 at 11:24:44PM +0200, Peter Rosin wrote:
> >> Bridges may affect the required bus output format of the encoder, in
> >> which case it may be wrong to use the output format of the panel or
> >> connector as is. Add infrastructure to address this problem.
> >
> > Bridges not only affect the format expected by the connector at the
> > end of the display pipeline, they may perform encoding/decoding
> > between protocols and their accepted input formats may be unrelated to
> > the connector at the end of the pipeline as there may an arbitrary
> > number of bridges in between.
> >
> > Eg.
> >
> > ENCODERCONNECTOR
> >   | |
> > |DU LVDS| ->lvds-> |THC63| ->rgb-> |ADV7511| ->hdmi-> |HDMI connector|
> >
> > The fact that THC63 wants a specific LVDS input format is unrelated to
> > the format required by the HDMI connector at the end of the pipeline.
> >
> > I would just state here that bridges need a way to report their
> > accepted media bus formats, and this patch extends the DRM Bridge APIs
> > to implement that.
>
> Yes, I can add some words, but I would like to clear up the naming
> before re-spinning.
>
> >>
> >> Signed-off-by: Peter Rosin 
> >> ---
> >>  drivers/gpu/drm/drm_bridge.c | 32 
> >>  include/drm/drm_bridge.h | 18 ++
> >>  2 files changed, 50 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> >> index 1638bfe9627c..f85e61b7416e 100644
> >> --- a/drivers/gpu/drm/drm_bridge.c
> >> +++ b/drivers/gpu/drm/drm_bridge.c
> >> @@ -348,6 +348,38 @@ void drm_bridge_enable(struct drm_bridge *bridge)
> >>  }
> >>  EXPORT_SYMBOL(drm_bridge_enable);
> >>
> >> +/**
> >> + * drm_bridge_input_formats - get the expected bus input format of the 
> >> bridge
> >
> > I may be biased by the V4L2 APIs, but this seems to me very much like
> > similar to g_fmt/s_fmt callbacks we have there. Bridges have an input and
>
> g_fmt/s_fmt says nothing to me. Graphics format? Source Format? I have
> no idea if those guesses are even close? So, that seems like poor naming
> to me. The only reason I see to go that way is for the sake of consistency.
>

Sorry, I wasn't clear here.

To me this operation seems like a "get format" and I see space for
future implementation of "set format" operations  and also
"enum_formats" to implement format negotiation between bridges...


> > an output formats, and that calls for something that takes that into
> > account, as well as they may have different input ports with different
> > accepted formats but I would for now simplify this to just 'g_fmt()'
>
> You mean rename the function to drm_bridge_g_fmt(), right?

Yeah, something like "drm_bridge_get_format(next_bridge)"

>
> As indicated above, I'm not that fond of it, but don't really care
> either. Maintainers?
>

I do not care much about the name too ofc, I think the real meat is
below...

> >> + * @bridge: bridge control structure
> >> + * @bus_formats: where to store a pointer to the bus input formats
> >> + *
> >> + * Calls the _bridge_funcs.input_formats op for the frist bridge in 
> >> the
> >> + * chain that has registered this op.
> >
> > I'm not sure passing the call down the pipeline is desirable. Each
> > component should make sure its output format is accepted as the next
> > bridge input format, passing the call to the next bridge is not
> > different that getting to the connector at the end of the pipeline and
> > return to the initial caller its supported format. Do you agree with this?
>
> Not passing down the call does one of two things. Either all bridges have
> to implement the call or all users have to walk the pipeline "manually".
> I don't like either or those options, so I still think it is good to
> pass the call down the pipeline.
>
> *time passes*
>
> Oh, you do not think it is useful for the bridge to have a callback but
> still report "no format change", and that the call down to pipeline
> should only happen for bridges that have not implemented the op. But I
> do think that is useful, see below.
>
> >> + *
> >> + * Note that the bridge passed should normally be the bridge closest to 
> >> the
> >> + * encoder, but possibly the bridge closest to an intermediate bridge in
> >> + * convoluted cases.
> >> + *
> >
> > As I see this, any bridge in the arbitrary long pipeline should call
> > this operation on next bridge if it supports different output formats.
> > Ie. I would not name here the encoder nor refer to the bridge position
> > in the pipeline.
>
> Ok, I can change that, it just seemed like a convoluted case to me.
> I mean, if this was a real issue and complicated 

[Bug 105747] TOPAZ: ring buffers not getting initialized with the amd-staging-drm-next branch

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105747

Nayan Deshmukh  changed:

   What|Removed |Added

 Status|RESOLVED|VERIFIED

--- Comment #5 from Nayan Deshmukh  ---
It is working alright. Sorry for the unnecessary noise.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105729] AMD Radeon flickering on Gnome Wayland after wake from suspend

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105729

--- Comment #2 from Jan Vlug  ---
> I believe it's the same,
> flickering when mouse movement occurs. However it's not flickering, it's
> more like it's locking the framebuffer and preventing any draws. Try running
> glxgears and moving your cursor in circles. I can get the gear to stop
> turning completely if I move my mouse quickly enough. Do we have the same
> bug? 

No it is different. When I start glxgears, there is no flickering as long as
the mouse pointer stays in the area with the gears. The gears spin without
disruption. However, as soon as the pointer leaves the gear area (i.e. touches
a window decoration) I see the flickering.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [bug report] drm/vmwgfx: Cursor update fixes

2018-03-27 Thread Thomas Hellstrom

Hi!

Thanks for the comments, I'll take a closer look.

/Thomas



On 03/27/2018 11:16 AM, Daniel Vetter wrote:

On Tue, Mar 27, 2018 at 12:05:38PM +0300, Dan Carpenter wrote:

Hello Thomas Hellstrom,

The patch 25db875401c8: "drm/vmwgfx: Cursor update fixes" from Jan
16, 2018, leads to the following static checker warning:

drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:513 
vmw_du_cursor_plane_atomic_check()
info: return a literal instead of 'ret'

drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
493  int vmw_du_cursor_plane_atomic_check(struct drm_plane *plane,
494   struct drm_plane_state *new_state)
495  {
496  int ret = 0;
497  struct vmw_surface *surface = NULL;
498  struct drm_framebuffer *fb = new_state->fb;
499
500  struct drm_rect src = drm_plane_state_src(new_state);
501  struct drm_rect dest = drm_plane_state_dest(new_state);
502
503  /* Turning off */
504  if (!fb)
505  return ret;
 ^^
This would be more clear if it were "return 0;"

506
507  ret = drm_plane_helper_check_update(plane, new_state->crtc, fb,
508  , ,
509  DRM_MODE_ROTATE_0,
510  
DRM_PLANE_HELPER_NO_SCALING,
511  
DRM_PLANE_HELPER_NO_SCALING,
512  true, true, 
_state->visible);
513  if (!ret)
514  return ret;

Same here.  With the current code, it almost looks like that the if
statement is reversed.  As a newbie to this subsystem but with a bit of
kernel experience, it would take me a while to figure out if the if
statement is deliberate or a bug.

It is a bug, drm_plane_helper_check_update returns negative errno's on
failure. Also, this code should use drm_atomic_helper_check_plane_state,
not the legacy wrapper, since vmwgfx is an atomic driver (but maybe that's
fixed in -next already).
-Daniel

515
516  /* A lot of the code assumes this */
517  if (new_state->crtc_w != 64 || new_state->crtc_h != 64) {
518  DRM_ERROR("Invalid cursor dimensions (%d, %d)\n",
519new_state->crtc_w, new_state->crtc_h);
520  ret = -EINVAL;
521  }
522
523  if (!vmw_framebuffer_to_vfb(fb)->dmabuf)
524  surface = vmw_framebuffer_to_vfbs(fb)->surface;
525
526  if (surface && !surface->snooper.image) {
527  DRM_ERROR("surface not suitable for cursor\n");
528  ret = -EINVAL;
529  }
530
531  return ret;
532  }

regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel=DwIBAg=uilaK90D4TOVoH58JNXRgQ=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ=EmKCi9L_95tppMT-q6ky7JkUBYYdk22nnaoiRUQRZ3c=acr_OiqUZeMipWTzfnf3FAppMYvjv-tetZPy2bYXVxg=



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

2018-03-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=105733

--- Comment #4 from Allan  ---
If you set amdgpu.dc=1 as a boot parameter and then try openning pavucontrol,
the screen hungs with artifacts (mouse cursor keeps moving) and you get this
error :


```
[  125.640254] amdgpu :0e:00.0: GPU fault detected: 147 0x04f00402
[  125.640259] amdgpu :0e:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x001C389E
[  125.640262] amdgpu :0e:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x04004002
[  125.640264] amdgpu :0e:00.0: VM fault (0x02, vmid 2) at page 1849502,
read from 'TC1' (0x54433100) (4)
[  125.640641] amdgpu :0e:00.0: GPU fault detected: 147 0x05004802
[  125.640643] amdgpu :0e:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x001C38A0
[  125.640644] amdgpu :0e:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x04048002
[  125.640646] amdgpu :0e:00.0: VM fault (0x02, vmid 2) at page 1849504,
read from 'TC4' (0x54433400) (72)
```

I'm using kernel 4.15.

Then when you request a poweroff from ssh the call trace appears again and
hungs the system, then you have to do a hard reset.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] gpu: drm: nouveau: Use list_for_each_entry_from_reverse

2018-03-27 Thread Arushi Singhal
On Tue, Mar 27, 2018 at 2:44 PM, Ben Skeggs  wrote:

> On 27 March 2018 at 19:11, Arushi Singhal
>  wrote:
> > It's better to use "list_for_each_entry_from_reverse" for iterating list
> > than "for loop" as it makes the code more clear to read.
> > This patch replace "for loop" with "list_for_each_entry_from_reverse"
> > and remove "cstate" variable as it is redundant in the code.
> I would prefer to also see "start" renamed to "cstate" with this change.
>
Hello Ben

Yes, using cstate is more accurate, as most of the functions are using
"cstate" variable.

Best
Arushi

>
> Ben.
>
> >
> > Signed-off-by: Arushi Singhal 
> > ---
> >  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 8 +++-
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > index 81c3567..5e56f74 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > @@ -113,7 +113,6 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate,
> >  {
> > struct nvkm_device *device = clk->subdev.device;
> > struct nvkm_volt *volt = device->volt;
> > -   struct nvkm_cstate *cstate;
> > int max_volt;
> >
> > if (!pstate || !start)
> > @@ -133,13 +132,12 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate,
> > max_volt = min(max_volt,
> >nvkm_volt_map(volt, volt->max2_id,
> clk->temp));
> >
> > -   for (cstate = start; >head != >list;
> > -cstate = list_prev_entry(cstate, head)) {
> > -   if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> > +   list_for_each_entry_from_reverse(start, >list, head) {
> > +   if (nvkm_cstate_valid(clk, start, max_volt, clk->temp))
> > break;
> > }
> >
> > -   return cstate;
> > +   return start;
> >  }
> >
> >  static struct nvkm_cstate *
> > --
> > 2.7.4
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH xserver 3/3] modesetting/drmmode: Use drmModeGetFB2

2018-03-27 Thread Daniel Stone
Hi Emil,

On 26 March 2018 at 16:22, Emil Velikov  wrote:
> On 23 March 2018 at 13:50, Daniel Stone  wrote:
>> Much like AddFB -> AddFB2, GetFB2 lets us get multiple buffers back as
>> well as modifier information. This lets us use -background none with
>> multiplanar formats, or modifiers which can't be inferred via a magic
>> side channel.
>>
> AFAICT there's nothing special (or wrong) with the new API.
>
> The flags field is a bit of an open question - should Xserver or
> libdrm manage the value passed to the kernel?
> Analogously - should we pass the flags back to the user (drmModeFB2::flags)?
>
> Current design seems perfectly fine IMHO, although others might disagree.

Thanks for looking at this! I think the flags question is a good one,
and that we should probably make the field RW: userspace declaring
what it supports (analagous to AddFB2), and the kernel overwriting
that with the intersection of what userspace and the kernel support
(not analogous, but since it's a getter rather than an add ...).

>> @@ -1090,31 +1094,63 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode, 
>> ScrnInfoPtr pScrn, int fbcon_id)
>>  if (pixmap)
>>  return pixmap;
>>
>> -fbcon = drmModeGetFB(drmmode->fd, fbcon_id);
>> -if (fbcon == NULL)
>> -return NULL;
>> +fbcon2 = drmModeGetFB2(drmmode->fd, fbcon_id);
>> +if (fbcon2) {
> Declare and initialize fbcon2 in one go - C99 feature that everyone
> has (even the people who use MSVC to build Xserver & friends).
> Then wrap the whole fbcon2 hunk in a #ifdef HAVE_GETFB2 + add a
> configure/meson check.

Sure; the only reason I didn't add an ifdef check is because there's
no version released with it yet.

> Alternatively add a weak drmModeGetFB2 function which returns NULL and
> forget all the above ;-)

If you have a good suggestion for implementing weak symbols then I'm
all ears, but last I saw from the Mesa experience no-one could figure
out how to do it properly.

>> +width = fbcon2->width;
>> +height = fbcon2->height;
>> +memcpy(handles, fbcon2->handles, sizeof(handles));
>> +memcpy(strides, fbcon2->pitches, sizeof(strides));
>> +memcpy(offsets, fbcon2->offsets, sizeof(offsets));
>> +modifier = fbcon2->modifier;
>> +switch (fbcon2->pixel_format) {
>> +case DRM_FORMAT_RGB565:
>> +depth = 16;
> Missing bpp?

Correct.

> Idea: Instead of introducing another format <> {depth, bpp} mapping,
> might as well add some helpers?

I can only see that mapping inside drmmode_create_bo, which is
different as it creates everything with an alpha channel, i.e.
s/XRGB/ARGB/. Are there some others I'm missing?

>> +break;
>> +case DRM_FORMAT_XRGB:
>> +depth = 24;
>> +bpp = 32;
>> +break;
>> +case DRM_FORMAT_XRGB2101010:
>> +depth = 30;
>> +bpp = 32;
>> +default:
>> +break;
> Error instead of silently continuing?

Sure.

Cheers,
Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 8/8] drm/arm/malidp: Added the late system pm functions

2018-03-27 Thread Daniel Vetter
On Tue, Mar 27, 2018 at 11:59 AM, Ayan Halder  wrote:
> On Tue, Mar 27, 2018 at 10:29:03AM +0200, Daniel Vetter wrote:
>> On Mon, Mar 26, 2018 at 06:03:20PM +0100, Ayan Kumar Halder wrote:
>> > malidp_pm_suspend_late checks if the runtime status is not suspended
>> > and if so, invokes malidp_runtime_pm_suspend which disables the
>> > display engine/core interrupts and the clocks. It sets the runtime status
>> > as suspended. Subsequently, malidp_pm_resume_early will invoke
>> > malidp_runtime_pm_resume which enables the clocks and the interrupts
>> > (previously disabled) and sets the runtime status as active.
>> >
>> > Signed-off-by: Ayan Kumar Halder 
>> > Change-Id: I5f8c3d28f076314a1c9da2a46760a9c37039ccda
>>
>> Why exactly do you need late/early hooks? If you have dependencies with
>> other devices, pls consider adding device_links instead. This here
>> shouldn't be necessary.
>> -Daniel
> We need to late/early hooks to disable malidp interrupts and the
> clocks.

Yes, but why this ordering constraint? Why can't you just disable the
interrupts/clocks in the normal suspend code. I see that the patch
does this, I want to understand why it does it.
-Daniel

>> > ---
>> >  drivers/gpu/drm/arm/malidp_drv.c | 17 +
>> >  1 file changed, 17 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c 
>> > b/drivers/gpu/drm/arm/malidp_drv.c
>> > index bd44a6d..f6124d8 100644
>> > --- a/drivers/gpu/drm/arm/malidp_drv.c
>> > +++ b/drivers/gpu/drm/arm/malidp_drv.c
>> > @@ -766,8 +766,25 @@ static int __maybe_unused malidp_pm_resume(struct 
>> > device *dev)
>> > return 0;
>> >  }
>> >
>> > +static int __maybe_unused malidp_pm_suspend_late(struct device *dev)
>> > +{
>> > +   if (!pm_runtime_status_suspended(dev)) {
>> > +   malidp_runtime_pm_suspend(dev);
>> > +   pm_runtime_set_suspended(dev);
>> > +   }
>> > +   return 0;
>> > +}
>> > +
>> > +static int __maybe_unused malidp_pm_resume_early(struct device *dev)
>> > +{
>> > +   malidp_runtime_pm_resume(dev);
>> > +   pm_runtime_set_active(dev);
>> > +   return 0;
>> > +}
>> > +
>> >  static const struct dev_pm_ops malidp_pm_ops = {
>> > SET_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend, malidp_pm_resume) \
>> > +   SET_LATE_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend_late, 
>> > malidp_pm_resume_early) \
>> > SET_RUNTIME_PM_OPS(malidp_runtime_pm_suspend, 
>> > malidp_runtime_pm_resume, NULL)
>> >  };
>> >
>> > --
>> > 2.7.4
>> >
>> > ___
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended 
> recipient, please notify the sender immediately and do not disclose the 
> contents to any other person, use it for any purpose, or store or copy the 
> information in any medium. Thank you.
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v6 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

2018-03-27 Thread Vladimir Zapolskiy
Hi Jacopo,

On 03/27/2018 01:10 PM, jacopo mondi wrote:
> Hi Vladimir,
> 
> On Tue, Mar 27, 2018 at 12:37:31PM +0300, Vladimir Zapolskiy wrote:
>> Hi Jacopo,
>>
>> On 03/27/2018 11:57 AM, jacopo mondi wrote:
>>> Hi Vladimir,
>>>
>>> On Tue, Mar 27, 2018 at 11:30:29AM +0300, Vladimir Zapolskiy wrote:
 Hi Sergei,

 On 03/27/2018 11:27 AM, Sergei Shtylyov wrote:
> Hello!
>
> On 3/27/2018 10:33 AM, jacopo mondi wrote:
> [...]
>>> Document Thine THC63LVD1024 LVDS decoder device tree bindings.
>>>
>>> Signed-off-by: Jacopo Mondi 
>>> Reviewed-by: Andrzej Hajda 
>>> Reviewed-by: Niklas Söderlund 
>>> 
>>> ---
>>>   .../bindings/display/bridge/thine,thc63lvd1024.txt | 66 
>>> +++
>>>   1 file changed, 66 insertions(+)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>>> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>>> new file mode 100644
>>> index 000..8225c6a
>>> --- /dev/null
>>> +++
>>> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>>> @@ -0,0 +1,66 @@
>>> +Thine Electronics THC63LVD1024 LVDS decoder
>>> +---
>>> +
>>> +The THC63LVD1024 is a dual link LVDS receiver designed to convert 
>>> LVDS
>>> streams
>>> +to parallel data outputs. The chip supports single/dual 
>>> input/output modes,
>>> +handling up to two two input LVDS stream and up to two digital 
>>> CMOS/TTL
>>> outputs.
>>> +
>>> +Single or dual operation modes, output data mapping and DDR output 
>>> modes
>>> are
>>> +configured through input signals and the chip does not expose any 
>>> control
>>> bus.
>>> +
>>> +Required properties:
>>> +- compatible: Shall be "thine,thc63lvd1024"
>>> +
>>> +Optional properties:
>>> +- vcc-supply: Power supply for TTL output and digital circuitry
>>> +- cvcc-supply: Power supply for TTL CLOCKOUT signal
>>> +- lvcc-supply: Power supply for LVDS inputs
>>> +- pvcc-supply: Power supply for PLL circuitry
>> As explained in a comment to one of the previous versions of this 
>> series, I'm
>> tempted to make vcc-supply mandatory and drop the three other power 
>> supplies
>> for now, as I believe there's very little chance they will be 
>> connected to
>> separately controllable regulators (all supplies use the same 
>> voltage). In the
>> very unlikely event that this occurs in design we need to support in 
>> the
>> future, the cvcc, lvcc and pvcc supplies can be added later as 
>> optional
>> without breaking backward compatibility.
> I'm okay with that.
>
>> Apart from that,
>>
>> Reviewed-by: Laurent Pinchart 
>>
>>> +- pdwn-gpios: Power down GPIO signal. Active low
> powerdown-gpios is the semi-standard name.
>
 right, I've also noticed it. If possible please avoid shortenings in
 property names.
>>>
>>> It is not shortening, it just follow pin name from decoder's datasheet.
>>>

>>> +- oe-gpios: Output enable GPIO signal. Active high
>>> +
 And this one is also a not ever met property name, please consider to
 rename it to 'enable-gpios', for instance display panels define it.
>>>
>>>
>>> Again, it follows datasheet naming scheme. Has something changed in DT
>>> conventions?
>>
>> Seconded. My understanding is that the property name should reflect
>> what reported in the the chip manual. For THC63LVD1024 the enable and
>> power down pins are named 'OE' and 'PDWN' respectively.
>
> But don't we need the vendor prefix in the prop names then, like
> "renesas,oe-gpios" then?
>

 Seconded, with a correction to "thine,oe-gpios".

>>>
>>> mmm, okay then...
>>>
>>> A grep for that semi-standard properties names in Documentation/
>>> returns only usage examples and no actual definitions, so I assume this
>>> is why they are semi-standard.
>>
>> Here we have to be specific about a particular property, let it be 'oe-gpios'
>> vs. 'enable-gpios' and let's collect some statistics:
>>
>> % grep -Hr oe-gpios Documentation/devicetree/bindings/* | wc -l
>> 0
>>
>> $ grep -Hr enable-gpios Documentation/devicetree/bindings/* | wc 

Re: [PATCH v2 4/5] drm: bridge: lvds-encoder: allow specifying the input bus format

2018-03-27 Thread jacopo mondi
Hi Peter, Laurent,
   thanks for the patches,

On Mon, Mar 26, 2018 at 11:24:46PM +0200, Peter Rosin wrote:
> If the bridge changes the bus format, allow this to be described in
> the bridge, instead of providing false information about the bus
> format of the connector or panel.
>
> Signed-off-by: Peter Rosin 
> ---
>  .../bindings/display/bridge/lvds-transmitter.txt   |  6 ++
>  drivers/gpu/drm/bridge/lvds-encoder.c  | 25 
> ++
>  2 files changed, 31 insertions(+)
>
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt 
> b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> index 50220190c203..8d40a2069252 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> @@ -30,6 +30,12 @@ Required properties:
>device-specific version corresponding to the device first
>followed by the generic version.
>
> +Optional properties:
> +
> +- interface-pix-fmt:
> +  List of valid input bus formats of the encoder. Recognized bus formats
> +  are listed in ../bus-format.txt
> +

This comments applies here and to [3/5] as well.

Are we sure we want the supported bridge input format defined in DT
bindings?

Again, I may be biased, but as I see this, each bridge driver should
list its supported formats with MEDIA_BUS_FMT_ fourcc codes and return
the currently 'active' one when requested by the preceding bridge.

I understand for this driver, being compatible with both a generic lvds
encoder and a specific chip, the supported input formats are
different, but I would have used the 'of_device_id.data' field for
this and leave this out from DT bindings.

This makes the translation routine here implemented not required if
I'm not wrong...

Thanks
   j

>  Required nodes:
>
>  This device has two video ports. Their connections are modeled using the OF
> diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c 
> b/drivers/gpu/drm/bridge/lvds-encoder.c
> index 75b0d3f6e4de..b78619b5560a 100644
> --- a/drivers/gpu/drm/bridge/lvds-encoder.c
> +++ b/drivers/gpu/drm/bridge/lvds-encoder.c
> @@ -9,6 +9,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #include 
> @@ -16,6 +17,8 @@
>  struct lvds_encoder {
>   struct drm_bridge bridge;
>   struct drm_bridge *panel_bridge;
> + int num_bus_formats;
> + u32 *bus_formats;
>  };
>
>  static int lvds_encoder_attach(struct drm_bridge *bridge)
> @@ -28,8 +31,22 @@ static int lvds_encoder_attach(struct drm_bridge *bridge)
>bridge);
>  }
>
> +static int lvds_encoder_input_formats(struct drm_bridge *bridge,
> +   const u32 **bus_formats)
> +{
> + struct lvds_encoder *lvds_encoder = container_of(bridge,
> +  struct lvds_encoder,
> +  bridge);
> +
> + if (lvds_encoder->num_bus_formats)
> + *bus_formats = lvds_encoder->bus_formats;
> +
> + return lvds_encoder->num_bus_formats;
> +}
> +
>  static struct drm_bridge_funcs funcs = {
>   .attach = lvds_encoder_attach,
> + .input_formats = lvds_encoder_input_formats,
>  };
>
>  static int lvds_encoder_probe(struct platform_device *pdev)
> @@ -39,6 +56,7 @@ static int lvds_encoder_probe(struct platform_device *pdev)
>   struct device_node *panel_node;
>   struct drm_panel *panel;
>   struct lvds_encoder *lvds_encoder;
> + int ret;
>
>   lvds_encoder = devm_kzalloc(>dev, sizeof(*lvds_encoder),
>   GFP_KERNEL);
> @@ -79,6 +97,12 @@ static int lvds_encoder_probe(struct platform_device *pdev)
>   if (IS_ERR(lvds_encoder->panel_bridge))
>   return PTR_ERR(lvds_encoder->panel_bridge);
>
> + ret = drm_of_bus_formats(pdev->dev.of_node, "interface-pix-fmt",
> +  _encoder->bus_formats);
> + if (ret < 0)
> + return ret;
> + lvds_encoder->num_bus_formats = ret;
> +
>   /* The panel_bridge bridge is attached to the panel's of_node,
>* but we need a bridge attached to our of_node for our user
>* to look up.
> @@ -96,6 +120,7 @@ static int lvds_encoder_remove(struct platform_device 
> *pdev)
>  {
>   struct lvds_encoder *lvds_encoder = platform_get_drvdata(pdev);
>
> + kfree(lvds_encoder->bus_formats);
>   drm_bridge_remove(_encoder->bridge);
>
>   return 0;
> --
> 2.11.0
>


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v6 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

2018-03-27 Thread jacopo mondi
Hi Vladimir,

On Tue, Mar 27, 2018 at 12:37:31PM +0300, Vladimir Zapolskiy wrote:
> Hi Jacopo,
>
> On 03/27/2018 11:57 AM, jacopo mondi wrote:
> > Hi Vladimir,
> >
> > On Tue, Mar 27, 2018 at 11:30:29AM +0300, Vladimir Zapolskiy wrote:
> >> Hi Sergei,
> >>
> >> On 03/27/2018 11:27 AM, Sergei Shtylyov wrote:
> >>> Hello!
> >>>
> >>> On 3/27/2018 10:33 AM, jacopo mondi wrote:
> >>> [...]
> > Document Thine THC63LVD1024 LVDS decoder device tree bindings.
> >
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Andrzej Hajda 
> > Reviewed-by: Niklas Söderlund 
> > 
> > ---
> >   .../bindings/display/bridge/thine,thc63lvd1024.txt | 66 
> > +++
> >   1 file changed, 66 insertions(+)
> >   create mode 100644
> > Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > new file mode 100644
> > index 000..8225c6a
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > @@ -0,0 +1,66 @@
> > +Thine Electronics THC63LVD1024 LVDS decoder
> > +---
> > +
> > +The THC63LVD1024 is a dual link LVDS receiver designed to convert 
> > LVDS
> > streams
> > +to parallel data outputs. The chip supports single/dual 
> > input/output modes,
> > +handling up to two two input LVDS stream and up to two digital 
> > CMOS/TTL
> > outputs.
> > +
> > +Single or dual operation modes, output data mapping and DDR output 
> > modes
> > are
> > +configured through input signals and the chip does not expose any 
> > control
> > bus.
> > +
> > +Required properties:
> > +- compatible: Shall be "thine,thc63lvd1024"
> > +
> > +Optional properties:
> > +- vcc-supply: Power supply for TTL output and digital circuitry
> > +- cvcc-supply: Power supply for TTL CLOCKOUT signal
> > +- lvcc-supply: Power supply for LVDS inputs
> > +- pvcc-supply: Power supply for PLL circuitry
>  As explained in a comment to one of the previous versions of this 
>  series, I'm
>  tempted to make vcc-supply mandatory and drop the three other power 
>  supplies
>  for now, as I believe there's very little chance they will be 
>  connected to
>  separately controllable regulators (all supplies use the same 
>  voltage). In the
>  very unlikely event that this occurs in design we need to support in 
>  the
>  future, the cvcc, lvcc and pvcc supplies can be added later as 
>  optional
>  without breaking backward compatibility.
> >>> I'm okay with that.
> >>>
>  Apart from that,
> 
>  Reviewed-by: Laurent Pinchart 
> 
> > +- pdwn-gpios: Power down GPIO signal. Active low
> >>> powerdown-gpios is the semi-standard name.
> >>>
> >> right, I've also noticed it. If possible please avoid shortenings in
> >> property names.
> >
> > It is not shortening, it just follow pin name from decoder's datasheet.
> >
> >>
> > +- oe-gpios: Output enable GPIO signal. Active high
> > +
> >> And this one is also a not ever met property name, please consider to
> >> rename it to 'enable-gpios', for instance display panels define it.
> >
> >
> > Again, it follows datasheet naming scheme. Has something changed in DT
> > conventions?
> 
>  Seconded. My understanding is that the property name should reflect
>  what reported in the the chip manual. For THC63LVD1024 the enable and
>  power down pins are named 'OE' and 'PDWN' respectively.
> >>>
> >>> But don't we need the vendor prefix in the prop names then, like
> >>> "renesas,oe-gpios" then?
> >>>
> >>
> >> Seconded, with a correction to "thine,oe-gpios".
> >>
> >
> > mmm, okay then...
> >
> > A grep for that semi-standard properties names in Documentation/
> > returns only usage examples and no actual definitions, so I assume this
> > is why they are semi-standard.
>
> Here we have to be specific about a particular property, let it be 'oe-gpios'
> vs. 'enable-gpios' and let's collect some statistics:
>
> % grep -Hr oe-gpios Documentation/devicetree/bindings/* | wc -l
> 0
>
> $ grep -Hr enable-gpios Documentation/devicetree/bindings/* | wc -l
> 86
>
> While 'thine,oe-gpios' would be correct, I see no reason to 

Re: [PATCH v3] drm/xen-front: Add support for Xen PV display frontend

2018-03-27 Thread Oleksandr Andrushchenko

On 03/27/2018 12:50 PM, Daniel Vetter wrote:

On Tue, Mar 27, 2018 at 11:34 AM, Oleksandr Andrushchenko
 wrote:

Hi, Daniel!


On 03/26/2018 03:46 PM, Oleksandr Andrushchenko wrote:

On 03/26/2018 11:18 AM, Daniel Vetter wrote:

On Fri, Mar 23, 2018 at 05:54:49PM +0200, Oleksandr Andrushchenko wrote:

My apologies, but I found a few more things that look strange and
should
be cleaned up. Sorry for this iterative review approach, but I think
we're
slowly getting there.

Thank you for reviewing!

Cheers, Daniel


---
+static int xen_drm_drv_dumb_create(struct drm_file *filp,
+struct drm_device *dev, struct drm_mode_create_dumb *args)
+{
+struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+struct drm_gem_object *obj;
+int ret;
+
+ret = xen_drm_front_gem_dumb_create(filp, dev, args);
+if (ret)
+goto fail;
+
+obj = drm_gem_object_lookup(filp, args->handle);
+if (!obj) {
+ret = -ENOENT;
+goto fail_destroy;
+}
+
+drm_gem_object_unreference_unlocked(obj);

You can't drop the reference while you keep using the object, someone
else
might sneak in and destroy your object. The unreference always must be
last.

Will fix, thank you

+
+/*
+ * In case of CONFIG_DRM_XEN_FRONTEND_CMA gem_obj is constructed
+ * via DRM CMA helpers and doesn't have ->pages allocated
+ * (xendrm_gem_get_pages will return NULL), but instead can
provide
+ * sg table
+ */
+if (xen_drm_front_gem_get_pages(obj))
+ret = xen_drm_front_dbuf_create_from_pages(
+drm_info->front_info,
+xen_drm_front_dbuf_to_cookie(obj),
+args->width, args->height, args->bpp,
+args->size,
+xen_drm_front_gem_get_pages(obj));
+else
+ret = xen_drm_front_dbuf_create_from_sgt(
+drm_info->front_info,
+xen_drm_front_dbuf_to_cookie(obj),
+args->width, args->height, args->bpp,
+args->size,
+xen_drm_front_gem_get_sg_table(obj));
+if (ret)
+goto fail_destroy;
+

The above also has another race: If you construct an object, then it
must
be fully constructed by the time you publish it to the wider world. In
gem
this is done by calling drm_gem_handle_create() - after that userspace
can
get at your object and do nasty things with it in a separate thread,
forcing your driver to Oops if the object isn't fully constructed yet.

That means you need to redo this code here to make sure that the gem
object is fully set up (including pages and sg tables) _before_
anything
calls drm_gem_handle_create().

You are correct, I need to rework this code

This probably means you also need to open-code the cma side, by first
calling drm_gem_cma_create(), then doing any additional setup, and
finally
doing the registration to userspace with drm_gem_handle_create as the
very
last thing.

Although I tend to avoid open-coding, but this seems the necessary
measure
here

Alternativet is to do the pages/sg setup only when you create an fb
(and
drop the pages again when the fb is destroyed), but that requires some
refcounting/locking in the driver.

Not sure this will work: nothing prevents you from attaching multiple
FBs to
a single dumb handle
So, not only ref-counting should be done here, but I also need to check
if
the dumb buffer,
we are attaching to, has been created already

No, you must make sure that no dumb buffer can be seen by anyone else
before it's fully created. If you don't register it in the file_priv idr
using drm_gem_handle_create, no one else can get at your buffer. Trying
to
paper over this race from all the other places breaks the gem core code
design, and is also much more fragile.

Yes, this is what I implement now, e.g. I do not create
any dumb handle until GEM is fully created. I was just
saying that alternative way when we do pages/sgt on FB
attach will not work in my case

So, I will rework with open-coding some stuff from CMA helpers


Aside: There's still a lot of indirection and jumping around which
makes
the code a bit hard to follow.

Probably I am not sure of which indirection we are talking about, could
you
please
specifically mark those annoying you?

I think it's the same indirection we talked about last time, it still
annoys me. But it's still ok if you prefer this way I think :-)

Ok, probably this is because I'm looking at the driver
from an editor, but you are from your mail client ;)

+
+static void xen_drm_drv_release(struct drm_device *dev)
+{
+struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+struct xen_drm_front_info *front_info = drm_info->front_info;
+
+drm_atomic_helper_shutdown(dev);
+drm_mode_config_cleanup(dev);
+
+xen_drm_front_evtchnl_free_all(front_info);
+dbuf_free_all(_info->dbuf_list);
+
+drm_dev_fini(dev);
+kfree(dev);
+
+/*
+ * Free now, as this release could be not due to rmmod, but
+ 

Re: [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook

2018-03-27 Thread Ville Syrjälä
On Sat, Mar 24, 2018 at 12:26:32PM -0500, David Lechner wrote:
> On 03/22/2018 03:27 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > We'll need access to the plane state during .atomic_enable().
> > 
> 
> Some more details in the commit message would be useful. It is
> not clear to me why this change is being made.

"tinydrm enable hook wants to play around with the new fb in
.atomic_enable(), thus we'll need access to the plane state."

Better? Worse?

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 8/8] drm/arm/malidp: Added the late system pm functions

2018-03-27 Thread Ayan Halder
On Tue, Mar 27, 2018 at 10:29:03AM +0200, Daniel Vetter wrote:
> On Mon, Mar 26, 2018 at 06:03:20PM +0100, Ayan Kumar Halder wrote:
> > malidp_pm_suspend_late checks if the runtime status is not suspended
> > and if so, invokes malidp_runtime_pm_suspend which disables the
> > display engine/core interrupts and the clocks. It sets the runtime status
> > as suspended. Subsequently, malidp_pm_resume_early will invoke
> > malidp_runtime_pm_resume which enables the clocks and the interrupts
> > (previously disabled) and sets the runtime status as active.
> >
> > Signed-off-by: Ayan Kumar Halder 
> > Change-Id: I5f8c3d28f076314a1c9da2a46760a9c37039ccda
>
> Why exactly do you need late/early hooks? If you have dependencies with
> other devices, pls consider adding device_links instead. This here
> shouldn't be necessary.
> -Daniel
We need to late/early hooks to disable malidp interrupts and the
clocks.
> > ---
> >  drivers/gpu/drm/arm/malidp_drv.c | 17 +
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c 
> > b/drivers/gpu/drm/arm/malidp_drv.c
> > index bd44a6d..f6124d8 100644
> > --- a/drivers/gpu/drm/arm/malidp_drv.c
> > +++ b/drivers/gpu/drm/arm/malidp_drv.c
> > @@ -766,8 +766,25 @@ static int __maybe_unused malidp_pm_resume(struct 
> > device *dev)
> > return 0;
> >  }
> >
> > +static int __maybe_unused malidp_pm_suspend_late(struct device *dev)
> > +{
> > +   if (!pm_runtime_status_suspended(dev)) {
> > +   malidp_runtime_pm_suspend(dev);
> > +   pm_runtime_set_suspended(dev);
> > +   }
> > +   return 0;
> > +}
> > +
> > +static int __maybe_unused malidp_pm_resume_early(struct device *dev)
> > +{
> > +   malidp_runtime_pm_resume(dev);
> > +   pm_runtime_set_active(dev);
> > +   return 0;
> > +}
> > +
> >  static const struct dev_pm_ops malidp_pm_ops = {
> > SET_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend, malidp_pm_resume) \
> > +   SET_LATE_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend_late, 
> > malidp_pm_resume_early) \
> > SET_RUNTIME_PM_OPS(malidp_runtime_pm_suspend, malidp_runtime_pm_resume, 
> > NULL)
> >  };
> >
> > --
> > 2.7.4
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v6 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver

2018-03-27 Thread Vladimir Zapolskiy
Hi Andrzej,

On 03/27/2018 10:28 AM, Andrzej Hajda wrote:
> On 27.03.2018 08:24, Vladimir Zapolskiy wrote:
>> Hi Jacopo,
>>
>> On 03/16/2018 05:16 PM, Jacopo Mondi wrote:
>>> Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel
>>> output converter.
>>>
>>> Signed-off-by: Jacopo Mondi 
>>> Reviewed-by: Andrzej Hajda 
>>> Reviewed-by: Niklas Söderlund 
>>> ---
>>>  drivers/gpu/drm/bridge/Kconfig|   6 +
>>>  drivers/gpu/drm/bridge/Makefile   |   1 +
>>>  drivers/gpu/drm/bridge/thc63lvd1024.c | 255 
>>> ++
>>>  3 files changed, 262 insertions(+)
>>>  create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c
>>>
>>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>>> index 3b99d5a..5815a20 100644
>>> --- a/drivers/gpu/drm/bridge/Kconfig
>>> +++ b/drivers/gpu/drm/bridge/Kconfig
>>> @@ -92,6 +92,12 @@ config DRM_SII9234
>>>   It is an I2C driver, that detects connection of MHL bridge
>>>   and starts encapsulation of HDMI signal.
>>>  
>>> +config DRM_THINE_THC63LVD1024
>>> +   tristate "Thine THC63LVD1024 LVDS decoder bridge"
>>> +   depends on OF
>>> +   ---help---
>>> + Thine THC63LVD1024 LVDS/parallel converter driver.
>>> +
>>>  config DRM_TOSHIBA_TC358767
>>> tristate "Toshiba TC358767 eDP bridge"
>>> depends on OF
>>> diff --git a/drivers/gpu/drm/bridge/Makefile 
>>> b/drivers/gpu/drm/bridge/Makefile
>>> index 373eb28..fd90b16 100644
>>> --- a/drivers/gpu/drm/bridge/Makefile
>>> +++ b/drivers/gpu/drm/bridge/Makefile
>>> @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>>>  obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>>>  obj-$(CONFIG_DRM_SII902X) += sii902x.o
>>>  obj-$(CONFIG_DRM_SII9234) += sii9234.o
>>> +obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
>>>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>>>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>>>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>>> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c 
>>> b/drivers/gpu/drm/bridge/thc63lvd1024.c
>>> new file mode 100644
>>> index 000..02a54c2
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
>>> @@ -0,0 +1,255 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * THC63LVD1024 LVDS to parallel data DRM bridge driver.
>>> + *
>>> + * Copyright (C) 2018 Jacopo Mondi 
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +enum {
>>> +   THC63_LVDS_IN0,
>>> +   THC63_LVDS_IN1,
>>> +   THC63_DIGITAL_OUT0,
>>> +   THC63_DIGITAL_OUT1,
>>> +};
>>> +
>>> +static const char * const thc63_reg_names[] = {
>>> +   "vcc", "lvcc", "pvcc", "cvcc",
>>> +};
>>> +
>>> +struct thc63_dev {
>>> +   struct device *dev;
>>> +
>>> +   struct regulator *vccs[ARRAY_SIZE(thc63_reg_names)];
>>> +
>>> +   struct gpio_desc *pdwn;
>>> +   struct gpio_desc *oe;
>>> +
>>> +   struct drm_bridge bridge;
>>> +   struct drm_bridge *next;
>>> +};
>>> +
>>> +static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
>>> +{
>>> +   return container_of(bridge, struct thc63_dev, bridge);
>>> +}
>>> +
>>> +static int thc63_attach(struct drm_bridge *bridge)
>>> +{
>>> +   struct thc63_dev *thc63 = to_thc63(bridge);
>>> +
>>> +   return drm_bridge_attach(bridge->encoder, thc63->next, bridge);
>>> +}
>>> +
>>> +static void thc63_enable(struct drm_bridge *bridge)
>>> +{
>>> +   struct thc63_dev *thc63 = to_thc63(bridge);
>>> +   struct regulator *vcc;
>>> +   int i;
>> unsigned int i;
> 
> Why? You are introducing silly bug this way, see few lines below.

You see that the comment was too terse to describe the context in full.
Thank you for exposing a flaw.

>>
>>> +
>>> +   for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
>>> +   vcc = thc63->vccs[i];
>>> +   if (!vcc)
>>> +   continue;
>>> +
>>> +   if (regulator_enable(vcc))
>>> +   goto error_vcc_enable;
>>> +   }
>>> +
>>> +   if (thc63->pdwn)
>>> +   gpiod_set_value(thc63->pdwn, 0);
>>> +
>>> +   if (thc63->oe)
>>> +   gpiod_set_value(thc63->oe, 1);
>>> +
>>> +   return;
>>> +
>>> +error_vcc_enable:
>>> +   dev_err(thc63->dev, "Failed to enable regulator %s\n",
>>> +   thc63_reg_names[i]);
>>> +
>>> +   for (i = i - 1; i >= 0; i--) {
> 
> Here, the loop will not end if you define i unsigned.
> 
> I know one can change the loop, to make it working with unsigned. But
> this clearly shows that using unsigned is more risky.
> What are advantages of unsigned vs int in this case. Are there some
> guidelines/discussions about it?
> 

The reason is pretty simple, like Geert said it might be a personal reason,
but a code pattern 

int i;
...
for (i = 0; i < ARRAY_SIZE(...); i++)

is my own red rag, because I've seen the pattern so many times, and every

Re: [PATCH v3] drm/xen-front: Add support for Xen PV display frontend

2018-03-27 Thread Daniel Vetter
On Tue, Mar 27, 2018 at 11:34 AM, Oleksandr Andrushchenko
 wrote:
> Hi, Daniel!
>
>
> On 03/26/2018 03:46 PM, Oleksandr Andrushchenko wrote:
>>
>> On 03/26/2018 11:18 AM, Daniel Vetter wrote:
>>>
>>> On Fri, Mar 23, 2018 at 05:54:49PM +0200, Oleksandr Andrushchenko wrote:
>
> My apologies, but I found a few more things that look strange and
> should
> be cleaned up. Sorry for this iterative review approach, but I think
> we're
> slowly getting there.

 Thank you for reviewing!
>
> Cheers, Daniel
>
>> ---
>> +static int xen_drm_drv_dumb_create(struct drm_file *filp,
>> +struct drm_device *dev, struct drm_mode_create_dumb *args)
>> +{
>> +struct xen_drm_front_drm_info *drm_info = dev->dev_private;
>> +struct drm_gem_object *obj;
>> +int ret;
>> +
>> +ret = xen_drm_front_gem_dumb_create(filp, dev, args);
>> +if (ret)
>> +goto fail;
>> +
>> +obj = drm_gem_object_lookup(filp, args->handle);
>> +if (!obj) {
>> +ret = -ENOENT;
>> +goto fail_destroy;
>> +}
>> +
>> +drm_gem_object_unreference_unlocked(obj);
>
> You can't drop the reference while you keep using the object, someone
> else
> might sneak in and destroy your object. The unreference always must be
> last.

 Will fix, thank you
>>
>> +
>> +/*
>> + * In case of CONFIG_DRM_XEN_FRONTEND_CMA gem_obj is constructed
>> + * via DRM CMA helpers and doesn't have ->pages allocated
>> + * (xendrm_gem_get_pages will return NULL), but instead can
>> provide
>> + * sg table
>> + */
>> +if (xen_drm_front_gem_get_pages(obj))
>> +ret = xen_drm_front_dbuf_create_from_pages(
>> +drm_info->front_info,
>> +xen_drm_front_dbuf_to_cookie(obj),
>> +args->width, args->height, args->bpp,
>> +args->size,
>> +xen_drm_front_gem_get_pages(obj));
>> +else
>> +ret = xen_drm_front_dbuf_create_from_sgt(
>> +drm_info->front_info,
>> +xen_drm_front_dbuf_to_cookie(obj),
>> +args->width, args->height, args->bpp,
>> +args->size,
>> +xen_drm_front_gem_get_sg_table(obj));
>> +if (ret)
>> +goto fail_destroy;
>> +
>
> The above also has another race: If you construct an object, then it
> must
> be fully constructed by the time you publish it to the wider world. In
> gem
> this is done by calling drm_gem_handle_create() - after that userspace
> can
> get at your object and do nasty things with it in a separate thread,
> forcing your driver to Oops if the object isn't fully constructed yet.
>
> That means you need to redo this code here to make sure that the gem
> object is fully set up (including pages and sg tables) _before_
> anything
> calls drm_gem_handle_create().

 You are correct, I need to rework this code
>
> This probably means you also need to open-code the cma side, by first
> calling drm_gem_cma_create(), then doing any additional setup, and
> finally
> doing the registration to userspace with drm_gem_handle_create as the
> very
> last thing.

 Although I tend to avoid open-coding, but this seems the necessary
 measure
 here
>
> Alternativet is to do the pages/sg setup only when you create an fb
> (and
> drop the pages again when the fb is destroyed), but that requires some
> refcounting/locking in the driver.

 Not sure this will work: nothing prevents you from attaching multiple
 FBs to
 a single dumb handle
 So, not only ref-counting should be done here, but I also need to check
 if
 the dumb buffer,
 we are attaching to, has been created already
>>>
>>> No, you must make sure that no dumb buffer can be seen by anyone else
>>> before it's fully created. If you don't register it in the file_priv idr
>>> using drm_gem_handle_create, no one else can get at your buffer. Trying
>>> to
>>> paper over this race from all the other places breaks the gem core code
>>> design, and is also much more fragile.
>>
>> Yes, this is what I implement now, e.g. I do not create
>> any dumb handle until GEM is fully created. I was just
>> saying that alternative way when we do pages/sgt on FB
>> attach will not work in my case

 So, I will rework with open-coding some stuff from CMA helpers

> Aside: There's still a lot of indirection and jumping around which
> makes
> the code a bit hard to follow.

 Probably I am not sure of which indirection we are talking about, could
 you
 please
 specifically mark those annoying you?
>>>
>>> I think it's the same 

Re: [PATCH v2 2/5] drm: bridge: add API to query the expected input formats of bridges

2018-03-27 Thread jacopo mondi
Hi Peter,
   thanks for the patches

On Mon, Mar 26, 2018 at 11:24:44PM +0200, Peter Rosin wrote:
> Bridges may affect the required bus output format of the encoder, in
> which case it may be wrong to use the output format of the panel or
> connector as is. Add infrastructure to address this problem.

Bridges not only affect the format expected by the connector at the
end of the display pipeline, they may perform encoding/decoding
between protocols and their accepted input formats may be unrelated to
the connector at the end of the pipeline as there may an arbitrary
number of bridges in between.

Eg.

ENCODERCONNECTOR
  | |
|DU LVDS| ->lvds-> |THC63| ->rgb-> |ADV7511| ->hdmi-> |HDMI connector|

The fact that THC63 wants a specific LVDS input format is unrelated to
the format required by the HDMI connector at the end of the pipeline.

I would just state here that bridges need a way to report their
accepted media bus formats, and this patch extends the DRM Bridge APIs
to implement that.

>
> Signed-off-by: Peter Rosin 
> ---
>  drivers/gpu/drm/drm_bridge.c | 32 
>  include/drm/drm_bridge.h | 18 ++
>  2 files changed, 50 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 1638bfe9627c..f85e61b7416e 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -348,6 +348,38 @@ void drm_bridge_enable(struct drm_bridge *bridge)
>  }
>  EXPORT_SYMBOL(drm_bridge_enable);
>
> +/**
> + * drm_bridge_input_formats - get the expected bus input format of the bridge

I may be biased by the V4L2 APIs, but this seems to me very much like
similar to g_fmt/s_fmt callbacks we have there. Bridges have an input and
an output formats, and that calls for something that takes that into
account, as well as they may have different input ports with different
accepted formats but I would for now simplify this to just 'g_fmt()'

> + * @bridge: bridge control structure
> + * @bus_formats: where to store a pointer to the bus input formats
> + *
> + * Calls the _bridge_funcs.input_formats op for the frist bridge in the
> + * chain that has registered this op.

I'm not sure passing the call down the pipeline is desirable. Each
component should make sure its output format is accepted as the next
bridge input format, passing the call to the next bridge is not
different that getting to the connector at the end of the pipeline and
return to the initial caller its supported format. Do you agree with this?

> + *
> + * Note that the bridge passed should normally be the bridge closest to the
> + * encoder, but possibly the bridge closest to an intermediate bridge in
> + * convoluted cases.
> + *

As I see this, any bridge in the arbitrary long pipeline should call
this operation on next bridge if it supports different output formats.
Ie. I would not name here the encoder nor refer to the bridge position
in the pipeline.

> + * RETURNS:
> + * The number of bus input formats the bridge accepts. Zero means that
> + * the chain of bridges are not converting the bus format and that the
> + * format of the drm_connector should be used.

How do we get to the connector format from a bridge that has maybe
other components in between in the pipeline?

If a bridge do not report any supported format, it won't implement
this callback and things will work as they work today.

> + */
> +int drm_bridge_input_formats(struct drm_bridge *bridge,
> +  const u32 **bus_formats)
> +{
> + int ret = 0;
> +
> + if (!bridge)
> + return 0;
> +
> + if (bridge->funcs->input_formats)
> + ret = bridge->funcs->input_formats(bridge, bus_formats);
> +
> + return ret ?: drm_bridge_input_formats(bridge->next, bus_formats);

See my comment on the call propagation down in the pipeline.

> +}
> +EXPORT_SYMBOL(drm_bridge_input_formats);
> +
>  #ifdef CONFIG_OF
>  /**
>   * of_drm_find_bridge - find the bridge corresponding to the device node in
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 682d01ba920c..ae8d3c4af0b8 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -220,6 +220,22 @@ struct drm_bridge_funcs {
>* The enable callback is optional.
>*/
>   void (*enable)(struct drm_bridge *bridge);
> +
> + /**
> +  * @input_formats:
> +  *
> +  * The callback reports the expected bus input formats of the bridge.
> +  *
> +  * The @input_formats callback is optional. The bridge is assumed to
> +  * not convert the bus format if the callback is not installed.
> +  *
> +  * RETURNS:
> +  *
> +  * Zero if the bridge does not convert the bus format, otherwise the
> +  * number of bus input formats returned in _formats.
> +  */
> + int (*input_formats)(struct 

Re: [Intel-gfx] [PATCH 15/23] drm: Stop updating plane->crtc/fb/old_fb on atomic drivers

2018-03-27 Thread Ville Syrjälä
On Tue, Mar 27, 2018 at 09:57:41AM +0200, Daniel Vetter wrote:
> On Thu, Mar 22, 2018 at 05:23:05PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > Stop playing around with plane->crtc/fb/old_fb with atomic
> > drivers. Make life a lot simpler when we don't have to do the
> > magic old_fb vs. fb dance around plane updates. That way we
> > can't risk plane->fb getting out of sync with plane->state->fb
> > and we're less likely to leak any refcounts as well.
> > 
> > Signed-off-by: Ville Syrjälä 
> 
> What's the reason for this patch being in the middle of the patch series?
> I figured it's savest if we put this at the end? If you need parts of this
> here, we definitely need to split out the WARN_ON hunk, since you haven't
> fixed up everything yet at this point here.

Yeah, the ordering is probably not great. I think I had some idea why
I had to do "cleanup drivers a bit, do core/helper stuff, cleanup some
more drivers, do other core/helper stuff". But I can't even recall that
reason now. Most likely I had just managed to confuse myself by that
time.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/drm_atomic.c| 55 
> > -
> >  drivers/gpu/drm/drm_atomic_helper.c | 15 +-
> >  drivers/gpu/drm/drm_crtc.c  |  8 --
> >  drivers/gpu/drm/drm_fb_helper.c |  7 -
> >  drivers/gpu/drm/drm_framebuffer.c   |  5 
> >  drivers/gpu/drm/drm_plane.c | 14 ++
> >  drivers/gpu/drm/drm_plane_helper.c  |  4 ++-
> >  include/drm/drm_atomic.h|  3 --
> >  8 files changed, 24 insertions(+), 87 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 7d25c42f22db..b16cc37e2adf 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -692,6 +692,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state 
> > *state,
> >  
> > WARN_ON(!state->acquire_ctx);
> >  
> > +   /* the legacy pointers should never be set */
> > +   WARN_ON(plane->fb);
> > +   WARN_ON(plane->old_fb);
> > +   WARN_ON(plane->crtc);
> > +
> > plane_state = drm_atomic_get_existing_plane_state(state, plane);
> > if (plane_state)
> > return plane_state;
> > @@ -2021,45 +2026,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
> > *state,
> > return ret;
> >  }
> >  
> > -/**
> > - * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb 
> > pointers.
> > - *
> > - * @dev: drm device to check.
> > - * @plane_mask: plane mask for planes that were updated.
> > - * @ret: return value, can be -EDEADLK for a retry.
> > - *
> > - * Before doing an update _plane.old_fb is set to _plane.fb, but 
> > before
> > - * dropping the locks old_fb needs to be set to NULL and plane->fb 
> > updated. This
> > - * is a common operation for each atomic update, so this call is split off 
> > as a
> > - * helper.
> > - */
> > -void drm_atomic_clean_old_fb(struct drm_device *dev,
> > -unsigned plane_mask,
> > -int ret)
> > -{
> > -   struct drm_plane *plane;
> > -
> > -   /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
> > -* locks (ie. while it is still safe to deref plane->state).  We
> > -* need to do this here because the driver entry points cannot
> > -* distinguish between legacy and atomic ioctls.
> > -*/
> > -   drm_for_each_plane_mask(plane, dev, plane_mask) {
> > -   if (ret == 0) {
> > -   struct drm_framebuffer *new_fb = plane->state->fb;
> > -   if (new_fb)
> > -   drm_framebuffer_get(new_fb);
> > -   plane->fb = new_fb;
> > -   plane->crtc = plane->state->crtc;
> > -
> > -   if (plane->old_fb)
> > -   drm_framebuffer_put(plane->old_fb);
> > -   }
> > -   plane->old_fb = NULL;
> > -   }
> > -}
> > -EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> > -
> >  /**
> >   * DOC: explicit fencing properties
> >   *
> > @@ -2280,9 +2246,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > unsigned int copied_objs, copied_props;
> > struct drm_atomic_state *state;
> > struct drm_modeset_acquire_ctx ctx;
> > -   struct drm_plane *plane;
> > struct drm_out_fence_state *fence_state;
> > -   unsigned plane_mask;
> > int ret = 0;
> > unsigned int i, j, num_fences;
> >  
> > @@ -2322,7 +2286,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
> >  
> >  retry:
> > -   plane_mask = 0;
> > copied_objs = 0;
> > copied_props = 0;
> > fence_state = NULL;
> > @@ -2393,12 +2356,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > copied_props++;
> > }
> >  
> > -   if (obj->type == DRM_MODE_OBJECT_PLANE && 

Re: [PATCH v6 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

2018-03-27 Thread Vladimir Zapolskiy
Hi Jacopo,

On 03/27/2018 11:57 AM, jacopo mondi wrote:
> Hi Vladimir,
> 
> On Tue, Mar 27, 2018 at 11:30:29AM +0300, Vladimir Zapolskiy wrote:
>> Hi Sergei,
>>
>> On 03/27/2018 11:27 AM, Sergei Shtylyov wrote:
>>> Hello!
>>>
>>> On 3/27/2018 10:33 AM, jacopo mondi wrote:
>>> [...]
> Document Thine THC63LVD1024 LVDS decoder device tree bindings.
>
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Andrzej Hajda 
> Reviewed-by: Niklas Söderlund 
> ---
>   .../bindings/display/bridge/thine,thc63lvd1024.txt | 66 
> +++
>   1 file changed, 66 insertions(+)
>   create mode 100644
> Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> new file mode 100644
> index 000..8225c6a
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> @@ -0,0 +1,66 @@
> +Thine Electronics THC63LVD1024 LVDS decoder
> +---
> +
> +The THC63LVD1024 is a dual link LVDS receiver designed to convert 
> LVDS
> streams
> +to parallel data outputs. The chip supports single/dual input/output 
> modes,
> +handling up to two two input LVDS stream and up to two digital 
> CMOS/TTL
> outputs.
> +
> +Single or dual operation modes, output data mapping and DDR output 
> modes
> are
> +configured through input signals and the chip does not expose any 
> control
> bus.
> +
> +Required properties:
> +- compatible: Shall be "thine,thc63lvd1024"
> +
> +Optional properties:
> +- vcc-supply: Power supply for TTL output and digital circuitry
> +- cvcc-supply: Power supply for TTL CLOCKOUT signal
> +- lvcc-supply: Power supply for LVDS inputs
> +- pvcc-supply: Power supply for PLL circuitry
 As explained in a comment to one of the previous versions of this 
 series, I'm
 tempted to make vcc-supply mandatory and drop the three other power 
 supplies
 for now, as I believe there's very little chance they will be 
 connected to
 separately controllable regulators (all supplies use the same 
 voltage). In the
 very unlikely event that this occurs in design we need to support in 
 the
 future, the cvcc, lvcc and pvcc supplies can be added later as optional
 without breaking backward compatibility.
>>> I'm okay with that.
>>>
 Apart from that,

 Reviewed-by: Laurent Pinchart 

> +- pdwn-gpios: Power down GPIO signal. Active low
>>> powerdown-gpios is the semi-standard name.
>>>
>> right, I've also noticed it. If possible please avoid shortenings in
>> property names.
>
> It is not shortening, it just follow pin name from decoder's datasheet.
>
>>
> +- oe-gpios: Output enable GPIO signal. Active high
> +
>> And this one is also a not ever met property name, please consider to
>> rename it to 'enable-gpios', for instance display panels define it.
>
>
> Again, it follows datasheet naming scheme. Has something changed in DT
> conventions?

 Seconded. My understanding is that the property name should reflect
 what reported in the the chip manual. For THC63LVD1024 the enable and
 power down pins are named 'OE' and 'PDWN' respectively.
>>>
>>> But don't we need the vendor prefix in the prop names then, like
>>> "renesas,oe-gpios" then?
>>>
>>
>> Seconded, with a correction to "thine,oe-gpios".
>>
> 
> mmm, okay then...
> 
> A grep for that semi-standard properties names in Documentation/
> returns only usage examples and no actual definitions, so I assume this
> is why they are semi-standard.

Here we have to be specific about a particular property, let it be 'oe-gpios'
vs. 'enable-gpios' and let's collect some statistics:

% grep -Hr oe-gpios Documentation/devicetree/bindings/* | wc -l
0

$ grep -Hr enable-gpios Documentation/devicetree/bindings/* | wc -l
86

While 'thine,oe-gpios' would be correct, I see no reason to introduce a vendor
specific property to define a pin with a common and well understood purpose.

If you go forward with the vendor specific prefix, apparently you can set the 
name
to 'thine,oe-gpio' (single) or even to 'thine,oe', or does the datasheet names
the pin as "OE GPIO" or "OE connected to a GPIO"? I guess no.

Standards do not define 

Re: [PATCH v3] drm/xen-front: Add support for Xen PV display frontend

2018-03-27 Thread Oleksandr Andrushchenko

Hi, Daniel!

On 03/26/2018 03:46 PM, Oleksandr Andrushchenko wrote:

On 03/26/2018 11:18 AM, Daniel Vetter wrote:

On Fri, Mar 23, 2018 at 05:54:49PM +0200, Oleksandr Andrushchenko wrote:
My apologies, but I found a few more things that look strange and 
should
be cleaned up. Sorry for this iterative review approach, but I 
think we're

slowly getting there.

Thank you for reviewing!

Cheers, Daniel


---
+static int xen_drm_drv_dumb_create(struct drm_file *filp,
+    struct drm_device *dev, struct drm_mode_create_dumb *args)
+{
+    struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+    struct drm_gem_object *obj;
+    int ret;
+
+    ret = xen_drm_front_gem_dumb_create(filp, dev, args);
+    if (ret)
+    goto fail;
+
+    obj = drm_gem_object_lookup(filp, args->handle);
+    if (!obj) {
+    ret = -ENOENT;
+    goto fail_destroy;
+    }
+
+    drm_gem_object_unreference_unlocked(obj);
You can't drop the reference while you keep using the object, 
someone else

might sneak in and destroy your object. The unreference always must be
last.

Will fix, thank you

+
+    /*
+ * In case of CONFIG_DRM_XEN_FRONTEND_CMA gem_obj is constructed
+ * via DRM CMA helpers and doesn't have ->pages allocated
+ * (xendrm_gem_get_pages will return NULL), but instead can 
provide

+ * sg table
+ */
+    if (xen_drm_front_gem_get_pages(obj))
+    ret = xen_drm_front_dbuf_create_from_pages(
+    drm_info->front_info,
+    xen_drm_front_dbuf_to_cookie(obj),
+    args->width, args->height, args->bpp,
+    args->size,
+    xen_drm_front_gem_get_pages(obj));
+    else
+    ret = xen_drm_front_dbuf_create_from_sgt(
+    drm_info->front_info,
+    xen_drm_front_dbuf_to_cookie(obj),
+    args->width, args->height, args->bpp,
+    args->size,
+    xen_drm_front_gem_get_sg_table(obj));
+    if (ret)
+    goto fail_destroy;
+
The above also has another race: If you construct an object, then 
it must
be fully constructed by the time you publish it to the wider world. 
In gem
this is done by calling drm_gem_handle_create() - after that 
userspace can

get at your object and do nasty things with it in a separate thread,
forcing your driver to Oops if the object isn't fully constructed yet.

That means you need to redo this code here to make sure that the gem
object is fully set up (including pages and sg tables) _before_ 
anything

calls drm_gem_handle_create().

You are correct, I need to rework this code

This probably means you also need to open-code the cma side, by first
calling drm_gem_cma_create(), then doing any additional setup, and 
finally
doing the registration to userspace with drm_gem_handle_create as 
the very

last thing.
Although I tend to avoid open-coding, but this seems the necessary 
measure

here
Alternativet is to do the pages/sg setup only when you create an fb 
(and

drop the pages again when the fb is destroyed), but that requires some
refcounting/locking in the driver.
Not sure this will work: nothing prevents you from attaching 
multiple FBs to

a single dumb handle
So, not only ref-counting should be done here, but I also need to 
check if

the dumb buffer,
we are attaching to, has been created already

No, you must make sure that no dumb buffer can be seen by anyone else
before it's fully created. If you don't register it in the file_priv idr
using drm_gem_handle_create, no one else can get at your buffer. 
Trying to

paper over this race from all the other places breaks the gem core code
design, and is also much more fragile.

Yes, this is what I implement now, e.g. I do not create
any dumb handle until GEM is fully created. I was just
saying that alternative way when we do pages/sgt on FB
attach will not work in my case

So, I will rework with open-coding some stuff from CMA helpers

Aside: There's still a lot of indirection and jumping around which 
makes

the code a bit hard to follow.
Probably I am not sure of which indirection we are talking about, 
could you

please
specifically mark those annoying you?

I think it's the same indirection we talked about last time, it still
annoys me. But it's still ok if you prefer this way I think :-)

Ok, probably this is because I'm looking at the driver
from an editor, but you are from your mail client ;)

+
+static void xen_drm_drv_release(struct drm_device *dev)
+{
+    struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+    struct xen_drm_front_info *front_info = drm_info->front_info;
+
+    drm_atomic_helper_shutdown(dev);
+    drm_mode_config_cleanup(dev);
+
+    xen_drm_front_evtchnl_free_all(front_info);
+    dbuf_free_all(_info->dbuf_list);
+
+    drm_dev_fini(dev);
+    kfree(dev);
+
+    /*
+ * Free now, as this release could be not due to rmmod, but
+ * due to the backend disconnect, making drm_info hang in
+ * memory until rmmod
+ */
+    

Re: [PATCH 02/10] drm/sun4i: Disable YUV channel when using the frontend and set interlace

2018-03-27 Thread Paul Kocialkowski
On Tue, 2018-03-27 at 11:18 +0200, Maxime Ripard wrote:
> On Tue, Mar 27, 2018 at 10:44:19AM +0200, Paul Kocialkowski wrote:
> > > > Also, is interlacing actually used on any of the video outputs
> > > > we
> > > > support? Perhaps RGB?
> > > 
> > > Composite would be a better guess :)
> > 
> > Oh and I was wondering what CVBS was about. Now I know!
> > It seems that we don't support it for now apparently, anyway.
> 
> What do you mean? We do support it.

Sorry, I think I was remembering the status for A10/A20 only. Glad to
see that it works on A13!

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 02/10] drm/sun4i: Disable YUV channel when using the frontend and set interlace

2018-03-27 Thread Maxime Ripard
On Tue, Mar 27, 2018 at 10:44:19AM +0200, Paul Kocialkowski wrote:
> > > Also, is interlacing actually used on any of the video outputs we
> > > support? Perhaps RGB?
> >
> > Composite would be a better guess :)
> 
> Oh and I was wondering what CVBS was about. Now I know!
> It seems that we don't support it for now apparently, anyway.

What do you mean? We do support it.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


  1   2   >