Re: [PATCH 3/6] drm/gem: use new ww_mutex_(un)lock_for_each macros
Am 14.06.19 um 14:59 schrieb Peter Zijlstra: On Fri, Jun 14, 2019 at 02:41:22PM +0200, Christian König wrote: Use the provided macros instead of implementing deadlock handling on our own. Signed-off-by: Christian König --- drivers/gpu/drm/drm_gem.c | 49 ++- 1 file changed, 12 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 50de138c89e0..6e4623d3bee2 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1307,51 +1307,26 @@ int drm_gem_lock_reservations(struct drm_gem_object **objs, int count, struct ww_acquire_ctx *acquire_ctx) { - int contended = -1; + struct ww_mutex *contended; int i, ret; ww_acquire_init(acquire_ctx, _ww_class); -retry: - if (contended != -1) { - struct drm_gem_object *obj = objs[contended]; - - ret = ww_mutex_lock_slow_interruptible(>resv->lock, - acquire_ctx); - if (ret) { - ww_acquire_done(acquire_ctx); - return ret; - } - } - - for (i = 0; i < count; i++) { - if (i == contended) - continue; - - ret = ww_mutex_lock_interruptible([i]->resv->lock, - acquire_ctx); - if (ret) { - int j; - - for (j = 0; j < i; j++) - ww_mutex_unlock([j]->resv->lock); - - if (contended != -1 && contended >= i) - ww_mutex_unlock([contended]->resv->lock); - - if (ret == -EDEADLK) { - contended = i; - goto retry; retry here, starts the whole locking loop. - } - - ww_acquire_done(acquire_ctx); - return ret; - } - } +#define ww_mutex_unlock_for_each(loop, pos, contended) \ + if (!IS_ERR(contended)) { \ + if (contended) \ + ww_mutex_unlock(contended); \ + contended = (pos); \ + loop { \ + if (contended == (pos)) \ + break; \ + ww_mutex_unlock(pos); \ + } \ + } + +#define ww_mutex_lock_for_each(loop, pos, contended, ret, intr, ctx) \ + for (contended = ERR_PTR(-ENOENT); ({ \ + __label__ relock, next; \ + ret = -ENOENT; \ + if (contended == ERR_PTR(-ENOENT)) \ + contended = NULL; \ + else if (contended == ERR_PTR(-EDEADLK))\ + contended = (pos); \ + else\ + goto next; \ + loop { \ + if (contended == (pos)) { \ + contended = NULL; \ + continue; \ + } \ +relock: \ + ret = !(intr) ? ww_mutex_lock(pos, ctx) : \ + ww_mutex_lock_interruptible(pos, ctx); \ + if (ret == -EDEADLK) { \ + ww_mutex_unlock_for_each(loop, pos, \ +contended);\ + contended = ERR_PTR(-EDEADLK); \ + goto relock;\ while relock here continues where it left of and doesn't restart @loop. Or am I reading this monstrosity the wrong way? contended = ERR_PTR(-EDEADLK) makes sure that the whole loop is restarted after retrying to acquire the lock. See the "if" above "loop". Christian. + } \ + break; \ +next: \ + continue; \ + }
Re: [PATCH 3/6] drm/gem: use new ww_mutex_(un)lock_for_each macros
On Fri, Jun 14, 2019 at 03:06:10PM +0200, Christian König wrote: > Am 14.06.19 um 14:59 schrieb Peter Zijlstra: > > +#define ww_mutex_lock_for_each(loop, pos, contended, ret, intr, ctx) \ > > + for (contended = ERR_PTR(-ENOENT); ({ \ > > + __label__ relock, next; \ > > + ret = -ENOENT; \ > > + if (contended == ERR_PTR(-ENOENT)) \ > > + contended = NULL; \ > > + else if (contended == ERR_PTR(-EDEADLK))\ > > + contended = (pos); \ > > + else\ > > + goto next; \ > > + loop { \ > > + if (contended == (pos)) { \ > > + contended = NULL; \ > > + continue; \ > > + } \ > > +relock: > > \ > > + ret = !(intr) ? ww_mutex_lock(pos, ctx) : \ > > + ww_mutex_lock_interruptible(pos, ctx); \ > > + if (ret == -EDEADLK) { \ > > + ww_mutex_unlock_for_each(loop, pos, \ > > +contended);\ > > + contended = ERR_PTR(-EDEADLK); \ > > + goto relock;\ > > > > while relock here continues where it left of and doesn't restart @loop. > > Or am I reading this monstrosity the wrong way? > > contended = ERR_PTR(-EDEADLK) makes sure that the whole loop is restarted > after retrying to acquire the lock. > > See the "if" above "loop". ARGH, the loop inside the loop... Yeah, maybe, brain hurts. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] gpu: ipu-v3: image-convert: Fix image downsize coefficients
On Tue, 2019-06-11 at 18:16 -0700, Steve Longerbeam wrote: > The output of the IC downsizer unit in both dimensions must be <= 1024 > before being passed to the IC resizer unit. This was causing corrupted > images when: > > input_dim > 1024, and > input_dim / 2 < output_dim < input_dim > > Some broken examples were 1920x1080 -> 1024x768 and 1920x1080 -> > 1280x1080. > > Fixes: 70b9b6b3bcb21 ("gpu: ipu-v3: image-convert: calculate per-tile > resize coefficients") > > Signed-off-by: Steve Longerbeam All applied on the imx-drm/fixes branch. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] gpu: ipu-v3: image-convert: Enable double write reduction
On Thu, 2019-06-13 at 18:02 -0700, Steve Longerbeam wrote: > For the write channels with 4:2:0 subsampled YUV formats, avoid chroma > overdraw by only writing chroma for even lines (skip odd chroma rows). > This reduces necessary write memory bandwidth by at least 25% (more > with rotation enabled). > > Signed-off-by: Steve Longerbeam Applied on imx-drm/next, thanks! regards Philipp
Re: [PATCH v10 01/11] drm/sun4i: dsi: Fix TCON DRQ set bits
On Fri, Jun 14, 2019 at 12:03:13PM +0530, Jagan Teki wrote: > On Thu, Jun 13, 2019 at 6:56 PM Maxime Ripard > wrote: > > > > On Wed, Jun 05, 2019 at 01:17:11PM +0530, Jagan Teki wrote: > > > On Tue, Jun 4, 2019 at 3:30 PM Maxime Ripard > > > wrote: > > > > > > > > On Wed, May 29, 2019 at 11:44:56PM +0530, Jagan Teki wrote: > > > > > On Wed, May 29, 2019 at 8:24 PM Maxime Ripard > > > > > wrote: > > > > > > > > > > > > On Fri, May 24, 2019 at 03:48:51PM +0530, Jagan Teki wrote: > > > > > > > On Fri, May 24, 2019 at 2:04 AM Maxime Ripard > > > > > > > wrote: > > > > > > > > > > > > > > > > On Mon, May 20, 2019 at 02:33:08PM +0530, Jagan Teki wrote: > > > > > > > > > According to "DRM kernel-internal display mode structure" in > > > > > > > > > include/drm/drm_modes.h the current driver is trying to > > > > > > > > > include > > > > > > > > > sync timings along with front porch value while checking and > > > > > > > > > computing drq set bits in non-burst mode. > > > > > > > > > > > > > > > > > > mode->hsync_end - mode->hdisplay => horizontal front porch + > > > > > > > > > sync > > > > > > > > > > > > > > > > > > With adding additional sync timings, the dsi controller leads > > > > > > > > > to > > > > > > > > > wrong drq set bits for "bananapi,s070wv20-ct16" panel which > > > > > > > > > indeed > > > > > > > > > trigger panel flip_done timed out as: > > > > > > > > > > > > > > > > > > WARNING: CPU: 0 PID: 31 at > > > > > > > > > drivers/gpu/drm/drm_atomic_helper.c:1429 > > > > > > > > > drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0 > > > > > > > > > [CRTC:46:crtc-0] vblank wait timed out > > > > > > > > > Modules linked in: > > > > > > > > > CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted > > > > > > > > > 5.1.0-next-20190514-00026-g01f0c75b902d-dirty #13 > > > > > > > > > Hardware name: Allwinner sun8i Family > > > > > > > > > Workqueue: events deferred_probe_work_func > > > > > > > > > [] (unwind_backtrace) from [] > > > > > > > > > (show_stack+0x10/0x14) > > > > > > > > > [] (show_stack) from [] > > > > > > > > > (dump_stack+0x84/0x98) > > > > > > > > > [] (dump_stack) from [] > > > > > > > > > (__warn+0xfc/0x114) > > > > > > > > > [] (__warn) from [] > > > > > > > > > (warn_slowpath_fmt+0x44/0x68) > > > > > > > > > [] (warn_slowpath_fmt) from [] > > > > > > > > > (drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0) > > > > > > > > > [] (drm_atomic_helper_wait_for_vblanks.part.1) > > > > > > > > > from [] > > > > > > > > > (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c) > > > > > > > > > [] (drm_atomic_helper_commit_tail_rpm) from > > > > > > > > > [] (commit_tail+0x40/0x6c) > > > > > > > > > [] (commit_tail) from [] > > > > > > > > > (drm_atomic_helper_commit+0xbc/0x128) > > > > > > > > > [] (drm_atomic_helper_commit) from [] > > > > > > > > > (restore_fbdev_mode_atomic+0x1cc/0x1dc) > > > > > > > > > [] (restore_fbdev_mode_atomic) from [] > > > > > > > > > (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0) > > > > > > > > > [] (drm_fb_helper_restore_fbdev_mode_unlocked) > > > > > > > > > from [] (drm_fb_helper_set_par+0x30/0x54) > > > > > > > > > [] (drm_fb_helper_set_par) from [] > > > > > > > > > (fbcon_init+0x560/0x5ac) > > > > > > > > > [] (fbcon_init) from [] > > > > > > > > > (visual_init+0xbc/0x104) > > > > > > > > > [] (visual_init) from [] > > > > > > > > > (do_bind_con_driver+0x1b0/0x390) > > > > > > > > > [] (do_bind_con_driver) from [] > > > > > > > > > (do_take_over_console+0x13c/0x1c4) > > > > > > > > > [] (do_take_over_console) from [] > > > > > > > > > (do_fbcon_takeover+0x74/0xcc) > > > > > > > > > [] (do_fbcon_takeover) from [] > > > > > > > > > (notifier_call_chain+0x44/0x84) > > > > > > > > > [] (notifier_call_chain) from [] > > > > > > > > > (__blocking_notifier_call_chain+0x48/0x60) > > > > > > > > > [] (__blocking_notifier_call_chain) from > > > > > > > > > [] (blocking_notifier_call_chain+0x18/0x20) > > > > > > > > > [] (blocking_notifier_call_chain) from > > > > > > > > > [] (register_framebuffer+0x1e0/0x2f8) > > > > > > > > > [] (register_framebuffer) from [] > > > > > > > > > (__drm_fb_helper_initial_config_and_unlock+0x2fc/0x50c) > > > > > > > > > [] (__drm_fb_helper_initial_config_and_unlock) > > > > > > > > > from [] (drm_fbdev_client_hotplug+0xe8/0x1b8) > > > > > > > > > [] (drm_fbdev_client_hotplug) from [] > > > > > > > > > (drm_fbdev_generic_setup+0x88/0x118) > > > > > > > > > [] (drm_fbdev_generic_setup) from [] > > > > > > > > > (sun4i_drv_bind+0x128/0x160) > > > > > > > > > [] (sun4i_drv_bind) from [] > > > > > > > > > (try_to_bring_up_master+0x164/0x1a0) > > > > > > > > > [] (try_to_bring_up_master) from [] > > > > > > > > > (__component_add+0x94/0x140) > > > > > > > > > [] (__component_add) from [] > > > > > > > > > (sun6i_dsi_probe+0x144/0x234) > > > > > > > > > [] (sun6i_dsi_probe) from [] > > > > > > > > > (platform_drv_probe+0x48/0x9c) > > > > > > > > > []
Re: [BUG] lockdep splat with kernfs lockdep annotations and slab mutex from drm patch??
Quoting Steven Rostedt (2019-06-14 14:39:14) > I'm trying to debug some code where I need KASAN and lockdep enabled > but I can't get past this splat unrelated to my code. I bisected it > down to this commit: > > 32eb6bcfdda9da ("drm/i915: Make request allocation caches global") > > To make sure it wasn't a bad bisect, I removed the patch and the splat > goes away. I add the patch back, and the splat returns. I did this > several times, so I have a large confidence that this is the cause of > the splat, or at least it is the commit that triggers whatever is going > on. > > Perhaps all the cache updates caused the slab_mutex to be called > inverse of the kernfs locking? > > Attached is my config. > > Also might be helpful, the splat happens: > > kernfs_fop_write() > ops->write == sysfs_kf_write >sysfs_kf_write() > ops->store = slab_attr_store More interestingly, static ssize_t slab_attr_store(struct kobject *kobj, struct attribute *attr, const char *buf, size_t len) { struct slab_attribute *attribute; struct kmem_cache *s; int err; attribute = to_slab_attr(attr); s = to_slab(kobj); if (!attribute->store) return -EIO; err = attribute->store(s, buf, len); #ifdef CONFIG_MEMCG if (slab_state >= FULL && err >= 0 && is_root_cache(s)) { struct kmem_cache *c; mutex_lock(_mutex); so it happens to hit the error + FULL case with the additional slabcaches? Anyway, according to lockdep, it is dangerous to use the slab_mutex inside slab_attr_store(). -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 01/12] drm/connector: Add documentation for drm_cmdline_mode
The struct drm_cmdline_mode holds the result of the command line parsers. However, it wasn't documented so far, so let's do that. Signed-off-by: Maxime Ripard --- include/drm/drm_connector.h | 86 +- 1 file changed, 84 insertions(+), 2 deletions(-) diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 47e749b74e5f..f9cfa96f5d7e 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -904,18 +904,100 @@ struct drm_connector_funcs { const struct drm_connector_state *state); }; -/* mode specified on the command line */ +/** + * struct drm_cmdline_mode - DRM Mode passed through the kernel command-line + * + * Each connector can have an initial mode with additional options + * passed through the kernel command line. This structure allows to + * express those parameters and will be filled by the command-line + * parser. + */ struct drm_cmdline_mode { + /** +* @specified: +* +* Has a mode been read from the command-line? +*/ bool specified; + + /** +* @refresh_specified: +* +* Did the mode has a preferred refresh rate? +*/ bool refresh_specified; + + /** +* @bpp_specified: +* +* Did the mode has a preferred BPP? +*/ bool bpp_specified; - int xres, yres; + + /** +* @xres: +* +* Active resolution on the X axis, in pixels. +*/ + int xres; + + /** +* @yres: +* +* Active resolution on the Y axis, in pixels. +*/ + int yres; + + /** +* @bpp: +* +* Bits per pixels for the mode. +*/ int bpp; + + /** +* @refresh: +* +* Refresh rate, in Hertz. +*/ int refresh; + + /** +* @rb: +* +* Do we need to use reduced blanking? +*/ bool rb; + + /** +* @interlace: +* +* The mode is interlaced. +*/ bool interlace; + + /** +* @cvt: +* +* The timings will be calculated using the VESA Coordinated +* Video Timings instead of looking up the mode from a table. +*/ bool cvt; + + /** +* @margins: +* +* Add margins to the mode calculation (1.8% of xres rounded +* down to 8 pixels and 1.8% of yres). +*/ bool margins; + + /** +* @force: +* +* Ignore the hotplug state of the connector, and force its +* state to one of the DRM_FORCE_* values. +*/ enum drm_connector_force force; }; -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 00/12] drm/vc4: Allow for more boot-time configuration
Hi, The proprietary stack for the RaspberryPi allows for a number of video parameters widely used by their users, but yet don't have any equivalents in the mainline kernel. Those options are detailed here: https://www.raspberrypi.org/documentation/configuration/config-txt/video.md While not all of them are desirable to have in the mainline kernel, some of them still have value, such as properties to initialise the overscan or rotation parameters. This series is an attempt to support those, and is based on a rewrite of the command line parser I did a couple of years ago that never reached upstream (due to a lack of time on my side). While this parser was initially made to deal with named modes (in order to support TV modes), it also allowed to extend it more easily, which is why it's resurrected. It relies on the series "drm/fb-helper: Move modesetting code to drm_client" by Noralf Trønnes found here: https://patchwork.freedesktop.org/series/58598/ Let me know what you think, Maxime Changes from v3: - Add documentation for drm_cmdline_mode and the new variables - Move the TV properties reset to a helper - Fix a missing X resolution or a missing Y resolution - Add more tests - Add the rotation to the orientation - Fix the reflection - Change the name of the drm_client_panel_rotation function - Rebased on top of current next Changes from v2: - Fixed some sparse warnings - Rebased on top of next and Noralf series - Moved the property initialisation to vc4 reset hook - Added documentation for the new drm_cmdline_mode - Renamed overscan to tv_margins to be consistent with the APIs Changes from v1: - Dropped the patches to deal with EDID - Added the unit test as selftest - Rebased on next Maxime Ripard (12): drm/connector: Add documentation for drm_cmdline_mode drm/client: Restrict the plane_state scope drm/client: Restrict the rotation check to the rotation itself drm/client: Change drm_client_panel_rotation name drm/modes: Rewrite the command line parser drm/modes: Support modes names on the command line drm/modes: Allow to specify rotation and reflection on the commandline drm/connector: Introduce a TV margins structure drm/atomic: Add a function to reset connector TV properties drm/modes: Parse overscan properties drm/selftests: Add command line parser selftests drm/vc4: hdmi: Set default state margin at reset drivers/gpu/drm/drm_atomic_state_helper.c | 18 +- drivers/gpu/drm/drm_client_modeset.c| 44 +- drivers/gpu/drm/drm_connector.c | 3 +- drivers/gpu/drm/drm_fb_helper.c | 2 +- drivers/gpu/drm/drm_modes.c | 469 +-- drivers/gpu/drm/selftests/Makefile | 2 +- drivers/gpu/drm/selftests/drm_cmdline_selftests.h | 55 +- drivers/gpu/drm/selftests/test-drm_cmdline_parser.c | 918 +- drivers/gpu/drm/vc4/vc4_hdmi.c | 8 +- include/drm/drm_atomic_state_helper.h | 1 +- include/drm/drm_client.h| 2 +- include/drm/drm_connector.h | 148 +- 12 files changed, 1529 insertions(+), 141 deletions(-) create mode 100644 drivers/gpu/drm/selftests/drm_cmdline_selftests.h create mode 100644 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c base-commit: 1123a3310ed6ad290be0fa4f2e995a7d68e108e2 -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Emil Velikov wrote: > From: Emil Velikov > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > render node. A seemingly deliberate design decision. > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > yet not all userspace checks if it's authenticated, but instead uses > uncommon assumptions. > > After days of digging through git log and testing, only a single (ab)use > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > assuming that failure implies lack of authentication. > > Affected versions are: > - the whole 18.2.x series, which is EOL > - the whole 18.3.x series, which is EOL > - the 19.0.x series, prior to 19.0.4 > > Add a special quirk for that case, thus we can drop DRM_AUTH bits as > mentioned earlier. > > Since all the affected userspace is EOL, we also add a kconfig option > to disable this quirk. > > The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT > > Cc: Alex Deucher > Cc: Christian König > Cc: amd-...@lists.freedesktop.org > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Emil Velikov > --- > drivers/gpu/drm/amd/amdgpu/Kconfig | 16 > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 12 +++- > drivers/gpu/drm/drm_ioctl.c | 5 + > include/drm/drm_ioctl.h | 17 + > 4 files changed, 49 insertions(+), 1 deletion(-) > Hi Christian, In the following, I would like to summarise and emphasize the need for DRM_AUTH removal. I would kindly ask you to spend a couple of minutes extra reading it. Today DRM drivers* do not make any distinction between primary and render node clients. Thus for a render capable driver, any premise of separation, security or otherwise imposed via DRM_AUTH is a fallacy. Considering the examples of flaky or outright missing drmAuth in prime open-source projects (mesa, kmscube, libva) we can reasonably assume other projects exbibit the same problem. For these and/or other reasons, two DRM drivers have dropped DRM_AUTH since day one. Since we are interested in providing consistent and predictable behaviour to user-space, dropping DRM_AUTH seems to be the most effective way forward. Of course, I'd be more than happy to hear for any other way to achieve the goal outlined. Based on the series, other maintainers agree with the idea/goal here. Amdgpu not following the same pattern would compromise predictability across drivers and complicate userspace, so I would kindly ask you to reconsider. Alternatively, I see two ways to special case: - New flag annotating amdgpu/radeon - akin to the one proposed earlier - Check for amdgpu/radeon in core DRM Now on your pain point - not allowing render iocts via the primary node, and how this patch could make it harder to achieve that goal. While I love the idea, there are number of obstacles that prevent us from doing so at this time: - Ensuring both old and new userspace does not regress - Drivers (QXL, others?) do not expose a render node - We want to avoid driver specific behaviour The only way forward that I can see is: - Address QXL/others to expose render nodes - Add a Kconfig toggle to disable !KMS ioctls via the primary node - Jump-start ^^ with distribution X - Fix user-space fallouts - X months down the line, flip the Kconfig - In case of regressions - revert the KConfig, goto Fix user-space... That said, the proposal will not conflict with the DRM_AUTH removal. If anything it is step 0.5 of the grand master plan. Tl;Dr: Removing DRM_AUTH is orthogonal to not allowing render iocts via the primary node. Here's an idea how to achieve the latter. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/16] cnic: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible modifiers of __GFP_NORETRY or __GFP_NOWARN. Signed-off-by: Christoph Hellwig --- drivers/net/ethernet/broadcom/cnic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c index 57dc3cbff36e..bd1c993680e5 100644 --- a/drivers/net/ethernet/broadcom/cnic.c +++ b/drivers/net/ethernet/broadcom/cnic.c @@ -1028,7 +1028,7 @@ static int __cnic_alloc_uio_rings(struct cnic_uio_dev *udev, int pages) udev->l2_ring_size = pages * CNIC_PAGE_SIZE; udev->l2_ring = dma_alloc_coherent(>pdev->dev, udev->l2_ring_size, >l2_ring_map, - GFP_KERNEL | __GFP_COMP); + GFP_KERNEL); if (!udev->l2_ring) return -ENOMEM; @@ -1036,7 +1036,7 @@ static int __cnic_alloc_uio_rings(struct cnic_uio_dev *udev, int pages) udev->l2_buf_size = CNIC_PAGE_ALIGN(udev->l2_buf_size); udev->l2_buf = dma_alloc_coherent(>pdev->dev, udev->l2_buf_size, >l2_buf_map, - GFP_KERNEL | __GFP_COMP); + GFP_KERNEL); if (!udev->l2_buf) { __cnic_free_uio_rings(udev); return -ENOMEM; -- 2.20.1
[PATCH 05/16] drm: don't mark pages returned from drm_pci_alloc reserved
We are not allowed to call virt_to_page on pages returned from dma_alloc_coherent, as in many cases the virtual address returned is aactually a kernel direct mapping. Also there generally is no need to mark dma memory as reserved. Signed-off-by: Christoph Hellwig --- drivers/gpu/drm/drm_bufs.c | 16 +--- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index 7418872d87c6..b640437ce90f 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -77,13 +77,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali return NULL; } - /* XXX - Is virt_to_page() legal for consistent mem? */ - /* Reserve */ - for (addr = (unsigned long)dmah->vaddr, sz = size; -sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) { - SetPageReserved(virt_to_page((void *)addr)); - } - return dmah; } @@ -97,16 +90,9 @@ void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) unsigned long addr; size_t sz; - if (dmah->vaddr) { - /* XXX - Is virt_to_page() legal for consistent mem? */ - /* Unreserve */ - for (addr = (unsigned long)dmah->vaddr, sz = dmah->size; -sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) { - ClearPageReserved(virt_to_page((void *)addr)); - } + if (dmah->vaddr) dma_free_coherent(>pdev->dev, dmah->size, dmah->vaddr, dmah->busaddr); - } } /** -- 2.20.1
[PATCH 11/16] s390/ism: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible modifiers of __GFP_NORETRY or __GFP_NOWARN. Signed-off-by: Christoph Hellwig --- drivers/s390/net/ism_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c index 4fc2056bd227..4ff5506fa4c6 100644 --- a/drivers/s390/net/ism_drv.c +++ b/drivers/s390/net/ism_drv.c @@ -241,7 +241,8 @@ static int ism_alloc_dmb(struct ism_dev *ism, struct smcd_dmb *dmb) dmb->cpu_addr = dma_alloc_coherent(>pdev->dev, dmb->dmb_len, >dma_addr, - GFP_KERNEL | __GFP_NOWARN | __GFP_NOMEMALLOC | __GFP_COMP | __GFP_NORETRY); + GFP_KERNEL | __GFP_NOWARN | + __GFP_NORETRY); if (!dmb->cpu_addr) clear_bit(dmb->sba_idx, ism->sba_bitmap); -- 2.20.1
Re: [RFC/WIP] drm/rockchip: Support CRTC gamma LUT
Hi Boris, Am Freitag, 14. Juni 2019, 15:53:20 CEST schrieb Boris Brezillon: > On Thu, 13 Jun 2019 16:22:44 -0300 > Ezequiel Garcia wrote: > > > > +static int vop_gamma_lut_request(struct device *dev, > > +struct resource *res, struct vop *vop) > > +{ > > + resource_size_t offset = vop->data->gamma_lut_addr_off; > > + resource_size_t size = VOP_GAMMA_LUT_SIZE * 4; > > + > > + /* > > +* Some SoCs (e.g. RK3288) have the gamma LUT address after > > +* the MMU registers, which means we can't request and ioremap > > +* the entire register set. Other (e.g. RK3399) have gamma LUT > > +* address before MMU. > > +* > > +* Therefore, we need to request and ioremap those that haven't > > +* been already. > > +*/ > > + if (vop->len >= (offset + size)) { > > + vop->lut_regs = vop->regs + offset; > > + return 0; > > + } > > + > > + if (!devm_request_mem_region(dev, res->start + offset, > > +size, dev_name(dev))) { > > + dev_warn(dev, "can't request gamma lut region\n"); > > + return -EBUSY; > > + } > > + > > + vop->lut_regs = devm_ioremap(dev, res->start + offset, size); > > + if (!vop->lut_regs) { > > + dev_err(dev, "can't ioremap gamma lut address\n"); > > + devm_release_mem_region(dev, res->start + offset, size); > > + return -ENOMEM; > > + } > > Can't we patch the resource just after calling plaform_get_resource() > (and before calling devm_ioremap_resource()) so we don't have to add > these devm_request_mem_region()+devm_ioremap() calls here? The issue is that on the older rk3288 socs the vops memory map has the mmu registers (which get mapped separately) in between the core and lut registers. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/panel: Add support for Raydium RM67191 panel driver
Hi Robert. On top of the feedback from Fabio here is a bit more. > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Divide include up in block in following order: #include #include #incude Within each block sort alphabetically. Do not use the deprecated drmP.h - replace it with the necessary includes. Use an empty line between each include block. > + > +/* Write Manufacture Command Set Control */ > +#define WRMAUCCTR 0xFE > + > +/* Manufacturer Command Set pages (CMD2) */ > +struct cmd_set_entry { > + u8 cmd; > + u8 param; > +}; > + > +/* > + * There is no description in the Reference Manual about these commands. > + * We received them from vendor, so just use them as is. > + */ > +static const struct cmd_set_entry manufacturer_cmd_set[] = { > + {0xFE, 0x0B}, > + {0x28, 0x40}, > + {0x29, 0x4F}, ... > + {0x51, 0x04}, > +}; > + > +static const u32 rad_bus_formats[] = { > + MEDIA_BUS_FMT_RGB888_1X24, > + MEDIA_BUS_FMT_RGB666_1X18, > + MEDIA_BUS_FMT_RGB565_1X16, > +}; > + > +struct rad_panel { > + struct drm_panel base; In the other raydium driver we name this "panel", which is a more descriptive name. > + struct mipi_dsi_device *dsi; > + > + struct gpio_desc *reset; > + struct backlight_device *backlight; > + > + bool prepared; > + bool enabled; > + > + struct videomode vm; > + u32 width_mm; > + u32 height_mm; > +}; > + > +static int rad_panel_prepare(struct drm_panel *panel) > +{ > + struct rad_panel *rad = to_rad_panel(panel); > + > + if (rad->prepared) > + return 0; > + > + if (rad->reset) { > + gpiod_set_value(rad->reset, 0); > + usleep_range(5000, 1); > + gpiod_set_value(rad->reset, 1); > + usleep_range(2, 25000); > + } > + > + rad->prepared = true; > + > + return 0; > +} > + > +static int rad_panel_unprepare(struct drm_panel *panel) > +{ > + struct rad_panel *rad = to_rad_panel(panel); > + struct device *dev = >dsi->dev; > + > + if (!rad->prepared) > + return 0; > + > + if (rad->enabled) { > + DRM_DEV_ERROR(dev, "Panel still enabled!\n"); > + return -EPERM; > + } This seems like overkill, what should trigger this? > + > + if (rad->reset) { > + gpiod_set_value(rad->reset, 0); > + usleep_range(15000, 17000); > + gpiod_set_value(rad->reset, 1); > + } > + > + rad->prepared = false; > + > + return 0; > +} > + > +static int rad_panel_enable(struct drm_panel *panel) > +{ > + struct rad_panel *rad = to_rad_panel(panel); > + struct mipi_dsi_device *dsi = rad->dsi; > + struct device *dev = >dev; > + int color_format = color_format_from_dsi_format(dsi->format); > + u16 brightness; > + int ret; > + > + if (rad->enabled) > + return 0; > + > + if (!rad->prepared) { > + DRM_DEV_ERROR(dev, "Panel not prepared!\n"); > + return -EPERM; > + } Seems like overkill. > + > + dsi->mode_flags |= MIPI_DSI_MODE_LPM; > + > + ret = rad_panel_push_cmd_list(dsi); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "Failed to send MCS (%d)\n", ret); > + goto fail; > + } > + > + /* Select User Command Set table (CMD1) */ > + ret = mipi_dsi_generic_write(dsi, (u8[]){ WRMAUCCTR, 0x00 }, 2); > + if (ret < 0) > + goto fail; > + > + /* Software reset */ > + ret = mipi_dsi_dcs_soft_reset(dsi); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "Failed to do Software Reset (%d)\n", ret); > + goto fail; > + } > + > + usleep_range(15000, 17000); > + > + /* Set DSI mode */ > + ret = mipi_dsi_generic_write(dsi, (u8[]){ 0xC2, 0x0B }, 2); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "Failed to set DSI mode (%d)\n", ret); > + goto fail; > + } > + /* Set tear ON */ > + ret = mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "Failed to set tear ON (%d)\n", ret); > + goto fail; > + } > + /* Set tear scanline */ > + ret = mipi_dsi_dcs_set_tear_scanline(dsi, 0x380); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "Failed to set tear scanline (%d)\n", ret); > + goto fail; > + } > + /* Set pixel format */ > + ret = mipi_dsi_dcs_set_pixel_format(dsi, color_format); > + DRM_DEV_DEBUG_DRIVER(dev, "Interface color format set to 0x%x\n", > + color_format); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "Failed to set pixel format (%d)\n", ret); > + goto fail; > + } > + /* Set display brightness */ > + brightness = rad->backlight->props.brightness; > + ret =
Re: [PATCH 2/2] drm: debugfs: make drm_debugfs_create_files() never fail
On Fri, Jun 14, 2019 at 11:51:10AM +0200, Greg Kroah-Hartman wrote: > As stated before, there is no need to care if a debugfs function > succeeds or not, and no code logic in the kernel should ever change > based on a debugfs function return value, so make > drm_debugfs_create_files() never fail. If it encounters an > odd/rare/impossible error (i.e. out of memory, or a duplicate debugfs > filename to be created), just keep on moving as if nothing improper had > happened. > > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Greg Kroah-Hartman Applied this one to drm-misc-next, thanks. -Daniel > --- > drivers/gpu/drm/drm_debugfs.c | 26 ++ > 1 file changed, 6 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index 515569002c86..009e1c0ac7b4 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -173,9 +173,8 @@ int drm_debugfs_create_files(const struct drm_info_list > *files, int count, >struct dentry *root, struct drm_minor *minor) > { > struct drm_device *dev = minor->dev; > - struct dentry *ent; > struct drm_info_node *tmp; > - int i, ret; > + int i; > > for (i = 0; i < count; i++) { > u32 features = files[i].driver_features; > @@ -185,22 +184,13 @@ int drm_debugfs_create_files(const struct drm_info_list > *files, int count, > continue; > > tmp = kmalloc(sizeof(struct drm_info_node), GFP_KERNEL); > - if (tmp == NULL) { > - ret = -1; > - goto fail; > - } > - ent = debugfs_create_file(files[i].name, S_IFREG | S_IRUGO, > - root, tmp, _debugfs_fops); > - if (!ent) { > - DRM_ERROR("Cannot create > /sys/kernel/debug/dri/%pd/%s\n", > - root, files[i].name); > - kfree(tmp); > - ret = -1; > - goto fail; > - } > + if (tmp == NULL) > + continue; > > tmp->minor = minor; > - tmp->dent = ent; > + tmp->dent = debugfs_create_file(files[i].name, > + S_IFREG | S_IRUGO, root, tmp, > + _debugfs_fops); > tmp->info_ent = [i]; > > mutex_lock(>debugfs_lock); > @@ -208,10 +198,6 @@ int drm_debugfs_create_files(const struct drm_info_list > *files, int count, > mutex_unlock(>debugfs_lock); > } > return 0; > - > -fail: > - drm_debugfs_remove_files(files, count, minor); > - return ret; > } > EXPORT_SYMBOL(drm_debugfs_create_files); > > -- > 2.22.0 > -- 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
Re: [BUG] lockdep splat with kernfs lockdep annotations and slab mutex from drm patch??
Hello, On Fri, Jun 14, 2019 at 04:08:33PM +0100, Chris Wilson wrote: > #ifdef CONFIG_MEMCG > if (slab_state >= FULL && err >= 0 && is_root_cache(s)) { > struct kmem_cache *c; > > mutex_lock(_mutex); > > so it happens to hit the error + FULL case with the additional slabcaches? > > Anyway, according to lockdep, it is dangerous to use the slab_mutex inside > slab_attr_store(). Didn't really look into the code but it looks like slab_mutex is held while trying to remove sysfs files. sysfs file removal flushes on-going accesses, so if a file operation then tries to grab a mutex which is held during removal, it leads to a deadlock. Thanks. -- tejun
Re: [pull] amdgpu drm-fixes-5.2
On Wed, Jun 12, 2019 at 09:18:56PM -0500, Alex Deucher wrote: > Hi Dave, Daniel, > > Fixes for 5.2: > - Extend previous vce fix for resume to uvd and vcn > - Fix bounds checking in ras debugfs interface > - Fix a regression on SI using amdgpu > > The following changes since commit 671e2ee5ee2127179ca884b439ab6001a623edd6: > > Merge branch 'linux-5.2' of git://github.com/skeggsb/linux into drm-fixes > (2019-06-07 17:16:00 +1000) Somehow missed this one, but just found it before I wanted to push out the -fixes pull to Linus ... > are available in the Git repository at: > > git://people.freedesktop.org/~agd5f/linux drm-fixes-5.2 Pulled, thanks. -Daniel > > for you to fetch changes up to f3a5231c8f14acd42845e9e60f506b4e948f0e68: > > drm/amdgpu: return 0 by default in amdgpu_pm_load_smu_firmware (2019-06-12 > 20:39:49 -0500) > > > Alex Deucher (1): > drm/amdgpu: return 0 by default in amdgpu_pm_load_smu_firmware > > Dan Carpenter (1): > drm/amdgpu: Fix bounds checking in amdgpu_ras_is_supported() > > Shirish S (1): > drm/amdgpu/{uvd,vcn}: fetch ring's read_ptr after alloc > > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 4 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 +++- > drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 5 - > drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 5 - > 5 files changed, 15 insertions(+), 5 deletions(-) -- 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
Re: [PATCH] DRM: Rockchip: correct rate in the struct drm_dp_link assignment
Hi, Am Mittwoch, 5. Juni 2019, 10:04:24 CEST schrieb sandor...@nxp.com: > From: Sandor Yu > > variable of rate in the struct drm_dp_link should assign to > 162000/27/54/81. > > Signed-off-by: Sandor Yu applied to drm-misc-next after fixing patch subject and description to be in line with subsystem conventions: https://cgit.freedesktop.org/drm/drm-misc/commit/?id=de85ec271a864c05e99ad5ffbed9e95d1b65c757 Thanks Heiko > --- > drivers/gpu/drm/rockchip/cdn-dp-reg.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c > b/drivers/gpu/drm/rockchip/cdn-dp-reg.c > index 6c8b14f..0a2aebe 100644 > --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c > +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c > @@ -543,7 +543,7 @@ static int cdn_dp_get_training_status(struct > cdn_dp_device *dp) > if (ret) > goto err_get_training_status; > > - dp->link.rate = status[0]; > + dp->link.rate = drm_dp_bw_code_to_link_rate(status[0]); > dp->link.num_lanes = status[1]; > > err_get_training_status: > @@ -647,7 +647,7 @@ int cdn_dp_config_video(struct cdn_dp_device *dp) > bit_per_pix = (video->color_fmt == YCBCR_4_2_2) ? > (video->color_depth * 2) : (video->color_depth * 3); > > - link_rate = drm_dp_bw_code_to_link_rate(dp->link.rate) / 1000; > + link_rate = dp->link.rate / 1000; > > ret = cdn_dp_reg_write(dp, BND_HSYNC2VSYNC, VIF_BYPASS_INTERLACE); > if (ret) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: move cpu_writers handling into vmwgfx
Thomas just a gentle ping on this. It's not that my live depends on this, but it would still be a nice to have cleanup. Thanks, Christian. Am 07.06.19 um 16:47 schrieb Christian König: This feature is only used by vmwgfx and superflous for everybody else. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 27 -- drivers/gpu/drm/ttm/ttm_bo_util.c| 1 - drivers/gpu/drm/ttm/ttm_execbuf_util.c | 7 + drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 35 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 ++ drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 8 ++ drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +++ include/drm/ttm/ttm_bo_api.h | 31 - 8 files changed, 45 insertions(+), 70 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index c7de667d482a..4ec055ffd6a7 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -153,7 +153,6 @@ static void ttm_bo_release_list(struct kref *list_kref) BUG_ON(kref_read(>list_kref)); BUG_ON(kref_read(>kref)); - BUG_ON(atomic_read(>cpu_writers)); BUG_ON(bo->mem.mm_node != NULL); BUG_ON(!list_empty(>lru)); BUG_ON(!list_empty(>ddestroy)); @@ -1308,7 +1307,6 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, kref_init(>kref); kref_init(>list_kref); - atomic_set(>cpu_writers, 0); INIT_LIST_HEAD(>lru); INIT_LIST_HEAD(>ddestroy); INIT_LIST_HEAD(>swap); @@ -1814,31 +1812,6 @@ int ttm_bo_wait(struct ttm_buffer_object *bo, } EXPORT_SYMBOL(ttm_bo_wait); -int ttm_bo_synccpu_write_grab(struct ttm_buffer_object *bo, bool no_wait) -{ - int ret = 0; - - /* -* Using ttm_bo_reserve makes sure the lru lists are updated. -*/ - - ret = ttm_bo_reserve(bo, true, no_wait, NULL); - if (unlikely(ret != 0)) - return ret; - ret = ttm_bo_wait(bo, true, no_wait); - if (likely(ret == 0)) - atomic_inc(>cpu_writers); - ttm_bo_unreserve(bo); - return ret; -} -EXPORT_SYMBOL(ttm_bo_synccpu_write_grab); - -void ttm_bo_synccpu_write_release(struct ttm_buffer_object *bo) -{ - atomic_dec(>cpu_writers); -} -EXPORT_SYMBOL(ttm_bo_synccpu_write_release); - /** * A buffer object shrink method that tries to swap out the first * buffer object on the bo_global::swap_lru list. diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 895d77d799e4..6f43f1f0de7c 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -511,7 +511,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, mutex_init(>base.wu_mutex); fbo->base.moving = NULL; drm_vma_node_reset(>base.vma_node); - atomic_set(>base.cpu_writers, 0); kref_init(>base.list_kref); kref_init(>base.kref); diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index 957ec375a4ba..80fa52b36d5c 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -113,12 +113,7 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, struct ttm_buffer_object *bo = entry->bo; ret = __ttm_bo_reserve(bo, intr, (ticket == NULL), ticket); - if (!ret && unlikely(atomic_read(>cpu_writers) > 0)) { - reservation_object_unlock(bo->resv); - - ret = -EBUSY; - - } else if (ret == -EALREADY && dups) { + if (ret == -EALREADY && dups) { struct ttm_validate_buffer *safe = entry; entry = list_prev_entry(entry, head); list_del(>head); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 5d5c2bce01f3..457861c5047f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -565,7 +565,7 @@ static void vmw_user_bo_ref_obj_release(struct ttm_base_object *base, switch (ref_type) { case TTM_REF_SYNCCPU_WRITE: - ttm_bo_synccpu_write_release(_bo->vbo.base); + atomic_dec(_bo->vbo.cpu_writers); break; default: WARN_ONCE(true, "Undefined buffer object reference release.\n"); @@ -681,12 +681,12 @@ static int vmw_user_bo_synccpu_grab(struct vmw_user_buffer_object *user_bo, struct ttm_object_file *tfile, uint32_t flags) { + bool nonblock = !!(flags & drm_vmw_synccpu_dontblock); struct ttm_buffer_object *bo = _bo->vbo.base; bool existed; int ret; if (flags & drm_vmw_synccpu_allow_cs) { - bool nonblock = !!(flags & drm_vmw_synccpu_dontblock); long lret;
Re: [PATCH 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel
On Fri, Jun 14, 2019 at 8:53 AM Robert Chiras wrote: > > Add dt-bindings documentation for Raydium RM67191 DSI panel. > > Signed-off-by: Robert Chiras > --- > .../bindings/display/panel/raydium,rm67191.txt | 42 > ++ > 1 file changed, 42 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt > > diff --git > a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt > b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt > new file mode 100644 > index 000..5a6268d > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt > @@ -0,0 +1,42 @@ > +Raydium RM67171 OLED LCD panel with MIPI-DSI protocol > + > +Required properties: > +- compatible: "raydium,rm67191" > +- reg: virtual channel for MIPI-DSI protocol > + must be <0> > +- dsi-lanes: number of DSI lanes to be used > + must be <3> or <4> > +- port:input port node with endpoint definition as > + defined in > Documentation/devicetree/bindings/graph.txt; > + the input port should be connected to a MIPI-DSI > device > + driver > + > +Optional properties: > +- reset-gpio: a GPIO spec for the RST_B GPIO pin reset-gpios (with the s in the end) is the recommendation. > +- display-timings: timings for the connected panel according to [1] This is not needed. > +- video-mode: 0 - burst-mode > + 1 - non-burst with sync event > + 2 - non-burst with sync pulse > + > +[1]: Documentation/devicetree/bindings/display/display-timing.txt This path does not exist. Also, could you try to align these bindings with the one from raydium,rm68200? There are power-supply and backlight optional properties there, which seem useful. > + > +Example: > + > + panel@0 { > + compatible = "raydium,rm67191"; > + reg = <0>; > + pinctrl-0 = <_mipi_dsi_0_1_en>; You should also pass pinctrl-names = "default"; if you use pinctrl-0. > + reset-gpio = < 7 GPIO_ACTIVE_HIGH>; Should be active low. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote: > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard > wrote: > > > > Hi, > > > > I've reordered the mail a bit to work on chunks > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote: > > > > I wish it was in your commit log in the first place, instead of having > > > > to exchange multiple mails over this. > > > > > > > > However, I don't think that's quite true, and it might be a bug in > > > > Allwinner's implementation (or rather something quite confusing). > > > > > > > > You're right that the lcd_rate and pll_rate seem to be generated from > > > > the pixel clock, and it indeed looks like the ratio between the pixel > > > > clock and the TCON dotclock is defined through the number of bits per > > > > lanes. > > > > > > > > However, in this case, dsi_rate is actually the same than lcd_rate, > > > > since pll_rate is going to be divided by dsi_div: > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L791 > > > > > > > > Since lcd_div is 1, it also means that in this case, dsi_rate == > > > > dclk_rate. > > > > > > > > The DSI module clock however, is always set to 148.5 MHz. Indeed, if > > > > we look at: > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L804 > > > > > > > > We can see that the rate in clk_info is used if it's different than > > > > 0. This is filled by disp_al_lcd_get_clk_info, which, in the case of a > > > > DSI panel, will hardcode it to 148.5 MHz: > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L164 > > > > > > Let me explain, something more. > > > > > > According to bsp there are clk_info.tcon_div which I will explain below. > > > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so it > > > is 6 for 24bpp and 4 lanes devices. > > > > > > PLL rate here depends on dsi_div (not tcon_div) > > > > > > Code here > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784 > > > > > > is computing the actual set rate, which depends on dsi_rate. > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div; > > > dsi_rate = pll_rate / clk_info.dsi_div; > > > > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set rate > > > for above link you mentioned. > > > > > > Here are the evidence with some prints. > > > > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043 > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a > > > > Ok, so we agree up to this point, and the prints confirm that the > > analysis above is the right one. > > > > > > So, the DSI clock is set to this here: > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L805 > > > > Your patch doesn't address that, so let's leave that one alone. > > Basically this is final pll set rate when sun4i_dotclock.c called the > desired rate with ccu_nkm.c so it ended the final rate with parent as > Line 8 of > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a If that's important to the driver, it should be set explicitly then, and not work by accident. > > > > The TCON *module* clock (the one in the clock controller) has been set > > > > to lcd_rate (so the pixel clock times the number of bits per lane) here: > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L800 > > > > > > > > And the PLL has been set to the same rate here: > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L794 > > > > > > > > Let's take a step back now: that function we were looking at, > > > > lcd_clk_config, is called by lcd_clk_enable, which is in turn called > > > > by disp_lcd_enable here: > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L1328 > > > > > > > > The next function being called is disp_al_lcd_cfg, and that function > > > > will hardcode the TCON dotclock divider to 4, here: > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L240 > > > > > > tcon_div from BSP point-of-view of there are two variants > > > 00) clk_info.tcon_div which is 4 and same is set the divider position > > > in SUN4I_TCON0_DCLK_REG (like above link refer) > > > 01) tcon_div which is 4 and used for edge timings computation > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L12 > > > > > > The real reason for 01) is again 4 is they set the divider to 4 in 00) > > > which is technically wrong because the
Re: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
On Fri, Jun 14, 2019 at 03:01:22PM +, David Laight wrote: > I'm pretty sure there is a lot of code out there that makes that assumption. > Without it many drivers will have to allocate almost double the > amount of memory they actually need in order to get the required alignment. > So instead of saving memory you'll actually make more be used. That code would already be broken on a lot of Linux platforms.
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #32 from Richard Thier --- If my card has no "hiz_ram" what that means? Can anyone explain if I am right? This is what I suspect so far: - Zcompression I get still, because the card has zmask_ram - No hierarchical Z (hiz) at all because there is no hiz_ram I do not see from the docs if there are specialties if enabing zmask_ram only, but not enabling hiz. -- 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 12/16] staging/comedi: mark as broken
On Fri, Jun 14, 2019 at 04:48:57PM +0200, Christoph Hellwig wrote: > On Fri, Jun 14, 2019 at 04:02:39PM +0200, Greg KH wrote: > > Perhaps a hint as to how we can fix this up? This is the first time > > I've heard of the comedi code not handling dma properly. > > It can be fixed by: > > a) never calling virt_to_page (or vmalloc_to_page for that matter) > on dma allocation > b) never remapping dma allocation with conflicting cache modes > (no remapping should be doable after a) anyway). Ok, fair enough, have any pointers of drivers/core code that does this correctly? I can put it on my todo list, but might take a week or so... Ian, want to look into doing this sooner? thanks, greg k-h
Re: [PATCH v3 0/4] drm/panfrost: Expose perf counters to userspace
On Fri, 14 Jun 2019 09:12:39 -0600 Rob Herring wrote: > On Wed, May 29, 2019 at 9:16 AM Alyssa Rosenzweig > wrote: > > > > Woohoo! Patches 1-3 are R-b; patch 4 is A-b. Exciting progress! Hoping > > to hear what Rob and Steven think :) > > All looks fine to me, but there's a kbuild error on patch 4 that needs > to be fixed. Yes, missing uintptr_t cast. I'll send a new version with that fixed. Thanks, Boris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: [PATCH v10 04/11] drm/sun4i: tcon: Compute DCLK dividers based on format, lanes
On Thu, Jun 13, 2019 at 7:28 PM Maxime Ripard wrote: > > On Wed, Jun 05, 2019 at 01:11:44PM +0530, Jagan Teki wrote: > > On Tue, Jun 4, 2019 at 8:00 PM Maxime Ripard > > wrote: > > > > > > On Fri, May 24, 2019 at 03:37:36PM +0530, Jagan Teki wrote: > > > > On Fri, May 24, 2019 at 2:18 AM Maxime Ripard > > > > wrote: > > > > > > > > > > On Mon, May 20, 2019 at 02:33:11PM +0530, Jagan Teki wrote: > > > > > > pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical > > > > > > MIPI clock topology in Allwinner DSI controller. > > > > > > > > > > > > TCON dotclock driver is computing the desired DCLK divider based on > > > > > > panel pixel clock along with input DCLK min, max divider values from > > > > > > tcon driver and that would eventually set the pll-mipi clock rate. > > > > > > > > > > > > The current code is passing dsi min and max divider value as 4 via > > > > > > tcon driver which would ended-up triggering below vblank wait timed > > > > > > out > > > > > > warning on "bananapi,s070wv20-ct16" panel. > > > > > > > > > > > > WARNING: CPU: 0 PID: 31 at > > > > > > drivers/gpu/drm/drm_atomic_helper.c:1429 > > > > > > drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0 > > > > > > [CRTC:46:crtc-0] vblank wait timed out > > > > > > Modules linked in: > > > > > > CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted > > > > > > 5.1.0-next-20190514-00025-g5186cdf10757-dirty #6 > > > > > > Hardware name: Allwinner sun8i Family > > > > > > Workqueue: events deferred_probe_work_func > > > > > > [] (unwind_backtrace) from [] > > > > > > (show_stack+0x10/0x14) > > > > > > [] (show_stack) from [] (dump_stack+0x84/0x98) > > > > > > [] (dump_stack) from [] (__warn+0xfc/0x114) > > > > > > [] (__warn) from [] > > > > > > (warn_slowpath_fmt+0x44/0x68) > > > > > > [] (warn_slowpath_fmt) from [] > > > > > > (drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0) > > > > > > [] (drm_atomic_helper_wait_for_vblanks.part.1) from > > > > > > [] (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c) > > > > > > [] (drm_atomic_helper_commit_tail_rpm) from [] > > > > > > (commit_tail+0x40/0x6c) > > > > > > [] (commit_tail) from [] > > > > > > (drm_atomic_helper_commit+0xbc/0x128) > > > > > > [] (drm_atomic_helper_commit) from [] > > > > > > (restore_fbdev_mode_atomic+0x1cc/0x1dc) > > > > > > [] (restore_fbdev_mode_atomic) from [] > > > > > > (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0) > > > > > > [] (drm_fb_helper_restore_fbdev_mode_unlocked) from > > > > > > [] (drm_fb_helper_set_par+0x30/0x54) > > > > > > [] (drm_fb_helper_set_par) from [] > > > > > > (fbcon_init+0x560/0x5ac) > > > > > > [] (fbcon_init) from [] > > > > > > (visual_init+0xbc/0x104) > > > > > > [] (visual_init) from [] > > > > > > (do_bind_con_driver+0x1b0/0x390) > > > > > > [] (do_bind_con_driver) from [] > > > > > > (do_take_over_console+0x13c/0x1c4) > > > > > > [] (do_take_over_console) from [] > > > > > > (do_fbcon_takeover+0x74/0xcc) > > > > > > [] (do_fbcon_takeover) from [] > > > > > > (notifier_call_chain+0x44/0x84) > > > > > > [] (notifier_call_chain) from [] > > > > > > (__blocking_notifier_call_chain+0x48/0x60) > > > > > > [] (__blocking_notifier_call_chain) from [] > > > > > > (blocking_notifier_call_chain+0x18/0x20) > > > > > > [] (blocking_notifier_call_chain) from [] > > > > > > (register_framebuffer+0x1e0/0x2f8) > > > > > > [] (register_framebuffer) from [] > > > > > > (__drm_fb_helper_initial_config_and_unlock+0x2fc/0x50c) > > > > > > [] (__drm_fb_helper_initial_config_and_unlock) from > > > > > > [] (drm_fbdev_client_hotplug+0xe8/0x1b8) > > > > > > [] (drm_fbdev_client_hotplug) from [] > > > > > > (drm_fbdev_generic_setup+0x88/0x118) > > > > > > [] (drm_fbdev_generic_setup) from [] > > > > > > (sun4i_drv_bind+0x128/0x160) > > > > > > [] (sun4i_drv_bind) from [] > > > > > > (try_to_bring_up_master+0x164/0x1a0) > > > > > > [] (try_to_bring_up_master) from [] > > > > > > (__component_add+0x94/0x140) > > > > > > [] (__component_add) from [] > > > > > > (sun6i_dsi_probe+0x144/0x234) > > > > > > [] (sun6i_dsi_probe) from [] > > > > > > (platform_drv_probe+0x48/0x9c) > > > > > > [] (platform_drv_probe) from [] > > > > > > (really_probe+0x1dc/0x2c8) > > > > > > [] (really_probe) from [] > > > > > > (driver_probe_device+0x60/0x160) > > > > > > [] (driver_probe_device) from [] > > > > > > (bus_for_each_drv+0x74/0xb8) > > > > > > [] (bus_for_each_drv) from [] > > > > > > (__device_attach+0xd0/0x13c) > > > > > > [] (__device_attach) from [] > > > > > > (bus_probe_device+0x84/0x8c) > > > > > > [] (bus_probe_device) from [] > > > > > > (deferred_probe_work_func+0x64/0x90) > > > > > > [] (deferred_probe_work_func) from [] > > > > > > (process_one_work+0x204/0x420) > > > > > > [] (process_one_work) from [] > > > > > > (worker_thread+0x274/0x5a0) > > > > > > [] (worker_thread) from [] > > > > > > (kthread+0x11c/0x14c) > > > > > > [] (kthread) from []
Re: [PATCH 2/2] drm/panel: Add support for Raydium RM67191 panel driver
Hi Robert, Minor comment. See inline: On Fri, Jun 14, 2019 at 2:52 PM Robert Chiras wrote: > > This patch adds Raydium RM67191 TFT LCD panel driver (MIPI-DSI > protocol). > > Signed-off-by: Robert Chiras > --- > drivers/gpu/drm/panel/Kconfig | 9 + > drivers/gpu/drm/panel/Makefile| 1 + > drivers/gpu/drm/panel/panel-raydium-rm67191.c | 730 > ++ > 3 files changed, 740 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index d9d931a..8be1ac1 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -159,6 +159,15 @@ config DRM_PANEL_RASPBERRYPI_TOUCHSCREEN > Pi 7" Touchscreen. To compile this driver as a module, > choose M here. > > +config DRM_PANEL_RAYDIUM_RM67191 > + tristate "Raydium RM67191 FHD 1080x1920 DSI video mode panel" > + depends on OF > + depends on DRM_MIPI_DSI > + depends on BACKLIGHT_CLASS_DEVICE > + help > + Say Y here if you want to enable support for Raydium RM67191 FHD > + (1080x1920) DSI panel. > + > config DRM_PANEL_RAYDIUM_RM68200 > tristate "Raydium RM68200 720x1280 DSI video mode panel" > depends on OF > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile > index fb0cb3a..1fc0f68 100644 > --- a/drivers/gpu/drm/panel/Makefile > +++ b/drivers/gpu/drm/panel/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += > panel-orisetech-otm8009a.o > obj-$(CONFIG_DRM_PANEL_OSD_OSD101T2587_53TS) += panel-osd-osd101t2587-53ts.o > obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += > panel-panasonic-vvx10f034n00.o > obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += > panel-raspberrypi-touchscreen.o > +obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o > obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o > obj-$(CONFIG_DRM_PANEL_ROCKTECH_JH057N00900) += panel-rocktech-jh057n00900.o > obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o > diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c > b/drivers/gpu/drm/panel/panel-raydium-rm67191.c > new file mode 100644 > index 000..75bfb03 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c > @@ -0,0 +1,730 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * i.MX drm driver - Raydium MIPI-DSI panel driver > + * > + * Copyright (C) 2017 NXP > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ Please remove the license text once you already added the SPDX identifier. Also preferred copyright for NXP is: Copyright 2019 NXP So, the file should look like this: // SPDX-License-Identifier: GPL-2.0 /* * i.MX drm driver - Raydium MIPI-DSI panel driver * * Copyright 2019 NXP */
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
Am 14.06.19 um 14:09 schrieb Emil Velikov: > On 2019/05/27, Emil Velikov wrote: > [SNIP] > Hi Christian, > > > In the following, I would like to summarise and emphasize the need for > DRM_AUTH removal. I would kindly ask you to spend a couple of minutes > extra reading it. > > > Today DRM drivers* do not make any distinction between primary and > render node clients. That is actually not 100% correct. We have a special case where a DRM master is allowed to change the priority of render node clients. > Thus for a render capable driver, any premise of > separation, security or otherwise imposed via DRM_AUTH is a fallacy. Yeah, that's what I agree on. I just don't think that removing DRM_AUTH now is the right direction to take. > Considering the examples of flaky or outright missing drmAuth in prime > open-source projects (mesa, kmscube, libva) we can reasonably assume > other projects exbibit the same problem. > > For these and/or other reasons, two DRM drivers have dropped DRM_AUTH > since day one. > > Since we are interested in providing consistent and predictable > behaviour to user-space, dropping DRM_AUTH seems to be the most > effective way forward. Well and what do you do with drivers which doesn't implement render nodes? > Of course, I'd be more than happy to hear for any other way to achieve > the goal outlined. > > Based on the series, other maintainers agree with the idea/goal here. > Amdgpu not following the same pattern would compromise predictability > across drivers and complicate userspace, so I would kindly ask you to > reconsider. > > Alternatively, I see two ways to special case: > - New flag annotating amdgpu/radeon - akin to the one proposed earlier > - Check for amdgpu/radeon in core DRM I perfectly agree that I don't want any special handling for amdgpu/radeon. My key point is that with DRM_AUTH we created an authorization and authentication mess in DRM which is functionality which doesn't belong into the DRM subsystem in the first place. > Now on your pain point - not allowing render iocts via the primary node, > and how this patch could make it harder to achieve that goal. > > While I love the idea, there are number of obstacles that prevent us > from doing so at this time: > - Ensuring both old and new userspace does not regress Yeah, agree totally. That's why I said we should probably start doing this for only for upcoming hardware generations. > - Drivers (QXL, others?) do not expose a render node Well isn't that is a rather big problem for the removal of DRM_AUTH in general? E.g. at least QXL would then expose functionality on the primary node without authentication. > - We want to avoid driver specific behaviour > > The only way forward that I can see is: > - Address QXL/others to expose render nodes > - Add a Kconfig toggle to disable !KMS ioctls via the primary node > - Jump-start ^^ with distribution X > - Fix user-space fallouts > - X months down the line, flip the Kconfig > - In case of regressions - revert the KConfig, goto Fix user-space... Well that at least sounds like a plan :) Let's to this! > That said, the proposal will not conflict with the DRM_AUTH removal. If > anything it is step 0.5 of the grand master plan. That's the point I strongly disagree on. By lowering the DRM_AUTH restriction you are encouraging userspace to use the shortcut and use the primary node for rendering instead of fixing the code and using the render node. So at the end of the day userspace will most likely completely drop support for the render node, simply for the reason that it became superfluous. You can just open up the primary node and get the same functionality. I absolutely can't understand how you can argue that this won't make it harder to cleanly separate rendering and primary node functionality. Regards, Christian. > > > Tl;Dr: Removing DRM_AUTH is orthogonal to not allowing render iocts via > the primary node. Here's an idea how to achieve the latter. > > > Thanks > Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] locking: add ww_mutex_(un)lock_for_each helpers
On Fri, Jun 14, 2019 at 02:41:20PM +0200, Christian König wrote: > The ww_mutex implementation allows for detection deadlocks when multiple > threads try to acquire the same set of locks in different order. > > The problem is that handling those deadlocks was the burden of the user of > the ww_mutex implementation and at least some users didn't got that right > on the first try. > > I'm not a big fan of macros, but it still better then implementing the same > logic at least halve a dozen times. So this patch adds two macros to lock > and unlock multiple ww_mutex instances with the necessary deadlock handling. > > Signed-off-by: Christian König > --- > include/linux/ww_mutex.h | 75 > 1 file changed, 75 insertions(+) > > diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h > index 3af7c0e03be5..a0d893b5b114 100644 > --- a/include/linux/ww_mutex.h > +++ b/include/linux/ww_mutex.h > @@ -369,4 +369,79 @@ static inline bool ww_mutex_is_locked(struct ww_mutex > *lock) > return mutex_is_locked(>base); > } > > +/** > + * ww_mutex_unlock_for_each - cleanup after error or contention > + * @loop: for loop code fragment iterating over all the locks > + * @pos: code fragment returning the currently handled lock > + * @contended: the last contended ww_mutex or NULL or ERR_PTR > + * > + * Helper to make cleanup after error or lock contention easier. > + * First unlock the last contended lock and then all other locked ones. > + */ > +#define ww_mutex_unlock_for_each(loop, pos, contended) \ > + if (!IS_ERR(contended)) { \ > + if (contended) \ > + ww_mutex_unlock(contended); \ > + contended = (pos); \ > + loop { \ > + if (contended == (pos)) \ > + break; \ > + ww_mutex_unlock(pos); \ > + } \ > + } > + > +/** > + * ww_mutex_lock_for_each - implement ww_mutex deadlock handling > + * @loop: for loop code fragment iterating over all the locks > + * @pos: code fragment returning the currently handled lock > + * @contended: ww_mutex pointer variable for state handling > + * @ret: int variable to store the return value of ww_mutex_lock() > + * @intr: If true ww_mutex_lock_interruptible() is used > + * @ctx: ww_acquire_ctx pointer for the locking > + * > + * This macro implements the necessary logic to lock multiple ww_mutex > + * instances. Lock contention with backoff and re-locking is handled by the > + * macro so that the loop body only need to handle other errors and > + * successfully acquired locks. > + * > + * With the @loop and @pos code fragment it is possible to apply this logic > + * to all kind of containers (array, list, tree, etc...) holding multiple > + * ww_mutex instances. > + * > + * @contended is used to hold the current state we are in. -ENOENT is used to > + * signal that we are just starting the handling. -EDEADLK means that the > + * current position is contended and we need to restart the loop after > locking > + * it. NULL means that there is no contention to be handled. Any other value > is > + * the last contended ww_mutex. > + */ > +#define ww_mutex_lock_for_each(loop, pos, contended, ret, intr, ctx) \ > + for (contended = ERR_PTR(-ENOENT); ({ \ > + __label__ relock, next; \ > + ret = -ENOENT; \ > + if (contended == ERR_PTR(-ENOENT)) \ > + contended = NULL; \ > + else if (contended == ERR_PTR(-EDEADLK))\ > + contended = (pos); \ > + else\ > + goto next; \ > + loop { \ > + if (contended == (pos)) { \ > + contended = NULL; \ > + continue; \ > + } \ > +relock: > \ > + ret = !(intr) ? ww_mutex_lock(pos, ctx) : \ > + ww_mutex_lock_interruptible(pos, ctx); \ > + if (ret == -EDEADLK) { \ > + ww_mutex_unlock_for_each(loop, pos, \ > + contended);\ > + contended =
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #29 from Richard Thier --- Okay this is weird for me: /* src/gallium/drivers/r300/r300_texture_desc.c */ 399 /* Get the HIZ buffer size in dwords. */ 400 hiz_numdw = (stride * height) / (8*8 * pipes); 401 402 /* Check whether we have enough HIZ memory. */ 403 if (hiz_numdw <= screen->caps.hiz_ram * pipes) { 404 tex->tex.hiz_dwords[i] = hiz_numdw; 405 tex->tex.hiz_stride_in_pixels[i] = stride; 406 } else { 407 tex->tex.hiz_dwords[i] = 0; 408 tex->tex.hiz_stride_in_pixels[i] = 0; 409 } (gdb) p hiz_numdw $35 = 1128 (gdb) p screen->caps.hiz_ram $36 = 0 (gdb) (!) Is it normal to have zero hiz_ram on this card??? Btw: info.r300_num_z_pipes == 1 info.r300_num_gb_pipes == 3 (this is in the pipes var) -- 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 12/16] staging/comedi: mark as broken
On Fri, Jun 14, 2019 at 03:47:22PM +0200, Christoph Hellwig wrote: > comedi_buf.c abuse the DMA API in gravely broken ways, as it assumes it > can call virt_to_page on the result, and the just remap it as uncached > using vmap. Disable the driver until this API abuse has been fixed. > > Signed-off-by: Christoph Hellwig > --- > drivers/staging/comedi/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig > index 049b659fa6ad..e7c021d76cfa 100644 > --- a/drivers/staging/comedi/Kconfig > +++ b/drivers/staging/comedi/Kconfig > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0 > config COMEDI > tristate "Data acquisition support (comedi)" > + depends on BROKEN Um, that's a huge sledgehammer. Perhaps a hint as to how we can fix this up? This is the first time I've heard of the comedi code not handling dma properly. thanks, greg k-h
Re: [PATCH v4 02/12] drm/client: Restrict the plane_state scope
Hi Jani, On Fri, Jun 14, 2019 at 03:28:59PM +0300, Jani Nikula wrote: > On Fri, 14 Jun 2019, Maxime Ripard wrote: > > The drm_client_modeset_commit_atomic function uses two times the > > plane_state variable in inner blocks of code, but the variable has a scope > > global to this function. > > > > This will lead to inadvertent devs to reuse the variable in the second > > block with the value left by the first, without any warning from the > > compiler since value would have been initialized. > > > > Fix this by moving the variable declaration to the proper scope. > > This is an improvement, but I'd consider renaming also to not shadow > variables. > > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/drm_client_modeset.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_client_modeset.c > > b/drivers/gpu/drm/drm_client_modeset.c > > index 006bf7390e7d..8264c3a732b0 100644 > > --- a/drivers/gpu/drm/drm_client_modeset.c > > +++ b/drivers/gpu/drm/drm_client_modeset.c > > @@ -861,7 +861,6 @@ EXPORT_SYMBOL(drm_client_panel_rotation); > > static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, > > bool active) > > { > > struct drm_device *dev = client->dev; > > - struct drm_plane_state *plane_state; > > struct drm_plane *plane; > > struct drm_atomic_state *state; > > struct drm_modeset_acquire_ctx ctx; > > @@ -879,6 +878,8 @@ static int drm_client_modeset_commit_atomic(struct > > drm_client_dev *client, bool > > state->acquire_ctx = > > retry: > > drm_for_each_plane(plane, dev) { > > + struct drm_plane_state *plane_state; > > + > > plane_state = drm_atomic_get_plane_state(state, plane); > > if (IS_ERR(plane_state)) { > > ret = PTR_ERR(plane_state); > > @@ -901,6 +902,8 @@ static int drm_client_modeset_commit_atomic(struct > > drm_client_dev *client, bool > > unsigned int rotation; > > > > if (drm_client_panel_rotation(mode_set, )) { > > + struct drm_plane_state *plane_state; > > + That's not super clear from that patch, but this variable will not shadow the first one. The code layout is pretty much this one: loop () { struct drm_plane_state *plane_state; [...] } loop () { loop () { struct drm_plane_state *plane_state; [...] } } so the shadowing doesn't exist Maxime -- Maxime Ripard, Bootlin 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 109955] amdgpu [RX Vega 64] system freeze while gaming
https://bugs.freedesktop.org/show_bug.cgi?id=109955 --- Comment #34 from Alex Deucher --- (In reply to Jiri Slaby from comment #33) > > amdgpu :1e:00.0: GPU reset(2) succeeded! > > [drm] Skip scheduling IBs! > > ... > > [drm] Skip scheduling IBs! > > [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! > > [drm] Skip scheduling IBs! > > ... > > [drm] Skip scheduling IBs! > > [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! > > [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! > > [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! The GPU reset was successful. You need to restart your desktop environment to recover. -- 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
[PATCH] video: fbdev: s3c-fb: add COMPILE_TEST support
Add COMPILE_TEST support to s3c-fb driver for better compile testing coverage. Cc: Jingoo Han Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/video/fbdev/Kconfig |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: b/drivers/video/fbdev/Kconfig === --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -1877,7 +1877,8 @@ config FB_TMIO_ACCELL config FB_S3C tristate "Samsung S3C framebuffer support" - depends on FB && (CPU_S3C2416 || ARCH_S3C64XX) + depends on FB && HAVE_CLK && HAS_IOMEM + depends on (CPU_S3C2416 || ARCH_S3C64XX) || COMPILE_TEST select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #30 from Richard Thier --- It seems it can be normal, but then my understanding of hyperZ was a bit off then. I will try adding hiz ram for it just to see what 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 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #16 from Gert Wollny --- I've updated the MR to add a CAP for support of TGSI_OPCODE_DIV to also check for it. -- 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
[PATCH -next] drm/bridge: sii902x: Make sii902x_audio_digital_mute static
Fix sparse warning: drivers/gpu/drm/bridge/sii902x.c:665:5: warning: symbol 'sii902x_audio_digital_mute' was not declared. Should it be static? Reported-by: Hulk Robot Signed-off-by: YueHaibing --- drivers/gpu/drm/bridge/sii902x.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index dd7aa46..c2f97e5 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -662,7 +662,8 @@ static void sii902x_audio_shutdown(struct device *dev, void *data) clk_disable_unprepare(sii902x->audio.mclk); } -int sii902x_audio_digital_mute(struct device *dev, void *data, bool enable) +static int sii902x_audio_digital_mute(struct device *dev, + void *data, bool enable) { struct sii902x *sii902x = dev_get_drvdata(dev); -- 2.7.4
Re: [v7 00/16] Add Plane Color Properties
On Thu, 28 Mar 2019 at 16:50, Uma Shankar wrote: > > This is how a typical display color hardware pipeline looks like: > +---+ > |RAM| > | +--++-++-+ | > | | FB 1 || FB 2 || FB N| | > | +--++-++-+ | > +---+ >| Plane Color Hardware Block | > ++ > | +---v-+ +---v---+ +---v--+ | > | | Plane A | | Plane B | | Plane N | | > | | DeGamma | | Degamma | | Degamma | | > | +---+-+ +---+---+ +---+--+ | > | | | || > | +---v-+ +---v---+ +---v--+ | > | |Plane A | | Plane B | | Plane N | | > | |CSC/CTM | | CSC/CTM | | CSC/CTM | | > | +---+-+ ++--+ ++-+ | > | | | | | > | +---v-+ +v--+ +v-+ | > | | Plane A | | Plane B | | Plane N | | > | | Gamma | | Gamma | | Gamma| | > | +---+-+ ++--+ ++-+ | > | | | | | > ++ > +--v--v---v---| > || || > || Pipe Blender|| > +++ > ||| > |+---v--+ | > || Pipe DeGamma| | > || | | > |+---+--+ | > ||Pipe Color | > |+---v--+ Hardware| > || Pipe CSC/CTM| | > || | | > |+---+--+ | > ||| > |+---v--+ | > || Pipe Gamma | | > || | | > |+---+--+ | > ||| > +-+ > | > v >Pipe Output > > This patch series adds properties for plane color features. It adds > properties for degamma used to linearize data, CSC used for gamut > conversion, and gamma used to again non-linearize data as per panel > supported color space. These can be utilize by user space to convert > planes from one format to another, one color space to another etc. > > Usersapce can take smart blending decisions and utilize these hardware > supported plane color features to get accurate color profile. The same > can help in consistent color quality from source to panel taking > advantage of advanced color features in hardware. > > These patches just add the property interfaces and enable helper > functions. > > This series adds Intel Gen9 specific plane gamma feature. We can > build up and add other platform/hardware specific implementation > on top of this series > > Note: This is just to get a design feedback whether these interfaces > look ok. Based on community feedback on interfaces, we will implement > IGT tests to validate plane color features. This is un-tested currently. > > Userspace implementation using these properties have been done in drm > hwcomposer by "Alexandru-Cosmin Gheorghe alexandru-cosmin.gheor...@arm.com" > from ARM. A merge request has been opened by Alexandru for drm_hwcomposer, > implementing the property changes for the same. Please review that as well: > https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_requests/25 > > v2: Dropped legacy gamma table for plane as suggested by Maarten. Added > Gen9/BDW plane gamma feature and rebase on tot. > > v3: Added a new drm_color_lut_ext structure to accommodate 32 bit precision > entries, pointed to by Brian, Starkey for HDR usecases. Addressed Sean,Paul > comments and moved plane color properties to drm_plane instead of > mode_config. Added property documentation as suggested by Daniel, Vetter. > Fixed a rebase fumble which occurred in v2, pointed by Emil Velikov. > > v4: Rebase > > v5: Added "Display Color Hardware Pipeline" flow to kernel > documentation as suggested by "Ville Syrjala" and "Brian Starkey". > Moved the property creation to drm_color_mgmt.c file to consolidate > all color operations at one place. Addressed Alexandru's review comments. > > v6: Rebase. Added support for ICL Color features. Enhanced Lut precision to > accept input values in u32.32 format. This is needed for higher precision > required in HDR data processing. > > v7: Fixed Lut roundup and extraction function in patch 1 and address > definitions for Degamma index in patch
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/06/14, Koenig, Christian wrote: > Am 14.06.19 um 17:53 schrieb Emil Velikov: > > On 2019/06/14, Koenig, Christian wrote: > >> Am 14.06.19 um 14:09 schrieb Emil Velikov: > >>> On 2019/05/27, Emil Velikov wrote: > >>> [SNIP] > >>> Hi Christian, > >>> > >>> > >>> In the following, I would like to summarise and emphasize the need for > >>> DRM_AUTH removal. I would kindly ask you to spend a couple of minutes > >>> extra reading it. > >>> > >>> > >>> Today DRM drivers* do not make any distinction between primary and > >>> render node clients. > >> That is actually not 100% correct. We have a special case where a DRM > >> master is allowed to change the priority of render node clients. > >> > > Can you provide a link? I cannot find that code. > > See amdgpu_sched_ioctl(). > > >>> Thus for a render capable driver, any premise of > >>> separation, security or otherwise imposed via DRM_AUTH is a fallacy. > >> Yeah, that's what I agree on. I just don't think that removing DRM_AUTH > >> now is the right direction to take. > >> > > Could have been clearer - I'm talking about DRM_AUTH | DRM_RENDER_ALLOW > > ioctls. > > > > That aside, can you propose an alternative solution that addresses this > > and the second point just below? > > Give me a few days to work on this, it's already Friday 6pm here. > Great thanks. Fwiw I was asking for a ideas/proposal, was not expecting you to write the patches ;-) Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EXT] Re: [PATCH 2/2] drm/panel: Add support for Raydium RM67191 panel driver
Hi Fabio, On Vi, 2019-06-14 at 09:27 -0300, Fabio Estevam wrote: > Hi Robert, > > On Fri, Jun 14, 2019 at 8:52 AM Robert Chiras > wrote: > > > > > --- /dev/null > > +++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c > > @@ -0,0 +1,730 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * i.MX drm driver - Raydium MIPI-DSI panel driver > > + * > > + * Copyright (C) 2017 NXP > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License > > + * as published by the Free Software Foundation; either version 2 > > + * of the License, or (at your option) any later version. > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > No need for this text as you are using SPDX tag. > > > > > +static int color_format_from_dsi_format(enum mipi_dsi_pixel_format > > format) > > +{ > > + switch (format) { > > + case MIPI_DSI_FMT_RGB565: > > + return 0x55; > > + case MIPI_DSI_FMT_RGB666: > > + case MIPI_DSI_FMT_RGB666_PACKED: > > + return 0x66; > > + case MIPI_DSI_FMT_RGB888: > > + return 0x77; > Could you use defines for these magic 0x55, 0x66 and 0x77 numbers? Those magic numbers mean exactly what their case statements are. They come from the panel documentation. I thought that the already existing defines (MIPI_DSI_FMT_) are self explanatory here, so using defines seemed redundant for me. But, if you think that using defines for them is better, I can do that. > > > > > +static int rad_panel_prepare(struct drm_panel *panel) > > +{ > > + struct rad_panel *rad = to_rad_panel(panel); > > + > > + if (rad->prepared) > > + return 0; > > + > > + if (rad->reset) { > > + gpiod_set_value(rad->reset, 0); > > + usleep_range(5000, 1); > > + gpiod_set_value(rad->reset, 1); > > + usleep_range(2, 25000); > This does not look correct. > > The correct way to do a reset with gpiod API is: > > gpiod_set_value(rad->reset, 1); > > delay > > gpiod_set_value(rad->reset, 0); > > I don't have the datasheet for the RM67191 panel, but I assume the > reset GPIO is active low. > > Since you inverted the polarity in the dts and inside the driver, you > got it right by accident. The GPIO is active high, and the above sequence was received from the panel vendor in the following form: SET_RESET_PIN(1); MDELAY(10); SET_RESET_PIN(0); MDELAY(5); SET_RESET_PIN(1); MDELAY(20); I got rid of the first transition to high since seemed redundant. Also, according to the manual reference, the RSTB pin needs to be active high while operating the display. > > You could also consider using gpiod_set_value_cansleep() variant > instead because the GPIO reset could be provided by an I2C GPIO > expander, for example. Thanks, will use this in the next revision. > > Also, when sleeping for more than 10ms, msleep is a better fit as per > Documentation/timers/timers-howto.txt. OK, I will use msleep. That documentation recommends using usleep_range for sleeps <20m, but also using msleep for >10ms (so I followed the recomendations from usleep_range) > > > > > + if (rad->reset) { > > + gpiod_set_value(rad->reset, 0); > > + usleep_range(15000, 17000); > > + gpiod_set_value(rad->reset, 1); > > + } > Another reset? Yes. This is tricky, since this GPIO is shared between the DSI controller and touch controller. The Android guys needs the touch controller to be active, while the display can be turned off. So this is why, after the display is turned off, the reset pin is put back to HIGH in order to keep the touch working.
Re: [RFC/WIP] drm/rockchip: Support CRTC gamma LUT
On Thu, 13 Jun 2019 16:22:44 -0300 Ezequiel Garcia wrote: > +static int vop_gamma_lut_request(struct device *dev, > + struct resource *res, struct vop *vop) > +{ > + resource_size_t offset = vop->data->gamma_lut_addr_off; > + resource_size_t size = VOP_GAMMA_LUT_SIZE * 4; > + > + /* > + * Some SoCs (e.g. RK3288) have the gamma LUT address after > + * the MMU registers, which means we can't request and ioremap > + * the entire register set. Other (e.g. RK3399) have gamma LUT > + * address before MMU. > + * > + * Therefore, we need to request and ioremap those that haven't > + * been already. > + */ > + if (vop->len >= (offset + size)) { > + vop->lut_regs = vop->regs + offset; > + return 0; > + } > + > + if (!devm_request_mem_region(dev, res->start + offset, > + size, dev_name(dev))) { > + dev_warn(dev, "can't request gamma lut region\n"); > + return -EBUSY; > + } > + > + vop->lut_regs = devm_ioremap(dev, res->start + offset, size); > + if (!vop->lut_regs) { > + dev_err(dev, "can't ioremap gamma lut address\n"); > + devm_release_mem_region(dev, res->start + offset, size); > + return -ENOMEM; > + } Can't we patch the resource just after calling plaform_get_resource() (and before calling devm_ioremap_resource()) so we don't have to add these devm_request_mem_region()+devm_ioremap() calls here? > + return 0; > +} ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 02/11] drm/sun4i: dsi: Update start value in video start delay
On Thu, Jun 13, 2019 at 01:34:04PM +0530, Jagan Teki wrote: > On Fri, May 31, 2019 at 12:23 AM Maxime Ripard > wrote: > > > > On Fri, May 24, 2019 at 03:55:42PM +0530, Jagan Teki wrote: > > > On Fri, May 24, 2019 at 2:07 AM Maxime Ripard > > > wrote: > > > > > > > > On Mon, May 20, 2019 at 02:33:09PM +0530, Jagan Teki wrote: > > > > > start value in video start delay computation done in below commit > > > > > is as per the legacy bsp drivers/video/sunxi/legacy.. > > > > > "drm/sun4i: dsi: Change the start delay calculation" > > > > > (sha1: da676c6aa6413d59ab0a80c97bbc273025e640b2) > > > > > > > > > > This existing start delay computation gives start value of 35, > > > > > for "bananapi,s070wv20-ct16" panel timings which indeed trigger > > > > > panel flip_done timed out as: > > > > > > > > > > WARNING: CPU: 0 PID: 31 at drivers/gpu/drm/drm_atomic_helper.c:1429 > > > > > drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0 > > > > > [CRTC:46:crtc-0] vblank wait timed out > > > > > Modules linked in: > > > > > CPU: 0 PID: 31 Comm: kworker/0:1 Tainted: GW > > > > > 5.1.0-next-20190514-00025-gf928bc7cc146 #15 > > > > > Hardware name: Allwinner sun8i Family > > > > > Workqueue: events deferred_probe_work_func > > > > > [] (unwind_backtrace) from [] > > > > > (show_stack+0x10/0x14) > > > > > [] (show_stack) from [] (dump_stack+0x84/0x98) > > > > > [] (dump_stack) from [] (__warn+0xfc/0x114) > > > > > [] (__warn) from [] (warn_slowpath_fmt+0x44/0x68) > > > > > [] (warn_slowpath_fmt) from [] > > > > > (drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0) > > > > > [] (drm_atomic_helper_wait_for_vblanks.part.1) from > > > > > [] (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c) > > > > > [] (drm_atomic_helper_commit_tail_rpm) from [] > > > > > (commit_tail+0x40/0x6c) > > > > > [] (commit_tail) from [] > > > > > (drm_atomic_helper_commit+0xbc/0x128) > > > > > [] (drm_atomic_helper_commit) from [] > > > > > (restore_fbdev_mode_atomic+0x1cc/0x1dc) > > > > > [] (restore_fbdev_mode_atomic) from [] > > > > > (drm_fb_helper_pan_display+0xac/0x1d0) > > > > > [] (drm_fb_helper_pan_display) from [] > > > > > (fb_pan_display+0xcc/0x134) > > > > > [] (fb_pan_display) from [] > > > > > (bit_update_start+0x14/0x30) > > > > > [] (bit_update_start) from [] > > > > > (fbcon_switch+0x3d8/0x4e0) > > > > > [] (fbcon_switch) from [] > > > > > (redraw_screen+0x174/0x238) > > > > > [] (redraw_screen) from [] > > > > > (fbcon_prepare_logo+0x3c4/0x400) > > > > > [] (fbcon_prepare_logo) from [] > > > > > (fbcon_init+0x3c8/0x5ac) > > > > > [] (fbcon_init) from [] (visual_init+0xbc/0x104) > > > > > [] (visual_init) from [] > > > > > (do_bind_con_driver+0x1b0/0x390) > > > > > [] (do_bind_con_driver) from [] > > > > > (do_take_over_console+0x13c/0x1c4) > > > > > [] (do_take_over_console) from [] > > > > > (do_fbcon_takeover+0x74/0xcc) > > > > > [] (do_fbcon_takeover) from [] > > > > > (notifier_call_chain+0x44/0x84) > > > > > [] (notifier_call_chain) from [] > > > > > (__blocking_notifier_call_chain+0x48/0x60) > > > > > [] (__blocking_notifier_call_chain) from [] > > > > > (blocking_notifier_call_chain+0x18/0x20) > > > > > [] (blocking_notifier_call_chain) from [] > > > > > (register_framebuffer+0x1e0/0x2f8) > > > > > [] (register_framebuffer) from [] > > > > > (__drm_fb_helper_initial_config_and_unlock+0x2fc/0x50c) > > > > > [] (__drm_fb_helper_initial_config_and_unlock) from > > > > > [] (drm_fbdev_client_hotplug+0xe8/0x1b8) > > > > > [] (drm_fbdev_client_hotplug) from [] > > > > > (drm_fbdev_generic_setup+0x88/0x118) > > > > > [] (drm_fbdev_generic_setup) from [] > > > > > (sun4i_drv_bind+0x128/0x160) > > > > > [] (sun4i_drv_bind) from [] > > > > > (try_to_bring_up_master+0x164/0x1a0) > > > > > [] (try_to_bring_up_master) from [] > > > > > (__component_add+0x94/0x140) > > > > > [] (__component_add) from [] > > > > > (sun6i_dsi_probe+0x144/0x234) > > > > > [] (sun6i_dsi_probe) from [] > > > > > (platform_drv_probe+0x48/0x9c) > > > > > [] (platform_drv_probe) from [] > > > > > (really_probe+0x1dc/0x2c8) > > > > > [] (really_probe) from [] > > > > > (driver_probe_device+0x60/0x160) > > > > > [] (driver_probe_device) from [] > > > > > (bus_for_each_drv+0x74/0xb8) > > > > > [] (bus_for_each_drv) from [] > > > > > (__device_attach+0xd0/0x13c) > > > > > [] (__device_attach) from [] > > > > > (bus_probe_device+0x84/0x8c) > > > > > [] (bus_probe_device) from [] > > > > > (deferred_probe_work_func+0x64/0x90) > > > > > [] (deferred_probe_work_func) from [] > > > > > (process_one_work+0x204/0x420) > > > > > [] (process_one_work) from [] > > > > > (worker_thread+0x274/0x5a0) > > > > > [] (worker_thread) from [] (kthread+0x11c/0x14c) > > > > > [] (kthread) from [] (ret_from_fork+0x14/0x2c) > > > > > Exception stack(0xde539fb0 to 0xde539ff8) > > > > > 9fa0: > > > > > > > > > >
[Bug 110783] Mesa 19.1 rc crashing MPV with VAAPI
https://bugs.freedesktop.org/show_bug.cgi?id=110783 --- Comment #15 from AngryPenguin --- Hi. I can confirm, this fix my issue with vaapi and vdpau. Thanks. -- 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
[PATCH][next] drm/mgag200: add in missing { } around if block
From: Colin Ian King There is an if block that is missing the { } curly brackets. Add these in. Addresses-Coverity: ("Structurally dead code") Fixes: 94dc57b10399 ("drm/mgag200: Rewrite cursor handling") Signed-off-by: Colin Ian King --- drivers/gpu/drm/mgag200/mgag200_cursor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c index f0c61a92351c..117eaedec7aa 100644 --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c @@ -99,10 +99,11 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc, /* Pin and map up-coming buffer to write colour indices */ ret = drm_gem_vram_pin(pixels_next, 0); - if (ret) + if (ret) { dev_err(>pdev->dev, "failed to pin cursor buffer: %d\n", ret); goto err_drm_gem_vram_kunmap_src; + } dst = drm_gem_vram_kmap(pixels_next, true, NULL); if (IS_ERR(dst)) { ret = PTR_ERR(dst); -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
On 14/06/2019 15:50, 'Christoph Hellwig' wrote: On Fri, Jun 14, 2019 at 02:15:44PM +, David Laight wrote: Does this still guarantee that requests for 16k will not cross a 16k boundary? It looks like you are losing the alignment parameter. The DMA API never gave you alignment guarantees to start with, and you can get not naturally aligned memory from many of our current implementations. Well, apart from the bit in DMA-API-HOWTO which has said this since forever (well, before Git history, at least): "The CPU virtual address and the DMA address are both guaranteed to be aligned to the smallest PAGE_SIZE order which is greater than or equal to the requested size. This invariant exists (for example) to guarantee that if you allocate a chunk which is smaller than or equal to 64 kilobytes, the extent of the buffer you receive will not cross a 64K boundary." That said, I don't believe this particular patch should make any appreciable difference - alloc_pages_exact() is still going to give back the same base address as the rounded up over-allocation would, and PAGE_ALIGN()ing the size passed to get_order() already seemed to be pointless. Robin.
Re: [PATCH 1/2] drm: debugfs: make drm_debugfs_remove_files() a void function
On Fri, Jun 14, 2019 at 04:59:08PM +0200, Daniel Vetter wrote: > On Fri, Jun 14, 2019 at 11:51:09AM +0200, Greg Kroah-Hartman wrote: > > The function can not fail, and no one checks the current return value, > > so just mark it as a void function so no one gets confused. > > > > Cc: Maarten Lankhorst > > Cc: Maxime Ripard > > Cc: Sean Paul > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: Greg Kroah-Hartman > > --- > > drivers/gpu/drm/drm_debugfs.c | 5 ++--- > > include/drm/drm_debugfs.h | 9 - > > 2 files changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > > index 6f2802e9bfb5..515569002c86 100644 > > --- a/drivers/gpu/drm/drm_debugfs.c > > +++ b/drivers/gpu/drm/drm_debugfs.c > > @@ -270,8 +270,8 @@ int drm_debugfs_init(struct drm_minor *minor, int > > minor_id, > > } > > > > > > -int drm_debugfs_remove_files(const struct drm_info_list *files, int count, > > -struct drm_minor *minor) > > +void drm_debugfs_remove_files(const struct drm_info_list *files, int count, > > + struct drm_minor *minor) > > We're trying to entirely nuke this function here, see the kerneldoc for > drm_debugfs_create_files(). Only user left is tegra, and we call the > "remove all debugfs files" and the ->early_unregister hooks all from the > same place. So this can all be garbage collected. It's mildly annoying > because we'd need to move the kfree from ->early_unregister into ->destroy > callbacks, because connectors are unregister before we throw away all the > debugfs files in drm_dev_unregister(). But imo that's cleaner anway. I would love to see this function gone, it can also make things a lot simpler from the point of view of creating the debugfs files as well, as no dentries will need to be saved. > Up for that? Sure, I can do that. I have a much larger patch messing with drm_debugfs_create_files() that I want you all to be in a good mood for when I submit it (it touches all drivers at once), so I might as well clean this up first :) Give me a week, I'm supposed to be writing my slides for a conference now, instead I'm procrastinating by writing debugfs cleanup patches, I need to stop... thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: debugfs: make drm_debugfs_remove_files() a void function
On Fri, Jun 14, 2019 at 5:20 PM Greg Kroah-Hartman wrote: > > On Fri, Jun 14, 2019 at 04:59:08PM +0200, Daniel Vetter wrote: > > On Fri, Jun 14, 2019 at 11:51:09AM +0200, Greg Kroah-Hartman wrote: > > > The function can not fail, and no one checks the current return value, > > > so just mark it as a void function so no one gets confused. > > > > > > Cc: Maarten Lankhorst > > > Cc: Maxime Ripard > > > Cc: Sean Paul > > > Cc: David Airlie > > > Cc: Daniel Vetter > > > Cc: dri-devel@lists.freedesktop.org > > > Signed-off-by: Greg Kroah-Hartman > > > --- > > > drivers/gpu/drm/drm_debugfs.c | 5 ++--- > > > include/drm/drm_debugfs.h | 9 - > > > 2 files changed, 6 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > > > index 6f2802e9bfb5..515569002c86 100644 > > > --- a/drivers/gpu/drm/drm_debugfs.c > > > +++ b/drivers/gpu/drm/drm_debugfs.c > > > @@ -270,8 +270,8 @@ int drm_debugfs_init(struct drm_minor *minor, int > > > minor_id, > > > } > > > > > > > > > -int drm_debugfs_remove_files(const struct drm_info_list *files, int > > > count, > > > -struct drm_minor *minor) > > > +void drm_debugfs_remove_files(const struct drm_info_list *files, int > > > count, > > > + struct drm_minor *minor) > > > > We're trying to entirely nuke this function here, see the kerneldoc for > > drm_debugfs_create_files(). Only user left is tegra, and we call the > > "remove all debugfs files" and the ->early_unregister hooks all from the > > same place. So this can all be garbage collected. It's mildly annoying > > because we'd need to move the kfree from ->early_unregister into ->destroy > > callbacks, because connectors are unregister before we throw away all the > > debugfs files in drm_dev_unregister(). But imo that's cleaner anway. > > I would love to see this function gone, it can also make things a lot > simpler from the point of view of creating the debugfs files as well, as > no dentries will need to be saved. > > > Up for that? > > Sure, I can do that. I have a much larger patch messing with > drm_debugfs_create_files() that I want you all to be in a good mood for > when I submit it (it touches all drivers at once), so I might as well > clean this up first :) Oh don't worry, we've had a pile of cleanup todo tasks in this area since a long time. You doing them all is going to make me a happy camper :-) Only thing to be aware of is that we have a bit a habit of dragging good contributors of refactoring/cleanup/fundamental work like this into the drm fold for good. You might get stuck ... > Give me a week, I'm supposed to be writing my slides for a conference > now, instead I'm procrastinating by writing debugfs cleanup patches, I > need to stop... :-) Cheers, Daniel -- 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 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/06/14, Koenig, Christian wrote: > Am 14.06.19 um 14:09 schrieb Emil Velikov: > > On 2019/05/27, Emil Velikov wrote: > > [SNIP] > > Hi Christian, > > > > > > In the following, I would like to summarise and emphasize the need for > > DRM_AUTH removal. I would kindly ask you to spend a couple of minutes > > extra reading it. > > > > > > Today DRM drivers* do not make any distinction between primary and > > render node clients. > > That is actually not 100% correct. We have a special case where a DRM > master is allowed to change the priority of render node clients. > Can you provide a link? I cannot find that code. > > Thus for a render capable driver, any premise of > > separation, security or otherwise imposed via DRM_AUTH is a fallacy. > > Yeah, that's what I agree on. I just don't think that removing DRM_AUTH > now is the right direction to take. > Could have been clearer - I'm talking about DRM_AUTH | DRM_RENDER_ALLOW ioctls. That aside, can you propose an alternative solution that addresses this and the second point just below? > > Considering the examples of flaky or outright missing drmAuth in prime > > open-source projects (mesa, kmscube, libva) we can reasonably assume > > other projects exbibit the same problem. > > > > For these and/or other reasons, two DRM drivers have dropped DRM_AUTH > > since day one. > > > > Since we are interested in providing consistent and predictable > > behaviour to user-space, dropping DRM_AUTH seems to be the most > > effective way forward. > > Well and what do you do with drivers which doesn't implement render nodes? > AFAICT there is a single non-DC driver which does not expose - QXL. I would expect it runs on a rather customised stack. Would be great to fix that, but ETIME and it's the only exception to the rule. > > Of course, I'd be more than happy to hear for any other way to achieve > > the goal outlined. > > > > Based on the series, other maintainers agree with the idea/goal here. > > Amdgpu not following the same pattern would compromise predictability > > across drivers and complicate userspace, so I would kindly ask you to > > reconsider. > > > > Alternatively, I see two ways to special case: > > - New flag annotating amdgpu/radeon - akin to the one proposed earlier > > - Check for amdgpu/radeon in core DRM > > I perfectly agree that I don't want any special handling for amdgpu/radeon. > > My key point is that with DRM_AUTH we created an authorization and > authentication mess in DRM which is functionality which doesn't belong > into the DRM subsystem in the first place. > Precisely and I've outlined below how to resolve this in the long run. > > Now on your pain point - not allowing render iocts via the primary node, > > and how this patch could make it harder to achieve that goal. > > > > While I love the idea, there are number of obstacles that prevent us > > from doing so at this time: > > - Ensuring both old and new userspace does not regress > > Yeah, agree totally. That's why I said we should probably start doing > this for only for upcoming hardware generations. > That will side-step the regression issue, but will enforce driver specific behaviour outlined before. > > - Drivers (QXL, others?) do not expose a render node > > Well isn't that is a rather big problem for the removal of DRM_AUTH in > general? > > E.g. at least QXL would then expose functionality on the primary node > without authentication. > With this series QXL remains functionally unchanged. I would love to fix that as well yet ETIME as mentioned above. > > - We want to avoid driver specific behaviour > > > > The only way forward that I can see is: > > - Address QXL/others to expose render nodes > > - Add a Kconfig toggle to disable !KMS ioctls via the primary node > > - Jump-start ^^ with distribution X > > - Fix user-space fallouts > > - X months down the line, flip the Kconfig > > - In case of regressions - revert the KConfig, goto Fix user-space... > > Well that at least sounds like a plan :) Let's to this! > We're talking about months if not years until it comes to fruition. We need something quicker. That said, I'm quite happy to take the first stab, yet this is not a replacement for this series. > > That said, the proposal will not conflict with the DRM_AUTH removal. If > > anything it is step 0.5 of the grand master plan. > > That's the point I strongly disagree on. > > By lowering the DRM_AUTH restriction you are encouraging userspace to > use the shortcut and use the primary node for rendering instead of > fixing the code and using the render node. > Have to disagree here. After working on the user-space side and fixing issues in various projects I can honestly say that most user-space is sane and developers _care_ about doing things correctly. > So at the end of the day userspace will most likely completely drop > support for the render node, simply for the reason that it became > superfluous. You can
[PATCH 2/2] drm/panel: Add support for Raydium RM67191 panel driver
This patch adds Raydium RM67191 TFT LCD panel driver (MIPI-DSI protocol). Signed-off-by: Robert Chiras --- drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-raydium-rm67191.c | 730 ++ 3 files changed, 740 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index d9d931a..8be1ac1 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -159,6 +159,15 @@ config DRM_PANEL_RASPBERRYPI_TOUCHSCREEN Pi 7" Touchscreen. To compile this driver as a module, choose M here. +config DRM_PANEL_RAYDIUM_RM67191 + tristate "Raydium RM67191 FHD 1080x1920 DSI video mode panel" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for Raydium RM67191 FHD + (1080x1920) DSI panel. + config DRM_PANEL_RAYDIUM_RM68200 tristate "Raydium RM68200 720x1280 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index fb0cb3a..1fc0f68 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o obj-$(CONFIG_DRM_PANEL_OSD_OSD101T2587_53TS) += panel-osd-osd101t2587-53ts.o obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen.o +obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o obj-$(CONFIG_DRM_PANEL_ROCKTECH_JH057N00900) += panel-rocktech-jh057n00900.o obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gpu/drm/panel/panel-raydium-rm67191.c new file mode 100644 index 000..75bfb03 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c @@ -0,0 +1,730 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * i.MX drm driver - Raydium MIPI-DSI panel driver + * + * Copyright (C) 2017 NXP + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* Write Manufacture Command Set Control */ +#define WRMAUCCTR 0xFE + +/* Manufacturer Command Set pages (CMD2) */ +struct cmd_set_entry { + u8 cmd; + u8 param; +}; + +/* + * There is no description in the Reference Manual about these commands. + * We received them from vendor, so just use them as is. + */ +static const struct cmd_set_entry manufacturer_cmd_set[] = { + {0xFE, 0x0B}, + {0x28, 0x40}, + {0x29, 0x4F}, + {0xFE, 0x0E}, + {0x4B, 0x00}, + {0x4C, 0x0F}, + {0x4D, 0x20}, + {0x4E, 0x40}, + {0x4F, 0x60}, + {0x50, 0xA0}, + {0x51, 0xC0}, + {0x52, 0xE0}, + {0x53, 0xFF}, + {0xFE, 0x0D}, + {0x18, 0x08}, + {0x42, 0x00}, + {0x08, 0x41}, + {0x46, 0x02}, + {0x72, 0x09}, + {0xFE, 0x0A}, + {0x24, 0x17}, + {0x04, 0x07}, + {0x1A, 0x0C}, + {0x0F, 0x44}, + {0xFE, 0x04}, + {0x00, 0x0C}, + {0x05, 0x08}, + {0x06, 0x08}, + {0x08, 0x08}, + {0x09, 0x08}, + {0x0A, 0xE6}, + {0x0B, 0x8C}, + {0x1A, 0x12}, + {0x1E, 0xE0}, + {0x29, 0x93}, + {0x2A, 0x93}, + {0x2F, 0x02}, + {0x31, 0x02}, + {0x33, 0x05}, + {0x37, 0x2D}, + {0x38, 0x2D}, + {0x3A, 0x1E}, + {0x3B, 0x1E}, + {0x3D, 0x27}, + {0x3F, 0x80}, + {0x40, 0x40}, + {0x41, 0xE0}, + {0x4F, 0x2F}, + {0x50, 0x1E}, + {0xFE, 0x06}, + {0x00, 0xCC}, + {0x05, 0x05}, + {0x07, 0xA2}, + {0x08, 0xCC}, + {0x0D, 0x03}, + {0x0F, 0xA2}, + {0x32, 0xCC}, + {0x37, 0x05}, + {0x39, 0x83}, + {0x3A, 0xCC}, + {0x41, 0x04}, + {0x43, 0x83}, + {0x44, 0xCC}, + {0x49, 0x05}, + {0x4B, 0xA2}, + {0x4C, 0xCC}, + {0x51, 0x03}, + {0x53, 0xA2}, + {0x75, 0xCC}, + {0x7A, 0x03}, + {0x7C, 0x83}, + {0x7D, 0xCC}, + {0x82, 0x02}, + {0x84, 0x83}, +
[PATCH v4 02/12] drm/client: Restrict the plane_state scope
The drm_client_modeset_commit_atomic function uses two times the plane_state variable in inner blocks of code, but the variable has a scope global to this function. This will lead to inadvertent devs to reuse the variable in the second block with the value left by the first, without any warning from the compiler since value would have been initialized. Fix this by moving the variable declaration to the proper scope. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_client_modeset.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index 006bf7390e7d..8264c3a732b0 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -861,7 +861,6 @@ EXPORT_SYMBOL(drm_client_panel_rotation); static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool active) { struct drm_device *dev = client->dev; - struct drm_plane_state *plane_state; struct drm_plane *plane; struct drm_atomic_state *state; struct drm_modeset_acquire_ctx ctx; @@ -879,6 +878,8 @@ static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool state->acquire_ctx = retry: drm_for_each_plane(plane, dev) { + struct drm_plane_state *plane_state; + plane_state = drm_atomic_get_plane_state(state, plane); if (IS_ERR(plane_state)) { ret = PTR_ERR(plane_state); @@ -901,6 +902,8 @@ static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool unsigned int rotation; if (drm_client_panel_rotation(mode_set, )) { + struct drm_plane_state *plane_state; + /* Cannot fail as we've already gotten the plane state above */ plane_state = drm_atomic_get_new_plane_state(state, primary); plane_state->rotation = rotation; -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 10/12] drm/modes: Parse overscan properties
Properly configuring the overscan properties might be needed for the initial setup of the framebuffer for display that still have overscan. Let's allow for more properties on the kernel command line to setup each margin. Reviewed-by: Noralf Trønnes Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_modes.c | 44 ++- include/drm/drm_connector.h | 12 +- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index b92b7df6784a..25d2ba595750 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1609,6 +1609,50 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len, } else if (!strncmp(option, "reflect_y", delim - option)) { rotation |= DRM_MODE_REFLECT_Y; sep = delim; + } else if (!strncmp(option, "margin_right", delim - option)) { + const char *value = delim + 1; + unsigned int margin; + + margin = simple_strtol(value, , 10); + + /* Make sure we have parsed something */ + if (sep == value) + return -EINVAL; + + mode->tv_margins.right = margin; + } else if (!strncmp(option, "margin_left", delim - option)) { + const char *value = delim + 1; + unsigned int margin; + + margin = simple_strtol(value, , 10); + + /* Make sure we have parsed something */ + if (sep == value) + return -EINVAL; + + mode->tv_margins.left = margin; + } else if (!strncmp(option, "margin_top", delim - option)) { + const char *value = delim + 1; + unsigned int margin; + + margin = simple_strtol(value, , 10); + + /* Make sure we have parsed something */ + if (sep == value) + return -EINVAL; + + mode->tv_margins.top = margin; + } else if (!strncmp(option, "margin_bottom", delim - option)) { + const char *value = delim + 1; + unsigned int margin; + + margin = simple_strtol(value, , 10); + + /* Make sure we have parsed something */ + if (sep == value) + return -EINVAL; + + mode->tv_margins.bottom = margin; } else { return -EINVAL; } diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index c58a35b34c1a..6841c46e6781 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -505,12 +505,7 @@ struct drm_connector_tv_margins { */ struct drm_tv_connector_state { enum drm_mode_subconnector subconnector; - struct { - unsigned int left; - unsigned int right; - unsigned int top; - unsigned int bottom; - } margins; + struct drm_connector_tv_margins margins; unsigned int mode; unsigned int brightness; unsigned int contrast; @@ -1039,6 +1034,11 @@ struct drm_cmdline_mode { * DRM_MODE_ROTATE_180 are supported at the moment. */ unsigned int rotation; + + /** +* @tv_margins: TV margins to apply to the mode. +*/ + struct drm_connector_tv_margins tv_margins; }; /** -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 12/12] drm/vc4: hdmi: Set default state margin at reset
Now that the TV margins are properly parsed and filled into drm_cmdline_mode, we just need to initialise the first state at reset to get those values and start using them. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hdmi.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 99fc8569e0f5..43442c5619a3 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -255,11 +255,17 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) return ret; } +static void vc4_hdmi_connector_reset(struct drm_connector *connector) +{ + drm_atomic_helper_connector_reset(connector); + drm_atomic_helper_connector_tv_reset(connector); +} + static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { .detect = vc4_hdmi_connector_detect, .fill_modes = drm_helper_probe_single_connector_modes, .destroy = vc4_hdmi_connector_destroy, - .reset = drm_atomic_helper_connector_reset, + .reset = vc4_hdmi_connector_reset, .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 07/12] drm/modes: Allow to specify rotation and reflection on the commandline
Rotations and reflections setup are needed in some scenarios to initialise properly the initial framebuffer. Some drivers already had a bunch of quirks to deal with this, such as either a private kernel command line parameter (omapdss) or on the device tree (various panels). In order to accomodate this, let's create a video mode parameter to deal with the rotation and reflexion. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_client_modeset.c | 20 +- drivers/gpu/drm/drm_modes.c | 110 ++-- include/drm/drm_connector.h | 9 ++- 3 files changed, 119 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index 33d4988f22ae..57937e38492c 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -824,6 +824,7 @@ bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation) { struct drm_connector *connector = modeset->connectors[0]; struct drm_plane *plane = modeset->crtc->primary; + struct drm_cmdline_mode *cmdline; u64 valid_mask = 0; unsigned int i; @@ -844,6 +845,25 @@ bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation) *rotation = DRM_MODE_ROTATE_0; } + /** +* The panel already defined the default rotation +* through its orientation. Whatever has been provided +* on the command line needs to be added to that. +* +* Unfortunately, the rotations are at different bit +* indices, so the math to add them up are not as +* trivial as they could. +*/ + cmdline = >cmdline_mode; + if (cmdline->specified) { + unsigned int panel_rot = ilog2(*rotation & DRM_MODE_ROTATE_MASK); + unsigned int cmdline_rot = ilog2(cmdline->rotation & DRM_MODE_ROTATE_MASK); + unsigned int sum_rot; + + sum_rot = (panel_rot + cmdline_rot) % 4; + *rotation = 1 << sum_rot; + } + /* * TODO: support 90 / 270 degree hardware rotation, * depending on the hardware this may require the framebuffer diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 429d3be17800..b92b7df6784a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1554,6 +1554,71 @@ static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int length, return 0; } +static int drm_mode_parse_cmdline_options(char *str, size_t len, + struct drm_connector *connector, + struct drm_cmdline_mode *mode) +{ + unsigned int rotation = 0; + char *sep = str; + + while ((sep = strchr(sep, ','))) { + char *delim, *option; + + option = sep + 1; + delim = strchr(option, '='); + if (!delim) { + delim = strchr(option, ','); + + if (!delim) + delim = str + len; + } + + if (!strncmp(option, "rotate", delim - option)) { + const char *value = delim + 1; + unsigned int deg; + + deg = simple_strtol(value, , 10); + + /* Make sure we have parsed something */ + if (sep == value) + return -EINVAL; + + switch (deg) { + case 0: + rotation |= DRM_MODE_ROTATE_0; + break; + + case 90: + rotation |= DRM_MODE_ROTATE_90; + break; + + case 180: + rotation |= DRM_MODE_ROTATE_180; + break; + + case 270: + rotation |= DRM_MODE_ROTATE_270; + break; + + default: + return -EINVAL; + } + } else if (!strncmp(option, "reflect_x", delim - option)) { + rotation |= DRM_MODE_REFLECT_X; + sep = delim; + } else if (!strncmp(option, "reflect_y", delim - option)) { + rotation |= DRM_MODE_REFLECT_Y; + sep = delim; + } else { + return -EINVAL; + } + } + + mode->rotation = rotation; + + return 0; +} + /** * drm_mode_parse_command_line_for_connector - parse command line modeline for connector * @mode_option: optional per connector mode option @@ -1581,9 +1646,10 @@ bool drm_mode_parse_command_line_for_connector(const char
[PATCH v4 05/12] drm/modes: Rewrite the command line parser
From: Maxime Ripard Rewrite the command line parser in order to get away from the state machine parsing the video mode lines. Hopefully, this will allow to extend it more easily to support named modes and / or properties set directly on the command line. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_modes.c | 325 +++-- 1 file changed, 210 insertions(+), 115 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 5a07a28fec6d..6debbd6c1763 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -30,6 +30,7 @@ * authorization from the copyright holder(s) and author(s). */ +#include #include #include #include @@ -1408,6 +1409,151 @@ void drm_connector_list_update(struct drm_connector *connector) } EXPORT_SYMBOL(drm_connector_list_update); +static int drm_mode_parse_cmdline_bpp(const char *str, char **end_ptr, + struct drm_cmdline_mode *mode) +{ + unsigned int bpp; + + if (str[0] != '-') + return -EINVAL; + + str++; + bpp = simple_strtol(str, end_ptr, 10); + if (*end_ptr == str) + return -EINVAL; + + mode->bpp = bpp; + mode->bpp_specified = true; + + return 0; +} + +static int drm_mode_parse_cmdline_refresh(const char *str, char **end_ptr, + struct drm_cmdline_mode *mode) +{ + unsigned int refresh; + + if (str[0] != '@') + return -EINVAL; + + str++; + refresh = simple_strtol(str, end_ptr, 10); + if (*end_ptr == str) + return -EINVAL; + + mode->refresh = refresh; + mode->refresh_specified = true; + + return 0; +} + +static int drm_mode_parse_cmdline_extra(const char *str, int length, + struct drm_connector *connector, + struct drm_cmdline_mode *mode) +{ + int i; + + for (i = 0; i < length; i++) { + switch (str[i]) { + case 'i': + mode->interlace = true; + break; + case 'm': + mode->margins = true; + break; + case 'D': + if (mode->force != DRM_FORCE_UNSPECIFIED) + return -EINVAL; + + if ((connector->connector_type != DRM_MODE_CONNECTOR_DVII) && + (connector->connector_type != DRM_MODE_CONNECTOR_HDMIB)) + mode->force = DRM_FORCE_ON; + else + mode->force = DRM_FORCE_ON_DIGITAL; + break; + case 'd': + if (mode->force != DRM_FORCE_UNSPECIFIED) + return -EINVAL; + + mode->force = DRM_FORCE_OFF; + break; + case 'e': + if (mode->force != DRM_FORCE_UNSPECIFIED) + return -EINVAL; + + mode->force = DRM_FORCE_ON; + break; + default: + return -EINVAL; + } + } + + return 0; +} + +static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int length, + bool extras, + struct drm_connector *connector, + struct drm_cmdline_mode *mode) +{ + const char *str_start = str; + bool rb = false, cvt = false; + int xres = 0, yres = 0; + int remaining, i; + char *end_ptr; + + xres = simple_strtol(str, _ptr, 10); + if (end_ptr == str) + return -EINVAL; + + if (end_ptr[0] != 'x') + return -EINVAL; + end_ptr++; + + str = end_ptr; + yres = simple_strtol(str, _ptr, 10); + if (end_ptr == str) + return -EINVAL; + + remaining = length - (end_ptr - str_start); + if (remaining < 0) + return -EINVAL; + + for (i = 0; i < remaining; i++) { + switch (end_ptr[i]) { + case 'M': + cvt = true; + break; + case 'R': + rb = true; + break; + default: + /* +* Try to pass that to our extras parsing +* function to handle the case where the +* extras are directly after the resolution +*/ + if (extras) { + int ret = drm_mode_parse_cmdline_extra(end_ptr + i, + 1, +
[PATCH v4 03/12] drm/client: Restrict the rotation check to the rotation itself
The drm_client_rotation has a check on the rotation value, but the reflections are also stored in the same variable, and the check doesn't take this into account. Therefore, even though we might have a valid rotation, if we're also using a reflection parameter, the test will fail for no particular reason. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_client_modeset.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index 8264c3a732b0..b4e5fb0a17cf 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -845,7 +845,8 @@ bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotat * depending on the hardware this may require the framebuffer * to be in a specific tiling format. */ - if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property) + if ((*rotation & DRM_MODE_ROTATE_MASK) != DRM_MODE_ROTATE_180 || + !plane->rotation_property) return false; for (i = 0; i < plane->rotation_property->num_values; i++) -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 09/12] drm/atomic: Add a function to reset connector TV properties
During the connector reset, if that connector has a TV property, it needs to be reset to the value provided on the command line. Provide a helper to do that. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_atomic_state_helper.c | 18 ++ include/drm/drm_atomic_state_helper.h | 1 + 2 files changed, 19 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index 97ab26679b96..f4f0ada9c152 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -376,6 +376,24 @@ void drm_atomic_helper_connector_reset(struct drm_connector *connector) EXPORT_SYMBOL(drm_atomic_helper_connector_reset); /** + * drm_atomic_helper_connector_tv_reset - Resets TV connector properties + * @connector: DRM connector + * + * Resets the TV-related properties attached to a connector. + */ +void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector) +{ + struct drm_cmdline_mode *cmdline = >cmdline_mode; + struct drm_connector_state *state = connector->state; + + state->tv.margins.left = cmdline->tv_margins.left; + state->tv.margins.right = cmdline->tv_margins.right; + state->tv.margins.top = cmdline->tv_margins.top; + state->tv.margins.bottom = cmdline->tv_margins.bottom; +} +EXPORT_SYMBOL(drm_atomic_helper_connector_tv_reset); + +/** * __drm_atomic_helper_connector_duplicate_state - copy atomic connector state * @connector: connector object * @state: atomic connector state diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h index 4e6d2e7a40b8..e4577cc11689 100644 --- a/include/drm/drm_atomic_state_helper.h +++ b/include/drm/drm_atomic_state_helper.h @@ -62,6 +62,7 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane, void __drm_atomic_helper_connector_reset(struct drm_connector *connector, struct drm_connector_state *conn_state); void drm_atomic_helper_connector_reset(struct drm_connector *connector); +void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector); void __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector, struct drm_connector_state *state); -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 11/12] drm/selftests: Add command line parser selftests
The command line parser is pretty tough to get right and very error prone, so let's add a selftest to try to catch any regression. Reviewed-by: Noralf Trønnes Signed-off-by: Maxime Ripard --- drivers/gpu/drm/selftests/Makefile | 2 +- drivers/gpu/drm/selftests/drm_cmdline_selftests.h | 55 +- drivers/gpu/drm/selftests/test-drm_cmdline_parser.c | 918 +- 3 files changed, 974 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/selftests/drm_cmdline_selftests.h create mode 100644 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c diff --git a/drivers/gpu/drm/selftests/Makefile b/drivers/gpu/drm/selftests/Makefile index 8ec64ecf0e36..aae88f8a016c 100644 --- a/drivers/gpu/drm/selftests/Makefile +++ b/drivers/gpu/drm/selftests/Makefile @@ -3,4 +3,4 @@ test-drm_modeset-y := test-drm_modeset_common.o test-drm_plane_helper.o \ test-drm_format.o test-drm_framebuffer.o \ test-drm_damage_helper.o -obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o +obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o test-drm_cmdline_parser.o diff --git a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h new file mode 100644 index ..b45824ec7c8f --- /dev/null +++ b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* List each unit test as selftest(function) + * + * The name is used as both an enum and expanded as igt__name to create + * a module parameter. It must be unique and legal for a C identifier. + * + * Tests are executed in order by igt/drm_mm + */ + +#define cmdline_test(test) selftest(test, test) + +cmdline_test(drm_cmdline_test_res) +cmdline_test(drm_cmdline_test_res_missing_x) +cmdline_test(drm_cmdline_test_res_missing_y) +cmdline_test(drm_cmdline_test_res_bad_y) +cmdline_test(drm_cmdline_test_res_missing_y_bpp) +cmdline_test(drm_cmdline_test_res_vesa) +cmdline_test(drm_cmdline_test_res_vesa_rblank) +cmdline_test(drm_cmdline_test_res_rblank) +cmdline_test(drm_cmdline_test_res_bpp) +cmdline_test(drm_cmdline_test_res_bad_bpp) +cmdline_test(drm_cmdline_test_res_refresh) +cmdline_test(drm_cmdline_test_res_bad_refresh) +cmdline_test(drm_cmdline_test_res_bpp_refresh) +cmdline_test(drm_cmdline_test_res_bpp_refresh_interlaced) +cmdline_test(drm_cmdline_test_res_bpp_refresh_margins) +cmdline_test(drm_cmdline_test_res_bpp_refresh_force_off) +cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on_off) +cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on) +cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on_analog) +cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on_digital) +cmdline_test(drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on) +cmdline_test(drm_cmdline_test_res_margins_force_on) +cmdline_test(drm_cmdline_test_res_vesa_margins) +cmdline_test(drm_cmdline_test_res_invalid_mode) +cmdline_test(drm_cmdline_test_res_bpp_wrong_place_mode) +cmdline_test(drm_cmdline_test_name) +cmdline_test(drm_cmdline_test_name_bpp) +cmdline_test(drm_cmdline_test_name_refresh) +cmdline_test(drm_cmdline_test_name_bpp_refresh) +cmdline_test(drm_cmdline_test_name_refresh_wrong_mode) +cmdline_test(drm_cmdline_test_name_refresh_invalid_mode) +cmdline_test(drm_cmdline_test_name_option) +cmdline_test(drm_cmdline_test_name_bpp_option) +cmdline_test(drm_cmdline_test_rotate_0) +cmdline_test(drm_cmdline_test_rotate_90) +cmdline_test(drm_cmdline_test_rotate_180) +cmdline_test(drm_cmdline_test_rotate_270) +cmdline_test(drm_cmdline_test_rotate_invalid_val) +cmdline_test(drm_cmdline_test_rotate_truncated) +cmdline_test(drm_cmdline_test_hmirror) +cmdline_test(drm_cmdline_test_vmirror) +cmdline_test(drm_cmdline_test_margin_options) +cmdline_test(drm_cmdline_test_multiple_options) +cmdline_test(drm_cmdline_test_invalid_option) diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c new file mode 100644 index ..d7e54154e988 --- /dev/null +++ b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c @@ -0,0 +1,918 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2019 Bootlin + */ + +#define pr_fmt(fmt) "drm_cmdline: " fmt + +#include +#include + +#include +#include + +#define TESTS "drm_cmdline_selftests.h" +#include "drm_selftest.h" +#include "test-drm_modeset_common.h" + +static int drm_cmdline_test_res(void *ignored) +{ + struct drm_connector connector = { }; + struct drm_cmdline_mode mode = { }; + + FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480", + , + )); + FAIL_ON(!mode.specified); + FAIL_ON(mode.xres != 720); + FAIL_ON(mode.yres != 480); + + FAIL_ON(mode.refresh_specified); + + FAIL_ON(mode.bpp_specified); + +
[PATCH v4 08/12] drm/connector: Introduce a TV margins structure
The TV margins has been defined as a structure inside the drm_connector_state structure so far. However, we will need it in other structures as well, so let's move that structure definition so that it can be reused. Signed-off-by: Maxime Ripard --- include/drm/drm_connector.h | 34 +- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 68a04169ea36..c58a35b34c1a 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -464,13 +464,37 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info, unsigned int num_formats); /** + * struct drm_connector_tv_margins - TV connector related margins + * + * Describes the margins in pixels to put around the image on TV + * connectors to deal with overscan. + */ +struct drm_connector_tv_margins { + /** +* @bottom: Bottom margin in pixels. +*/ + unsigned int bottom; + + /** +* @left: Left margin in pixels. +*/ + unsigned int left; + + /** +* @right: Right margin in pixels. +*/ + unsigned int right; + + /** +* @top: Top margin in pixels. +*/ + unsigned int top; +}; + +/** * struct drm_tv_connector_state - TV connector related states * @subconnector: selected subconnector - * @margins: margins (all margins are expressed in pixels) - * @margins.left: left margin - * @margins.right: right margin - * @margins.top: top margin - * @margins.bottom: bottom margin + * @margins: TV margins * @mode: TV mode * @brightness: brightness in percent * @contrast: contrast in percent -- git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
ww_mutex deadlock handling cleanup
The ww_mutex implementation allows drivers to acquire locks on a set of locking primitives with detection of deadlocks. If a deadlock happens we can then backoff and reacquire the contended lock until we should finally be able to acquire all necessary locks for an operation. The problem is that handling those deadlocks was the burden of the user of the ww_mutex implementation and at least some users didn't got that right on the first try. So this patch adds two macros to lock and unlock multiple ww_mutex instances with the necessary deadlock handling. I'm not a big fan of macros, but it still better then implementing the same logic at least halve a dozen times. Please note that only the first two patches are more than compile tested. And this only cleans up the tip of the iceberg, just enough to show the potential of those two macros. Please review and/or comment, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] drm/etnaviv: use new ww_mutex_(un)lock_for_each macros
Use the provided macros instead of implementing deadlock handling on our own. Signed-off-by: Christian König --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 55 +--- 1 file changed, 14 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index e054f09ac828..923b8a71f80d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -118,55 +118,28 @@ static void submit_unlock_object(struct etnaviv_gem_submit *submit, int i) static int submit_lock_objects(struct etnaviv_gem_submit *submit, struct ww_acquire_ctx *ticket) { - int contended, slow_locked = -1, i, ret = 0; - -retry: - for (i = 0; i < submit->nr_bos; i++) { - struct drm_gem_object *obj = >bos[i].obj->base; - - if (slow_locked == i) - slow_locked = -1; - - contended = i; + struct ww_mutex *contended; + int i, ret = 0; - if (!(submit->bos[i].flags & BO_LOCKED)) { - ret = ww_mutex_lock_interruptible(>resv->lock, - ticket); - if (ret == -EALREADY) - DRM_ERROR("BO at index %u already on submit list\n", - i); - if (ret) - goto fail; - submit->bos[i].flags |= BO_LOCKED; - } + ww_mutex_lock_for_each(for (i = 0; i < submit->nr_bos; i++), + >bos[i].obj->base.resv->lock, + contended, ret, true, ticket) { + if (ret == -EALREADY) + DRM_ERROR("BO at index %u already on submit list\n", i); + if (ret) + goto fail; } + for (i = 0; i < submit->nr_bos; i++) + submit->bos[i].flags |= BO_LOCKED; ww_acquire_done(ticket); return 0; fail: - for (; i >= 0; i--) - submit_unlock_object(submit, i); - - if (slow_locked > 0) - submit_unlock_object(submit, slow_locked); - - if (ret == -EDEADLK) { - struct drm_gem_object *obj; - - obj = >bos[contended].obj->base; - - /* we lost out in a seqno race, lock and retry.. */ - ret = ww_mutex_lock_slow_interruptible(>resv->lock, - ticket); - if (!ret) { - submit->bos[contended].flags |= BO_LOCKED; - slow_locked = contended; - goto retry; - } - } - + ww_mutex_unlock_for_each(for (i = 0; i < submit->nr_bos; i++), +>bos[i].obj->base.resv->lock, +contended); return ret; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6] locking: add ww_mutex_(un)lock_for_each helpers
The ww_mutex implementation allows for detection deadlocks when multiple threads try to acquire the same set of locks in different order. The problem is that handling those deadlocks was the burden of the user of the ww_mutex implementation and at least some users didn't got that right on the first try. I'm not a big fan of macros, but it still better then implementing the same logic at least halve a dozen times. So this patch adds two macros to lock and unlock multiple ww_mutex instances with the necessary deadlock handling. Signed-off-by: Christian König --- include/linux/ww_mutex.h | 75 1 file changed, 75 insertions(+) diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h index 3af7c0e03be5..a0d893b5b114 100644 --- a/include/linux/ww_mutex.h +++ b/include/linux/ww_mutex.h @@ -369,4 +369,79 @@ static inline bool ww_mutex_is_locked(struct ww_mutex *lock) return mutex_is_locked(>base); } +/** + * ww_mutex_unlock_for_each - cleanup after error or contention + * @loop: for loop code fragment iterating over all the locks + * @pos: code fragment returning the currently handled lock + * @contended: the last contended ww_mutex or NULL or ERR_PTR + * + * Helper to make cleanup after error or lock contention easier. + * First unlock the last contended lock and then all other locked ones. + */ +#define ww_mutex_unlock_for_each(loop, pos, contended) \ + if (!IS_ERR(contended)) { \ + if (contended) \ + ww_mutex_unlock(contended); \ + contended = (pos); \ + loop { \ + if (contended == (pos)) \ + break; \ + ww_mutex_unlock(pos); \ + } \ + } + +/** + * ww_mutex_lock_for_each - implement ww_mutex deadlock handling + * @loop: for loop code fragment iterating over all the locks + * @pos: code fragment returning the currently handled lock + * @contended: ww_mutex pointer variable for state handling + * @ret: int variable to store the return value of ww_mutex_lock() + * @intr: If true ww_mutex_lock_interruptible() is used + * @ctx: ww_acquire_ctx pointer for the locking + * + * This macro implements the necessary logic to lock multiple ww_mutex + * instances. Lock contention with backoff and re-locking is handled by the + * macro so that the loop body only need to handle other errors and + * successfully acquired locks. + * + * With the @loop and @pos code fragment it is possible to apply this logic + * to all kind of containers (array, list, tree, etc...) holding multiple + * ww_mutex instances. + * + * @contended is used to hold the current state we are in. -ENOENT is used to + * signal that we are just starting the handling. -EDEADLK means that the + * current position is contended and we need to restart the loop after locking + * it. NULL means that there is no contention to be handled. Any other value is + * the last contended ww_mutex. + */ +#define ww_mutex_lock_for_each(loop, pos, contended, ret, intr, ctx) \ + for (contended = ERR_PTR(-ENOENT); ({ \ + __label__ relock, next; \ + ret = -ENOENT; \ + if (contended == ERR_PTR(-ENOENT)) \ + contended = NULL; \ + else if (contended == ERR_PTR(-EDEADLK))\ + contended = (pos); \ + else\ + goto next; \ + loop { \ + if (contended == (pos)) { \ + contended = NULL; \ + continue; \ + } \ +relock: \ + ret = !(intr) ? ww_mutex_lock(pos, ctx) : \ + ww_mutex_lock_interruptible(pos, ctx); \ + if (ret == -EDEADLK) { \ + ww_mutex_unlock_for_each(loop, pos, \ +contended);\ + contended = ERR_PTR(-EDEADLK); \ + goto relock;\ + } \ +
[PATCH 6/6] drm/vc4: use new ww_mutex_(un)lock_for_each macros
Use the provided macros instead of implementing deadlock handling on our own. Signed-off-by: Christian König --- drivers/gpu/drm/vc4/vc4_gem.c | 56 --- 1 file changed, 13 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index d9311be32a4f..628b3a8bcf6a 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -584,53 +584,17 @@ vc4_lock_bo_reservations(struct drm_device *dev, struct vc4_exec_info *exec, struct ww_acquire_ctx *acquire_ctx) { - int contended_lock = -1; - int i, ret; + struct ww_mutex *contended; struct drm_gem_object *bo; + int i, ret; ww_acquire_init(acquire_ctx, _ww_class); -retry: - if (contended_lock != -1) { - bo = >bo[contended_lock]->base; - ret = ww_mutex_lock_slow_interruptible(>resv->lock, - acquire_ctx); - if (ret) { - ww_acquire_done(acquire_ctx); - return ret; - } - } - - for (i = 0; i < exec->bo_count; i++) { - if (i == contended_lock) - continue; - - bo = >bo[i]->base; - - ret = ww_mutex_lock_interruptible(>resv->lock, acquire_ctx); - if (ret) { - int j; - - for (j = 0; j < i; j++) { - bo = >bo[j]->base; - ww_mutex_unlock(>resv->lock); - } - - if (contended_lock != -1 && contended_lock >= i) { - bo = >bo[contended_lock]->base; - - ww_mutex_unlock(>resv->lock); - } - - if (ret == -EDEADLK) { - contended_lock = i; - goto retry; - } - - ww_acquire_done(acquire_ctx); - return ret; - } - } + ww_mutex_lock_for_each(for (i = 0; i < exec->bo_count; i++), + >bo[i]->base.resv->lock, contended, ret, + true, acquire_ctx) + if (ret) + goto error; ww_acquire_done(acquire_ctx); @@ -648,6 +612,12 @@ vc4_lock_bo_reservations(struct drm_device *dev, } return 0; + +error: + ww_mutex_unlock_for_each(for (i = 0; i < exec->bo_count; i++), +>bo[i]->base.resv->lock, contended); + ww_acquire_done(acquire_ctx); + return ret; } /* Queues a struct vc4_exec_info for execution. If no job is -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] drm/ttm: use new ww_mutex_(un)lock_for_each macros
Use the provided macros instead of implementing deadlock handling on our own. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_execbuf_util.c | 87 +- 1 file changed, 30 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index 957ec375a4ba..3c3ac6c94d7f 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -33,16 +33,6 @@ #include #include -static void ttm_eu_backoff_reservation_reverse(struct list_head *list, - struct ttm_validate_buffer *entry) -{ - list_for_each_entry_continue_reverse(entry, list, head) { - struct ttm_buffer_object *bo = entry->bo; - - reservation_object_unlock(bo->resv); - } -} - static void ttm_eu_del_from_lru_locked(struct list_head *list) { struct ttm_validate_buffer *entry; @@ -96,8 +86,9 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, struct list_head *list, bool intr, struct list_head *dups, bool del_lru) { - struct ttm_bo_global *glob; struct ttm_validate_buffer *entry; + struct ww_mutex *contended; + struct ttm_bo_global *glob; int ret; if (list_empty(list)) @@ -109,68 +100,39 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, if (ticket) ww_acquire_init(ticket, _ww_class); - list_for_each_entry(entry, list, head) { + ww_mutex_lock_for_each(list_for_each_entry(entry, list, head), + >bo->resv->lock, contended, ret, + intr, ticket) + { struct ttm_buffer_object *bo = entry->bo; - ret = __ttm_bo_reserve(bo, intr, (ticket == NULL), ticket); if (!ret && unlikely(atomic_read(>cpu_writers) > 0)) { reservation_object_unlock(bo->resv); - ret = -EBUSY; + goto error; + } - } else if (ret == -EALREADY && dups) { + if (ret == -EALREADY && dups) { struct ttm_validate_buffer *safe = entry; + entry = list_prev_entry(entry, head); list_del(>head); list_add(>head, dups); continue; } - if (!ret) { - if (!entry->num_shared) - continue; - - ret = reservation_object_reserve_shared(bo->resv, - entry->num_shared); - if (!ret) - continue; - } + if (unlikely(ret)) + goto error; - /* uh oh, we lost out, drop every reservation and try -* to only reserve this buffer, then start over if -* this succeeds. -*/ - ttm_eu_backoff_reservation_reverse(list, entry); - - if (ret == -EDEADLK) { - if (intr) { - ret = ww_mutex_lock_slow_interruptible(>resv->lock, - ticket); - } else { - ww_mutex_lock_slow(>resv->lock, ticket); - ret = 0; - } - } + if (!entry->num_shared) + continue; - if (!ret && entry->num_shared) - ret = reservation_object_reserve_shared(bo->resv, - entry->num_shared); - - if (unlikely(ret != 0)) { - if (ret == -EINTR) - ret = -ERESTARTSYS; - if (ticket) { - ww_acquire_done(ticket); - ww_acquire_fini(ticket); - } - return ret; + ret = reservation_object_reserve_shared(bo->resv, + entry->num_shared); + if (unlikely(ret)) { + reservation_object_unlock(bo->resv); + goto error; } - - /* move this item to the front of the list, -* forces correct iteration of the loop without keeping track -*/ - list_del(>head); - list_add(>head, list); } if (del_lru) { @@ -179,6 +141,17 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, spin_unlock(>lru_lock); } return 0; + +error: +
[PATCH 5/6] drm/lima: use new ww_mutex_(un)lock_for_each macros
Use the provided macros instead of implementing deadlock handling on our own. Signed-off-by: Christian König --- drivers/gpu/drm/lima/lima_gem.c | 41 + 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c index 477c0f73..6a51f100c29e 100644 --- a/drivers/gpu/drm/lima/lima_gem.c +++ b/drivers/gpu/drm/lima/lima_gem.c @@ -151,43 +151,24 @@ static int lima_gem_sync_bo(struct lima_sched_task *task, struct lima_bo *bo, static int lima_gem_lock_bos(struct lima_bo **bos, u32 nr_bos, struct ww_acquire_ctx *ctx) { - int i, ret = 0, contended, slow_locked = -1; + struct ww_mutex *contended; + int i, ret = 0; ww_acquire_init(ctx, _ww_class); -retry: - for (i = 0; i < nr_bos; i++) { - if (i == slow_locked) { - slow_locked = -1; - continue; - } - - ret = ww_mutex_lock_interruptible([i]->gem.resv->lock, ctx); - if (ret < 0) { - contended = i; - goto err; - } - } + ww_mutex_lock_for_each(for (i = 0; i < nr_bos; i++), + [i]->gem.resv->lock, contended, ret, true, + ctx) + if (ret) + goto error; ww_acquire_done(ctx); - return 0; -err: - for (i--; i >= 0; i--) - ww_mutex_unlock([i]->gem.resv->lock); - - if (slow_locked >= 0) - ww_mutex_unlock([slow_locked]->gem.resv->lock); + return 0; - if (ret == -EDEADLK) { - /* we lost out in a seqno race, lock and retry.. */ - ret = ww_mutex_lock_slow_interruptible( - [contended]->gem.resv->lock, ctx); - if (!ret) { - slow_locked = contended; - goto retry; - } - } +error: + ww_mutex_unlock_for_each(for (i = 0; i < nr_bos; i++), +[i]->gem.resv->lock, contended); ww_acquire_fini(ctx); return ret; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/6] drm/gem: use new ww_mutex_(un)lock_for_each macros
Use the provided macros instead of implementing deadlock handling on our own. Signed-off-by: Christian König --- drivers/gpu/drm/drm_gem.c | 49 ++- 1 file changed, 12 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 50de138c89e0..6e4623d3bee2 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1307,51 +1307,26 @@ int drm_gem_lock_reservations(struct drm_gem_object **objs, int count, struct ww_acquire_ctx *acquire_ctx) { - int contended = -1; + struct ww_mutex *contended; int i, ret; ww_acquire_init(acquire_ctx, _ww_class); -retry: - if (contended != -1) { - struct drm_gem_object *obj = objs[contended]; - - ret = ww_mutex_lock_slow_interruptible(>resv->lock, - acquire_ctx); - if (ret) { - ww_acquire_done(acquire_ctx); - return ret; - } - } - - for (i = 0; i < count; i++) { - if (i == contended) - continue; - - ret = ww_mutex_lock_interruptible([i]->resv->lock, - acquire_ctx); - if (ret) { - int j; - - for (j = 0; j < i; j++) - ww_mutex_unlock([j]->resv->lock); - - if (contended != -1 && contended >= i) - ww_mutex_unlock([contended]->resv->lock); - - if (ret == -EDEADLK) { - contended = i; - goto retry; - } - - ww_acquire_done(acquire_ctx); - return ret; - } - } + ww_mutex_lock_for_each(for (i = 0; i < count; i++), + [i]->resv->lock, contended, ret, true, + acquire_ctx) + if (ret) + goto error; ww_acquire_done(acquire_ctx); return 0; + +error: + ww_mutex_unlock_for_each(for (i = 0; i < count; i++), +[i]->resv->lock, contended); + ww_acquire_done(acquire_ctx); + return ret; } EXPORT_SYMBOL(drm_gem_lock_reservations); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/WIP] drm/rockchip: Support CRTC gamma LUT
On Fri, 14 Jun 2019 16:03:28 +0200 Heiko Stübner wrote: > Hi Boris, > > Am Freitag, 14. Juni 2019, 15:53:20 CEST schrieb Boris Brezillon: > > On Thu, 13 Jun 2019 16:22:44 -0300 > > Ezequiel Garcia wrote: > > > > > > > +static int vop_gamma_lut_request(struct device *dev, > > > + struct resource *res, struct vop *vop) > > > +{ > > > + resource_size_t offset = vop->data->gamma_lut_addr_off; > > > + resource_size_t size = VOP_GAMMA_LUT_SIZE * 4; > > > + > > > + /* > > > + * Some SoCs (e.g. RK3288) have the gamma LUT address after > > > + * the MMU registers, which means we can't request and ioremap > > > + * the entire register set. Other (e.g. RK3399) have gamma LUT > > > + * address before MMU. > > > + * > > > + * Therefore, we need to request and ioremap those that haven't > > > + * been already. > > > + */ > > > + if (vop->len >= (offset + size)) { > > > + vop->lut_regs = vop->regs + offset; > > > + return 0; > > > + } > > > + > > > + if (!devm_request_mem_region(dev, res->start + offset, > > > + size, dev_name(dev))) { > > > + dev_warn(dev, "can't request gamma lut region\n"); > > > + return -EBUSY; > > > + } > > > + > > > + vop->lut_regs = devm_ioremap(dev, res->start + offset, size); > > > + if (!vop->lut_regs) { > > > + dev_err(dev, "can't ioremap gamma lut address\n"); > > > + devm_release_mem_region(dev, res->start + offset, size); > > > + return -ENOMEM; > > > + } > > > > Can't we patch the resource just after calling plaform_get_resource() > > (and before calling devm_ioremap_resource()) so we don't have to add > > these devm_request_mem_region()+devm_ioremap() calls here? > > The issue is that on the older rk3288 socs the vops memory map has > the mmu registers (which get mapped separately) in between the core > and lut registers. Sorry for the noise, I thought the MMU was registered directly by the VOP driver and we could avoid the 3 ioremap and merge things into a single one, but it seems the MMU driver is a separate thing. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
Am 14.06.19 um 17:53 schrieb Emil Velikov: > On 2019/06/14, Koenig, Christian wrote: >> Am 14.06.19 um 14:09 schrieb Emil Velikov: >>> On 2019/05/27, Emil Velikov wrote: >>> [SNIP] >>> Hi Christian, >>> >>> >>> In the following, I would like to summarise and emphasize the need for >>> DRM_AUTH removal. I would kindly ask you to spend a couple of minutes >>> extra reading it. >>> >>> >>> Today DRM drivers* do not make any distinction between primary and >>> render node clients. >> That is actually not 100% correct. We have a special case where a DRM >> master is allowed to change the priority of render node clients. >> > Can you provide a link? I cannot find that code. See amdgpu_sched_ioctl(). >>> Thus for a render capable driver, any premise of >>> separation, security or otherwise imposed via DRM_AUTH is a fallacy. >> Yeah, that's what I agree on. I just don't think that removing DRM_AUTH >> now is the right direction to take. >> > Could have been clearer - I'm talking about DRM_AUTH | DRM_RENDER_ALLOW > ioctls. > > That aside, can you propose an alternative solution that addresses this > and the second point just below? Give me a few days to work on this, it's already Friday 6pm here. Christian. > >>> Considering the examples of flaky or outright missing drmAuth in prime >>> open-source projects (mesa, kmscube, libva) we can reasonably assume >>> other projects exbibit the same problem. >>> >>> For these and/or other reasons, two DRM drivers have dropped DRM_AUTH >>> since day one. >>> >>> Since we are interested in providing consistent and predictable >>> behaviour to user-space, dropping DRM_AUTH seems to be the most >>> effective way forward. >> Well and what do you do with drivers which doesn't implement render nodes? >> > AFAICT there is a single non-DC driver which does not expose - QXL. > I would expect it runs on a rather customised stack. > > Would be great to fix that, but ETIME and it's the only exception to the > rule. > > >>> Of course, I'd be more than happy to hear for any other way to achieve >>> the goal outlined. >>> >>> Based on the series, other maintainers agree with the idea/goal here. >>> Amdgpu not following the same pattern would compromise predictability >>> across drivers and complicate userspace, so I would kindly ask you to >>> reconsider. >>> >>> Alternatively, I see two ways to special case: >>>- New flag annotating amdgpu/radeon - akin to the one proposed earlier >>>- Check for amdgpu/radeon in core DRM >> I perfectly agree that I don't want any special handling for amdgpu/radeon. >> >> My key point is that with DRM_AUTH we created an authorization and >> authentication mess in DRM which is functionality which doesn't belong >> into the DRM subsystem in the first place. >> > Precisely and I've outlined below how to resolve this in the long run. > >>> Now on your pain point - not allowing render iocts via the primary node, >>> and how this patch could make it harder to achieve that goal. >>> >>> While I love the idea, there are number of obstacles that prevent us >>> from doing so at this time: >>>- Ensuring both old and new userspace does not regress >> Yeah, agree totally. That's why I said we should probably start doing >> this for only for upcoming hardware generations. >> > That will side-step the regression issue, but will enforce driver > specific behaviour outlined before. > >>>- Drivers (QXL, others?) do not expose a render node >> Well isn't that is a rather big problem for the removal of DRM_AUTH in >> general? >> >> E.g. at least QXL would then expose functionality on the primary node >> without authentication. >> > With this series QXL remains functionally unchanged. I would love to fix > that as well yet ETIME as mentioned above. > >>>- We want to avoid driver specific behaviour >>> >>> The only way forward that I can see is: >>>- Address QXL/others to expose render nodes >>>- Add a Kconfig toggle to disable !KMS ioctls via the primary node >>>- Jump-start ^^ with distribution X >>>- Fix user-space fallouts >>>- X months down the line, flip the Kconfig >>>- In case of regressions - revert the KConfig, goto Fix user-space... >> Well that at least sounds like a plan :) Let's to this! >> > We're talking about months if not years until it comes to fruition. We > need something quicker. > > That said, I'm quite happy to take the first stab, yet this is not a > replacement for this series. > >>> That said, the proposal will not conflict with the DRM_AUTH removal. If >>> anything it is step 0.5 of the grand master plan. >> That's the point I strongly disagree on. >> >> By lowering the DRM_AUTH restriction you are encouraging userspace to >> use the shortcut and use the primary node for rendering instead of >> fixing the code and using the render node. >> > Have to disagree here. After working on the user-space side and fixing > issues in various projects I can honestly say that most user-space is > sane and
[PATCH] video: fbdev: imxfb: fix sparse warnings about using incorrect types
Use ->screen_buffer instead of ->screen_base to fix sparse warnings. [ Please see commit 17a7b0b4d974 ("fb.h: Provide alternate screen_base pointer") for details. ] Reported-by: kbuild test robot Cc: Shawn Guo Cc: Sascha Hauer Cc: Pengutronix Kernel Team Cc: Fabio Estevam Cc: Uwe Kleine-König Cc: NXP Linux Team Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/video/fbdev/imxfb.c |9 - 1 file changed, 4 insertions(+), 5 deletions(-) Index: b/drivers/video/fbdev/imxfb.c === --- a/drivers/video/fbdev/imxfb.c +++ b/drivers/video/fbdev/imxfb.c @@ -974,9 +974,8 @@ static int imxfb_probe(struct platform_d } fbi->map_size = PAGE_ALIGN(info->fix.smem_len); - info->screen_base = dma_alloc_wc(>dev, fbi->map_size, ->map_dma, GFP_KERNEL); - + info->screen_buffer = dma_alloc_wc(>dev, fbi->map_size, + >map_dma, GFP_KERNEL); if (!info->screen_base) { dev_err(>dev, "Failed to allocate video RAM: %d\n", ret); ret = -ENOMEM; @@ -1046,7 +1045,7 @@ failed_cmap: if (pdata && pdata->exit) pdata->exit(fbi->pdev); failed_platform_init: - dma_free_wc(>dev, fbi->map_size, info->screen_base, + dma_free_wc(>dev, fbi->map_size, info->screen_buffer, fbi->map_dma); failed_map: iounmap(fbi->regs); @@ -1077,7 +1076,7 @@ static int imxfb_remove(struct platform_ pdata = dev_get_platdata(>dev); if (pdata && pdata->exit) pdata->exit(fbi->pdev); - dma_free_wc(>dev, fbi->map_size, info->screen_base, + dma_free_wc(>dev, fbi->map_size, info->screen_buffer, fbi->map_dma); iounmap(fbi->regs); release_mem_region(res->start, resource_size(res)); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #26 from Richard Thier --- Okay it is still bad now despite I have compiled with "-O0" flags actually... To be honest I have no idea actually why it started working last time. Also now that I step with the debugger I see that a lot of code paths that I imagine should be taken are never ever taken. Also when going randomly in the code with gdb I have found the following things. I just want to document my findings so far with gdb breakpoints in the driver. These are maybe not general for everyone, but this is what happens on my very machine and card when debugging r300 code with gdb... WHAT HAPPENS IN r300_blit.c === You can roughly follow this stuff here: https://github.com/anholt/mesa/blob/master/src/gallium/drivers/r300/r300_blit.c 0.) Setting of zmask_clear=1 and hiz_clear=0 always happen -- 262 /* Use fast Z clear. 263 * The zbuffer must be in micro-tiled mode, otherwise it locks up. */ 264 if (buffers & PIPE_CLEAR_DEPTHSTENCIL) { 265 boolean zmask_clear, hiz_clear; 266 267 /* If both depth and stencil are present, they must be cleared together. */ 268 if (fb->zsbuf->texture->format == PIPE_FORMAT_S8_UINT_Z24_UNORM && 269 (buffers & PIPE_CLEAR_DEPTHSTENCIL) != PIPE_CLEAR_DEPTHSTENCIL) { 270 zmask_clear = FALSE; 271 hiz_clear = FALSE; 272 } else { /* ALWAYS HAPPENS: */ 273 zmask_clear = r300_fast_zclear_allowed(r300, buffers); 274 hiz_clear = r300_hiz_clear_allowed(r300); 275 /* FIXME: only for testing! */ 276 /*zmask_clear = FALSE;*/ 277 /*zmask_clear = TRUE; // - this alone looks bad, bothfalse look good, zmaks_clear only false hiz_clear untouched is good */ 278 /*hiz_clear = FALSE;*/ 279 /*hiz_clear = TRUE; // enabling this and falsing zmask_clear shows picture but FPS is lower in glxgears...*/ We always go where I marked it with "/* ALWAYS HAPPENS */". r300_fast_zclear_allowed(r300, buffers) return 1 r300_hiz_clear_allowed(r300) returns 0 So: zmask_clear == 1 hiz_clear == 0 ALWAYS. As I understand the hiz_clear is the thing that clears the "hierarchical Z" buffer and I suspect this should be returning true instead. The other for zmask clearn is the one that clears the compessed z-buffer. I had a bit of a hard time to figure out what these mean, but using the r5xx docs (despite this card is r300) helped a lot. So zmask is a lossless compressed z-buffer ram and hiz ram seem to be on the higher hiearchy level. I have checked what is the body of the latter: return 300_resource(fb->zsbuf->texture)->tex.hiz_dwords[fb->zsbuf->u.tex.level] != 0; ^^and this body never returns true for some reason. I think this should be nonzero when HyperZ is properly going on isn't it? I am just running glxgears and nothing fancy. 1.) "Enabling" HyperZ seem to happen properly - 282 /* If we need Hyper-Z. */ 283 if (zmask_clear || hiz_clear) { 284 /* Try to obtain the access to Hyper-Z buffers if we don't have one. */ 285 if (!r300->hyperz_enabled && 286 (r300->screen->caps.is_r500 || debug_get_option_hyperz())) { 287 r300->hyperz_enabled = 288 r300->rws->cs_request_feature(r300->cs, 289 RADEON_FID_R300_HYPERZ_ACCESS, 290 TRUE); 291 if (r300->hyperz_enabled) { 292/* Need to emit HyperZ buffer regs for the first time. */ 293r300_mark_fb_state_dirty(r300, R300_CHANGED_HYPERZ_FLAG); 294 } 295 } On the first run we get into the cs_request_feature stuff and then the r300_mark_fb_state_dirty function properly. hiz_clear is never true here, but the zmask_clear boolean is - see above. The debug_get_option_hyperz() is the cause why this happens as that is the one that checks the environment variable. 297 /* Setup Hyper-Z clears. */ 298 if (r300->hyperz_enabled) { 299 if (zmask_clear) { 300 hyperz_dcv = hyperz->zb_depthclearvalue = 301 r300_depth_clear_value(fb->zsbuf->format, depth, stencil); 302 303 r300_mark_atom_dirty(r300, >zmask_clear); 304 r300_mark_atom_dirty(r300, >gpu_flush); 305 buffers &= ~PIPE_CLEAR_DEPTHSTENCIL; 306 } 307 308 if (hiz_clear) { 309 r300->hiz_clear_value = r300_hiz_clear_value(depth); 310 r300_mark_atom_dirty(r300, >hiz_clear); 311
Re: [linux-sunxi] [PATCH 2/9] drm/sun4i: tcon: Add TCON LCD support for R40
On Fri, Jun 14, 2019 at 4:32 PM Chen-Yu Tsai wrote: > > On Fri, Jun 14, 2019 at 6:56 PM Jagan Teki wrote: > > > > On Fri, Jun 14, 2019 at 9:05 AM Chen-Yu Tsai wrote: > > > > > > On Fri, Jun 14, 2019 at 11:19 AM Chen-Yu Tsai wrote: > > > > > > > > On Fri, Jun 14, 2019 at 2:53 AM Jagan Teki > > > > wrote: > > > > > > > > > > TCON LCD0, LCD1 in allwinner R40, are used for managing > > > > > LCD interfaces like RGB, LVDS and DSI. > > > > > > > > > > Like TCON TV0, TV1 these LCD0, LCD1 are also managed via > > > > > tcon top. > > > > > > > > > > Add support for it, in tcon driver. > > > > > > > > > > Signed-off-by: Jagan Teki > > > > > > > > Reviewed-by: Chen-Yu Tsai > > > > > > I take that back. > > > > > > The TCON output muxing (which selects whether TCON LCD or TCON TV > > > outputs to the GPIO pins) > > > is not supported yet. Please at least add TODO notes, or ideally, > > > > Are you referring about port selection? it is support in > > sun8i_tcon_top_de_config. > > No. That's supported as you already pointed out. That only selects > which mixer outputs to which TCON. > > I'm talking about the GPIOD and GPIOH bits, which select which of > LCD or TV TCON outputs to the LCD function pins. Keep in mind that > the TV TCON's H/V SYNC signals are used in combination with the > TV encoder RGB outputs for VGA. Got it, so do I need to add TODO on sun8i_r40_lcd_quirks struct? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/panel: Add support for Raydium RM67191 panel driver
Hi Robert, On Fri, Jun 14, 2019 at 8:52 AM Robert Chiras wrote: > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c > @@ -0,0 +1,730 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * i.MX drm driver - Raydium MIPI-DSI panel driver > + * > + * Copyright (C) 2017 NXP > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. No need for this text as you are using SPDX tag. > +static int color_format_from_dsi_format(enum mipi_dsi_pixel_format format) > +{ > + switch (format) { > + case MIPI_DSI_FMT_RGB565: > + return 0x55; > + case MIPI_DSI_FMT_RGB666: > + case MIPI_DSI_FMT_RGB666_PACKED: > + return 0x66; > + case MIPI_DSI_FMT_RGB888: > + return 0x77; Could you use defines for these magic 0x55, 0x66 and 0x77 numbers? > +static int rad_panel_prepare(struct drm_panel *panel) > +{ > + struct rad_panel *rad = to_rad_panel(panel); > + > + if (rad->prepared) > + return 0; > + > + if (rad->reset) { > + gpiod_set_value(rad->reset, 0); > + usleep_range(5000, 1); > + gpiod_set_value(rad->reset, 1); > + usleep_range(2, 25000); This does not look correct. The correct way to do a reset with gpiod API is: gpiod_set_value(rad->reset, 1); delay gpiod_set_value(rad->reset, 0); I don't have the datasheet for the RM67191 panel, but I assume the reset GPIO is active low. Since you inverted the polarity in the dts and inside the driver, you got it right by accident. You could also consider using gpiod_set_value_cansleep() variant instead because the GPIO reset could be provided by an I2C GPIO expander, for example. Also, when sleeping for more than 10ms, msleep is a better fit as per Documentation/timers/timers-howto.txt. > + if (rad->reset) { > + gpiod_set_value(rad->reset, 0); > + usleep_range(15000, 17000); > + gpiod_set_value(rad->reset, 1); > + } Another reset? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 --- Comment #27 from Richard Thier --- This is also always happening: 434 /* Enable fastfill and/or hiz. 435 * 436 * If we cleared zmask/hiz, it's in use now. The Hyper-Z state update 437 * looks if zmask/hiz is in use and programs hardware accordingly. */ 438 if (r300->zmask_in_use || r300->hiz_in_use) { 439 r300_mark_atom_dirty(r300, >hyperz_state); 440 } So we do get into the body of the if as zmasn_in_use is true here. -- 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 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)
https://bugs.freedesktop.org/show_bug.cgi?id=110897 Richard Thier changed: What|Removed |Added Priority|low |medium -- 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 3/6] drm/gem: use new ww_mutex_(un)lock_for_each macros
On Fri, Jun 14, 2019 at 02:41:22PM +0200, Christian König wrote: > Use the provided macros instead of implementing deadlock handling on our own. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/drm_gem.c | 49 ++- > 1 file changed, 12 insertions(+), 37 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 50de138c89e0..6e4623d3bee2 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1307,51 +1307,26 @@ int > drm_gem_lock_reservations(struct drm_gem_object **objs, int count, > struct ww_acquire_ctx *acquire_ctx) > { > - int contended = -1; > + struct ww_mutex *contended; > int i, ret; > > ww_acquire_init(acquire_ctx, _ww_class); > > -retry: > - if (contended != -1) { > - struct drm_gem_object *obj = objs[contended]; > - > - ret = ww_mutex_lock_slow_interruptible(>resv->lock, > -acquire_ctx); > - if (ret) { > - ww_acquire_done(acquire_ctx); > - return ret; > - } > - } > - > - for (i = 0; i < count; i++) { > - if (i == contended) > - continue; > - > - ret = ww_mutex_lock_interruptible([i]->resv->lock, > - acquire_ctx); > - if (ret) { > - int j; > - > - for (j = 0; j < i; j++) > - ww_mutex_unlock([j]->resv->lock); > - > - if (contended != -1 && contended >= i) > - ww_mutex_unlock([contended]->resv->lock); > - > - if (ret == -EDEADLK) { > - contended = i; > - goto retry; retry here, starts the whole locking loop. > - } > - > - ww_acquire_done(acquire_ctx); > - return ret; > - } > - } +#define ww_mutex_unlock_for_each(loop, pos, contended) \ + if (!IS_ERR(contended)) { \ + if (contended) \ + ww_mutex_unlock(contended); \ + contended = (pos); \ + loop { \ + if (contended == (pos)) \ + break; \ + ww_mutex_unlock(pos); \ + } \ + } + +#define ww_mutex_lock_for_each(loop, pos, contended, ret, intr, ctx) \ + for (contended = ERR_PTR(-ENOENT); ({ \ + __label__ relock, next; \ + ret = -ENOENT; \ + if (contended == ERR_PTR(-ENOENT)) \ + contended = NULL; \ + else if (contended == ERR_PTR(-EDEADLK))\ + contended = (pos); \ + else\ + goto next; \ + loop { \ + if (contended == (pos)) { \ + contended = NULL; \ + continue; \ + } \ +relock: \ + ret = !(intr) ? ww_mutex_lock(pos, ctx) : \ + ww_mutex_lock_interruptible(pos, ctx); \ + if (ret == -EDEADLK) { \ + ww_mutex_unlock_for_each(loop, pos, \ +contended);\ + contended = ERR_PTR(-EDEADLK); \ + goto relock;\ while relock here continues where it left of and doesn't restart @loop. Or am I reading this monstrosity the wrong way? + } \ + break; \ +next: \ + continue; \ + } \ + }), ret != -ENOENT;) + > + ww_mutex_lock_for_each(for (i = 0; i < count; i++), > +
[PATCH 08/16] IB/qib: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible modifiers of __GFP_NORETRY or __GFP_NOWARN. Signed-off-by: Christoph Hellwig --- drivers/infiniband/hw/qib/qib_iba6120.c | 2 +- drivers/infiniband/hw/qib/qib_init.c| 20 +++- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/drivers/infiniband/hw/qib/qib_iba6120.c b/drivers/infiniband/hw/qib/qib_iba6120.c index 531d8a1db2c3..d8a0b8993d22 100644 --- a/drivers/infiniband/hw/qib/qib_iba6120.c +++ b/drivers/infiniband/hw/qib/qib_iba6120.c @@ -2076,7 +2076,7 @@ static void alloc_dummy_hdrq(struct qib_devdata *dd) dd->cspec->dummy_hdrq = dma_alloc_coherent(>pcidev->dev, dd->rcd[0]->rcvhdrq_size, >cspec->dummy_hdrq_phys, - GFP_ATOMIC | __GFP_COMP); + GFP_ATOMIC); if (!dd->cspec->dummy_hdrq) { qib_devinfo(dd->pcidev, "Couldn't allocate dummy hdrq\n"); /* fallback to just 0'ing */ diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c index d4fd8a6cff7b..072885a6684d 100644 --- a/drivers/infiniband/hw/qib/qib_init.c +++ b/drivers/infiniband/hw/qib/qib_init.c @@ -1547,18 +1547,13 @@ int qib_create_rcvhdrq(struct qib_devdata *dd, struct qib_ctxtdata *rcd) if (!rcd->rcvhdrq) { dma_addr_t phys_hdrqtail; - gfp_t gfp_flags; - amt = ALIGN(dd->rcvhdrcnt * dd->rcvhdrentsize * sizeof(u32), PAGE_SIZE); - gfp_flags = (rcd->ctxt >= dd->first_user_ctxt) ? - GFP_USER : GFP_KERNEL; old_node_id = dev_to_node(>pcidev->dev); set_dev_node(>pcidev->dev, rcd->node_id); rcd->rcvhdrq = dma_alloc_coherent( - >pcidev->dev, amt, >rcvhdrq_phys, - gfp_flags | __GFP_COMP); + >pcidev->dev, amt, >rcvhdrq_phys, GFP_KERNEL); set_dev_node(>pcidev->dev, old_node_id); if (!rcd->rcvhdrq) { @@ -1578,7 +1573,7 @@ int qib_create_rcvhdrq(struct qib_devdata *dd, struct qib_ctxtdata *rcd) set_dev_node(>pcidev->dev, rcd->node_id); rcd->rcvhdrtail_kvaddr = dma_alloc_coherent( >pcidev->dev, PAGE_SIZE, _hdrqtail, - gfp_flags); + GFP_KERNEL); set_dev_node(>pcidev->dev, old_node_id); if (!rcd->rcvhdrtail_kvaddr) goto bail_free; @@ -1622,17 +1617,8 @@ int qib_setup_eagerbufs(struct qib_ctxtdata *rcd) struct qib_devdata *dd = rcd->dd; unsigned e, egrcnt, egrperchunk, chunk, egrsize, egroff; size_t size; - gfp_t gfp_flags; int old_node_id; - /* -* GFP_USER, but without GFP_FS, so buffer cache can be -* coalesced (we hope); otherwise, even at order 4, -* heavy filesystem activity makes these fail, and we can -* use compound pages. -*/ - gfp_flags = __GFP_RECLAIM | __GFP_IO | __GFP_COMP; - egrcnt = rcd->rcvegrcnt; egroff = rcd->rcvegr_tid_base; egrsize = dd->rcvegrbufsize; @@ -1664,7 +1650,7 @@ int qib_setup_eagerbufs(struct qib_ctxtdata *rcd) rcd->rcvegrbuf[e] = dma_alloc_coherent(>pcidev->dev, size, >rcvegrbuf_phys[e], - gfp_flags); + GFP_KERNEL); set_dev_node(>pcidev->dev, old_node_id); if (!rcd->rcvegrbuf[e]) goto bail_rcvegrbuf_phys; -- 2.20.1
[PATCH 16/16] dma-mapping: use exact allocation in dma_alloc_contiguous
Many architectures (e.g. arm, m68 and sh) have always used exact allocation in their dma coherent allocator, which avoids a lot of memory waste especially for larger allocations. Lift this behavior into the generic allocator so that dma-direct and the generic IOMMU code benefit from this behavior as well. Signed-off-by: Christoph Hellwig --- include/linux/dma-contiguous.h | 8 +--- kernel/dma/contiguous.c| 17 +++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h index c05d4e661489..2e542e314acf 100644 --- a/include/linux/dma-contiguous.h +++ b/include/linux/dma-contiguous.h @@ -161,15 +161,17 @@ static inline struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp) { int node = dev ? dev_to_node(dev) : NUMA_NO_NODE; - size_t align = get_order(PAGE_ALIGN(size)); + void *cpu_addr = alloc_pages_exact_node(node, size, gfp); - return alloc_pages_node(node, gfp, align); + if (!cpu_addr) + return NULL; + return virt_to_page(p); } static inline void dma_free_contiguous(struct device *dev, struct page *page, size_t size) { - __free_pages(page, get_order(size)); + free_pages_exact(page_address(page), get_order(size)); } #endif diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c index bfc0c17f2a3d..84f41eea2741 100644 --- a/kernel/dma/contiguous.c +++ b/kernel/dma/contiguous.c @@ -232,9 +232,8 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp) { int node = dev ? dev_to_node(dev) : NUMA_NO_NODE; size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT; - size_t align = get_order(PAGE_ALIGN(size)); - struct page *page = NULL; struct cma *cma = NULL; + void *cpu_addr; if (dev && dev->cma_area) cma = dev->cma_area; @@ -243,14 +242,20 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp) /* CMA can be used only in the context which permits sleeping */ if (cma && gfpflags_allow_blocking(gfp)) { + size_t align = get_order(PAGE_ALIGN(size)); + struct page *page; + align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT); page = cma_alloc(cma, count, align, gfp & __GFP_NOWARN); + if (page) + return page; } /* Fallback allocation of normal pages */ - if (!page) - page = alloc_pages_node(node, gfp, align); - return page; + cpu_addr = alloc_pages_exact_node(node, size, gfp); + if (!cpu_addr) + return NULL; + return virt_to_page(cpu_addr); } /** @@ -267,7 +272,7 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp) void dma_free_contiguous(struct device *dev, struct page *page, size_t size) { if (!cma_release(dev_get_cma_area(dev), page, size >> PAGE_SHIFT)) - __free_pages(page, get_order(size)); + free_pages_exact(page_address(page), get_order(size)); } /* -- 2.20.1
[PATCH 12/16] staging/comedi: mark as broken
comedi_buf.c abuse the DMA API in gravely broken ways, as it assumes it can call virt_to_page on the result, and the just remap it as uncached using vmap. Disable the driver until this API abuse has been fixed. Signed-off-by: Christoph Hellwig --- drivers/staging/comedi/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig index 049b659fa6ad..e7c021d76cfa 100644 --- a/drivers/staging/comedi/Kconfig +++ b/drivers/staging/comedi/Kconfig @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 config COMEDI tristate "Data acquisition support (comedi)" + depends on BROKEN help Enable support for a wide range of data acquisition devices for Linux. -- 2.20.1
[PATCH 07/16] IB/hfi1: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible modifiers of __GFP_NORETRY or __GFP_NOWARN. Signed-off-by: Christoph Hellwig --- drivers/infiniband/hw/hfi1/init.c | 22 +++--- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c index 71cb9525c074..ff9d106ee784 100644 --- a/drivers/infiniband/hw/hfi1/init.c +++ b/drivers/infiniband/hw/hfi1/init.c @@ -1846,17 +1846,10 @@ int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd) u64 reg; if (!rcd->rcvhdrq) { - gfp_t gfp_flags; - amt = rcvhdrq_size(rcd); - if (rcd->ctxt < dd->first_dyn_alloc_ctxt || rcd->is_vnic) - gfp_flags = GFP_KERNEL; - else - gfp_flags = GFP_USER; rcd->rcvhdrq = dma_alloc_coherent(>pcidev->dev, amt, - >rcvhdrq_dma, - gfp_flags | __GFP_COMP); + >rcvhdrq_dma, GFP_KERNEL); if (!rcd->rcvhdrq) { dd_dev_err(dd, @@ -1870,7 +1863,7 @@ int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd) rcd->rcvhdrtail_kvaddr = dma_alloc_coherent(>pcidev->dev, PAGE_SIZE, >rcvhdrqtailaddr_dma, - gfp_flags); + GFP_KERNEL); if (!rcd->rcvhdrtail_kvaddr) goto bail_free; } @@ -1926,19 +1919,10 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd) { struct hfi1_devdata *dd = rcd->dd; u32 max_entries, egrtop, alloced_bytes = 0; - gfp_t gfp_flags; u16 order, idx = 0; int ret = 0; u16 round_mtu = roundup_pow_of_two(hfi1_max_mtu); - /* -* GFP_USER, but without GFP_FS, so buffer cache can be -* coalesced (we hope); otherwise, even at order 4, -* heavy filesystem activity makes these fail, and we can -* use compound pages. -*/ - gfp_flags = __GFP_RECLAIM | __GFP_IO | __GFP_COMP; - /* * The minimum size of the eager buffers is a groups of MTU-sized * buffers. @@ -1969,7 +1953,7 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd) dma_alloc_coherent(>pcidev->dev, rcd->egrbufs.rcvtid_size, >egrbufs.buffers[idx].dma, - gfp_flags); + GFP_KERNEL); if (rcd->egrbufs.buffers[idx].addr) { rcd->egrbufs.buffers[idx].len = rcd->egrbufs.rcvtid_size; -- 2.20.1
[PATCH 10/16] iwlwifi: stop passing bogus gfp flags arguments to dma_alloc_coherent
dma_alloc_coherent is not just the page allocator. The only valid arguments to pass are either GFP_ATOMIC or GFP_ATOMIC with possible modifiers of __GFP_NORETRY or __GFP_NOWARN. Signed-off-by: Christoph Hellwig --- drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 3 +-- drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c index 5f52e40a2903..323dc5d5ee88 100644 --- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c +++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c @@ -2361,8 +2361,7 @@ iwl_fw_dbg_buffer_allocation(struct iwl_fw_runtime *fwrt, u32 size) virtual_addr = dma_alloc_coherent(fwrt->trans->dev, size, _addr, - GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO | - __GFP_COMP); + GFP_KERNEL | __GFP_NOWARN); /* TODO: alloc fragments if needed */ if (!virtual_addr) diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c index 803fcbac4152..22a47f928dc8 100644 --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c @@ -210,8 +210,7 @@ static void iwl_pcie_alloc_fw_monitor_block(struct iwl_trans *trans, for (power = max_power; power >= min_power; power--) { size = BIT(power); cpu_addr = dma_alloc_coherent(trans->dev, size, , - GFP_KERNEL | __GFP_NOWARN | - __GFP_ZERO | __GFP_COMP); + GFP_KERNEL | __GFP_NOWARN); if (!cpu_addr) continue; -- 2.20.1
[PATCH 15/16] dma-mapping: clear __GFP_COMP in dma_alloc_attrs
Lift the code to clear __GFP_COMP from arm into the common DMA allocator path. For one this fixes the various other patches that call alloc_pages_exact or split_page in case a bogus driver passes the argument, and it also prepares for doing exact allocation in the generic dma-direct allocator. Signed-off-by: Christoph Hellwig --- arch/arm/mm/dma-mapping.c | 17 - kernel/dma/mapping.c | 9 + 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 0a75058c11f3..86135feb2c05 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -759,14 +759,6 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, if (mask < 0xULL) gfp |= GFP_DMA; - /* -* Following is a work-around (a.k.a. hack) to prevent pages -* with __GFP_COMP being passed to split_page() which cannot -* handle them. The real problem is that this flag probably -* should be 0 on ARM as it is not supported on this -* platform; see CONFIG_HUGETLBFS. -*/ - gfp &= ~(__GFP_COMP); args.gfp = gfp; *handle = DMA_MAPPING_ERROR; @@ -1527,15 +1519,6 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size, return __iommu_alloc_simple(dev, size, gfp, handle, coherent_flag, attrs); - /* -* Following is a work-around (a.k.a. hack) to prevent pages -* with __GFP_COMP being passed to split_page() which cannot -* handle them. The real problem is that this flag probably -* should be 0 on ARM as it is not supported on this -* platform; see CONFIG_HUGETLBFS. -*/ - gfp &= ~(__GFP_COMP); - pages = __iommu_alloc_buffer(dev, size, gfp, attrs, coherent_flag); if (!pages) return NULL; diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index f7afdadb6770..4b618e1abbc1 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -252,6 +252,15 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle, /* let the implementation decide on the zone to allocate from: */ flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM); + /* +* __GFP_COMP interacts badly with splitting up a larger order +* allocation. But as our allocations might not even come from the +* page allocator, the callers can't rely on the fact that they +* even get pages, never mind which kind. +*/ + if (WARN_ON_ONCE(flag & __GFP_COMP)) + flag &= ~__GFP_COMP; + if (dma_is_direct(ops)) cpu_addr = dma_direct_alloc(dev, size, dma_handle, flag, attrs); else if (ops->alloc) -- 2.20.1
[PATCH 14/16] mm: use alloc_pages_exact_node to implement alloc_pages_exact
No need to duplicate the logic over two functions that are almost the same. Signed-off-by: Christoph Hellwig --- include/linux/gfp.h | 5 +++-- mm/page_alloc.c | 39 +++ 2 files changed, 10 insertions(+), 34 deletions(-) diff --git a/include/linux/gfp.h b/include/linux/gfp.h index 4274ea6bc72b..c616a23a3f81 100644 --- a/include/linux/gfp.h +++ b/include/linux/gfp.h @@ -530,9 +530,10 @@ extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order, extern unsigned long __get_free_pages(gfp_t gfp_mask, unsigned int order); extern unsigned long get_zeroed_page(gfp_t gfp_mask); -void *alloc_pages_exact(size_t size, gfp_t gfp_mask); void free_pages_exact(void *virt, size_t size); -void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask); +void *alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask); +#define alloc_pages_exact(size, gfp_mask) \ + alloc_pages_exact_node(NUMA_NO_NODE, size, gfp_mask) #define __get_free_page(gfp_mask) \ __get_free_pages((gfp_mask), 0) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index dd2fed66b656..dec68bd21a71 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -4859,34 +4859,6 @@ static void *make_alloc_exact(unsigned long addr, unsigned int order, return (void *)addr; } -/** - * alloc_pages_exact - allocate an exact number physically-contiguous pages. - * @size: the number of bytes to allocate - * @gfp_mask: GFP flags for the allocation, must not contain __GFP_COMP - * - * This function is similar to alloc_pages(), except that it allocates the - * minimum number of pages to satisfy the request. alloc_pages() can only - * allocate memory in power-of-two pages. - * - * This function is also limited by MAX_ORDER. - * - * Memory allocated by this function must be released by free_pages_exact(). - * - * Return: pointer to the allocated area or %NULL in case of error. - */ -void *alloc_pages_exact(size_t size, gfp_t gfp_mask) -{ - unsigned int order = get_order(size); - unsigned long addr; - - if (WARN_ON_ONCE(gfp_mask & __GFP_COMP)) - gfp_mask &= ~__GFP_COMP; - - addr = __get_free_pages(gfp_mask, order); - return make_alloc_exact(addr, order, size); -} -EXPORT_SYMBOL(alloc_pages_exact); - /** * alloc_pages_exact_node - allocate an exact number of physically-contiguous *pages on a node. @@ -4894,12 +4866,15 @@ EXPORT_SYMBOL(alloc_pages_exact); * @size: the number of bytes to allocate * @gfp_mask: GFP flags for the allocation, must not contain __GFP_COMP * - * Like alloc_pages_exact(), but try to allocate on node nid first before falling - * back. + * This function is similar to alloc_pages_node(), except that it allocates the + * minimum number of pages to satisfy the request while alloc_pages() can only + * allocate memory in power-of-two pages. This function is also limited by + * MAX_ORDER. * - * Return: pointer to the allocated area or %NULL in case of error. + * Returns a pointer to the allocated area or %NULL in case of error, memory + * allocated by this function must be released by free_pages_exact(). */ -void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask) +void *alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask) { unsigned int order = get_order(size); struct page *p; -- 2.20.1
[PATCH 13/16] mm: rename alloc_pages_exact_nid to alloc_pages_exact_node
This fits in with the naming scheme used by alloc_pages_node. Signed-off-by: Christoph Hellwig --- include/linux/gfp.h | 2 +- mm/page_alloc.c | 4 ++-- mm/page_ext.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/linux/gfp.h b/include/linux/gfp.h index fb07b503dc45..4274ea6bc72b 100644 --- a/include/linux/gfp.h +++ b/include/linux/gfp.h @@ -532,7 +532,7 @@ extern unsigned long get_zeroed_page(gfp_t gfp_mask); void *alloc_pages_exact(size_t size, gfp_t gfp_mask); void free_pages_exact(void *virt, size_t size); -void * __meminit alloc_pages_exact_nid(int nid, size_t size, gfp_t gfp_mask); +void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask); #define __get_free_page(gfp_mask) \ __get_free_pages((gfp_mask), 0) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index d66bc8abe0af..dd2fed66b656 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -4888,7 +4888,7 @@ void *alloc_pages_exact(size_t size, gfp_t gfp_mask) EXPORT_SYMBOL(alloc_pages_exact); /** - * alloc_pages_exact_nid - allocate an exact number of physically-contiguous + * alloc_pages_exact_node - allocate an exact number of physically-contiguous *pages on a node. * @nid: the preferred node ID where memory should be allocated * @size: the number of bytes to allocate @@ -4899,7 +4899,7 @@ EXPORT_SYMBOL(alloc_pages_exact); * * Return: pointer to the allocated area or %NULL in case of error. */ -void * __meminit alloc_pages_exact_nid(int nid, size_t size, gfp_t gfp_mask) +void * __meminit alloc_pages_exact_node(int nid, size_t size, gfp_t gfp_mask) { unsigned int order = get_order(size); struct page *p; diff --git a/mm/page_ext.c b/mm/page_ext.c index d8f1aca4ad43..bca6bb316714 100644 --- a/mm/page_ext.c +++ b/mm/page_ext.c @@ -215,7 +215,7 @@ static void *__meminit alloc_page_ext(size_t size, int nid) gfp_t flags = GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN; void *addr = NULL; - addr = alloc_pages_exact_nid(nid, size, flags); + addr = alloc_pages_exact_node(nid, size, flags); if (addr) { kmemleak_alloc(addr, size, 1, flags); return addr; -- 2.20.1
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019-06-14 2:55 p.m., Koenig, Christian wrote: > Am 14.06.19 um 14:09 schrieb Emil Velikov: > >> That said, the proposal will not conflict with the DRM_AUTH removal. If >> anything it is step 0.5 of the grand master plan. > > That's the point I strongly disagree on. > > By lowering the DRM_AUTH restriction you are encouraging userspace to > use the shortcut and use the primary node for rendering instead of > fixing the code and using the render node. > > So at the end of the day userspace will most likely completely drop > support for the render node, simply for the reason that it became > superfluous. You can just open up the primary node and get the same > functionality. > > I absolutely can't understand how you can argue that this won't make it > harder to cleanly separate rendering and primary node functionality. FWIW, I agree with Christian on this. Also FWIW, the solution I'm working on for https://bugs.freedesktop.org/110903 should allow making libdrm_amdgpu always use a render node for rendering. For backwards compatibility it'll probably require adding a new variant of amdgpu_device_initialize though, since existing users may rely on the first amdgpu_device for a GPU using the DRM file description passed to amdgpu_device_initialize for rendering. -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: move cpu_writers handling into vmwgfx
Hi, Christian, On Fri, 2019-06-14 at 14:58 +0200, Christian König wrote: > Thomas just a gentle ping on this. > > It's not that my live depends on this, but it would still be a nice > to > have cleanup. > > Thanks, > Christian. > I thought I had answered this, but I can't find it in my outgoing folder. Sorry about that. In principle I'm fine with it, but the vmwgfx part needs some changes: 1) We need to operate on struct vmwgfx_buffer_object rather than struct vmwgfx_user_buffer_object. Not all buffer objects are user buffer objects... 2) Need to look at the moving the list verifying or at least its calls into the vmwgfx_validate.c code. I hopefully can have a quick look at this next week. /Thomas > Am 07.06.19 um 16:47 schrieb Christian König: > > This feature is only used by vmwgfx and superflous for everybody > > else. > > > > Signed-off-by: Christian König > > --- > > drivers/gpu/drm/ttm/ttm_bo.c | 27 -- > > drivers/gpu/drm/ttm/ttm_bo_util.c| 1 - > > drivers/gpu/drm/ttm/ttm_execbuf_util.c | 7 + > > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 35 > > > > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 ++ > > drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 8 ++ > > drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +++ > > include/drm/ttm/ttm_bo_api.h | 31 - > > > > 8 files changed, 45 insertions(+), 70 deletions(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c > > b/drivers/gpu/drm/ttm/ttm_bo.c > > index c7de667d482a..4ec055ffd6a7 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > > @@ -153,7 +153,6 @@ static void ttm_bo_release_list(struct kref > > *list_kref) > > > > BUG_ON(kref_read(>list_kref)); > > BUG_ON(kref_read(>kref)); > > - BUG_ON(atomic_read(>cpu_writers)); > > BUG_ON(bo->mem.mm_node != NULL); > > BUG_ON(!list_empty(>lru)); > > BUG_ON(!list_empty(>ddestroy)); > > @@ -1308,7 +1307,6 @@ int ttm_bo_init_reserved(struct ttm_bo_device > > *bdev, > > > > kref_init(>kref); > > kref_init(>list_kref); > > - atomic_set(>cpu_writers, 0); > > INIT_LIST_HEAD(>lru); > > INIT_LIST_HEAD(>ddestroy); > > INIT_LIST_HEAD(>swap); > > @@ -1814,31 +1812,6 @@ int ttm_bo_wait(struct ttm_buffer_object > > *bo, > > } > > EXPORT_SYMBOL(ttm_bo_wait); > > > > -int ttm_bo_synccpu_write_grab(struct ttm_buffer_object *bo, bool > > no_wait) > > -{ > > - int ret = 0; > > - > > - /* > > -* Using ttm_bo_reserve makes sure the lru lists are updated. > > -*/ > > - > > - ret = ttm_bo_reserve(bo, true, no_wait, NULL); > > - if (unlikely(ret != 0)) > > - return ret; > > - ret = ttm_bo_wait(bo, true, no_wait); > > - if (likely(ret == 0)) > > - atomic_inc(>cpu_writers); > > - ttm_bo_unreserve(bo); > > - return ret; > > -} > > -EXPORT_SYMBOL(ttm_bo_synccpu_write_grab); > > - > > -void ttm_bo_synccpu_write_release(struct ttm_buffer_object *bo) > > -{ > > - atomic_dec(>cpu_writers); > > -} > > -EXPORT_SYMBOL(ttm_bo_synccpu_write_release); > > - > > /** > >* A buffer object shrink method that tries to swap out the first > >* buffer object on the bo_global::swap_lru list. > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c > > b/drivers/gpu/drm/ttm/ttm_bo_util.c > > index 895d77d799e4..6f43f1f0de7c 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > > @@ -511,7 +511,6 @@ static int ttm_buffer_object_transfer(struct > > ttm_buffer_object *bo, > > mutex_init(>base.wu_mutex); > > fbo->base.moving = NULL; > > drm_vma_node_reset(>base.vma_node); > > - atomic_set(>base.cpu_writers, 0); > > > > kref_init(>base.list_kref); > > kref_init(>base.kref); > > diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c > > b/drivers/gpu/drm/ttm/ttm_execbuf_util.c > > index 957ec375a4ba..80fa52b36d5c 100644 > > --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c > > +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c > > @@ -113,12 +113,7 @@ int ttm_eu_reserve_buffers(struct > > ww_acquire_ctx *ticket, > > struct ttm_buffer_object *bo = entry->bo; > > > > ret = __ttm_bo_reserve(bo, intr, (ticket == NULL), > > ticket); > > - if (!ret && unlikely(atomic_read(>cpu_writers) > > > 0)) { > > - reservation_object_unlock(bo->resv); > > - > > - ret = -EBUSY; > > - > > - } else if (ret == -EALREADY && dups) { > > + if (ret == -EALREADY && dups) { > > struct ttm_validate_buffer *safe = entry; > > entry = list_prev_entry(entry, head); > > list_del(>head); > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > > b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > > index 5d5c2bce01f3..457861c5047f 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > > @@ -565,7
[PATCH 1/3] video: fbdev: s3c-fb: return -ENOMEM on framebuffer_alloc() failure
Fix error code from -ENOENT to -ENOMEM. Cc: Jingoo Han Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/video/fbdev/s3c-fb.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/drivers/video/fbdev/s3c-fb.c === --- a/drivers/video/fbdev/s3c-fb.c +++ b/drivers/video/fbdev/s3c-fb.c @@ -1191,7 +1191,7 @@ static int s3c_fb_probe_win(struct s3c_f palette_size * sizeof(u32), sfb->dev); if (!fbinfo) { dev_err(sfb->dev, "failed to allocate framebuffer\n"); - return -ENOENT; + return -ENOMEM; } windata = sfb->pdata->win[win_no];
Re: [PATCH 19/20] drm: rcar-du: crtc: Register GAMMA_LUT properties
On Fri, Jun 14, 2019 at 11:27:45AM +0200, Jacopo Mondi wrote: > Hi Daniel, > > On Fri, Jun 14, 2019 at 10:42:51AM +0200, Daniel Vetter wrote: > > On Fri, Jun 14, 2019 at 10:15:52AM +0200, Jacopo Mondi wrote: > > > Hi Laurent, > > >thanks for review > > > > > > On Fri, Jun 07, 2019 at 03:03:04PM +0300, Laurent Pinchart wrote: > > > > Hi Jacopo, > > > > > > > > Thank you for the patch. > > > > > > > > On Thu, Jun 06, 2019 at 04:22:19PM +0200, Jacopo Mondi wrote: > > > > > Enable the GAMMA_LUT KMS property using the framework helpers to > > > > > register the proeprty and the associated gamma table size maximum > > > > > size. > > > > > > > > > > Signed-off-by: Jacopo Mondi > > > > > --- > > > > > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > > > > > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > > > > > index e6d3df37c827..c920fb5dba65 100644 > > > > > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > > > > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > > > > > @@ -1207,6 +1207,9 @@ int rcar_du_crtc_create(struct rcar_du_group > > > > > *rgrp, unsigned int swindex, > > > > > rcdu->cmms[swindex]) { > > > > > rcrtc->cmm = rcdu->cmms[swindex]; > > > > > rgrp->cmms_mask |= BIT(hwindex % 2); > > > > > + > > > > > + drm_mode_crtc_set_gamma_size(crtc, CMM_GAMMA_LUT_SIZE); > > > > > + drm_crtc_enable_color_mgmt(crtc, 0, false, > > > > > CMM_GAMMA_LUT_SIZE); > > > > > > > > This change looks good, but you also need to add support for legacy API. > > > > According to the function's documentation, > > > > > > > > * Drivers should use drm_atomic_helper_legacy_gamma_set() to implement > > > > the > > > > * legacy _crtc_funcs.gamma_set callback. > > > > > > > > > > Drivers 'shuld' or drivers 'shall' ? > > > Isn't this required only to support the 'legacy APIs' ? Do we want that? > > > > You're calling drm_mode_crtc_set_gamma_size, which is only useful for the > > legacy ioctls. should here = assuming your hw supports something that > > legacy gamma ioctl can use. Feel free to patch up the docs. > > Oh, I see. I should either leave the old API alone without calling > drm_mode_crtc_set_gamma_size(), or set the .gamma_set callback to > point to drm_atomic_helper_legacy_gamma_set(), which translates the > old gamma table interface to a blob property and attach it to a crtc > state which is then commited and applied through the atomic helpers. > > So I would change the doc to prescribe that if the driver intends to > support the legacy SETGAMMA/GETGAMMA IOCTLs it should declare the > gamma table size with drm_mode_crtc_set_gamma_size() first, and set > the .gamma_set crtc callback to drm_atomic_helper_legacy_gamma_set(), > which translates the legacy interface to a GAMMA_LUT property blob > and commit it. > > If that works, I'll make a small patch to the documentation in v2. Not quite what I meant: You should support the legacy gamma ioctl, if your gamma ramp can be squared into support that. Which generally means yes. We've been thinking about just wiring this all up by default, but there's some drivers who use a 256 entry legacy gamma ramp (for backwards compat with old X11 userspace), but advertise much bigger tables through the new ioctl. So it's not quite as simple. But except if you have some really strange hw there's just no good reason not to support the old legacy ioctl. We also don't just support the new atomic ioctl on new drivers, they all still support the older interfaces. This is the same. That's what I meant should be clarified if it's not yet clear in docs, plus maybe a new todo entry in Documentation/gpu/todo.rst. -Daniel > > Thanks > j > > > > -Daniel > > > > > > > > Thanks > > >j > > > > > > > > } > > > > > > > > > > drm_crtc_helper_add(crtc, _helper_funcs); > > > > > > > > > > > > > -- > > > > Regards, > > > > > > > > Laurent Pinchart > > > > > > > > -- > > 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 2/3] video: fbdev: intelfb: return -ENOMEM on framebuffer_alloc() failure
Fix error code from -ENODEV to -ENOMEM. Cc: Maik Broemme Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/video/fbdev/intelfb/intelfbdrv.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/drivers/video/fbdev/intelfb/intelfbdrv.c === --- a/drivers/video/fbdev/intelfb/intelfbdrv.c +++ b/drivers/video/fbdev/intelfb/intelfbdrv.c @@ -493,7 +493,7 @@ static int intelfb_pci_register(struct p info = framebuffer_alloc(sizeof(struct intelfb_info), >dev); if (!info) { ERR_MSG("Could not allocate memory for intelfb_info.\n"); - return -ENODEV; + return -ENOMEM; } if (fb_alloc_cmap(>cmap, 256, 1) < 0) { ERR_MSG("Could not allocate cmap for intelfb_info.\n"); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd: fix hotplug race at startup
On Fri, Jun 14, 2019 at 07:29:23PM +0800, Young Xiao wrote: > We should check mode_config_initialized flag in amdgpu_hotplug_work_func. > > See commit 7f98ca454ad3 ("drm/radeon: fix hotplug race at startup") for > details. > > Signed-off-by: Young Xiao <92siuy...@gmail.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > index af4c3b1..13186d6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > @@ -85,6 +85,9 @@ static void amdgpu_hotplug_work_func(struct work_struct > *work) > struct drm_mode_config *mode_config = >mode_config; > struct drm_connector *connector; > > + if (!adev->mode_info.mode_config_initialized) > + return; I think you want to delay your hotplug initialization until you're ready to serve hotplug events, this here is fairly racy ... -Daniel > + > mutex_lock(_config->mutex); > list_for_each_entry(connector, _config->connector_list, head) > amdgpu_connector_hotplug(connector); > -- > 2.7.4 > -- 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
Re: [PATCH v3 0/4] drm/panfrost: Expose perf counters to userspace
On Wed, May 29, 2019 at 9:16 AM Alyssa Rosenzweig wrote: > > Woohoo! Patches 1-3 are R-b; patch 4 is A-b. Exciting progress! Hoping > to hear what Rob and Steven think :) All looks fine to me, but there's a kbuild error on patch 4 that needs to be fixed. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 12/16] staging/comedi: mark as broken
On Fri, Jun 14, 2019 at 05:30:32PM +0200, Greg KH wrote: > On Fri, Jun 14, 2019 at 04:48:57PM +0200, Christoph Hellwig wrote: > > On Fri, Jun 14, 2019 at 04:02:39PM +0200, Greg KH wrote: > > > Perhaps a hint as to how we can fix this up? This is the first time > > > I've heard of the comedi code not handling dma properly. > > > > It can be fixed by: > > > > a) never calling virt_to_page (or vmalloc_to_page for that matter) > > on dma allocation > > b) never remapping dma allocation with conflicting cache modes > > (no remapping should be doable after a) anyway). > > Ok, fair enough, have any pointers of drivers/core code that does this > correctly? I can put it on my todo list, but might take a week or so... Just about everyone else. They just need to remove the vmap and either do one large allocation, or live with the fact that they need helpers to access multiple array elements instead of one net vmap, which most of the users already seem to do anyway, with just a few using the vmap (which might explain why we didn't see blowups yet).
Re: [pull] amdgpu drm-fixes-5.2
On Fri, Jun 14, 2019 at 5:49 PM Daniel Vetter wrote: > > On Wed, Jun 12, 2019 at 09:18:56PM -0500, Alex Deucher wrote: > > Hi Dave, Daniel, > > > > Fixes for 5.2: > > - Extend previous vce fix for resume to uvd and vcn > > - Fix bounds checking in ras debugfs interface > > - Fix a regression on SI using amdgpu > > > > The following changes since commit 671e2ee5ee2127179ca884b439ab6001a623edd6: > > > > Merge branch 'linux-5.2' of git://github.com/skeggsb/linux into drm-fixes > > (2019-06-07 17:16:00 +1000) > > Somehow missed this one, but just found it before I wanted to push out the > -fixes pull to Linus ... > > > are available in the Git repository at: > > > > git://people.freedesktop.org/~agd5f/linux drm-fixes-5.2 > > Pulled, thanks. Was a bit a lie, the script was still running, and complained that I didn't add a proper merge commit message. Can you pls use annotated tags, then the tooling we use makes this happen automatically? dim pull-request for cheatsheet, if you need one. Cheers, Daniel > -Daniel > > > > > for you to fetch changes up to f3a5231c8f14acd42845e9e60f506b4e948f0e68: > > > > drm/amdgpu: return 0 by default in amdgpu_pm_load_smu_firmware > > (2019-06-12 20:39:49 -0500) > > > > > > Alex Deucher (1): > > drm/amdgpu: return 0 by default in amdgpu_pm_load_smu_firmware > > > > Dan Carpenter (1): > > drm/amdgpu: Fix bounds checking in amdgpu_ras_is_supported() > > > > Shirish S (1): > > drm/amdgpu/{uvd,vcn}: fetch ring's read_ptr after alloc > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 4 ++-- > > drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 ++ > > drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 +++- > > drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 5 - > > drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 5 - > > 5 files changed, 15 insertions(+), 5 deletions(-) > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- 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
[PATCH 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel
Add dt-bindings documentation for Raydium RM67191 DSI panel. Signed-off-by: Robert Chiras --- .../bindings/display/panel/raydium,rm67191.txt | 42 ++ 1 file changed, 42 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt diff --git a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt new file mode 100644 index 000..5a6268d --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt @@ -0,0 +1,42 @@ +Raydium RM67171 OLED LCD panel with MIPI-DSI protocol + +Required properties: +- compatible: "raydium,rm67191" +- reg: virtual channel for MIPI-DSI protocol + must be <0> +- dsi-lanes: number of DSI lanes to be used + must be <3> or <4> +- port:input port node with endpoint definition as + defined in Documentation/devicetree/bindings/graph.txt; + the input port should be connected to a MIPI-DSI device + driver + +Optional properties: +- reset-gpio: a GPIO spec for the RST_B GPIO pin +- pinctrl-0phandle to the pin settings for the reset pin +- width-mm:physical panel width [mm] +- height-mm: physical panel height [mm] +- display-timings: timings for the connected panel according to [1] +- video-mode: 0 - burst-mode + 1 - non-burst with sync event + 2 - non-burst with sync pulse + +[1]: Documentation/devicetree/bindings/display/display-timing.txt + +Example: + + panel@0 { + compatible = "raydium,rm67191"; + reg = <0>; + pinctrl-0 = <_mipi_dsi_0_1_en>; + reset-gpio = < 7 GPIO_ACTIVE_HIGH>; + dsi-lanes = <4>; + width-mm = <68>; + height-mm = <121>; + + port { + panel_in: endpoint { + remote-endpoint = <_out>; + }; + }; + }; -- 2.7.4
[PATCH 0/2] Add DSI panel driver for Raydium RM67191
This patch-set contains the DRM panel driver and dt-bindings documentation for the DSI driven panel: Raydium RM67191. Robert Chiras (2): dt-bindings: display: panel: Add support for Raydium RM67191 panel drm/panel: Add support for Raydium RM67191 panel driver .../bindings/display/panel/raydium,rm67191.txt | 42 ++ drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm67191.c | 730 + 4 files changed, 782 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c -- 2.7.4
[Bug 108987] [CI][DRMTIP]igt@perf@short-reads - incomplete
https://bugs.freedesktop.org/show_bug.cgi?id=108987 --- Comment #2 from CI Bug Log --- The CI Bug Log issue associated to this bug has been archived. New failures matching the above filters will not be associated to this bug anymore. -- 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 108987] [CI][DRMTIP]igt@perf@short-reads - incomplete
https://bugs.freedesktop.org/show_bug.cgi?id=108987 Lakshmi changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |WORKSFORME --- Comment #1 from Lakshmi --- Seen only once drmtip_150 (6 months, 2 weeks old). Closing as WORKSFORME. -- 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] drm/rockchip: dw_hdmi: add basic rk3228 support
Am Donnerstag, 23. Mai 2019, 00:46:29 CEST schrieb Justin Swartz: > Like the RK3328, RK322x SoCs offer a Synopsis DesignWare HDMI transmitter > and an Innosilicon HDMI PHY. > > Add a new dw_hdmi_plat_data struct, rk3228_hdmi_drv_data. > Assign a set of mostly generic rk3228_hdmi_phy_ops functions. > Add dw_hdmi_rk3228_setup_hpd() to enable the HDMI HPD and DDC lines. > > Signed-off-by: Justin Swartz applied to drm-misc-next Thanks Heiko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] locking: add ww_mutex_(un)lock_for_each helpers
Am 14.06.19 um 14:56 schrieb Peter Zijlstra: On Fri, Jun 14, 2019 at 02:41:20PM +0200, Christian König wrote: The ww_mutex implementation allows for detection deadlocks when multiple threads try to acquire the same set of locks in different order. The problem is that handling those deadlocks was the burden of the user of the ww_mutex implementation and at least some users didn't got that right on the first try. I'm not a big fan of macros, but it still better then implementing the same logic at least halve a dozen times. So this patch adds two macros to lock and unlock multiple ww_mutex instances with the necessary deadlock handling. Signed-off-by: Christian König --- include/linux/ww_mutex.h | 75 1 file changed, 75 insertions(+) diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h index 3af7c0e03be5..a0d893b5b114 100644 --- a/include/linux/ww_mutex.h +++ b/include/linux/ww_mutex.h @@ -369,4 +369,79 @@ static inline bool ww_mutex_is_locked(struct ww_mutex *lock) return mutex_is_locked(>base); } +/** + * ww_mutex_unlock_for_each - cleanup after error or contention + * @loop: for loop code fragment iterating over all the locks + * @pos: code fragment returning the currently handled lock + * @contended: the last contended ww_mutex or NULL or ERR_PTR + * + * Helper to make cleanup after error or lock contention easier. + * First unlock the last contended lock and then all other locked ones. + */ +#define ww_mutex_unlock_for_each(loop, pos, contended) \ + if (!IS_ERR(contended)) { \ + if (contended) \ + ww_mutex_unlock(contended); \ + contended = (pos); \ + loop { \ + if (contended == (pos)) \ + break; \ + ww_mutex_unlock(pos); \ + } \ + } + +/** + * ww_mutex_lock_for_each - implement ww_mutex deadlock handling + * @loop: for loop code fragment iterating over all the locks + * @pos: code fragment returning the currently handled lock + * @contended: ww_mutex pointer variable for state handling + * @ret: int variable to store the return value of ww_mutex_lock() + * @intr: If true ww_mutex_lock_interruptible() is used + * @ctx: ww_acquire_ctx pointer for the locking + * + * This macro implements the necessary logic to lock multiple ww_mutex + * instances. Lock contention with backoff and re-locking is handled by the + * macro so that the loop body only need to handle other errors and + * successfully acquired locks. + * + * With the @loop and @pos code fragment it is possible to apply this logic + * to all kind of containers (array, list, tree, etc...) holding multiple + * ww_mutex instances. + * + * @contended is used to hold the current state we are in. -ENOENT is used to + * signal that we are just starting the handling. -EDEADLK means that the + * current position is contended and we need to restart the loop after locking + * it. NULL means that there is no contention to be handled. Any other value is + * the last contended ww_mutex. + */ +#define ww_mutex_lock_for_each(loop, pos, contended, ret, intr, ctx) \ + for (contended = ERR_PTR(-ENOENT); ({ \ + __label__ relock, next; \ + ret = -ENOENT; \ + if (contended == ERR_PTR(-ENOENT)) \ + contended = NULL; \ + else if (contended == ERR_PTR(-EDEADLK))\ + contended = (pos); \ + else\ + goto next; \ + loop { \ + if (contended == (pos)) { \ + contended = NULL; \ + continue; \ + } \ +relock: \ + ret = !(intr) ? ww_mutex_lock(pos, ctx) : \ + ww_mutex_lock_interruptible(pos, ctx); \ + if (ret == -EDEADLK) { \ + ww_mutex_unlock_for_each(loop, pos, \ +contended);\ + contended = ERR_PTR(-EDEADLK); \ + goto relock;
Re: [PATCH 3/6] drm/gem: use new ww_mutex_(un)lock_for_each macros
On Fri, Jun 14, 2019 at 02:41:22PM +0200, Christian König wrote: > Use the provided macros instead of implementing deadlock handling on our own. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/drm_gem.c | 49 ++- > 1 file changed, 12 insertions(+), 37 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 50de138c89e0..6e4623d3bee2 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1307,51 +1307,26 @@ int > drm_gem_lock_reservations(struct drm_gem_object **objs, int count, > struct ww_acquire_ctx *acquire_ctx) > { > - int contended = -1; > + struct ww_mutex *contended; > int i, ret; > > ww_acquire_init(acquire_ctx, _ww_class); > > -retry: > - if (contended != -1) { > - struct drm_gem_object *obj = objs[contended]; > - > - ret = ww_mutex_lock_slow_interruptible(>resv->lock, > -acquire_ctx); > - if (ret) { > - ww_acquire_done(acquire_ctx); > - return ret; > - } > - } > - > - for (i = 0; i < count; i++) { > - if (i == contended) > - continue; > - > - ret = ww_mutex_lock_interruptible([i]->resv->lock, > - acquire_ctx); > - if (ret) { > - int j; > - > - for (j = 0; j < i; j++) > - ww_mutex_unlock([j]->resv->lock); > - > - if (contended != -1 && contended >= i) > - ww_mutex_unlock([contended]->resv->lock); > - > - if (ret == -EDEADLK) { > - contended = i; > - goto retry; > - } > - > - ww_acquire_done(acquire_ctx); > - return ret; > - } > - } I note all the sites you use this on are simple idx iterators; so how about something like so: int ww_mutex_unlock_all(int count, void *data, struct ww_mutex *(*func)(int, void *)) { int i; for (i = 0; i < count; i++) { lock = func(i, data); ww_mutex_unlock(lock); } } int ww_mutex_lock_all(int count, struct ww_acquire_context *acquire_ctx, bool intr, void *data, struct ww_mutex *(*func)(int, void *)) { int i, ret, contended = -1; struct ww_mutex *lock; retry: if (contended != -1) { lock = func(contended, data); if (intr) ret = ww_mutex_lock_slow_interruptible(lock, acquire_ctx); else ret = ww_mutex_lock_slow(lock, acquire_ctx), 0; if (ret) { ww_acquire_done(acquire_ctx); return ret; } } for (i = 0; i < count; i++) { if (i == contended) continue; lock = func(i, data); if (intr) ret = ww_mutex_lock_interruptible(lock, acquire_ctx); else ret = ww_mutex_lock(lock, acquire_ctx), 0; if (ret) { ww_mutex_unlock_all(i, data, func); if (contended > i) { lock = func(contended, data); ww_mutex_unlock(lock); } if (ret == -EDEADLK) { contended = i; goto retry; } ww_acquire_done(acquire_ctx); return ret; } } ww_acquire_done(acquire_ctx); return 0; } > + ww_mutex_lock_for_each(for (i = 0; i < count; i++), > +[i]->resv->lock, contended, ret, true, > +acquire_ctx) > + if (ret) > + goto error; which then becomes: struct ww_mutex *gem_ww_mutex_func(int i, void *data) { struct drm_gem_object **objs = data; return [i]->resv->lock; } ret = ww_mutex_lock_all(count, acquire_ctx, true, objs, gem_ww_mutex_func); > ww_acquire_done(acquire_ctx); > > return 0; > + > +error: > + ww_mutex_unlock_for_each(for (i = 0; i < count; i++), > + [i]->resv->lock, contended); > + ww_acquire_done(acquire_ctx); > + return ret; > } > EXPORT_SYMBOL(drm_gem_lock_reservations); > > -- > 2.17.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EXT] Re: [PATCH 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel
Hi Fabio, On Vi, 2019-06-14 at 09:59 -0300, Fabio Estevam wrote: > On Fri, Jun 14, 2019 at 8:53 AM Robert Chiras > wrote: > > > > > > Add dt-bindings documentation for Raydium RM67191 DSI panel. > > > > Signed-off-by: Robert Chiras > > --- > > .../bindings/display/panel/raydium,rm67191.txt | 42 > > ++ > > 1 file changed, 42 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt > > > > diff --git > > a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.t > > xt > > b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.t > > xt > > new file mode 100644 > > index 000..5a6268d > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.t > > xt > > @@ -0,0 +1,42 @@ > > +Raydium RM67171 OLED LCD panel with MIPI-DSI protocol > > + > > +Required properties: > > +- compatible: "raydium,rm67191" > > +- reg: virtual channel for MIPI-DSI protocol > > + must be <0> > > +- dsi-lanes: number of DSI lanes to be used > > + must be <3> or <4> > > +- port:input port node with endpoint definition as > > + defined in > > Documentation/devicetree/bindings/graph.txt; > > + the input port should be connected to a > > MIPI-DSI device > > + driver > > + > > +Optional properties: > > +- reset-gpio: a GPIO spec for the RST_B GPIO pin > reset-gpios (with the s in the end) is the recommendation. > > > > > +- display-timings: timings for the connected panel according > > to [1] > This is not needed. Well, I know that the panel timings are already hard-coded into the driver, but on 850D, we have two display controllers: eLCDDIF and DCSS. While eLCDIF works just fine with the display-timings received (and undocumented) from panel vendor, with DCSS we had some issues and we had to tweak the display-timings. This is why I added this property, for a special case where we have to use different timings without changing the driver (just a different dtb file). Do you think this is a bad practice? If yes, then what mechanism of doing that do you recommend? > > > > > +- video-mode: 0 - burst-mode > > + 1 - non-burst with sync event > > + 2 - non-burst with sync pulse > > + > > +[1]: Documentation/devicetree/bindings/display/display-timing.txt > This path does not exist. Right. Will update the path. > > Also, could you try to align these bindings with the one from > raydium,rm68200? > > There are power-supply and backlight optional properties there, which > seem useful. This panel is OLED, while the rm68200 is LCD (from what I've noticed). Meaning this panel backligth is also controlled by the DSI controller, not by a separate backlight LED driver. I will consider, instead, adding support for a power-supply (if possible). > > > > > + > > +Example: > > + > > + panel@0 { > > + compatible = "raydium,rm67191"; > > + reg = <0>; > > + pinctrl-0 = <_mipi_dsi_0_1_en>; > You should also pass pinctrl-names = "default"; if you use pinctrl-0. Thanks. Will do that > > > > > + reset-gpio = < 7 GPIO_ACTIVE_HIGH>; > Should be active low. But, the GPIO is active high.
Re: [EXT] Re: [PATCH 2/2] drm/panel: Add support for Raydium RM67191 panel driver
On Fri, Jun 14, 2019 at 10:29 AM Robert Chiras wrote: > The GPIO is active high, and the above sequence was received from the > panel vendor in the following form: > SET_RESET_PIN(1); > MDELAY(10); > SET_RESET_PIN(0); > MDELAY(5); > SET_RESET_PIN(1); > MDELAY(20); > I got rid of the first transition to high since seemed redundant. > Also, according to the manual reference, the RSTB pin needs to be > active high while operating the display. That's exactly my point :-) In normal operation the GPIO reset needs to be high. During reset the GPIO reset needs to be low., which means that the GPIO reset is "active low". So you should invert both the dts and the driver to behave correctly.