Re: [PATCH 3/6] drm/gem: use new ww_mutex_(un)lock_for_each macros

2019-06-14 Thread Christian König

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

2019-06-14 Thread Peter Zijlstra
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

2019-06-14 Thread Philipp Zabel
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

2019-06-14 Thread Philipp Zabel
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

2019-06-14 Thread Maxime Ripard
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??

2019-06-14 Thread Chris Wilson
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Emil Velikov
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Heiko Stübner
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

2019-06-14 Thread Sam Ravnborg
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

2019-06-14 Thread Daniel Vetter
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??

2019-06-14 Thread Tejun Heo
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

2019-06-14 Thread Daniel Vetter
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

2019-06-14 Thread Heiko Stuebner
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

2019-06-14 Thread Christian König

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

2019-06-14 Thread Fabio Estevam
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread 'Christoph Hellwig'
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?)

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread Greg KH
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

2019-06-14 Thread Boris Brezillon
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

2019-06-14 Thread Jagan Teki
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

2019-06-14 Thread Daniel Baluta
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

2019-06-14 Thread Koenig, Christian
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

2019-06-14 Thread 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 = 

[Bug 110897] HyperZ is broken for r300 (bad z for some micro and macrotiles?)

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread Greg KH
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread Bartlomiej Zolnierkiewicz
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?)

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread YueHaibing
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

2019-06-14 Thread Ezequiel Garcia
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

2019-06-14 Thread Emil Velikov
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

2019-06-14 Thread Robert Chiras
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

2019-06-14 Thread 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?

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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread Colin King
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

2019-06-14 Thread Robin Murphy

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

2019-06-14 Thread Greg Kroah-Hartman
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

2019-06-14 Thread Daniel Vetter
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

2019-06-14 Thread 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.

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

2019-06-14 Thread Robert Chiras
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Maxime Ripard
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

2019-06-14 Thread Christian König
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

2019-06-14 Thread Christian König
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

2019-06-14 Thread Christian König
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

2019-06-14 Thread Christian König
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

2019-06-14 Thread Christian König
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

2019-06-14 Thread Christian König
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

2019-06-14 Thread Christian König
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

2019-06-14 Thread Boris Brezillon
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

2019-06-14 Thread Koenig, Christian
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

2019-06-14 Thread Bartlomiej Zolnierkiewicz
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?)

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread Jagan Teki
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

2019-06-14 Thread Fabio Estevam
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?)

2019-06-14 Thread bugzilla-daemon
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?)

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread 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?

+   }   \
+   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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Michel Dänzer
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

2019-06-14 Thread Thomas Hellstrom
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

2019-06-14 Thread Bartlomiej Zolnierkiewicz
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

2019-06-14 Thread Daniel Vetter
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

2019-06-14 Thread Bartlomiej Zolnierkiewicz
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

2019-06-14 Thread Daniel Vetter
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

2019-06-14 Thread Rob Herring
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

2019-06-14 Thread Christoph Hellwig
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

2019-06-14 Thread Daniel Vetter
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

2019-06-14 Thread Robert Chiras
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

2019-06-14 Thread Robert Chiras
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

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread bugzilla-daemon
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

2019-06-14 Thread Heiko Stuebner
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

2019-06-14 Thread Christian König

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

2019-06-14 Thread 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;
> - }
> -
> - 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

2019-06-14 Thread Robert Chiras
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

2019-06-14 Thread Fabio Estevam
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.


  1   2   3   4   >