Re: [RFC,3/3] drm/komeda: Allow non-component drm_bridge only endpoints

2019-10-23 Thread james qian wang (Arm Technology China)
On Tue, Oct 22, 2019 at 10:50:42AM +0200, Daniel Vetter wrote:
> On Tue, Oct 22, 2019 at 10:48 AM Russell King - ARM Linux admin
>  wrote:
> >
> > On Tue, Oct 22, 2019 at 10:42:10AM +0200, Daniel Vetter wrote:
> > > On Thu, Oct 17, 2019 at 12:41:37PM +0100, Russell King - ARM Linux admin 
> > > wrote:
> > > > On Thu, Oct 17, 2019 at 10:48:12AM +, Brian Starkey wrote:
> > > > > On Thu, Oct 17, 2019 at 10:21:03AM +, james qian wang (Arm 
> > > > > Technology China) wrote:
> > > > > > On Thu, Oct 17, 2019 at 08:20:56AM +, Brian Starkey wrote:
> > > > > > > On Thu, Oct 17, 2019 at 03:07:59AM +, james qian wang (Arm 
> > > > > > > Technology China) wrote:
> > > > > > > > On Wed, Oct 16, 2019 at 04:22:07PM +, Brian Starkey wrote:
> > > > > > > > >
> > > > > > > > > If James is strongly against merging this, maybe we just swap
> > > > > > > > > wholesale to bridge? But for me, the pragmatic approach would 
> > > > > > > > > be this
> > > > > > > > > stop-gap.
> > > > > > > > >
> > > > > > > >
> > > > > > > > This is a good idea, and I vote +ULONG_MAX :)
> > > > > > > >
> > > > > > > > and I also checked tda998x driver, it supports bridge. so swap 
> > > > > > > > the
> > > > > > > > wholesale to brige is perfect. :)
> > > > > > > >
> > > > > > >
> > > > > > > Well, as Mihail wrote, it's definitely not perfect.
> > > > > > >
> > > > > > > Today, if you rmmod tda998x with the DPU driver still loaded,
> > > > > > > everything will be unbound gracefully.
> > > > > > >
> > > > > > > If we swap to bridge, then rmmod'ing tda998x (or any other bridge
> > > > > > > driver the DPU is using) with the DPU driver still loaded will 
> > > > > > > result
> > > > > > > in a crash.
> > > > > >
> > > > > > I haven't read the bridge code, but seems this is a bug of 
> > > > > > drm_bridge,
> > > > > > since if the bridge is still in using by others, the rmmod should 
> > > > > > fail
> > > > > >
> > > > >
> > > > > Correct, but there's no fix for that today. You can also take a look
> > > > > at the thread linked from Mihail's cover letter.
> > > > >
> > > > > > And personally opinion, if the bridge doesn't handle the dependence.
> > > > > > for us:
> > > > > >
> > > > > > - add such support to bridge
> > > > >
> > > > > That would certainly be helpful. I don't know if there's consensus on
> > > > > how to do that.
> > > > >
> > > > > >   or
> > > > > > - just do the insmod/rmmod in correct order.
> > > > > >
> > > > > > > So, there really are proper benefits to sticking with the 
> > > > > > > component
> > > > > > > code for tda998x, which is why I'd like to understand why you're 
> > > > > > > so
> > > > > > > against this patch?
> > > > > > >
> > > > > >
> > > > > > This change handles two different connectors in komeda internally, 
> > > > > > compare
> > > > > > with one interface, it increases the complexity, more risk of bug 
> > > > > > and more
> > > > > > cost of maintainance.
> > > > > >
> > > > >
> > > > > Well, it's only about how to bind the drivers - two different methods
> > > > > of binding, not two different connectors. I would argue that carrying
> > > > > our out-of-tree patches to support both platforms is a larger
> > > > > maintenance burden.
> > > > >
> > > > > Honestly this looks like a win-win to me. We get the superior approach
> > > > > when its supported, and still get to support bridges which are more
> > > > > common.
> > > > >
> > > > > As/when improvements are made to the bridge code we can remove the
> > > > > component bits and not lose anything.
> > > >
> > > > There was an idea a while back about using the device links code to
> > > > solve the bridge issue - but at the time the device links code wasn't
> > > > up to the job.  I think that's been resolved now, but I haven't been
> > > > able to confirm it.  I did propose some patches for bridge at the
> > > > time but they probably need updating.
> > >
> > > I think the only patches that existed where for panel, and we only
> > > discussed the bridge case. At least I can only find patches for panel,not
> > > bridge, but might be missing something.
> >
> > I had a patches, which is why I raised the problem with the core:
> >
> > 6961edfee26d bridge hacks using device links
> >
> > but it never went further than an experiment at the time because of the
> > problems in the core.  As it was a hack, it never got posted.  Seems
> > that kernel tree (for the cubox) is still 5.2 based, so has a lot of
> > patches and might need updating to a more recent base before anything
> > can be tested.
> 
> 
> For reference, the panel patch:
> 
> https://patchwork.kernel.org/patch/10364873/
> 
> And the huge discussion around bridges, that resulted in Rafael
> Wyzocki fixing all the core issues:
> 
> https://www.spinics.net/lists/dri-devel/msg201927.html
> 
> James, do you want to look into this for bridges?
> 

Hi Daniel:

It's my honour. but I don't have much time in the next 3 weeks.

And I talked with Mihail, he will help to check this 

[Bug 109955] amdgpu [RX Vega 64] system freeze while gaming

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109955

--- Comment #121 from Mauro Gaspari  ---
(In reply to blppt from comment #120)
> I dont have anything to attach here, but same issue here, ubuntu 19.04,
> kernel 5.4-rc3, vega64 W/C, Mesa 19.3.0 -- it only seems to occur with DXVK
> and not D9VK for some reason.
> 
> Example: GW2 (DX9 game) will work perfectly under heavy load in WvW with
> massive zergs for hours with no crash, but FFXIV (DX11) will always lock the
> entire system up after a time.
> 
> That being said, when you force the top clock using
> 
> echo manual > /sys/class/drm/card0/device/power_dpm_force_performance_level
> 
> and
> 
> echo 7 > /sys/class/drm/card0/device/pp_dpm_sclk
> 
> FFXIV no longer locks the system at all. It does eat up a good deal more
> watts according to my UPS meter though, so resetting to auto is necessary
> IMHO.
> 
> So, it sounds like you guys are on the right track with the whole "power
> management" thing being the culprit. Just wanted to add my experience to
> this.
> 
> (and yes, echoing the guy above, the exact same system is stable in windows
> 10, so its not a hardware issue).

I agree with this. I am having much better experience myself even without
commands to force the power performance level by doing:
- change game to windowed or full-screen borderless (fixed window)
- disable vsync
- disable frame limiter

by doing the above 3, it seems that GPU is forced into max power state all the
time while playing. I have been using this method for a few days with DXVK
games and I had no freeze so far.

But again this is just a temporary workaround. So is the command to manually
force high power performance level. Hopefully a permanent fix comes with
AMDGPU/Kernel updates.

-- 
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 V5 4/6] mdev: introduce virtio device and its device ops

2019-10-23 Thread Jason Wang


On 2019/10/24 上午5:57, Alex Williamson wrote:

On Wed, 23 Oct 2019 21:07:50 +0800
Jason Wang  wrote:


This patch implements basic support for mdev driver that supports
virtio transport for kernel virtio driver.

Signed-off-by: Jason Wang 
---
  drivers/vfio/mdev/mdev_core.c|  20 
  drivers/vfio/mdev/mdev_private.h |   2 +
  include/linux/mdev.h |   6 ++
  include/linux/virtio_mdev_ops.h  | 159 +++
  4 files changed, 187 insertions(+)
  create mode 100644 include/linux/virtio_mdev_ops.h

diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index 555bd61d8c38..9b00c3513120 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -76,6 +76,26 @@ const struct vfio_mdev_device_ops *mdev_get_vfio_ops(struct 
mdev_device *mdev)
  }
  EXPORT_SYMBOL(mdev_get_vfio_ops);
  
+/* Specify the virtio device ops for the mdev device, this

+ * must be called during create() callback for virtio mdev device.
+ */
+void mdev_set_virtio_ops(struct mdev_device *mdev,
+const struct virtio_mdev_device_ops *virtio_ops)
+{
+   mdev_set_class(mdev, MDEV_CLASS_ID_VIRTIO);
+   mdev->virtio_ops = virtio_ops;
+}
+EXPORT_SYMBOL(mdev_set_virtio_ops);
+
+/* Get the virtio device ops for the mdev device. */
+const struct virtio_mdev_device_ops *
+mdev_get_virtio_ops(struct mdev_device *mdev)
+{
+   WARN_ON(mdev->class_id != MDEV_CLASS_ID_VIRTIO);
+   return mdev->virtio_ops;
+}
+EXPORT_SYMBOL(mdev_get_virtio_ops);
+
  struct device *mdev_dev(struct mdev_device *mdev)
  {
return >dev;
diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h
index 0770410ded2a..7b47890c34e7 100644
--- a/drivers/vfio/mdev/mdev_private.h
+++ b/drivers/vfio/mdev/mdev_private.h
@@ -11,6 +11,7 @@
  #define MDEV_PRIVATE_H
  
  #include 

+#include 
  
  int  mdev_bus_register(void);

  void mdev_bus_unregister(void);
@@ -38,6 +39,7 @@ struct mdev_device {
u16 class_id;
union {
const struct vfio_mdev_device_ops *vfio_ops;
+   const struct virtio_mdev_device_ops *virtio_ops;
};
  };
  
diff --git a/include/linux/mdev.h b/include/linux/mdev.h

index 4625f1a11014..9b69b0bbebfd 100644
--- a/include/linux/mdev.h
+++ b/include/linux/mdev.h
@@ -17,6 +17,7 @@
  
  struct mdev_device;

  struct vfio_mdev_device_ops;
+struct virtio_mdev_device_ops;
  
  /*

   * Called by the parent device driver to set the device which represents
@@ -112,6 +113,10 @@ void mdev_set_class(struct mdev_device *mdev, u16 id);
  void mdev_set_vfio_ops(struct mdev_device *mdev,
   const struct vfio_mdev_device_ops *vfio_ops);
  const struct vfio_mdev_device_ops *mdev_get_vfio_ops(struct mdev_device 
*mdev);
+void mdev_set_virtio_ops(struct mdev_device *mdev,
+const struct virtio_mdev_device_ops *virtio_ops);
+const struct virtio_mdev_device_ops *
+mdev_get_virtio_ops(struct mdev_device *mdev);
  
  extern struct bus_type mdev_bus_type;
  
@@ -127,6 +132,7 @@ struct mdev_device *mdev_from_dev(struct device *dev);
  
  enum {

MDEV_CLASS_ID_VFIO = 1,
+   MDEV_CLASS_ID_VIRTIO = 2,
/* New entries must be added here */
  };
  
diff --git a/include/linux/virtio_mdev_ops.h b/include/linux/virtio_mdev_ops.h

new file mode 100644
index ..d417b41f2845
--- /dev/null
+++ b/include/linux/virtio_mdev_ops.h
@@ -0,0 +1,159 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Virtio mediated device driver
+ *
+ * Copyright 2019, Red Hat Corp.
+ * Author: Jason Wang 
+ */
+#ifndef _LINUX_VIRTIO_MDEV_H
+#define _LINUX_VIRTIO_MDEV_H
+
+#include 
+#include 
+#include 
+
+#define VIRTIO_MDEV_DEVICE_API_STRING  "virtio-mdev"
+#define VIRTIO_MDEV_F_VERSION_1 0x1
+
+struct virtio_mdev_callback {
+   irqreturn_t (*callback)(void *data);
+   void *private;
+};
+
+/**
+ * struct vfio_mdev_device_ops - Structure to be registered for each
+ * mdev device to register the device for virtio/vhost drivers.
+ *
+ * The device ops that is supported by VIRTIO_MDEV_F_VERSION_1, the
+ * callbacks are mandatory unless explicity mentioned.

If the version of the callbacks is returned by a callback within the
structure defined by the version... isn't that a bit circular?  This
seems redundant to me versus the class id.  The fact that the parent
driver defines the device as MDEV_CLASS_ID_VIRTIO should tell us this
already.  If it was incremented, we'd need an MDEV_CLASS_ID_VIRTIOv2,
which the virtio-mdev bus driver could add to its id table and handle
differently.



My understanding is versions are only allowed to increase monotonically, 
this seems less flexible than features. E.g we have features A, B, C, 
mdev device can choose to support only a subset. E.g when mdev device 
can support dirty page logging, it can add a new feature bit for driver 
to know that it support new device ops. MDEV_CLASS_ID_VIRTIOv2 may only 
be 

Re: [PATCH v2] panfrost: Properly undo pm_runtime_enable when deferring a probe

2019-10-23 Thread Chen-Yu Tsai
On Wed, Oct 23, 2019 at 8:22 PM Tomeu Vizoso  wrote:
>
> When deferring the probe because of a missing regulator, we were calling
> pm_runtime_disable even if pm_runtime_enable wasn't called.
>
> Move the call to pm_runtime_disable to the right place.
>
> Signed-off-by: Tomeu Vizoso 
> Reported-by: Chen-Yu Tsai 
> Cc: Robin Murphy 
> Fixes: f4a3c6a44b35 ("drm/panfrost: Disable PM on probe failure")

Tested-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V5 2/6] modpost: add support for mdev class id

2019-10-23 Thread Jason Wang


On 2019/10/24 上午5:42, Alex Williamson wrote:

On Wed, 23 Oct 2019 21:07:48 +0800
Jason Wang  wrote:


Add support to parse mdev class id table.

Reviewed-by: Parav Pandit 
Signed-off-by: Jason Wang 
---
  drivers/vfio/mdev/vfio_mdev.c |  2 ++
  scripts/mod/devicetable-offsets.c |  3 +++
  scripts/mod/file2alias.c  | 10 ++
  3 files changed, 15 insertions(+)

diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
index 7b24ee9cb8dd..cb701cd646f0 100644
--- a/drivers/vfio/mdev/vfio_mdev.c
+++ b/drivers/vfio/mdev/vfio_mdev.c
@@ -125,6 +125,8 @@ static const struct mdev_class_id id_table[] = {
{ 0 },
  };
  
+MODULE_DEVICE_TABLE(mdev, id_table);

+

Two questions, first we have:

#define MODULE_DEVICE_TABLE(type, name) \
extern typeof(name) __mod_##type##__##name##_device_table   \
   __attribute__ ((unused, alias(__stringify(name

Therefore we're defining __mod_mdev__id_table_device_table with alias
id_table.  When the virtio mdev bus driver is added in 5/6 it uses the
same name value.  I see virtio types all register this way (virtio,
id_table), so I assume there's no conflict, but pci types mostly (not
entirely) seem to use unique names.  Is there a preference to one way
or the other or it simply doesn't matter?



It looks to me that those symbol were local, so it doesn't matter. But 
if you wish I can switch to use unique name.






  static struct mdev_driver vfio_mdev_driver = {
.name   = "vfio_mdev",
.probe  = vfio_mdev_probe,
diff --git a/scripts/mod/devicetable-offsets.c 
b/scripts/mod/devicetable-offsets.c
index 054405b90ba4..6cbb1062488a 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -231,5 +231,8 @@ int main(void)
DEVID(wmi_device_id);
DEVID_FIELD(wmi_device_id, guid_string);
  
+	DEVID(mdev_class_id);

+   DEVID_FIELD(mdev_class_id, id);
+
return 0;
  }
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index c91eba751804..d365dfe7c718 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1335,6 +1335,15 @@ static int do_wmi_entry(const char *filename, void 
*symval, char *alias)
return 1;
  }
  
+/* looks like: "mdev:cN" */

+static int do_mdev_entry(const char *filename, void *symval, char *alias)
+{
+   DEF_FIELD(symval, mdev_class_id, id);
+
+   sprintf(alias, "mdev:c%02X", id);

A lot of entries call add_wildcard() here, should we?  Sorry for the
basic questions, I haven't played in this code.  Thanks,



It's really good question. My understanding is we won't have a module 
that can deal with all kinds of classes like CLASS_ID_ANY. So there's 
probably no need for the wildcard.


Thanks




Alex


+   return 1;
+}
+
  /* Does namelen bytes of name exactly match the symbol? */
  static bool sym_is(const char *name, unsigned namelen, const char *symbol)
  {
@@ -1407,6 +1416,7 @@ static const struct devtable devtable[] = {
{"typec", SIZE_typec_device_id, do_typec_entry},
{"tee", SIZE_tee_client_device_id, do_tee_entry},
{"wmi", SIZE_wmi_device_id, do_wmi_entry},
+   {"mdev", SIZE_mdev_class_id, do_mdev_entry},
  };
  
  /* Create MODULE_ALIAS() statements.


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

Re: [PATCH V5 1/6] mdev: class id support

2019-10-23 Thread Jason Wang


On 2019/10/24 上午5:42, Alex Williamson wrote:

On Wed, 23 Oct 2019 21:07:47 +0800
Jason Wang  wrote:


Mdev bus only supports vfio driver right now, so it doesn't implement
match method. But in the future, we may add drivers other than vfio,
the first driver could be virtio-mdev. This means we need to add
device class id support in bus match method to pair the mdev device
and mdev driver correctly.

So this patch adds id_table to mdev_driver and class_id for mdev
device with the match method for mdev bus.

Signed-off-by: Jason Wang 
---
  .../driver-api/vfio-mediated-device.rst   |  5 +
  drivers/gpu/drm/i915/gvt/kvmgt.c  |  1 +
  drivers/s390/cio/vfio_ccw_ops.c   |  1 +
  drivers/s390/crypto/vfio_ap_ops.c |  1 +
  drivers/vfio/mdev/mdev_core.c | 18 +++
  drivers/vfio/mdev/mdev_driver.c   | 22 +++
  drivers/vfio/mdev/mdev_private.h  |  1 +
  drivers/vfio/mdev/vfio_mdev.c |  6 +
  include/linux/mdev.h  |  8 +++
  include/linux/mod_devicetable.h   |  8 +++
  samples/vfio-mdev/mbochs.c|  1 +
  samples/vfio-mdev/mdpy.c  |  1 +
  samples/vfio-mdev/mtty.c  |  1 +
  13 files changed, 74 insertions(+)

diff --git a/Documentation/driver-api/vfio-mediated-device.rst 
b/Documentation/driver-api/vfio-mediated-device.rst
index 25eb7d5b834b..6709413bee29 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
* @probe: called when new device created
* @remove: called when device removed
* @driver: device driver structure
+  * @id_table: the ids serviced by this driver
*/
   struct mdev_driver {
 const char *name;
 int  (*probe)  (struct device *dev);
 void (*remove) (struct device *dev);
 struct device_driverdriver;
+const struct mdev_class_id *id_table;
   };
  
  A mediated bus driver for mdev should use this structure in the function calls

@@ -170,6 +172,9 @@ that a driver should use to unregister itself with the mdev 
core driver::
  
  	extern void mdev_unregister_device(struct device *dev);
  
+It is also required to specify the class_id in create() callback through::

+
+   int mdev_set_class(struct mdev_device *mdev, u16 id);
  
  Mediated Device Management Interface Through sysfs

  ==
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 343d79c1cb7e..6420f0dbd31b 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -678,6 +678,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct 
mdev_device *mdev)
 dev_name(mdev_dev(mdev)));
ret = 0;
  
+	mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);

  out:
return ret;
  }
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index f0d71ab77c50..cf2c013ae32f 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -129,6 +129,7 @@ static int vfio_ccw_mdev_create(struct kobject *kobj, 
struct mdev_device *mdev)
   private->sch->schid.ssid,
   private->sch->schid.sch_no);
  
+	mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);

return 0;
  }
  
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c

index 5c0f53c6dde7..07c31070afeb 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -343,6 +343,7 @@ static int vfio_ap_mdev_create(struct kobject *kobj, struct 
mdev_device *mdev)
list_add(_mdev->node, _dev->mdev_list);
mutex_unlock(_dev->lock);
  
+	mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);

return 0;
  }
  
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c

index b558d4cfd082..3a9c52d71b4e 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -45,6 +45,16 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data)
  }
  EXPORT_SYMBOL(mdev_set_drvdata);
  
+/* Specify the class for the mdev device, this must be called during

+ * create() callback.
+ */
+void mdev_set_class(struct mdev_device *mdev, u16 id)
+{
+   WARN_ON(mdev->class_id);
+   mdev->class_id = id;
+}
+EXPORT_SYMBOL(mdev_set_class);
+
  struct device *mdev_dev(struct mdev_device *mdev)
  {
return >dev;
@@ -135,6 +145,7 @@ static int mdev_device_remove_cb(struct device *dev, void 
*data)
   * mdev_register_device : Register a device
   * @dev: device structure representing parent device.
   * @ops: Parent device operation structure to be registered.
+ * @id: class id.
   *
   * Add device to list of registered parent 

[pull] amdgpu drm-fixes-5.4

2019-10-23 Thread Alex Deucher
Hi Dave, Daniel,

Fixes for 5.4 for amdgpu.

The following changes since commit 5c1e34b5159ec65bf33e2c1a62fa7158132c10cf:

  Merge tag 'drm-misc-fixes-2019-10-17' of 
git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2019-10-18 06:40:28 
+1000)

are available in the Git repository at:

  git://people.freedesktop.org/~agd5f/linux tags/drm-fixes-5.4-2019-10-23

for you to fetch changes up to ee027828c40faa92a7ef4c2b0641bbb3f4be95d3:

  drm/amdgpu/vce: fix allocation size in enc ring test (2019-10-17 17:12:34 
-0400)


drm-fixes-5.4-2019-10-23:

amdgpu:
- Fix suspend/resume issue related to multi-media engines
- Fix memory leak in user ptr code related to hmm conversion
- Fix possible VM faults when allocating page table memory
- Fix error handling in bo list ioctl


Alex Deucher (4):
  drm/amdgpu/uvd6: fix allocation size in enc ring test (v2)
  drm/amdgpu/uvd7: fix allocation size in enc ring test (v2)
  drm/amdgpu/vcn: fix allocation size in enc ring test
  drm/amdgpu/vce: fix allocation size in enc ring test

Christian König (2):
  drm/amdgpu: fix potential VM faults
  drm/amdgpu: fix error handling in amdgpu_bo_list_create

Philip Yang (1):
  drm/amdgpu: user pages array memory leak fix

 drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c |  7 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  8 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 20 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 35 +++--
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c   | 31 -
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c   | 33 ++-
 8 files changed, 92 insertions(+), 46 deletions(-)
___
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-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109955

--- Comment #120 from bl...@yahoo.com ---
I dont have anything to attach here, but same issue here, ubuntu 19.04, kernel
5.4-rc3, vega64 W/C, Mesa 19.3.0 -- it only seems to occur with DXVK and not
D9VK for some reason.

Example: GW2 (DX9 game) will work perfectly under heavy load in WvW with
massive zergs for hours with no crash, but FFXIV (DX11) will always lock the
entire system up after a time.

That being said, when you force the top clock using

echo manual > /sys/class/drm/card0/device/power_dpm_force_performance_level

and

echo 7 > /sys/class/drm/card0/device/pp_dpm_sclk

FFXIV no longer locks the system at all. It does eat up a good deal more watts
according to my UPS meter though, so resetting to auto is necessary IMHO.

So, it sounds like you guys are on the right track with the whole "power
management" thing being the culprit. Just wanted to add my experience to this.

(and yes, echoing the guy above, the exact same system is stable in windows 10,
so its not a hardware issue).

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

[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #149 from Seba Pe  ---
(In reply to Jaap Buurman from comment #136)
> Has anyone tried AMD's closed source OpenGL driver to see if that one is
> stable?

I've been running AMDGPU-PRO without issues while waiting for a fix for this
(5700XT). OpenGL apps appear to work fine. With Vulkan I've had a crash to
desktop but I haven't tested it that much. No freezes at least.

-- 
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] drm/dp: Add function to parse EDID descriptors for adaptive sync limits

2019-10-23 Thread Manasi Navare
Adaptive Sync is a VESA feature so add a DRM core helper to parse
the EDID's detailed descritors to obtain the adaptive sync monitor range.
Store this info as part fo drm_display_info so it can be used
across all drivers.
This part of the code is stripped out of amdgpu's function
amdgpu_dm_update_freesync_caps() to make it generic and be used
across all DRM drivers

Cc: Ville Syrjälä 
Cc: Harry Wentland 
Cc: Clinton A Taylor 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/drm_edid.c  | 49 +
 include/drm/drm_connector.h | 25 +++
 include/drm/drm_edid.h  |  2 ++
 3 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 474ac04d5600..97dd1200773e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4707,6 +4707,52 @@ static void drm_parse_cea_ext(struct drm_connector 
*connector,
}
 }
 
+void drm_get_adaptive_sync_limits(struct drm_connector *connector,
+ const struct edid *edid)
+{
+   struct drm_display_info *info = >display_info;
+   const struct detailed_timing *timing;
+   const struct detailed_non_pixel *data;
+   const struct detailed_data_monitor_range *range;
+   int i;
+
+   /*
+* Restrict Adaptive Sync only for dp and edp
+*/
+   if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
+   connector->connector_type != DRM_MODE_CONNECTOR_eDP)
+   return;
+
+   if (edid->version <= 1 && !(edid->version == 1 && edid->revision > 1))
+   return;
+
+   for (i = 0; i < 4; i++) {
+   timing  = >detailed_timings[i];
+   data= >data.other_data;
+   range   = >data.range;
+   /*
+* Check if monitor has continuous frequency mode
+*/
+   if (data->type != EDID_DETAIL_MONITOR_RANGE)
+   continue;
+   /*
+* Check for flag range limits only. If flag == 1 then
+* no additional timing information provided.
+* Default GTF, GTF Secondary curve and CVT are not
+* supported
+*/
+   if (range->flags != 1)
+   continue;
+
+   info->adaptive_sync.min_vfreq = range->min_vfreq;
+   info->adaptive_sync.max_vfreq = range->max_vfreq;
+   info->adaptive_sync.pixel_clock_mhz =
+   range->pixel_clock_mhz * 10;
+   break;
+   }
+}
+EXPORT_SYMBOL(drm_get_adaptive_sync_limits);
+
 /* A connector has no EDID information, so we've got no EDID to compute quirks 
from. Reset
  * all of the values which would have been set from EDID
  */
@@ -4728,6 +4774,7 @@ drm_reset_display_info(struct drm_connector *connector)
memset(>hdmi, 0, sizeof(info->hdmi));
 
info->non_desktop = 0;
+   memset(>adaptive_sync, 0, sizeof(info->adaptive_sync));
 }
 
 u32 drm_add_display_info(struct drm_connector *connector, const struct edid 
*edid)
@@ -4743,6 +4790,8 @@ u32 drm_add_display_info(struct drm_connector *connector, 
const struct edid *edi
 
info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
 
+   drm_get_adaptive_sync_limits(connector, edid);
+
DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
 
if (edid->revision < 3)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 5f8c3389d46f..a27a84270d8d 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -254,6 +254,26 @@ enum drm_panel_orientation {
DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
 };
 
+/**
+ * struct drm_adaptive_sync_info - Panel's Adaptive Sync capabilities for
+ * _display_info
+ *
+ * This struct is used to store a Panel's Adaptive Sync capabilities
+ * as parsed from EDID's detailed monitor range descriptor block.
+ *
+ * @min_vfreq: This is the min supported refresh rate in Hz from
+ * EDID's detailed monitor range.
+ * @max_vfreq: This is the max supported refresh rate in Hz from
+ * EDID's detailed monitor range
+ * @pixel_clock_mhz: This is the dotclock in MHz from
+ *   EDID's detailed monitor range
+ */
+struct drm_adaptive_sync_info {
+   int min_vfreq;
+   int max_vfreq;
+   int pixel_clock_mhz;
+};
+
 /*
  * This is a consolidated colorimetry list supported by HDMI and
  * DP protocol standard. The respective connectors will register
@@ -465,6 +485,11 @@ struct drm_display_info {
 * @non_desktop: Non desktop display (HMD).
 */
bool non_desktop;
+
+   /**
+* @adaptive_sync: Adaptive Sync capabilities of the DP/eDP sink
+*/
+   struct drm_adaptive_sync_info adaptive_sync;
 };
 
 int drm_display_info_set_bus_formats(struct drm_display_info *info,
diff --git a/include/drm/drm_edid.h 

Re: [PATCH 5/6] drm/mediatek: Convert to use CMA helpers

2019-10-23 Thread Rob Herring
On Wed, Oct 23, 2019 at 4:06 PM CK Hu  wrote:
>
> Hi, Rob:
>
> On Wed, 2019-10-23 at 12:42 -0500, Rob Herring wrote:
> > On Tue, Oct 22, 2019 at 12:07 PM Matthias Brugger
> >  wrote:
> > >
> > > Hi Rob,
> > >
> > > On 21/10/2019 23:45, Rob Herring wrote:
> > > > The only reason the Mediatek driver doesn't use the CMA helpers is it
> > > > sets DMA_ATTR_NO_KERNEL_MAPPING and does a vmap() on demand. Using
> > > > vmap() is not even guaranteed to work as DMA buffers may not have a
> > > > struct page. Now that the CMA helpers support setting
> > > > DMA_ATTR_NO_KERNEL_MAPPING as needed or not, convert Mediatek driver to
> > > > use CMA helpers.
> > > >
> > > > Cc: CK Hu 
> > > > Cc: Philipp Zabel 
> > > > Cc: David Airlie 
> > > > Cc: Daniel Vetter 
> > > > Cc: Matthias Brugger 
> > > > Cc: linux-arm-ker...@lists.infradead.org
> > > > Cc: linux-media...@lists.infradead.org
> > > > Signed-off-by: Rob Herring 
> > > > ---
> > >
> > > I tested this on my Chromebook with some patches on top of v5.4-rc1 [1], 
> > > which
> > > work. If I add your patches on top of that, the system does not boot up.
> > > Unfortunately I don't have a serial console, so I wasn't able to see if 
> > > there is
> > > any error message.
> >
> > Thanks for testing. I'm based on drm-misc-next, but don't see anything
> > obvious there that would matter. There are some mmap changes, but I
> > think they shouldn't matter.
> >
> > Did you have fbcon enabled? That may give more clues about where the 
> > problem is.
>
> There are priv->dma_dev for dma device, but it is not drm device. In
> mt8173.dtsi [1], there are mmsys device and ovl device, mmsys device is
> drm device and ovl device is mmsys's sub device which provide dma
> function, so ovl is the priv->dma_dev. I think your patch directly use
> drm device for dma operation and this would cause dma function fail.
> Please use priv->dma_dev for dma operation.

Right, thanks for catching that. Either we'll need to make CMA GEM
object have a struct device ptr or adjust the drm_device.dev to have
the necessary DMA setup.

One question though, why do you use CMA when you have an IOMMU? That's
not optimal as CMA size may be limited. Or you don't always have an
IOMMU?

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

[PATCH] drm/prime: Fix mmap fake offset for drm_gem_object_funcs.mmap

2019-10-23 Thread Rob Herring
Commit c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs")
introduced a GEM object mmap() hook which is expected to subtract the
fake offset from vm_pgoff. However, for mmap() on dmabufs, there is not
a fake offset. To fix this, we need to add the fake offset just like the
driver->fops->mmap() code path.

Fixes: c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs")
Cc: Gerd Hoffmann 
Cc: Daniel Vetter 
Signed-off-by: Rob Herring 
---
I ran into this while working on converting vgem to shmem helpers and
the IGT vgem_basic dmabuf-mmap test failed. This fixes shmem, but I
have checked any other users of the new mmap hook.

Rob

 drivers/gpu/drm/drm_prime.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 0814211b0f3f..5d06690a2e9d 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -713,6 +713,8 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct 
vm_area_struct *vma)
struct file *fil;
int ret;
 
+   vma->vm_pgoff += drm_vma_node_start(>vma_node);
+
if (obj->funcs && obj->funcs->mmap) {
ret = obj->funcs->mmap(obj, vma);
if (ret)
@@ -737,8 +739,6 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct 
vm_area_struct *vma)
if (ret)
goto out;
 
-   vma->vm_pgoff += drm_vma_node_start(>vma_node);
-
ret = obj->dev->driver->fops->mmap(fil, vma);
 
drm_vma_node_revoke(>vma_node, priv);
-- 
2.20.1

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

Re: [PATCH V5 4/6] mdev: introduce virtio device and its device ops

2019-10-23 Thread Alex Williamson
On Wed, 23 Oct 2019 21:07:50 +0800
Jason Wang  wrote:

> This patch implements basic support for mdev driver that supports
> virtio transport for kernel virtio driver.
> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/vfio/mdev/mdev_core.c|  20 
>  drivers/vfio/mdev/mdev_private.h |   2 +
>  include/linux/mdev.h |   6 ++
>  include/linux/virtio_mdev_ops.h  | 159 +++
>  4 files changed, 187 insertions(+)
>  create mode 100644 include/linux/virtio_mdev_ops.h
> 
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index 555bd61d8c38..9b00c3513120 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -76,6 +76,26 @@ const struct vfio_mdev_device_ops 
> *mdev_get_vfio_ops(struct mdev_device *mdev)
>  }
>  EXPORT_SYMBOL(mdev_get_vfio_ops);
>  
> +/* Specify the virtio device ops for the mdev device, this
> + * must be called during create() callback for virtio mdev device.
> + */
> +void mdev_set_virtio_ops(struct mdev_device *mdev,
> +  const struct virtio_mdev_device_ops *virtio_ops)
> +{
> + mdev_set_class(mdev, MDEV_CLASS_ID_VIRTIO);
> + mdev->virtio_ops = virtio_ops;
> +}
> +EXPORT_SYMBOL(mdev_set_virtio_ops);
> +
> +/* Get the virtio device ops for the mdev device. */
> +const struct virtio_mdev_device_ops *
> +mdev_get_virtio_ops(struct mdev_device *mdev)
> +{
> + WARN_ON(mdev->class_id != MDEV_CLASS_ID_VIRTIO);
> + return mdev->virtio_ops;
> +}
> +EXPORT_SYMBOL(mdev_get_virtio_ops);
> +
>  struct device *mdev_dev(struct mdev_device *mdev)
>  {
>   return >dev;
> diff --git a/drivers/vfio/mdev/mdev_private.h 
> b/drivers/vfio/mdev/mdev_private.h
> index 0770410ded2a..7b47890c34e7 100644
> --- a/drivers/vfio/mdev/mdev_private.h
> +++ b/drivers/vfio/mdev/mdev_private.h
> @@ -11,6 +11,7 @@
>  #define MDEV_PRIVATE_H
>  
>  #include 
> +#include 
>  
>  int  mdev_bus_register(void);
>  void mdev_bus_unregister(void);
> @@ -38,6 +39,7 @@ struct mdev_device {
>   u16 class_id;
>   union {
>   const struct vfio_mdev_device_ops *vfio_ops;
> + const struct virtio_mdev_device_ops *virtio_ops;
>   };
>  };
>  
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index 4625f1a11014..9b69b0bbebfd 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -17,6 +17,7 @@
>  
>  struct mdev_device;
>  struct vfio_mdev_device_ops;
> +struct virtio_mdev_device_ops;
>  
>  /*
>   * Called by the parent device driver to set the device which represents
> @@ -112,6 +113,10 @@ void mdev_set_class(struct mdev_device *mdev, u16 id);
>  void mdev_set_vfio_ops(struct mdev_device *mdev,
>  const struct vfio_mdev_device_ops *vfio_ops);
>  const struct vfio_mdev_device_ops *mdev_get_vfio_ops(struct mdev_device 
> *mdev);
> +void mdev_set_virtio_ops(struct mdev_device *mdev,
> +  const struct virtio_mdev_device_ops *virtio_ops);
> +const struct virtio_mdev_device_ops *
> +mdev_get_virtio_ops(struct mdev_device *mdev);
>  
>  extern struct bus_type mdev_bus_type;
>  
> @@ -127,6 +132,7 @@ struct mdev_device *mdev_from_dev(struct device *dev);
>  
>  enum {
>   MDEV_CLASS_ID_VFIO = 1,
> + MDEV_CLASS_ID_VIRTIO = 2,
>   /* New entries must be added here */
>  };
>  
> diff --git a/include/linux/virtio_mdev_ops.h b/include/linux/virtio_mdev_ops.h
> new file mode 100644
> index ..d417b41f2845
> --- /dev/null
> +++ b/include/linux/virtio_mdev_ops.h
> @@ -0,0 +1,159 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Virtio mediated device driver
> + *
> + * Copyright 2019, Red Hat Corp.
> + * Author: Jason Wang 
> + */
> +#ifndef _LINUX_VIRTIO_MDEV_H
> +#define _LINUX_VIRTIO_MDEV_H
> +
> +#include 
> +#include 
> +#include 
> +
> +#define VIRTIO_MDEV_DEVICE_API_STRING"virtio-mdev"
> +#define VIRTIO_MDEV_F_VERSION_1 0x1
> +
> +struct virtio_mdev_callback {
> + irqreturn_t (*callback)(void *data);
> + void *private;
> +};
> +
> +/**
> + * struct vfio_mdev_device_ops - Structure to be registered for each
> + * mdev device to register the device for virtio/vhost drivers.
> + *
> + * The device ops that is supported by VIRTIO_MDEV_F_VERSION_1, the
> + * callbacks are mandatory unless explicity mentioned.

If the version of the callbacks is returned by a callback within the
structure defined by the version... isn't that a bit circular?  This
seems redundant to me versus the class id.  The fact that the parent
driver defines the device as MDEV_CLASS_ID_VIRTIO should tell us this
already.  If it was incremented, we'd need an MDEV_CLASS_ID_VIRTIOv2,
which the virtio-mdev bus driver could add to its id table and handle
differently.

> + *
> + * @set_vq_address:  Set the address of virtqueue
> + *   @mdev: mediated device
> + *   @idx: virtqueue index
> + *   @desc_area: 

Re: [PATCH V5 2/6] modpost: add support for mdev class id

2019-10-23 Thread Alex Williamson
On Wed, 23 Oct 2019 21:07:48 +0800
Jason Wang  wrote:

> Add support to parse mdev class id table.
> 
> Reviewed-by: Parav Pandit 
> Signed-off-by: Jason Wang 
> ---
>  drivers/vfio/mdev/vfio_mdev.c |  2 ++
>  scripts/mod/devicetable-offsets.c |  3 +++
>  scripts/mod/file2alias.c  | 10 ++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
> index 7b24ee9cb8dd..cb701cd646f0 100644
> --- a/drivers/vfio/mdev/vfio_mdev.c
> +++ b/drivers/vfio/mdev/vfio_mdev.c
> @@ -125,6 +125,8 @@ static const struct mdev_class_id id_table[] = {
>   { 0 },
>  };
>  
> +MODULE_DEVICE_TABLE(mdev, id_table);
> +

Two questions, first we have:

#define MODULE_DEVICE_TABLE(type, name) \
extern typeof(name) __mod_##type##__##name##_device_table   \
  __attribute__ ((unused, alias(__stringify(name

Therefore we're defining __mod_mdev__id_table_device_table with alias
id_table.  When the virtio mdev bus driver is added in 5/6 it uses the
same name value.  I see virtio types all register this way (virtio,
id_table), so I assume there's no conflict, but pci types mostly (not
entirely) seem to use unique names.  Is there a preference to one way
or the other or it simply doesn't matter?

>  static struct mdev_driver vfio_mdev_driver = {
>   .name   = "vfio_mdev",
>   .probe  = vfio_mdev_probe,
> diff --git a/scripts/mod/devicetable-offsets.c 
> b/scripts/mod/devicetable-offsets.c
> index 054405b90ba4..6cbb1062488a 100644
> --- a/scripts/mod/devicetable-offsets.c
> +++ b/scripts/mod/devicetable-offsets.c
> @@ -231,5 +231,8 @@ int main(void)
>   DEVID(wmi_device_id);
>   DEVID_FIELD(wmi_device_id, guid_string);
>  
> + DEVID(mdev_class_id);
> + DEVID_FIELD(mdev_class_id, id);
> +
>   return 0;
>  }
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index c91eba751804..d365dfe7c718 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1335,6 +1335,15 @@ static int do_wmi_entry(const char *filename, void 
> *symval, char *alias)
>   return 1;
>  }
>  
> +/* looks like: "mdev:cN" */
> +static int do_mdev_entry(const char *filename, void *symval, char *alias)
> +{
> + DEF_FIELD(symval, mdev_class_id, id);
> +
> + sprintf(alias, "mdev:c%02X", id);

A lot of entries call add_wildcard() here, should we?  Sorry for the
basic questions, I haven't played in this code.  Thanks,

Alex

> + return 1;
> +}
> +
>  /* Does namelen bytes of name exactly match the symbol? */
>  static bool sym_is(const char *name, unsigned namelen, const char *symbol)
>  {
> @@ -1407,6 +1416,7 @@ static const struct devtable devtable[] = {
>   {"typec", SIZE_typec_device_id, do_typec_entry},
>   {"tee", SIZE_tee_client_device_id, do_tee_entry},
>   {"wmi", SIZE_wmi_device_id, do_wmi_entry},
> + {"mdev", SIZE_mdev_class_id, do_mdev_entry},
>  };
>  
>  /* Create MODULE_ALIAS() statements.

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

Re: [PATCH V5 1/6] mdev: class id support

2019-10-23 Thread Alex Williamson
On Wed, 23 Oct 2019 21:07:47 +0800
Jason Wang  wrote:

> Mdev bus only supports vfio driver right now, so it doesn't implement
> match method. But in the future, we may add drivers other than vfio,
> the first driver could be virtio-mdev. This means we need to add
> device class id support in bus match method to pair the mdev device
> and mdev driver correctly.
> 
> So this patch adds id_table to mdev_driver and class_id for mdev
> device with the match method for mdev bus.
> 
> Signed-off-by: Jason Wang 
> ---
>  .../driver-api/vfio-mediated-device.rst   |  5 +
>  drivers/gpu/drm/i915/gvt/kvmgt.c  |  1 +
>  drivers/s390/cio/vfio_ccw_ops.c   |  1 +
>  drivers/s390/crypto/vfio_ap_ops.c |  1 +
>  drivers/vfio/mdev/mdev_core.c | 18 +++
>  drivers/vfio/mdev/mdev_driver.c   | 22 +++
>  drivers/vfio/mdev/mdev_private.h  |  1 +
>  drivers/vfio/mdev/vfio_mdev.c |  6 +
>  include/linux/mdev.h  |  8 +++
>  include/linux/mod_devicetable.h   |  8 +++
>  samples/vfio-mdev/mbochs.c|  1 +
>  samples/vfio-mdev/mdpy.c  |  1 +
>  samples/vfio-mdev/mtty.c  |  1 +
>  13 files changed, 74 insertions(+)
> 
> diff --git a/Documentation/driver-api/vfio-mediated-device.rst 
> b/Documentation/driver-api/vfio-mediated-device.rst
> index 25eb7d5b834b..6709413bee29 100644
> --- a/Documentation/driver-api/vfio-mediated-device.rst
> +++ b/Documentation/driver-api/vfio-mediated-device.rst
> @@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
>* @probe: called when new device created
>* @remove: called when device removed
>* @driver: device driver structure
> +  * @id_table: the ids serviced by this driver
>*/
>   struct mdev_driver {
>const char *name;
>int  (*probe)  (struct device *dev);
>void (*remove) (struct device *dev);
>struct device_driverdriver;
> +  const struct mdev_class_id *id_table;
>   };
>  
>  A mediated bus driver for mdev should use this structure in the function 
> calls
> @@ -170,6 +172,9 @@ that a driver should use to unregister itself with the 
> mdev core driver::
>  
>   extern void mdev_unregister_device(struct device *dev);
>  
> +It is also required to specify the class_id in create() callback through::
> +
> + int mdev_set_class(struct mdev_device *mdev, u16 id);
>  
>  Mediated Device Management Interface Through sysfs
>  ==
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 343d79c1cb7e..6420f0dbd31b 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -678,6 +678,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct 
> mdev_device *mdev)
>dev_name(mdev_dev(mdev)));
>   ret = 0;
>  
> + mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
>  out:
>   return ret;
>  }
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> index f0d71ab77c50..cf2c013ae32f 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -129,6 +129,7 @@ static int vfio_ccw_mdev_create(struct kobject *kobj, 
> struct mdev_device *mdev)
>  private->sch->schid.ssid,
>  private->sch->schid.sch_no);
>  
> + mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
>   return 0;
>  }
>  
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
> b/drivers/s390/crypto/vfio_ap_ops.c
> index 5c0f53c6dde7..07c31070afeb 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -343,6 +343,7 @@ static int vfio_ap_mdev_create(struct kobject *kobj, 
> struct mdev_device *mdev)
>   list_add(_mdev->node, _dev->mdev_list);
>   mutex_unlock(_dev->lock);
>  
> + mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
>   return 0;
>  }
>  
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index b558d4cfd082..3a9c52d71b4e 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -45,6 +45,16 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data)
>  }
>  EXPORT_SYMBOL(mdev_set_drvdata);
>  
> +/* Specify the class for the mdev device, this must be called during
> + * create() callback.
> + */
> +void mdev_set_class(struct mdev_device *mdev, u16 id)
> +{
> + WARN_ON(mdev->class_id);
> + mdev->class_id = id;
> +}
> +EXPORT_SYMBOL(mdev_set_class);
> +
>  struct device *mdev_dev(struct mdev_device *mdev)
>  {
>   return >dev;
> @@ -135,6 +145,7 @@ static int mdev_device_remove_cb(struct device *dev, void 
> *data)
>   * mdev_register_device : Register a device
>   * @dev: device structure representing parent device.
>   * 

Re: [PATCH 5/6] drm/mediatek: Convert to use CMA helpers

2019-10-23 Thread CK Hu
Hi, Rob:

On Wed, 2019-10-23 at 12:42 -0500, Rob Herring wrote:
> On Tue, Oct 22, 2019 at 12:07 PM Matthias Brugger
>  wrote:
> >
> > Hi Rob,
> >
> > On 21/10/2019 23:45, Rob Herring wrote:
> > > The only reason the Mediatek driver doesn't use the CMA helpers is it
> > > sets DMA_ATTR_NO_KERNEL_MAPPING and does a vmap() on demand. Using
> > > vmap() is not even guaranteed to work as DMA buffers may not have a
> > > struct page. Now that the CMA helpers support setting
> > > DMA_ATTR_NO_KERNEL_MAPPING as needed or not, convert Mediatek driver to
> > > use CMA helpers.
> > >
> > > Cc: CK Hu 
> > > Cc: Philipp Zabel 
> > > Cc: David Airlie 
> > > Cc: Daniel Vetter 
> > > Cc: Matthias Brugger 
> > > Cc: linux-arm-ker...@lists.infradead.org
> > > Cc: linux-media...@lists.infradead.org
> > > Signed-off-by: Rob Herring 
> > > ---
> >
> > I tested this on my Chromebook with some patches on top of v5.4-rc1 [1], 
> > which
> > work. If I add your patches on top of that, the system does not boot up.
> > Unfortunately I don't have a serial console, so I wasn't able to see if 
> > there is
> > any error message.
> 
> Thanks for testing. I'm based on drm-misc-next, but don't see anything
> obvious there that would matter. There are some mmap changes, but I
> think they shouldn't matter.
> 
> Did you have fbcon enabled? That may give more clues about where the problem 
> is.

There are priv->dma_dev for dma device, but it is not drm device. In
mt8173.dtsi [1], there are mmsys device and ovl device, mmsys device is
drm device and ovl device is mmsys's sub device which provide dma
function, so ovl is the priv->dma_dev. I think your patch directly use
drm device for dma operation and this would cause dma function fail.
Please use priv->dma_dev for dma operation.

Regards,
CK

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.4-rc4
> 
> Rob


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

[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #148 from Shmerl  ---
(In reply to Daniel Suarez from comment #147)
> 
> You shouldn't be using a 5700 XT in a system that demands 100% uptime, I
> have had mine randomly hang in the night without Firefox even being open,
> only qbittorrent and discord

For the reference, common UI toolkits (GTK and Qt) use OpenGL rendering too.

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #147 from Daniel Suarez  ---
(In reply to Jaap Buurman from comment #144)
> I need as close as 100% uptime on this machine as possible, so I don't
> really have the time to add applications over time until the problem is
> fixed. I need stability now. So a systemwide setting is fine for me, even if
> it might result in big performance losses. I'll wait until a proper fix is
> found.
> 
> Do you happen to know whether it will require two lines to set both debug
> options, or does the environment variable expect the values to be
> comma-separated?

You shouldn't be using a 5700 XT in a system that demands 100% uptime, I have
had mine randomly hang in the night without Firefox even being open, only
qbittorrent and discord

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #146 from Shmerl  ---
You can also use Your $HOME/.profile for setting session wide variables.

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #145 from Shmerl  ---
According to

man environment

The /etc/environment file specifies the environment variables to be set. The
file must consist of simple NAME=VALUE pairs on separate lines.

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #144 from Jaap Buurman  ---
I need as close as 100% uptime on this machine as possible, so I don't really
have the time to add applications over time until the problem is fixed. I need
stability now. So a systemwide setting is fine for me, even if it might result
in big performance losses. I'll wait until a proper fix is found.

Do you happen to know whether it will require two lines to set both debug
options, or does the environment variable expect the values to be
comma-separated?

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #143 from Shmerl  ---
(In reply to Jaap Buurman from comment #142)
> How can I set both AMD_DEBUG=nongg and AMD_DEBUG=nodma in the
> /etc/environment file? Do they need to be on two separate lines, or will the
> second line simply overwrite the first one by setting the same environment
> variable? Do they need to be comma separated maybe?

It's probably better to avoid a wide setting like that. If you know some
applications that hangs (like Firefox or specific game), just set that when
launching it (you can for example add it to .desktop file or some start
script).

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #142 from Jaap Buurman  ---
How can I set both AMD_DEBUG=nongg and AMD_DEBUG=nodma in the /etc/environment
file? Do they need to be on two separate lines, or will the second line simply
overwrite the first one by setting the same environment variable? Do they need
to be comma separated maybe?

-- 
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 hmm 00/15] Consolidate the mmu notifier interval_tree and locking

2019-10-23 Thread Jerome Glisse
On Mon, Oct 21, 2019 at 07:06:00PM +, Jason Gunthorpe wrote:
> On Mon, Oct 21, 2019 at 02:40:41PM -0400, Jerome Glisse wrote:
> > On Tue, Oct 15, 2019 at 03:12:27PM -0300, Jason Gunthorpe wrote:
> > > From: Jason Gunthorpe 
> > > 
> > > 8 of the mmu_notifier using drivers (i915_gem, radeon_mn, umem_odp, hfi1,
> > > scif_dma, vhost, gntdev, hmm) drivers are using a common pattern where
> > > they only use invalidate_range_start/end and immediately check the
> > > invalidating range against some driver data structure to tell if the
> > > driver is interested. Half of them use an interval_tree, the others are
> > > simple linear search lists.
> > > 
> > > Of the ones I checked they largely seem to have various kinds of races,
> > > bugs and poor implementation. This is a result of the complexity in how
> > > the notifier interacts with get_user_pages(). It is extremely difficult to
> > > use it correctly.
> > > 
> > > Consolidate all of this code together into the core mmu_notifier and
> > > provide a locking scheme similar to hmm_mirror that allows the user to
> > > safely use get_user_pages() and reliably know if the page list still
> > > matches the mm.
> > > 
> > > This new arrangment plays nicely with the !blockable mode for
> > > OOM. Scanning the interval tree is done such that the intersection test
> > > will always succeed, and since there is no invalidate_range_end exposed to
> > > drivers the scheme safely allows multiple drivers to be subscribed.
> > > 
> > > Four places are converted as an example of how the new API is used.
> > > Four are left for future patches:
> > >  - i915_gem has complex locking around destruction of a registration,
> > >needs more study
> > >  - hfi1 (2nd user) needs access to the rbtree
> > >  - scif_dma has a complicated logic flow
> > >  - vhost's mmu notifiers are already being rewritten
> > > 
> > > This is still being tested, but I figured to send it to start getting help
> > > from the xen, amd and hfi drivers which I cannot test here.
> > 
> > It might be a good oportunity to also switch those users to
> > hmm_range_fault() instead of GUP as GUP is pointless for those
> > users. In fact the GUP is an impediment to normal mm operations.
> 
> I think vhost can use hmm_range_fault
> 
> hfi1 does actually need to have the page pin, it doesn't fence DMA
> during invalidate.
> 
> i915_gem feels alot like amdgpu, so probably it would benefit
> 
> No idea about scif_dma
> 
> > I will test on nouveau.
> 
> Thanks, hopefully it still works, I think Ralph was able to do some
> basic checks. But it is a pretty complicated series, I probably made
> some mistakes.

So it seems to work ok with nouveau, will let tests run in loop thought
there are not very advance test.

> 
> FWIW, I know that nouveau gets a lockdep splat now from Daniel
> Vetter's recent changes, it tries to do GFP_KERENEL allocations under
> a lock also held by the invalidate_range_start path.

I have not seen any splat so far, is it throught some new kernel config ?

Cheers,
Jérôme

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

Re: [PATCH] drm/syncobj: extend syncobj query ability v3

2019-10-23 Thread Sean Paul
On Tue, Jul 30, 2019 at 9:22 AM Chunming Zhou  wrote:
>
>
> 在 2019/7/30 21:17, Koenig, Christian 写道:
> > Am 30.07.19 um 15:02 schrieb Chunming Zhou:
> >> user space needs a flexiable query ability.
> >> So that umd can get last signaled or submitted point.
> >> v2:
> >> add sanitizer checking.
> >> v3:
> >> rebase
> >>
> >> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> >> Signed-off-by: Chunming Zhou 
> >> Cc: Lionel Landwerlin 
> >> Cc: Christian König 
> >> Reviewed-by: Lionel Landwerlin 
> > Reviewed-by: Christian König 
> >
> > Lionel is the Intel code using this already public? Or David any chance
> > that we can get a public amdvlk release using this?
>
> In latest public amdvlk, We should be able to see how timeline syncobj
> is used in it.
>

I couldn't find this anywhere, could you please provide a link?

Sean

>
> -David
>
> >
> > Christian.
> >
> >> ---
> >>drivers/gpu/drm/drm_syncobj.c | 37 +--
> >>include/uapi/drm/drm.h|  3 ++-
> >>2 files changed, 24 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> >> index cecff2e447b1..d4432f1513ac 100644
> >> --- a/drivers/gpu/drm/drm_syncobj.c
> >> +++ b/drivers/gpu/drm/drm_syncobj.c
> >> @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
> >> *dev, void *data,
> >>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> >>  return -EOPNOTSUPP;
> >>
> >> -if (args->pad != 0)
> >> +if (args->flags != 0)
> >>  return -EINVAL;
> >>
> >>  if (args->count_handles == 0)
> >> @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
> >> void *data,
> >>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> >>  return -EOPNOTSUPP;
> >>
> >> -if (args->pad != 0)
> >> +if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
> >>  return -EINVAL;
> >>
> >>  if (args->count_handles == 0)
> >> @@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device 
> >> *dev, void *data,
> >>  fence = drm_syncobj_fence_get(syncobjs[i]);
> >>  chain = to_dma_fence_chain(fence);
> >>  if (chain) {
> >> -struct dma_fence *iter, *last_signaled = NULL;
> >> -
> >> -dma_fence_chain_for_each(iter, fence) {
> >> -if (iter->context != fence->context) {
> >> -dma_fence_put(iter);
> >> -/* It is most likely that timeline has
> >> - * unorder points. */
> >> -break;
> >> +struct dma_fence *iter, *last_signaled =
> >> +dma_fence_get(fence);
> >> +
> >> +if (args->flags &
> >> +DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> >> +point = fence->seqno;
> >> +} else {
> >> +dma_fence_chain_for_each(iter, fence) {
> >> +if (iter->context != fence->context) {
> >> +dma_fence_put(iter);
> >> +/* It is most likely that 
> >> timeline has
> >> +* unorder points. */
> >> +break;
> >> +}
> >> +dma_fence_put(last_signaled);
> >> +last_signaled = dma_fence_get(iter);
> >>  }
> >> -dma_fence_put(last_signaled);
> >> -last_signaled = dma_fence_get(iter);
> >> +point = dma_fence_is_signaled(last_signaled) ?
> >> +last_signaled->seqno :
> >> +
> >> to_dma_fence_chain(last_signaled)->prev_seqno;
> >>  }
> >> -point = dma_fence_is_signaled(last_signaled) ?
> >> -last_signaled->seqno :
> >> -to_dma_fence_chain(last_signaled)->prev_seqno;
> >>  dma_fence_put(last_signaled);
> >>  } else {
> >>  point = 0;
> >>  }
> >> +dma_fence_put(fence);
> >>  ret = copy_to_user([i], , sizeof(uint64_t));
> >>  ret = ret ? -EFAULT : 0;
> >>  if (ret)
> >> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> >> index 661d73f9a919..fd987ce24d9f 100644
> >> --- a/include/uapi/drm/drm.h
> >> +++ b/include/uapi/drm/drm.h
> >> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
> >>  __u32 pad;
> >>};
> >>
> >> +#define 

[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #141 from Pierre-Eric Pelloux-Prayer 
 ---
Thanks for the quake 2 trace, I could reproduce the same hang here.

If anyone has a reliable way to trigger the issue, the most helpful thing to do
for now is an apitrace capture.

The umr log were helpful (thanks!) but I don't need more of them at the moment.

I don't think radv uses SDMA at all, so they cannot be affected by this issue. 
For radeonsi the AMD_DEBUG=nodma environment variable is a workaround until we
figure out a proper fix.

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #140 from Shmerl  ---
For individual games, I recommend opening separate bugs for each title.

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #139 from Daniel Suarez  ---
I get instant hangs when playing Space Engineers, the moment I load into a
world it completely hands my system, cannot even enter TTY. 

Tested with Manjaro and Mesa-git along with all the other packages recommended
in https://wiki.archlinux.org/index.php/Navi_10

-- 
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] drm/mediatek: Add RGB[A] variants to published plane formats

2019-10-23 Thread Sean Paul
From: Sean Paul 

These formats are handled in the rdma code, but for some reason they're
not published as supported formats for the planes. So add them to the
list.

Cc: Nicolas Boichat 
Cc: Daniele Castagna 
Cc: Miguel Casas 
Tested-by: Miguel Casas 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/mediatek/mtk_drm_plane.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c 
b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 584a9ecadce6..49d59470cc11 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -20,6 +20,12 @@
 static const u32 formats[] = {
DRM_FORMAT_XRGB,
DRM_FORMAT_ARGB,
+   DRM_FORMAT_BGRX,
+   DRM_FORMAT_BGRA,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_RGB888,
+   DRM_FORMAT_BGR888,
DRM_FORMAT_RGB565,
DRM_FORMAT_UYVY,
DRM_FORMAT_YUYV,
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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

[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #138 from Shmerl  ---
(In reply to Alexandr Kára from comment #137)
> It hangs reproducibly with RADV (Vulkan) as well. As an example, many people
> report crashes with Return of the Tomb Raider.

It could be a bug in radv as well related to Navi hazards.

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #137 from Alexandr Kára  ---
It hangs reproducibly with RADV (Vulkan) as well. As an example, many people
report crashes with Return of the Tomb Raider.

-- 
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/v3d: Fix memory leak in v3d_submit_cl_ioctl

2019-10-23 Thread Daniel Vetter
On Wed, Oct 23, 2019 at 11:16:01AM +0200, Daniel Vetter wrote:
> On Tue, Oct 22, 2019 at 10:53:57PM -0500, Navid Emamdoost wrote:
> > On Tue, Oct 22, 2019 at 4:36 AM Daniel Vetter  wrote:
> > >
> > > On Mon, Oct 21, 2019 at 01:52:49PM -0500, Navid Emamdoost wrote:
> > > > In the impelementation of v3d_submit_cl_ioctl() there are two memory
> > > > leaks. One is when allocation for bin fails, and the other is when bin
> > > > initialization fails. If kcalloc fails to allocate memory for bin then
> > > > render->base should be put. Also, if v3d_job_init() fails to initialize
> > > > bin->base then allocated memory for bin should be released.
> > > >
> > > > Fixes: a783a09ee76d ("drm/v3d: Refactor job management.")
> > > > Signed-off-by: Navid Emamdoost 
> > > > ---
> > > >  drivers/gpu/drm/v3d/v3d_gem.c | 5 -
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/v3d/v3d_gem.c 
> > > > b/drivers/gpu/drm/v3d/v3d_gem.c
> > > > index 5d80507b539b..19c092d75266 100644
> > > > --- a/drivers/gpu/drm/v3d/v3d_gem.c
> > > > +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> > > > @@ -557,13 +557,16 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void 
> > > > *data,
> > > >
> > > >   if (args->bcl_start != args->bcl_end) {
> > > >   bin = kcalloc(1, sizeof(*bin), GFP_KERNEL);
> > > > - if (!bin)
> > > > + if (!bin) {
> > > > + v3d_job_put(>base);
> > >
> > > The job isn't initialized yet, this doesn't work.
> > Do you mean we have to release render via kfree() here?
> > 
> > >
> > > >   return -ENOMEM;
> > > > + }
> > > >
> > > >   ret = v3d_job_init(v3d, file_priv, >base,
> > > >  v3d_job_free, args->in_sync_bcl);
> > > >   if (ret) {
> > > >   v3d_job_put(>base);
> > >
> > > v3d_job_put will call kfree, if you chase the callchain long enough (in
> > > v3d_job_free). So no bug here, this would lead to a double kfree and
> > > crash.
> > Yes, v3d_job_put() takes care of render,
> > 
> > > -Daniel
> > >
> > > > + kfree(bin);
> > but how about leaking bin?
> 
> Sorry, I totally missed that this is bin, no render. Patch looks correct
> to me.
> 
> Reviewed-by: Daniel Vetter 

Double-checked with Eric and applied to drm-misc-fixes.
-Daniel

> 
> > 
> > > >   return ret;
> > > >   }
> > > >
> > > > --
> > > > 2.17.1
> > > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> > 
> > 
> > 
> > -- 
> > Navid.
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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


[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #136 from Jaap Buurman  ---
Has anyone tried AMD's closed source OpenGL driver to see if that one is
stable?

-- 
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 1/2] drm/nouveau: Move the declaration of struct nouveau_conn_atom up a bit

2019-10-23 Thread Hans de Goede
Place the declaration of struct nouveau_conn_atom above that of
struct nouveau_connector. This commit makes no changes to the moved
block what so ever, it just moves it up a bit.

This is a preparation patch to fix some issues with connector handling
on pre nv50 displays (which do not use atomic modesetting).

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/nouveau/nouveau_connector.h | 110 ++--
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h 
b/drivers/gpu/drm/nouveau/nouveau_connector.h
index f43a8d63aef8..de9588420884 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -29,6 +29,7 @@
 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -44,6 +45,60 @@ struct dcb_output;
 struct nouveau_backlight;
 #endif
 
+#define nouveau_conn_atom(p)   
\
+   container_of((p), struct nouveau_conn_atom, state)
+
+struct nouveau_conn_atom {
+   struct drm_connector_state state;
+
+   struct {
+   /* The enum values specifically defined here match nv50/gf119
+* hw values, and the code relies on this.
+*/
+   enum {
+   DITHERING_MODE_OFF = 0x00,
+   DITHERING_MODE_ON = 0x01,
+   DITHERING_MODE_DYNAMIC2X2 = 0x10 | DITHERING_MODE_ON,
+   DITHERING_MODE_STATIC2X2 = 0x18 | DITHERING_MODE_ON,
+   DITHERING_MODE_TEMPORAL = 0x20 | DITHERING_MODE_ON,
+   DITHERING_MODE_AUTO
+   } mode;
+   enum {
+   DITHERING_DEPTH_6BPC = 0x00,
+   DITHERING_DEPTH_8BPC = 0x02,
+   DITHERING_DEPTH_AUTO
+   } depth;
+   } dither;
+
+   struct {
+   int mode;   /* DRM_MODE_SCALE_* */
+   struct {
+   enum {
+   UNDERSCAN_OFF,
+   UNDERSCAN_ON,
+   UNDERSCAN_AUTO,
+   } mode;
+   u32 hborder;
+   u32 vborder;
+   } underscan;
+   bool full;
+   } scaler;
+
+   struct {
+   int color_vibrance;
+   int vibrant_hue;
+   } procamp;
+
+   union {
+   struct {
+   bool dither:1;
+   bool scaler:1;
+   bool procamp:1;
+   };
+   u8 mask;
+   } set;
+};
+
 struct nouveau_connector {
struct drm_connector base;
enum dcb_connector_type type;
@@ -121,61 +176,6 @@ extern int nouveau_ignorelid;
 extern int nouveau_duallink;
 extern int nouveau_hdmimhz;
 
-#include 
-#define nouveau_conn_atom(p)   
\
-   container_of((p), struct nouveau_conn_atom, state)
-
-struct nouveau_conn_atom {
-   struct drm_connector_state state;
-
-   struct {
-   /* The enum values specifically defined here match nv50/gf119
-* hw values, and the code relies on this.
-*/
-   enum {
-   DITHERING_MODE_OFF = 0x00,
-   DITHERING_MODE_ON = 0x01,
-   DITHERING_MODE_DYNAMIC2X2 = 0x10 | DITHERING_MODE_ON,
-   DITHERING_MODE_STATIC2X2 = 0x18 | DITHERING_MODE_ON,
-   DITHERING_MODE_TEMPORAL = 0x20 | DITHERING_MODE_ON,
-   DITHERING_MODE_AUTO
-   } mode;
-   enum {
-   DITHERING_DEPTH_6BPC = 0x00,
-   DITHERING_DEPTH_8BPC = 0x02,
-   DITHERING_DEPTH_AUTO
-   } depth;
-   } dither;
-
-   struct {
-   int mode;   /* DRM_MODE_SCALE_* */
-   struct {
-   enum {
-   UNDERSCAN_OFF,
-   UNDERSCAN_ON,
-   UNDERSCAN_AUTO,
-   } mode;
-   u32 hborder;
-   u32 vborder;
-   } underscan;
-   bool full;
-   } scaler;
-
-   struct {
-   int color_vibrance;
-   int vibrant_hue;
-   } procamp;
-
-   union {
-   struct {
-   bool dither:1;
-   bool scaler:1;
-   bool procamp:1;
-   };
-   u8 mask;
-   } set;
-};
-
 void nouveau_conn_attach_properties(struct drm_connector *);
 void nouveau_conn_reset(struct drm_connector *);
 struct drm_connector_state *
-- 
2.23.0

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

[PATCH 2/2] drm/nouveau: Fix drm-core using atomic code-paths on pre-nv50 hardware

2019-10-23 Thread Hans de Goede
We do not support atomic modesetting on pre-nv50 hardware, but until now
our connector code was setting drm_connector->state on pre-nv50 hardware.

This causes the core to enter atomic modesetting paths in at least:

1. drm_connector_get_encoder(), returning connector->state->best_encoder
which is always 0, causing us to always report 0 as encoder_id in
the drmModeConnector struct returned by drmModeGetConnector().

2. drm_encoder_get_crtc(), returning NULL because uses_atomic get set,
causing us to always report 0 as crtc_id in the drmModeEncoder struct
returned by drmModeGetEncoder()

Which in turn confuses userspace, at least plymouth thinks that the pipe
has changed because of this and tries to reconfigure it unnecessarily.

More in general we should not set drm_connector->state in the non-atomic
code as this violates the drm-core's expectations.

This commit fixes this by using a nouveau_conn_atom struct embedded in the
nouveau_connector struct for property handling in the non-atomic case.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1706557
Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 28 +++--
 drivers/gpu/drm/nouveau/nouveau_connector.h |  6 +
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 94dfa2e5a9ab..805b341db165 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -245,14 +245,22 @@ nouveau_conn_atomic_duplicate_state(struct drm_connector 
*connector)
 void
 nouveau_conn_reset(struct drm_connector *connector)
 {
+   struct nouveau_connector *nv_connector = nouveau_connector(connector);
struct nouveau_conn_atom *asyc;
 
-   if (WARN_ON(!(asyc = kzalloc(sizeof(*asyc), GFP_KERNEL
-   return;
+   if (drm_drv_uses_atomic_modeset(connector->dev)) {
+   if (WARN_ON(!(asyc = kzalloc(sizeof(*asyc), GFP_KERNEL
+   return;
+
+   if (connector->state)
+   nouveau_conn_atomic_destroy_state(connector,
+ connector->state);
+
+   __drm_atomic_helper_connector_reset(connector, >state);
+   } else {
+   asyc = _connector->properties_state;
+   }
 
-   if (connector->state)
-   nouveau_conn_atomic_destroy_state(connector, connector->state);
-   __drm_atomic_helper_connector_reset(connector, >state);
asyc->dither.mode = DITHERING_MODE_AUTO;
asyc->dither.depth = DITHERING_DEPTH_AUTO;
asyc->scaler.mode = DRM_MODE_SCALE_NONE;
@@ -276,8 +284,14 @@ void
 nouveau_conn_attach_properties(struct drm_connector *connector)
 {
struct drm_device *dev = connector->dev;
-   struct nouveau_conn_atom *armc = nouveau_conn_atom(connector->state);
struct nouveau_display *disp = nouveau_display(dev);
+   struct nouveau_connector *nv_connector = nouveau_connector(connector);
+   struct nouveau_conn_atom *armc;
+
+   if (drm_drv_uses_atomic_modeset(connector->dev))
+   armc = nouveau_conn_atom(connector->state);
+   else
+   armc = _connector->properties_state;
 
/* Init DVI-I specific properties. */
if (connector->connector_type == DRM_MODE_CONNECTOR_DVII)
@@ -749,9 +763,9 @@ static int
 nouveau_connector_set_property(struct drm_connector *connector,
   struct drm_property *property, uint64_t value)
 {
-   struct nouveau_conn_atom *asyc = nouveau_conn_atom(connector->state);
struct nouveau_connector *nv_connector = nouveau_connector(connector);
struct nouveau_encoder *nv_encoder = nv_connector->detected_encoder;
+   struct nouveau_conn_atom *asyc = _connector->properties_state;
struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
int ret;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h 
b/drivers/gpu/drm/nouveau/nouveau_connector.h
index de9588420884..2d70dea4018b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -118,6 +118,12 @@ struct nouveau_connector {
 #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
struct nouveau_backlight *backlight;
 #endif
+   /*
+* Our connector property code expects a nouveau_conn_atom struct
+* even on pre-nv50 where we do not support atomic. This embedded
+* version gets used in the non atomic modeset case.
+*/ 
+   struct nouveau_conn_atom properties_state;
 };
 
 static inline struct nouveau_connector *nouveau_connector(
-- 
2.23.0

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

[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #135 from Shmerl  ---
(In reply to Benjamin Neff from comment #134)
> 
> I don't know if OpenGL is involved in all of my freezes, I had freezes while
> watching a video on youtube in chromium, also while just browsing the web in
> firefox, once it crashed during rendering file preview images of photos in
> nautilus, and once it crashed while the screen was locked with i3lock. It
> didn't freeze during games yet.
> 

Both Firefox and Chromium are only using OpenGL paths, they aren't using
Vulkan. So it's safe to assume it's going through radeonsi.

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #134 from Benjamin Neff  ---
I had the freeze with different versions of mesa-git, but I didn't update that
since two days, I can try with the current master and see if it still freezes.
I don't have a way to reproduce it, it is usually random and I never had two 
similar freezes.

I don't know if OpenGL is involved in all of my freezes, I had freezes while
watching a video on youtube in chromium, also while just browsing the web in
firefox, once it crashed during rendering file preview images of photos in
nautilus, and once it crashed while the screen was locked with i3lock. It
didn't freeze during games yet.

I have always a browser open, so it could just be a browser doing something in
the background. The process mentioned in the error message is usually Xorg
itself, but once it was chrome.

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #133 from Shmerl  ---
(In reply to Jaap Buurman from comment #132)
>
> I am beginning to suspect that RadeonSI might be to blame instead of the
> AMDGPU kernel driver. Does anyone agree with that notion?

That's my suspicion as well, since I haven't gotten any hangs so far with
radv/llvm or radv/aco when playing games, especially after aco added several
GPU hazard mitigations for Navi:

https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/amd/compiler/README

I wonder if the above hangs are related to similar GPU hardware bugs in Navi,
that weren't yet worked around in radeonsi.

I even opened one bug like that, but closed it due to assuming it's really
amdgpu problem. Not sure if I should re-open it:

https://gitlab.freedesktop.org/mesa/mesa/issues/1910

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #132 from Jaap Buurman  ---
Many people are experiencing the hangs with OpenGL: Quake with the OpenGL
renderer, Chrome/Chromium, Firefox, etc.

I am beginning to suspect that RadeonSI might be to blame instead of the AMDGPU
kernel driver. Does anyone agree with that notion?

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #131 from Shmerl  ---
Does it also hang with Mesa master?

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

Re: [PATCH v2] panfrost: Properly undo pm_runtime_enable when deferring a probe

2019-10-23 Thread Rob Herring
On Wed, Oct 23, 2019 at 10:49 AM Steven Price  wrote:
>
> On 23/10/2019 13:21, Tomeu Vizoso wrote:
> > When deferring the probe because of a missing regulator, we were calling
> > pm_runtime_disable even if pm_runtime_enable wasn't called.
> >
> > Move the call to pm_runtime_disable to the right place.
> >
> > Signed-off-by: Tomeu Vizoso 
> > Reported-by: Chen-Yu Tsai 
> > Cc: Robin Murphy 
> > Fixes: f4a3c6a44b35 ("drm/panfrost: Disable PM on probe failure")
>
> As Robin pointed out this should be:
>
> Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")
>
> But other than that,
>
> Reviewed-by: Steven Price 

Applied with Fixes fixed. Looks like we just missed this weeks fixes...

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

Re: [PATCH 5/6] drm/mediatek: Convert to use CMA helpers

2019-10-23 Thread Rob Herring
On Tue, Oct 22, 2019 at 12:07 PM Matthias Brugger
 wrote:
>
> Hi Rob,
>
> On 21/10/2019 23:45, Rob Herring wrote:
> > The only reason the Mediatek driver doesn't use the CMA helpers is it
> > sets DMA_ATTR_NO_KERNEL_MAPPING and does a vmap() on demand. Using
> > vmap() is not even guaranteed to work as DMA buffers may not have a
> > struct page. Now that the CMA helpers support setting
> > DMA_ATTR_NO_KERNEL_MAPPING as needed or not, convert Mediatek driver to
> > use CMA helpers.
> >
> > Cc: CK Hu 
> > Cc: Philipp Zabel 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: Matthias Brugger 
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: linux-media...@lists.infradead.org
> > Signed-off-by: Rob Herring 
> > ---
>
> I tested this on my Chromebook with some patches on top of v5.4-rc1 [1], which
> work. If I add your patches on top of that, the system does not boot up.
> Unfortunately I don't have a serial console, so I wasn't able to see if there 
> is
> any error message.

Thanks for testing. I'm based on drm-misc-next, but don't see anything
obvious there that would matter. There are some mmap changes, but I
think they shouldn't matter.

Did you have fbcon enabled? That may give more clues about where the problem is.

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

[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #130 from yam...@yamagi.org ---
Created attachment 145800
  --> https://bugs.freedesktop.org/attachment.cgi?id=145800=edit
sdma0 after q2 crash

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #128 from yam...@yamagi.org ---
(In reply to yamagi from comment #124)
> Interestingly I've got the problem the other way round. My 5700XT was
> running fine since I got it about two weeks ago. This is Arch Linux, I've
> run Mesa 19.2.1 and llvm-libs 9.0.0 since day one. The card was stable with
> 5.4-RC2 and 5.4-RC3, not a single hang in about 10 hours The Witcher 3 under
> wine + dxvk and Yamagi Quake II with OpenGL 3.2 renderer. After I upgraded
> to 5.4-RC4 I've seen several GPU hangs. The last one, and the only one
> that's still in the logs was:
> 
> [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled
> seq=85270, emitted seq=85272
> 
> That one was in Yamagi Quake II, but I had hangs on the desktop and in The
> Witcher 3. I have no umr reports so far. I've just compiled the tool and
> will see if I can get some.


As promised, some more informations:

For me the crash is fairly easy to reproduce with Linux 5.4-RC4. All it takes
is Yamagi Quake II (Revision 1232289, can be found at
https://github.com/yquake2/yquake2) with OpenGL 3.2 renderer. The old OpenGL
1.4 doesn't trigger it. Start the game, it's a good idea to set set timedemo
mode to 1, and just let it cycle through the demo loop until it crashes. I used
'./quake +set timedemo 1 +set vid_renderer gl3'. I've never experienced this
crash in the wild with Linux 5.4-RC3 until I learned that I can trigger with
the Quake II demo loop. In Linux 5.4-RC3 it usually takes somewhere between 20
to 30 cycles through loop to trigger, with 5.4-RC4 only 5 to 10 cycles. So
something changed between RC3 and RC4 that made it more likely.

I suspect some kind of timing issue. The demo loop is deterministic, it
generates exactly the same API calls each time it's run. While the crash always
happens while the loading screen is up, it never occures at the same one.
Sometimes it's in the fifth iteration, the next time at the 12th and so on.
Putting apitrace (adds some latency!) onto it, makes it much less likely to
occure. To the point I thought that it's a heisenbug. The same goes for cycling
through the loop without timedemo mode enabled (~20 FPS in normal mode, ~1000
FPS in timedemo mode).

I made an apitrace for easier reproduction. It's a little bit big for bugzilla,
so I've uploaded it here: https://deponie.yamagi.org/temp/quake2.trace.xz
Replaying it usually triggers the crash during the first or second run.

The exact software versions were:
* Linux 5.4-RC4 with https://bugzilla.freedesktop.org/attachment.cgi?id=145323
and https://bugzilla.freedesktop.org/attachment.cgi?id=145734 applied.
* Mesa 19.2.1-2
* LLVM 9.0.0

dmesg output after a crash in Quake IIs demo loop is:
[  122.294181] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout,
signaled seq=177737, emitted seq=177739
[  122.294256] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information:
process glretrace pid 1302 thread glretrace:cs0 pid 1303
[  122.294257] [drm] GPU recovery disabled.

dmesg output after a crash by replaying the apitrace is:
[  266.695388] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout,
signaled seq=27598, emitted seq=27600
[  266.695463] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information:
process glretrace pid 1372 thread glretrace:cs0 pid 1373
[  266.695465] [drm] GPU recovery disabled.

I'm attaching the state of sdma0 is both cases.

I hope this helps to find the root cause of this. If can provide more
informations don't hesitate to ask.

-- 
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 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #129 from yam...@yamagi.org ---
Created attachment 145799
  --> https://bugs.freedesktop.org/attachment.cgi?id=145799=edit
sdma0 after apitrace crash

-- 
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 2/6] drm: Introduce DRM_MODE_DUMB_KERNEL_MAP flag

2019-10-23 Thread Rob Herring
On Wed, Oct 23, 2019 at 9:28 AM Laurent Pinchart
 wrote:
>
> Hi Rob,
>
> On Tue, Oct 22, 2019 at 07:42:06AM -0500, Rob Herring wrote:
> > On Tue, Oct 22, 2019 at 6:14 AM Laurent Pinchart wrote:
> > > On Mon, Oct 21, 2019 at 04:45:46PM -0500, Rob Herring wrote:
> > > > Introduce a new flag, DRM_MODE_DUMB_KERNEL_MAP, for struct
> > > > drm_mode_create_dumb. This flag is for internal kernel use to indicate
> > > > if dumb buffer allocation needs a kernel mapping. This is needed only 
> > > > for
> > > > CMA where creating a kernel mapping or not has to be decided at 
> > > > allocation
> > > > time because creating a mapping on demand (with vmap()) is not 
> > > > guaranteed
> > > > to work. Several drivers are using CMA, but not the CMA helpers because
> > > > they distinguish between kernel and userspace allocations to create a
> > > > kernel mapping or not.
> > > >
> > > > Update the callers of drm_mode_dumb_create() to set
> > > > drm_mode_dumb_create.flags to appropriate defaults. Currently, flags can
> > > > be set to anything by userspace, but is unused within the kernel. Let's
> > > > force flags to zero (no kernel mapping) for userspace callers by 
> > > > default.
> > > > For in kernel clients, set DRM_MODE_DUMB_KERNEL_MAP by default. Drivers
> > > > can override this as needed.
> > > >
> > > > Cc: David Airlie 
> > > > Cc: Daniel Vetter 
> > > > Cc: Maarten Lankhorst 
> > > > Cc: Maxime Ripard 
> > > > Cc: Sean Paul 
> > > > Signed-off-by: Rob Herring 
> > > > ---
> > > >  drivers/gpu/drm/drm_client.c   | 1 +
> > > >  drivers/gpu/drm/drm_dumb_buffers.c | 5 -
> > > >  include/uapi/drm/drm_mode.h| 2 ++
> > > >  3 files changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> > > > index d9a2e3695525..dbfc8061b392 100644
> > > > --- a/drivers/gpu/drm/drm_client.c
> > > > +++ b/drivers/gpu/drm/drm_client.c
> > > > @@ -264,6 +264,7 @@ drm_client_buffer_create(struct drm_client_dev 
> > > > *client, u32 width, u32 height, u
> > > >   dumb_args.width = width;
> > > >   dumb_args.height = height;
> > > >   dumb_args.bpp = info->cpp[0] * 8;
> > > > + dumb_args.flags = DRM_MODE_DUMB_KERNEL_MAP;
> > > >   ret = drm_mode_create_dumb(dev, _args, client->file);
> > > >   if (ret)
> > > >   goto err_delete;
> > > > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c 
> > > > b/drivers/gpu/drm/drm_dumb_buffers.c
> > > > index d18a740fe0f1..74a13f14c173 100644
> > > > --- a/drivers/gpu/drm/drm_dumb_buffers.c
> > > > +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> > > > @@ -97,7 +97,10 @@ int drm_mode_create_dumb(struct drm_device *dev,
> > > >  int drm_mode_create_dumb_ioctl(struct drm_device *dev,
> > > >  void *data, struct drm_file *file_priv)
> > > >  {
> > > > - return drm_mode_create_dumb(dev, data, file_priv);
> > > > + struct drm_mode_create_dumb *args = data;
> > > > +
> > > > + args->flags = 0;
> > >
> > > I would prefer returning an error if flags is set to a non-zero value,
> > > to catch userspace errors early, but I'm also worried it would break
> > > existing userspace by uncovering existing bugs. Still, if we later add
> > > flags that userspace can set, those existing bugs will be triggered, so
> > > we'll have to deal with them anyway, and we could already give it a try.
> >
> > I would like that too, but the comment just above this code tells me
> > that's likely to break things:
> >
> > /*
> >  * handle, pitch and size are output parameters. Zero them out to
> >  * prevent drivers from accidentally using uninitialized data. Since
> >  * not all existing userspace is clearing these fields properly we
> >  * cannot reject IOCTL with garbage in them.
> >  */
> >
> > Maybe userspace does correctly zero out input params.
>
> But if that holds true, it means that we will never be able to add
> userspace flags, as doing so could break applications for the same
> reason. The flag field should thus be marked as deprecated for userspace
> usage. I wonder how big the risk is.

Good point. I guess another option is add a WARN(flags != 0) and see
what happens.

The commit adding the comment was f60859522a83 ("drm: Sanitize
DRM_IOCTL_MODE_CREATE_DUMB input"). Maybe Thierry has some comment?

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

[PATCH v2] drm/msm/dsi: Implement qcom,dsi-phy-regulator-ldo-mode for 28nm PHY

2019-10-23 Thread Stephan Gerhold
The DSI PHY regulator supports two regulator modes: LDO and DCDC.
This mode can be selected using the "qcom,dsi-phy-regulator-ldo-mode"
device tree property.

However, at the moment only the 20nm PHY driver actually implements
that option. Add a check in the 28nm PHY driver to program the
registers correctly for LDO mode.

Tested-by: Nikita Travkin  # l8150
Signed-off-by: Stephan Gerhold 
---
Changes in v2: Move DCDC/LDO code into separate methods
v1: 
https://lore.kernel.org/linux-arm-msm/20191021163425.83697-1-step...@gerhold.net/

This is needed to make the display work on Longcheer L8150,
which has recently gained mainline support in:
https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?id=16e8e8072108426029f0c16dff7fbe77fae3df8f

This patch is based on code from the downstream kernel:
https://source.codeaurora.org/quic/la/kernel/msm-3.10/tree/drivers/video/msm/mdss/msm_mdss_io_8974.c?h=LA.BR.1.2.9.1-02310-8x16.0#n152

The LDO regulator configuration is taken from msm8916-qrd.dtsi:
https://source.codeaurora.org/quic/la/kernel/msm-3.10/tree/arch/arm/boot/dts/qcom/msm8916-qrd.dtsi?h=LA.BR.1.2.9.1-02310-8x16.0#n56
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 42 +-
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
index b3f678f6c2aa..b384ea20f359 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
@@ -39,15 +39,10 @@ static void dsi_28nm_dphy_set_timing(struct msm_dsi_phy 
*phy,
DSI_28nm_PHY_TIMING_CTRL_11_TRIG3_CMD(0));
 }
 
-static void dsi_28nm_phy_regulator_ctrl(struct msm_dsi_phy *phy, bool enable)
+static void dsi_28nm_phy_regulator_enable_dcdc(struct msm_dsi_phy *phy)
 {
void __iomem *base = phy->reg_base;
 
-   if (!enable) {
-   dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CAL_PWR_CFG, 0);
-   return;
-   }
-
dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_0, 0x0);
dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CAL_PWR_CFG, 1);
dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_5, 0);
@@ -56,6 +51,39 @@ static void dsi_28nm_phy_regulator_ctrl(struct msm_dsi_phy 
*phy, bool enable)
dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_1, 0x9);
dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_0, 0x7);
dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_4, 0x20);
+   dsi_phy_write(phy->base + REG_DSI_28nm_PHY_LDO_CNTRL, 0x00);
+}
+
+static void dsi_28nm_phy_regulator_enable_ldo(struct msm_dsi_phy *phy)
+{
+   void __iomem *base = phy->reg_base;
+
+   dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_0, 0x0);
+   dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CAL_PWR_CFG, 0);
+   dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_5, 0x7);
+   dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_3, 0);
+   dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_2, 0x1);
+   dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_1, 0x1);
+   dsi_phy_write(base + REG_DSI_28nm_PHY_REGULATOR_CTRL_4, 0x20);
+
+   if (phy->cfg->type == MSM_DSI_PHY_28NM_LP)
+   dsi_phy_write(phy->base + REG_DSI_28nm_PHY_LDO_CNTRL, 0x05);
+   else
+   dsi_phy_write(phy->base + REG_DSI_28nm_PHY_LDO_CNTRL, 0x0d);
+}
+
+static void dsi_28nm_phy_regulator_ctrl(struct msm_dsi_phy *phy, bool enable)
+{
+   if (!enable) {
+   dsi_phy_write(phy->reg_base +
+ REG_DSI_28nm_PHY_REGULATOR_CAL_PWR_CFG, 0);
+   return;
+   }
+
+   if (phy->regulator_ldo_mode)
+   dsi_28nm_phy_regulator_enable_ldo(phy);
+   else
+   dsi_28nm_phy_regulator_enable_dcdc(phy);
 }
 
 static int dsi_28nm_phy_enable(struct msm_dsi_phy *phy, int src_pll_id,
@@ -77,8 +105,6 @@ static int dsi_28nm_phy_enable(struct msm_dsi_phy *phy, int 
src_pll_id,
 
dsi_28nm_phy_regulator_ctrl(phy, true);
 
-   dsi_phy_write(base + REG_DSI_28nm_PHY_LDO_CNTRL, 0x00);
-
dsi_28nm_dphy_set_timing(phy, timing);
 
dsi_phy_write(base + REG_DSI_28nm_PHY_CTRL_1, 0x00);
-- 
2.23.0



Re: [PATCH hmm 00/15] Consolidate the mmu notifier interval_tree and locking

2019-10-23 Thread Jerome Glisse
On Wed, Oct 23, 2019 at 11:32:16AM +0200, Christian König wrote:
> Am 23.10.19 um 11:08 schrieb Daniel Vetter:
> > On Tue, Oct 22, 2019 at 03:01:13PM +, Jason Gunthorpe wrote:
> > > On Tue, Oct 22, 2019 at 09:57:35AM +0200, Daniel Vetter wrote:
> > > 
> > > > > The unusual bit in all of this is using a lock's critical region to
> > > > > 'protect' data for read, but updating that same data before the lock's
> > > > > critical secion. ie relying on the unlock barrier to 'release' program
> > > > > ordered stores done before the lock's own critical region, and the
> > > > > lock side barrier to 'acquire' those stores.
> > > > I think this unusual use of locks as barriers for other unlocked 
> > > > accesses
> > > > deserves comments even more than just normal barriers. Can you pls add
> > > > them? I think the design seeems sound ...
> > > > 
> > > > Also the comment on the driver's lock hopefully prevents driver
> > > > maintainers from moving the driver_lock around in a way that would very
> > > > subtle break the scheme, so I think having the acquire barrier commented
> > > > in each place would be really good.
> > > There is already a lot of documentation, I think it would be helpful
> > > if you could suggest some specific places where you think an addition
> > > would help? I think the perspective of someone less familiar with this
> > > design would really improve the documentation
> > Hm I just meant the usual recommendation that "barriers must have comments
> > explaining what they order, and where the other side of the barrier is".
> > Using unlock/lock as a barrier imo just makes that an even better idea.
> > Usually what I do is something like "we need to order $this against $that
> > below, and the other side of this barrier is in function()." With maybe a
> > bit more if it's not obvious how things go wrong if the orderin is broken.
> > 
> > Ofc seqlock.h itself skimps on that rule and doesn't bother explaining its
> > barriers :-/
> > 
> > > I've been tempted to force the driver to store the seq number directly
> > > under the driver lock - this makes the scheme much clearer, ie
> > > something like this:
> > > 
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c 
> > > b/drivers/gpu/drm/nouveau/nouveau_svm.c
> > > index 712c99918551bc..738fa670dcfb19 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> > > @@ -488,7 +488,8 @@ struct svm_notifier {
> > >   };
> > >   static bool nouveau_svm_range_invalidate(struct mmu_range_notifier *mrn,
> > > -const struct mmu_notifier_range 
> > > *range)
> > > +const struct mmu_notifier_range 
> > > *range,
> > > +unsigned long seq)
> > >   {
> > >  struct svm_notifier *sn =
> > >  container_of(mrn, struct svm_notifier, notifier);
> > > @@ -504,6 +505,7 @@ static bool nouveau_svm_range_invalidate(struct 
> > > mmu_range_notifier *mrn,
> > >  mutex_lock(>svmm->mutex);
> > >  else if (!mutex_trylock(>svmm->mutex))
> > >  return false;
> > > +   mmu_range_notifier_update_seq(mrn, seq);
> > >  mutex_unlock(>svmm->mutex);
> > >  return true;
> > >   }
> > > 
> > > 
> > > At the cost of making the driver a bit more complex, what do you
> > > think?
> > Hm, spinning this further ... could we initialize the mmu range notifier
> > with a pointer to the driver lock, so that we could put a
> > lockdep_assert_held into mmu_range_notifier_update_seq? I think that would
> > make this scheme substantially more driver-hacker proof :-)
> 
> Going another step further what hinders us to put the lock into the mmu
> range notifier itself and have _lock()/_unlock() helpers?
> 
> I mean having the lock in the driver only makes sense when the driver would
> be using the same lock for multiple things, e.g. multiple MMU range
> notifiers under the same lock. But I really don't see that use case here.

I actualy do, nouveau use one lock to protect the page table and that's the
lock that matter. You can have multiple range for a single page table, idea
being only a sub-set of the process address space is ever accessed by the
GPU and those it is better to focus on this sub-set and track invalidation in
a finer grain.

Cheers,
Jérôme

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

[Bug 204241] amdgpu fails to resume from suspend

2019-10-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=204241

David (dav@gmx.com) changed:

   What|Removed |Added

 CC||dav@gmx.com

--- Comment #37 from David (dav@gmx.com) ---
*** Bug 204965 has been marked as a duplicate of this bug. ***

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

[Bug 204965] [amdgpu]] *ERROR* ring gfx test failed (-110) upon wake from sleep, no video or frozen video

2019-10-23 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=204965

David (dav@gmx.com) changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |DUPLICATE

--- Comment #4 from David (dav@gmx.com) ---


*** This bug has been marked as a duplicate of bug 204241 ***

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

RE: [PATCH V5 1/6] mdev: class id support

2019-10-23 Thread Parav Pandit



> -Original Message-
> From: Jason Wang 
> Sent: Wednesday, October 23, 2019 8:08 AM
> To: k...@vger.kernel.org; linux-s...@vger.kernel.org; linux-
> ker...@vger.kernel.org; dri-devel@lists.freedesktop.org; intel-
> g...@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org;
> kwankh...@nvidia.com; alex.william...@redhat.com; m...@redhat.com;
> tiwei@intel.com
> Cc: virtualizat...@lists.linux-foundation.org; net...@vger.kernel.org;
> coh...@redhat.com; maxime.coque...@redhat.com;
> cunming.li...@intel.com; zhihong.w...@intel.com;
> rob.mil...@broadcom.com; xiao.w.w...@intel.com;
> haotian.w...@sifive.com; zhen...@linux.intel.com; zhi.a.w...@intel.com;
> jani.nik...@linux.intel.com; joonas.lahti...@linux.intel.com;
> rodrigo.v...@intel.com; airl...@linux.ie; dan...@ffwll.ch;
> far...@linux.ibm.com; pa...@linux.ibm.com; seb...@linux.ibm.com;
> ober...@linux.ibm.com; heiko.carst...@de.ibm.com; g...@linux.ibm.com;
> borntrae...@de.ibm.com; akrow...@linux.ibm.com; fre...@linux.ibm.com;
> lingshan@intel.com; Ido Shamay ;
> epere...@redhat.com; l...@redhat.com; Parav Pandit
> ; christophe.de.dinec...@gmail.com;
> kevin.t...@intel.com; stefa...@redhat.com; Jason Wang
> 
> Subject: [PATCH V5 1/6] mdev: class id support
> 
> Mdev bus only supports vfio driver right now, so it doesn't implement match
> method. But in the future, we may add drivers other than vfio, the first 
> driver
> could be virtio-mdev. This means we need to add device class id support in bus
> match method to pair the mdev device and mdev driver correctly.
> 
> So this patch adds id_table to mdev_driver and class_id for mdev device with
> the match method for mdev bus.
> 
> Signed-off-by: Jason Wang 
> ---
>  .../driver-api/vfio-mediated-device.rst   |  5 +
>  drivers/gpu/drm/i915/gvt/kvmgt.c  |  1 +
>  drivers/s390/cio/vfio_ccw_ops.c   |  1 +
>  drivers/s390/crypto/vfio_ap_ops.c |  1 +
>  drivers/vfio/mdev/mdev_core.c | 18 +++
>  drivers/vfio/mdev/mdev_driver.c   | 22 +++
>  drivers/vfio/mdev/mdev_private.h  |  1 +
>  drivers/vfio/mdev/vfio_mdev.c |  6 +
>  include/linux/mdev.h  |  8 +++
>  include/linux/mod_devicetable.h   |  8 +++
>  samples/vfio-mdev/mbochs.c|  1 +
>  samples/vfio-mdev/mdpy.c  |  1 +
>  samples/vfio-mdev/mtty.c  |  1 +
>  13 files changed, 74 insertions(+)
> 
> diff --git a/Documentation/driver-api/vfio-mediated-device.rst
> b/Documentation/driver-api/vfio-mediated-device.rst
> index 25eb7d5b834b..6709413bee29 100644
> --- a/Documentation/driver-api/vfio-mediated-device.rst
> +++ b/Documentation/driver-api/vfio-mediated-device.rst
> @@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
>* @probe: called when new device created
>* @remove: called when device removed
>* @driver: device driver structure
> +  * @id_table: the ids serviced by this driver
>*/
>   struct mdev_driver {
>const char *name;
>int  (*probe)  (struct device *dev);
>void (*remove) (struct device *dev);
>struct device_driverdriver;
> +  const struct mdev_class_id *id_table;
>   };
> 
>  A mediated bus driver for mdev should use this structure in the function 
> calls
> @@ -170,6 +172,9 @@ that a driver should use to unregister itself with the
> mdev core driver::
> 
>   extern void mdev_unregister_device(struct device *dev);
> 
> +It is also required to specify the class_id in create() callback through::
> +
> + int mdev_set_class(struct mdev_device *mdev, u16 id);
> 
>  Mediated Device Management Interface Through sysfs
> ==
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 343d79c1cb7e..6420f0dbd31b 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -678,6 +678,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct
> mdev_device *mdev)
>dev_name(mdev_dev(mdev)));
>   ret = 0;
> 
> + mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
>  out:
>   return ret;
>  }
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> index f0d71ab77c50..cf2c013ae32f 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -129,6 +129,7 @@ static int vfio_ccw_mdev_create(struct kobject *kobj,
> struct mdev_device *mdev)
>  private->sch->schid.ssid,
>  private->sch->schid.sch_no);
> 
> + mdev_set_class(mdev, MDEV_CLASS_ID_VFIO);
>   return 0;
>  }
> 
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
> b/drivers/s390/crypto/vfio_ap_ops.c
> index 5c0f53c6dde7..07c31070afeb 100644
> --- 

Re: [PATCH v7 0/9] backlight: gpio: simplify the driver

2019-10-23 Thread Daniel Thompson
On Tue, Oct 22, 2019 at 11:29:54AM +0200, Bartosz Golaszewski wrote:
> wt., 22 paź 2019 o 10:36 Bartosz Golaszewski  napisał(a):
> >
> > From: Bartosz Golaszewski 
> >
> > While working on my other series related to gpio-backlight[1] I noticed
> > that we could simplify the driver if we made the only user of platform
> > data use GPIO lookups and device properties. This series tries to do
> > that.
> >
> > First two patches contain minor fixes. Third patch makes the driver
> > explicitly drive the GPIO line. Fourth patch adds all necessary data
> > structures to ecovec24. Patch 5/9 unifies much of the code for both
> > pdata and non-pdata cases. Patches 6-7/9 remove unused platform data
> > fields. Last two patches contain additional improvements for the GPIO
> > backlight driver while we're already modifying it.
> >
> > I don't have access to this HW but hopefully this works. Only compile
> > tested.
> >
> > [1] https://lkml.org/lkml/2019/6/25/900
> >
> > v1 -> v2:
> > - rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy
> > - added additional two patches with minor improvements
> >
> > v2 -> v3:
> > - in patch 7/7: used initializers to set values for pdata and dev local vars
> >
> > v3 -> v4:
> > - rebased on top of v5.4-rc1
> > - removed changes that are no longer relevant after commit ec665b756e6f
> >   ("backlight: gpio-backlight: Correct initial power state handling")
> > - added patch 7/7
> >
> > v4 -> v5:
> > - in patch 7/7: added a comment replacing the name of the function being
> >   pulled into probe()
> >
> > v5 -> v6:
> > - added a patch making the driver explicitly set the direction of the GPIO
> >   to output
> > - added a patch removing a redundant newline
> >
> > v6 -> v7:
> > - renamed the function calculating the new GPIO value for status update
> > - collected more tags
> >
> > Bartosz Golaszewski (9):
> >   backlight: gpio: remove unneeded include
> >   backlight: gpio: remove stray newline
> >   backlight: gpio: explicitly set the direction of the GPIO
> >   sh: ecovec24: add additional properties to the backlight device
> >   backlight: gpio: simplify the platform data handling
> >   sh: ecovec24: don't set unused fields in platform data
> >   backlight: gpio: remove unused fields from platform data
> >   backlight: gpio: use a helper variable for >dev
> >   backlight: gpio: pull gpio_backlight_initial_power_state() into probe
> >
> >  arch/sh/boards/mach-ecovec24/setup.c |  33 +++--
> >  drivers/video/backlight/gpio_backlight.c | 128 +++
> >  include/linux/platform_data/gpio_backlight.h |   3 -
> >  3 files changed, 69 insertions(+), 95 deletions(-)
> >
> > --
> > 2.23.0
> >
> 
> Lee, Daniel, Jingoo,
> 
> Jacopo is travelling until November 1st and won't be able to test this
> again before this date. Do you think you can pick it up and in case
> anything's broken on SH, we can fix it after v5.5-rc1, so that it
> doesn't miss another merge window?

Outside of holidays and other emergencies Lee usually collects up the
patches for backlight. I'm not sure when he plans to close things for
v5.5 .


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

Re: [PATCH v2] drm/amdgpu: Add DC feature mask to disable fractional pwm

2019-10-23 Thread Li, Sun peng (Leo)
On 2019-10-23 10:19 a.m., Lukáš Krejčí wrote:
> On Mon, 2019-10-21 at 15:44 -0400, sunpeng...@amd.com wrote:
>> From: Leo Li 
>>
>> [Why]
>>
>> Some LED panel drivers might not like fractional PWM. In such cases,
>> backlight flickering may be observed.
>>
>> [How]
>>
>> Add a DC feature mask to disable fractional PWM, and associate it with
>> the preexisting dc_config flag.
>>
>> The flag is only plumbed through the dmcu firmware, so plumb it through
>> the driver path as well.
>>
>> To disable, add the following to the linux cmdline:
>> amdgpu.dcfeaturemask=0x4
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204957
>> Signed-off-by: Leo Li 
>> ---
>>
>> v2: Add bugzilla link
>>
>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
>>  drivers/gpu/drm/amd/display/dc/dce/dce_abm.c  | 4 
>>  drivers/gpu/drm/amd/include/amd_shared.h  | 1 +
>>  3 files changed, 8 insertions(+)
> 
> Tested on Ubuntu 19.04, vanilla v5.3.7 kernel with the patch applied
> and amdgpu.dcfeaturemask=0x4 added to the kernel command line, no
> issues after 8 reboots. (The issue could be reproduced without
> amdgpu.dcfeaturemask=0x4 on first boot.)
> 
> Tested-by: Lukáš Krejčí 

Applied, thanks!
Leo
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] panfrost: Properly undo pm_runtime_enable when deferring a probe

2019-10-23 Thread Steven Price
On 23/10/2019 13:21, Tomeu Vizoso wrote:
> When deferring the probe because of a missing regulator, we were calling
> pm_runtime_disable even if pm_runtime_enable wasn't called.
> 
> Move the call to pm_runtime_disable to the right place.
> 
> Signed-off-by: Tomeu Vizoso 
> Reported-by: Chen-Yu Tsai 
> Cc: Robin Murphy 
> Fixes: f4a3c6a44b35 ("drm/panfrost: Disable PM on probe failure")

As Robin pointed out this should be:

Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")

But other than that,

Reviewed-by: Steven Price 

> ---
>  drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c 
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index bc2ddeb55f5d..f21bc8a7ee3a 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -556,11 +556,11 @@ static int panfrost_probe(struct platform_device *pdev)
>   return 0;
>  
>  err_out2:
> + pm_runtime_disable(pfdev->dev);
>   panfrost_devfreq_fini(pfdev);
>  err_out1:
>   panfrost_device_fini(pfdev);
>  err_out0:
> - pm_runtime_disable(pfdev->dev);
>   drm_dev_put(ddev);
>   return err;
>  }
> 

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

[PATCH v3 04/21] drm/bridge: Rename bridge helpers targeting a bridge chain

2019-10-23 Thread Boris Brezillon
Change the prefix of bridge helpers targeting a bridge chain from
drm_bridge_ to drm_bridge_chain_ to better reflect the fact that
the operation will happen on all elements of chain, starting at the
bridge passed in argument.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* None

Changes in v2:
* Pass te bridge, not the encoder, so we can later act on a sub-chain
  instead of the whole chain
---
 drivers/gpu/drm/drm_atomic_helper.c |  19 +++--
 drivers/gpu/drm/drm_bridge.c| 125 ++--
 drivers/gpu/drm/drm_probe_helper.c  |   2 +-
 drivers/gpu/drm/mediatek/mtk_hdmi.c |   4 +-
 include/drm/drm_bridge.h|  64 +++---
 5 files changed, 112 insertions(+), 102 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 587052751b48..cf678be58fa4 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -435,8 +435,9 @@ mode_fixup(struct drm_atomic_state *state)
encoder = new_conn_state->best_encoder;
funcs = encoder->helper_private;
 
-   ret = drm_bridge_mode_fixup(encoder->bridge, 
_crtc_state->mode,
-   _crtc_state->adjusted_mode);
+   ret = drm_bridge_chain_mode_fixup(encoder->bridge,
+   _crtc_state->mode,
+   _crtc_state->adjusted_mode);
if (!ret) {
DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
return -EINVAL;
@@ -501,7 +502,7 @@ static enum drm_mode_status mode_valid_path(struct 
drm_connector *connector,
return ret;
}
 
-   ret = drm_bridge_mode_valid(encoder->bridge, mode);
+   ret = drm_bridge_chain_mode_valid(encoder->bridge, mode);
if (ret != MODE_OK) {
DRM_DEBUG_ATOMIC("[BRIDGE] mode_valid() failed\n");
return ret;
@@ -1020,7 +1021,7 @@ disable_outputs(struct drm_device *dev, struct 
drm_atomic_state *old_state)
 * Each encoder has at most one connector (since we always steal
 * it away), so we won't call disable hooks twice.
 */
-   drm_atomic_bridge_disable(encoder->bridge, old_state);
+   drm_atomic_bridge_chain_disable(encoder->bridge, old_state);
 
/* Right function depends upon target state. */
if (funcs) {
@@ -1034,7 +1035,8 @@ disable_outputs(struct drm_device *dev, struct 
drm_atomic_state *old_state)
funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
}
 
-   drm_atomic_bridge_post_disable(encoder->bridge, old_state);
+   drm_atomic_bridge_chain_post_disable(encoder->bridge,
+old_state);
}
 
for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, 
new_crtc_state, i) {
@@ -1215,7 +1217,8 @@ crtc_set_mode(struct drm_device *dev, struct 
drm_atomic_state *old_state)
funcs->mode_set(encoder, mode, adjusted_mode);
}
 
-   drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode);
+   drm_bridge_chain_mode_set(encoder->bridge, mode,
+ adjusted_mode);
}
 }
 
@@ -1332,7 +1335,7 @@ void drm_atomic_helper_commit_modeset_enables(struct 
drm_device *dev,
 * Each encoder has at most one connector (since we always steal
 * it away), so we won't call enable hooks twice.
 */
-   drm_atomic_bridge_pre_enable(encoder->bridge, old_state);
+   drm_atomic_bridge_chain_pre_enable(encoder->bridge, old_state);
 
if (funcs) {
if (funcs->atomic_enable)
@@ -1343,7 +1346,7 @@ void drm_atomic_helper_commit_modeset_enables(struct 
drm_device *dev,
funcs->commit(encoder);
}
 
-   drm_atomic_bridge_enable(encoder->bridge, old_state);
+   drm_atomic_bridge_chain_enable(encoder->bridge, old_state);
}
 
drm_atomic_helper_commit_writebacks(dev, old_state);
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index cba537c99e43..54c874493c57 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -172,8 +172,8 @@ void drm_bridge_detach(struct drm_bridge *bridge)
  */
 
 /**
- * drm_bridge_mode_fixup - fixup proposed mode for all bridges in the
- *encoder chain
+ * drm_bridge_chain_mode_fixup - fixup proposed mode for all bridges in the
+ *  encoder chain
  * @bridge: bridge control structure
  * @mode: desired mode to be set for the bridge
  * @adjusted_mode: updated mode that works for this bridge
@@ -186,9 +186,9 @@ void drm_bridge_detach(struct drm_bridge *bridge)
  * 

[PATCH v3 18/21] drm/bridge: panel: Propage bus format/flags

2019-10-23 Thread Boris Brezillon
So that the previous bridge element in the chain knows which input
format the panel bridge expects.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* Adjust things to match the new bus-format negotiation approach
* Use drm_atomic_helper_bridge_propagate_bus_fmt
* Don't implement ->atomic_check() (the core now takes care of bus
  flags propagation)

Changes in v2:
* Adjust things to match the new bus-format negotiation approach
---
 drivers/gpu/drm/bridge/panel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index f4e293e7cf64..a70c363a2bd0 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -127,6 +127,7 @@ static const struct drm_bridge_funcs 
panel_bridge_bridge_funcs = {
.enable = panel_bridge_enable,
.disable = panel_bridge_disable,
.post_disable = panel_bridge_post_disable,
+   .atomic_get_input_bus_fmts = drm_atomic_helper_bridge_propagate_bus_fmt,
 };
 
 /**
-- 
2.21.0

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

[PATCH v3 19/21] drm/panel: simple: Add support for Toshiba LTA089AC29000 panel

2019-10-23 Thread Boris Brezillon
Add a new entry for the Toshiba LTA089AC29000 panel.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* None
---
 drivers/gpu/drm/panel/panel-simple.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 5d487686d25c..27c92b44bd95 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2937,6 +2937,39 @@ static const struct panel_desc toshiba_lt089ac29000 = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
 };
 
+static const struct drm_display_mode toshiba_lta089ac29000_mode = {
+   .clock = 79500,
+   .hdisplay = 1280,
+   .hsync_start = 1280 + 192,
+   .hsync_end = 1280 + 192 + 128,
+   .htotal = 1280 + 192 + 128 + 64,
+   .vdisplay = 768,
+   .vsync_start = 768 + 20,
+   .vsync_end = 768 + 20 + 7,
+   .vtotal = 768 + 20 + 7 + 3,
+   .vrefresh = 60,
+};
+
+static const struct panel_desc toshiba_lta089ac29000 = {
+   .modes = _lta089ac29000_mode,
+   .num_modes = 1,
+   .size = {
+   .width = 194,
+   .height = 116,
+   },
+   /*
+* FIXME:
+* The panel supports 2 bus formats: jeida-24 and jeida-18. The
+* mode is selected through the 8b6b_SEL pin. If anyone ever needs
+* support for jeida-18 we should probably parse the 'data-mapping'
+* property.
+* In the unlikely event where 8b6b_SEL is connected to a GPIO, we'd
+* need a new infra to allow bus format negotiation at the panel level.
+*/
+   .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA,
+   .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
+};
+
 static const struct drm_display_mode tpk_f07a_0102_mode = {
.clock = 33260,
.hdisplay = 800,
@@ -3392,6 +3425,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "toshiba,lt089ac29000",
.data = _lt089ac29000,
+   }, {
+   .compatible = "toshiba,lta089ac29000",
+   .data = _lta089ac29000,
}, {
.compatible = "tpk,f07a-0102",
.data = _f07a_0102,
-- 
2.21.0

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

[PATCH v3 01/21] drm/vc4: Declare the DSI encoder as a bridge element

2019-10-23 Thread Boris Brezillon
Encoder drivers will progressively transition to the drm_bridge
interface in place of the drm_encoder one.

Let's start with the VC4 driver, and use the ->pre_{enable,disable}()
hooks to get rid of the hack resetting encoder->bridge.next (which was
needed to control the encoder/bridge enable/disable sequence).

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* Embed a drm_bridge object in vc4_dsi since drm_encoder no longer has
  a dummy bridge

Changes in v2:
* New patch (replaces "drm/vc4: Get rid of the dsi->bridge field")
---
 drivers/gpu/drm/vc4/vc4_dsi.c | 88 +--
 1 file changed, 52 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index c9ba83ed49b9..49f8a313e759 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -498,7 +498,11 @@ struct vc4_dsi {
 
struct mipi_dsi_host dsi_host;
struct drm_encoder *encoder;
-   struct drm_bridge *bridge;
+
+   /* Embed a bridge object so we can implement bridge funcs instead of
+* encoder ones.
+*/
+   struct drm_bridge bridge;
 
void __iomem *regs;
 
@@ -543,6 +547,11 @@ struct vc4_dsi {
struct debugfs_regset32 regset;
 };
 
+static inline struct vc4_dsi *bridge_to_vc4_dsi(struct drm_bridge *bridge)
+{
+   return container_of(bridge, struct vc4_dsi, bridge);
+}
+
 #define host_to_dsi(host) container_of(host, struct vc4_dsi, dsi_host)
 
 static inline void
@@ -747,16 +756,11 @@ dsi_esc_timing(u32 ns)
return DIV_ROUND_UP(ns, ESC_TIME_NS);
 }
 
-static void vc4_dsi_encoder_disable(struct drm_encoder *encoder)
+static void vc4_dsi_bridge_post_disable(struct drm_bridge *bridge)
 {
-   struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
-   struct vc4_dsi *dsi = vc4_encoder->dsi;
+   struct vc4_dsi *dsi = bridge_to_vc4_dsi(bridge);
struct device *dev = >pdev->dev;
 
-   drm_bridge_disable(dsi->bridge);
-   vc4_dsi_ulps(dsi, true);
-   drm_bridge_post_disable(dsi->bridge);
-
clk_disable_unprepare(dsi->pll_phy_clock);
clk_disable_unprepare(dsi->escape_clock);
clk_disable_unprepare(dsi->pixel_clock);
@@ -764,6 +768,13 @@ static void vc4_dsi_encoder_disable(struct drm_encoder 
*encoder)
pm_runtime_put(dev);
 }
 
+static void vc4_dsi_bridge_disable(struct drm_bridge *bridge)
+{
+   struct vc4_dsi *dsi = bridge_to_vc4_dsi(bridge);
+
+   vc4_dsi_ulps(dsi, true);
+}
+
 /* Extends the mode's blank intervals to handle BCM2835's integer-only
  * DSI PLL divider.
  *
@@ -777,12 +788,11 @@ static void vc4_dsi_encoder_disable(struct drm_encoder 
*encoder)
  * higher-than-expected clock rate to the panel, but that's what the
  * firmware does too.
  */
-static bool vc4_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
-  const struct drm_display_mode *mode,
-  struct drm_display_mode *adjusted_mode)
+static bool vc4_dsi_bridge_mode_fixup(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode,
+ struct drm_display_mode *adjusted_mode)
 {
-   struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
-   struct vc4_dsi *dsi = vc4_encoder->dsi;
+   struct vc4_dsi *dsi = bridge_to_vc4_dsi(bridge);
struct clk *phy_parent = clk_get_parent(dsi->pll_phy_clock);
unsigned long parent_rate = clk_get_rate(phy_parent);
unsigned long pixel_clock_hz = mode->clock * 1000;
@@ -816,11 +826,11 @@ static bool vc4_dsi_encoder_mode_fixup(struct drm_encoder 
*encoder,
return true;
 }
 
-static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
+static void vc4_dsi_bridge_pre_enable(struct drm_bridge *bridge)
 {
+   struct drm_encoder *encoder = bridge->encoder;
struct drm_display_mode *mode = >crtc->state->adjusted_mode;
-   struct vc4_dsi_encoder *vc4_encoder = to_vc4_dsi_encoder(encoder);
-   struct vc4_dsi *dsi = vc4_encoder->dsi;
+   struct vc4_dsi *dsi = bridge_to_vc4_dsi(bridge);
struct device *dev = >pdev->dev;
bool debug_dump_regs = false;
unsigned long hs_clock;
@@ -1054,8 +1064,12 @@ static void vc4_dsi_encoder_enable(struct drm_encoder 
*encoder)
}
 
vc4_dsi_ulps(dsi, false);
+}
 
-   drm_bridge_pre_enable(dsi->bridge);
+static void vc4_dsi_bridge_enable(struct drm_bridge *bridge)
+{
+   struct vc4_dsi *dsi = bridge_to_vc4_dsi(bridge);
+   bool debug_dump_regs = false;
 
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
DSI_PORT_WRITE(DISP0_CTRL,
@@ -1072,8 +1086,6 @@ static void vc4_dsi_encoder_enable(struct drm_encoder 
*encoder)
   DSI_DISP0_ENABLE);
}
 
-   drm_bridge_enable(dsi->bridge);
-
if (debug_dump_regs) {
struct drm_printer p = 

[PATCH v3 21/21] ARM: dts: imx: imx51-zii-rdu1: Fix the display pipeline definition

2019-10-23 Thread Boris Brezillon
The current definition does not represent the exact display pipeline we
have on the board: the LVDS panel is actually connected through a
parallel -> LVDS bridge. Let's fix that so the driver can select the
proper bus format on the CRTC end.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* None

Changes in v2:
* None
---
 arch/arm/boot/dts/imx51-zii-rdu1.dts | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx51-zii-rdu1.dts 
b/arch/arm/boot/dts/imx51-zii-rdu1.dts
index 3596060f52e7..3fb84ea7f993 100644
--- a/arch/arm/boot/dts/imx51-zii-rdu1.dts
+++ b/arch/arm/boot/dts/imx51-zii-rdu1.dts
@@ -95,6 +95,28 @@
reg = <1>;
 
display_out: endpoint {
+   remote-endpoint = <_encoder_in>;
+   };
+   };
+   };
+
+   lvds-encoder {
+   compatible = "lvds-encoder";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   bus-width = <24>;
+   lvds_encoder_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   data-mapping = "jeida-24";
+   lvds_encoder_out: endpoint {
remote-endpoint = <_in>;
};
};
@@ -110,7 +132,7 @@
 
port {
panel_in: endpoint {
-   remote-endpoint = <_out>;
+   remote-endpoint = <_encoder_out>;
};
};
};
-- 
2.21.0

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

[PATCH v3 05/21] drm/bridge: Introduce drm_bridge_chain_get_next_bridge()

2019-10-23 Thread Boris Brezillon
And use it in drivers accessing the bridge->next field directly.
This is part of our attempt to make the bridge chain a double-linked list
based on the generic list helpers.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* Inline drm_bridge_chain_get_next_bridge() (Suggested by Laurent)

Changes in v2:
* Kill the last/first helpers (they're not really needed)
* Drop the !bridge || !bridge->encoder test
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c |  3 ++-
 drivers/gpu/drm/mediatek/mtk_hdmi.c |  6 --
 drivers/gpu/drm/omapdrm/omap_drv.c  |  4 ++--
 drivers/gpu/drm/omapdrm/omap_encoder.c  |  3 ++-
 drivers/gpu/drm/vc4/vc4_dsi.c   |  4 +++-
 include/drm/drm_bridge.h| 13 +
 6 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 3915f50b005e..005c67894b78 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1593,9 +1593,10 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host 
*host,
  struct mipi_dsi_device *device)
 {
struct exynos_dsi *dsi = host_to_dsi(host);
-   struct drm_bridge *out_bridge = dsi->bridge.next;
struct drm_device *drm = dsi->encoder.dev;
+   struct drm_bridge *out_bridge;
 
+   out_bridge = drm_bridge_chain_get_next_bridge(>bridge);
if (dsi->panel) {
mutex_lock(>mode_config.mutex);
exynos_dsi_disable(>bridge);
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c 
b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index ea68b5adccbe..cfaa5aab8876 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1238,16 +1238,18 @@ static int mtk_hdmi_conn_mode_valid(struct 
drm_connector *conn,
struct drm_display_mode *mode)
 {
struct mtk_hdmi *hdmi = hdmi_ctx_from_conn(conn);
+   struct drm_bridge *next_bridge;
 
dev_dbg(hdmi->dev, "xres=%d, yres=%d, refresh=%d, intl=%d clock=%d\n",
mode->hdisplay, mode->vdisplay, mode->vrefresh,
!!(mode->flags & DRM_MODE_FLAG_INTERLACE), mode->clock * 1000);
 
-   if (hdmi->bridge.next) {
+   next_bridge = drm_bridge_chain_get_next_bridge(>bridge);
+   if (next_bridge) {
struct drm_display_mode adjusted_mode;
 
drm_mode_copy(_mode, mode);
-   if (!drm_bridge_chain_mode_fixup(hdmi->bridge.next, mode,
+   if (!drm_bridge_chain_mode_fixup(next_bridge, mode,
 _mode))
return MODE_BAD;
}
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
b/drivers/gpu/drm/omapdrm/omap_drv.c
index b3e22c890c51..865164fe28dc 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -217,8 +217,8 @@ static int omap_display_id(struct omap_dss_device *output)
} else if (output->bridge) {
struct drm_bridge *bridge = output->bridge;
 
-   while (bridge->next)
-   bridge = bridge->next;
+   while (drm_bridge_chain_get_next_bridge(bridge))
+   bridge = drm_bridge_chain_get_next_bridge(bridge);
 
node = bridge->of_node;
} else if (output->panel) {
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c 
b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 24bbe9f2a32e..8ca54081997e 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -126,7 +126,8 @@ static void omap_encoder_mode_set(struct drm_encoder 
*encoder,
for (dssdev = output; dssdev; dssdev = dssdev->next)
omap_encoder_update_videomode_flags(, dssdev->bus_flags);
 
-   for (bridge = output->bridge; bridge; bridge = bridge->next) {
+   for (bridge = output->bridge; bridge;
+bridge = drm_bridge_chain_get_next_bridge(bridge)) {
if (!bridge->timings)
continue;
 
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 49f8a313e759..49c47185aff0 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -1644,8 +1644,10 @@ static void vc4_dsi_unbind(struct device *dev, struct 
device *master,
struct drm_device *drm = dev_get_drvdata(master);
struct vc4_dev *vc4 = to_vc4_dev(drm);
struct vc4_dsi *dsi = dev_get_drvdata(dev);
+   struct drm_bridge *bridge;
 
-   if (dsi->bridge.next)
+   bridge = drm_bridge_chain_get_next_bridge(>bridge);
+   if (bridge)
pm_runtime_disable(dev);
 
vc4_dsi_encoder_destroy(dsi->encoder);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 726435baf4ad..8aeba83fcf31 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -409,6 +409,19 @@ struct drm_bridge 

[PATCH v3 14/21] drm/bridge: Add the necessary bits to support bus format negotiation

2019-10-23 Thread Boris Brezillon
drm_bridge_state is extended to describe the input and output bus
configuration. This bus configuration is exposed through the
drm_bus_cfg struct which contains 2 properties: the bus format and
the bus flags.

Bus format negotiation is automated by the core, drivers just have
to implement the ->atomic_get_{output,input}_bus_fmts() hooks if they
want to take part to this negotiation. Negotiation happens in reserve
order, starting from the last element of the chain (the one directly
connected to the display) up to the first element of the chain (the one
connected to the encoder).
During this negotiation all supported formats are tested until we find
one that works, meaning that the formats array should be in decreasing
preference order (assuming the driver has a preference order).

Note that the bus format negotiation works even if some elements in the
chain don't implement the ->atomic_get_{output,input}_bus_fmts() hooks.
In that case, the core advertises only MEDIA_BUS_FMT_FIXED and let
the previous bridge element decide what to do (most of the time, bridge
drivers will pick a default bus format of extract this piece of
information from somewhere else, like a FW property).

Bus flags negotiation is left to drivers which can simply propagate the
flags from the input of the next bridge element if there's no conversion
done inside the bridge, or tweak them if the bridge does some kind of
signal inversion.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* Fix the commit message (Reported by Laurent)
* Document the fact that bus formats should not be directly modified by
  drivers (Suggested by Laurent)
* Document the fact that format order matters (Suggested by Laurent)
* Propagate bus flags by default
* Document the fact that drivers can tweak bus flags if needed
* Let ->atomic_get_{output,input}_bus_fmts() allocate the bus format
  array (Suggested by Laurent)
* Add a drm_atomic_helper_bridge_propagate_bus_fmt()
* Mandate that bridge drivers return accurate input_fmts even if they
  are known to be the first element in the bridge chain

Changes in v2:
* Rework things to support more complex use cases
---
 drivers/gpu/drm/drm_bridge.c | 257 ++-
 include/drm/drm_bridge.h | 106 +++
 2 files changed, 362 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 990e056296bd..6022fb3d406a 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -641,13 +641,251 @@ static int drm_atomic_bridge_check(struct drm_bridge 
*bridge,
return 0;
 }
 
+static int select_bus_fmt_recursive(struct drm_bridge *first,
+   struct drm_bridge *cur,
+   struct drm_crtc_state *crtc_state,
+   struct drm_connector_state *conn_state,
+   u32 out_bus_fmt)
+{
+   struct drm_bridge_state *cur_state;
+   unsigned int num_in_bus_fmts, i;
+   struct drm_bridge *prev;
+   u32 *in_bus_fmts;
+   int ret;
+
+   prev = drm_bridge_chain_get_prev_bridge(cur);
+   cur_state = drm_atomic_get_new_bridge_state(crtc_state->state, cur);
+   if (WARN_ON(!cur_state))
+   return -EINVAL;
+
+   /*
+* Bus format negotiation is not supported by this bridge, let's pass
+* MEDIA_BUS_FMT_FIXED to the previous bridge in the chain and hope
+* that it can handle this situation gracefully (by providing
+* appropriate default values).
+*/
+   if (!cur->funcs->atomic_get_input_bus_fmts) {
+   if (cur != first) {
+   ret = select_bus_fmt_recursive(first, prev, crtc_state,
+  conn_state,
+  MEDIA_BUS_FMT_FIXED);
+   if (ret)
+   return ret;
+   }
+
+   cur_state->input_bus_cfg.fmt = MEDIA_BUS_FMT_FIXED;
+   cur_state->output_bus_cfg.fmt = out_bus_fmt;
+   return 0;
+   }
+
+   in_bus_fmts = cur->funcs->atomic_get_input_bus_fmts(cur, cur_state,
+   crtc_state,
+   conn_state,
+   out_bus_fmt,
+   _in_bus_fmts);
+   if (!num_in_bus_fmts)
+   return -ENOTSUPP;
+   else if (!in_bus_fmts)
+   return -ENOMEM;
+
+   if (first == cur) {
+   cur_state->input_bus_cfg.fmt = in_bus_fmts[0];
+   cur_state->output_bus_cfg.fmt = out_bus_fmt;
+   kfree(in_bus_fmts);
+   return 0;
+   }
+
+   for (i = 0; i < num_in_bus_fmts; i++) {
+   ret = select_bus_fmt_recursive(first, prev, 

[PATCH v3 03/21] drm/exynos: Declare the DSI encoder as a bridge element

2019-10-23 Thread Boris Brezillon
Encoder drivers will progressively transition to the drm_bridge
interface in place of the drm_encoder one.

Converting the Exynos DSI encoder driver to this approach allows us to
use the ->pre_{enable,disable}()  hooks and get rid of the hack
resetting encoder->bridge.next (which was needed to control the
encoder/bridge enable/disable sequence).

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* Embed a drm_bridge object in exynos_dsi since drm_encoder no longer
  has a dummy bridge

Changes in v2:
* New patch (replacement for "drm/exynos: Get rid of exynos_dsi->out_bridge")
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 89 +++--
 1 file changed, 55 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 72726f2c7a9f..3915f50b005e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -252,10 +252,10 @@ struct exynos_dsi_driver_data {
 
 struct exynos_dsi {
struct drm_encoder encoder;
+   struct drm_bridge bridge;
struct mipi_dsi_host dsi_host;
struct drm_connector connector;
struct drm_panel *panel;
-   struct drm_bridge *out_bridge;
struct device *dev;
 
void __iomem *reg_base;
@@ -291,6 +291,11 @@ static inline struct exynos_dsi *encoder_to_dsi(struct 
drm_encoder *e)
return container_of(e, struct exynos_dsi, encoder);
 }
 
+static inline struct exynos_dsi *bridge_to_dsi(struct drm_bridge *b)
+{
+   return container_of(b, struct exynos_dsi, bridge);
+}
+
 enum reg_idx {
DSIM_STATUS_REG,/* Status register */
DSIM_SWRST_REG, /* Software reset register */
@@ -1374,25 +1379,38 @@ static void exynos_dsi_unregister_te_irq(struct 
exynos_dsi *dsi)
}
 }
 
-static void exynos_dsi_enable(struct drm_encoder *encoder)
+static void exynos_dsi_pre_enable(struct drm_bridge *bridge)
 {
-   struct exynos_dsi *dsi = encoder_to_dsi(encoder);
+   struct exynos_dsi *dsi = bridge_to_dsi(bridge);
int ret;
 
if (dsi->state & DSIM_STATE_ENABLED)
return;
 
pm_runtime_get_sync(dsi->dev);
-   dsi->state |= DSIM_STATE_ENABLED;
 
if (dsi->panel) {
ret = drm_panel_prepare(dsi->panel);
if (ret < 0)
goto err_put_sync;
-   } else {
-   drm_bridge_pre_enable(dsi->out_bridge);
}
 
+   dsi->state |= DSIM_STATE_ENABLED;
+   return;
+
+err_put_sync:
+   pm_runtime_put(dsi->dev);
+}
+
+static void exynos_dsi_enable(struct drm_bridge *bridge)
+{
+   struct exynos_dsi *dsi = bridge_to_dsi(bridge);
+   int ret;
+
+   if (!(dsi->state & DSIM_STATE_ENABLED) ||
+   (dsi->state & DSIM_STATE_VIDOUT_AVAILABLE))
+   return;
+
exynos_dsi_set_display_mode(dsi);
exynos_dsi_set_display_enable(dsi, true);
 
@@ -1400,8 +1418,6 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
ret = drm_panel_enable(dsi->panel);
if (ret < 0)
goto err_display_disable;
-   } else {
-   drm_bridge_enable(dsi->out_bridge);
}
 
dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
@@ -1410,28 +1426,30 @@ static void exynos_dsi_enable(struct drm_encoder 
*encoder)
 err_display_disable:
exynos_dsi_set_display_enable(dsi, false);
drm_panel_unprepare(dsi->panel);
-
-err_put_sync:
-   dsi->state &= ~DSIM_STATE_ENABLED;
-   pm_runtime_put(dsi->dev);
 }
 
-static void exynos_dsi_disable(struct drm_encoder *encoder)
+static void exynos_dsi_disable(struct drm_bridge *bridge)
 {
-   struct exynos_dsi *dsi = encoder_to_dsi(encoder);
+   struct exynos_dsi *dsi = bridge_to_dsi(bridge);
+
+   if (!(dsi->state & DSIM_STATE_VIDOUT_AVAILABLE))
+   return;
+
+   drm_panel_disable(dsi->panel);
+   exynos_dsi_set_display_enable(dsi, false);
+   dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
+}
+
+static void exynos_dsi_post_disable(struct drm_bridge *bridge)
+{
+   struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 
if (!(dsi->state & DSIM_STATE_ENABLED))
return;
 
-   dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
-
-   drm_panel_disable(dsi->panel);
-   drm_bridge_disable(dsi->out_bridge);
-   exynos_dsi_set_display_enable(dsi, false);
drm_panel_unprepare(dsi->panel);
-   drm_bridge_post_disable(dsi->out_bridge);
-   dsi->state &= ~DSIM_STATE_ENABLED;
pm_runtime_put_sync(dsi->dev);
+   dsi->state &= ~DSIM_STATE_ENABLED;
 }
 
 static enum drm_connector_status
@@ -1499,9 +1517,11 @@ static int exynos_dsi_create_connector(struct 
drm_encoder *encoder)
return 0;
 }
 
-static const struct drm_encoder_helper_funcs exynos_dsi_encoder_helper_funcs = 
{
+static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
+   .pre_enable = 

[PATCH v3 10/21] drm/bridge: Clarify the atomic enable/disable hooks semantics

2019-10-23 Thread Boris Brezillon
The [pre_]enable/[post_]disable hooks are passed the old atomic state.
Update the doc and rename the arguments to make it clear.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* New patch
---
 drivers/gpu/drm/drm_bridge.c | 24 
 include/drm/drm_bridge.h | 16 
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index c53966767782..ca74bfe028c9 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -447,7 +447,7 @@ EXPORT_SYMBOL(drm_bridge_chain_enable);
 /**
  * drm_atomic_bridge_chain_disable - disables all bridges in the encoder chain
  * @bridge: bridge control structure
- * @state: atomic state being committed
+ * @old_state: old atomic state
  *
  * Calls _bridge_funcs.atomic_disable (falls back on
  * _bridge_funcs.disable) op for all the bridges in the encoder chain,
@@ -457,7 +457,7 @@ EXPORT_SYMBOL(drm_bridge_chain_enable);
  * Note: the bridge passed should be the one closest to the encoder
  */
 void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge,
-struct drm_atomic_state *state)
+struct drm_atomic_state *old_state)
 {
struct drm_encoder *encoder;
struct drm_bridge *iter;
@@ -469,7 +469,7 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge 
*bridge,
list_for_each_entry_reverse(iter, >bridge_chain,
chain_node) {
if (iter->funcs->atomic_disable)
-   iter->funcs->atomic_disable(iter, state);
+   iter->funcs->atomic_disable(iter, old_state);
else if (iter->funcs->disable)
iter->funcs->disable(iter);
 
@@ -483,7 +483,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable);
  * drm_atomic_bridge_chain_post_disable - cleans up after disabling all bridges
  *   in the encoder chain
  * @bridge: bridge control structure
- * @state: atomic state being committed
+ * @old_state: old atomic state
  *
  * Calls _bridge_funcs.atomic_post_disable (falls back on
  * _bridge_funcs.post_disable) op for all the bridges in the encoder chain,
@@ -493,7 +493,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable);
  * Note: the bridge passed should be the one closest to the encoder
  */
 void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
- struct drm_atomic_state *state)
+ struct drm_atomic_state *old_state)
 {
struct drm_encoder *encoder;
 
@@ -504,7 +504,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge 
*bridge,
list_for_each_entry_from(bridge, >bridge_chain,
 chain_node) {
if (bridge->funcs->atomic_post_disable)
-   bridge->funcs->atomic_post_disable(bridge, state);
+   bridge->funcs->atomic_post_disable(bridge, old_state);
else if (bridge->funcs->post_disable)
bridge->funcs->post_disable(bridge);
}
@@ -515,7 +515,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable);
  * drm_atomic_bridge_chain_pre_enable - prepares for enabling all bridges in
  * the encoder chain
  * @bridge: bridge control structure
- * @state: atomic state being committed
+ * @old_state: old atomic state
  *
  * Calls _bridge_funcs.atomic_pre_enable (falls back on
  * _bridge_funcs.pre_enable) op for all the bridges in the encoder chain,
@@ -525,7 +525,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable);
  * Note: the bridge passed should be the one closest to the encoder
  */
 void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge,
-   struct drm_atomic_state *state)
+   struct drm_atomic_state *old_state)
 {
struct drm_encoder *encoder;
struct drm_bridge *iter;
@@ -537,7 +537,7 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge 
*bridge,
list_for_each_entry_reverse(iter, >encoder->bridge_chain,
chain_node) {
if (iter->funcs->atomic_pre_enable)
-   iter->funcs->atomic_pre_enable(iter, state);
+   iter->funcs->atomic_pre_enable(iter, old_state);
else if (iter->funcs->pre_enable)
iter->funcs->pre_enable(iter);
 
@@ -550,7 +550,7 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable);
 /**
  * drm_atomic_bridge_chain_enable - enables all bridges in the encoder chain
  * @bridge: bridge control structure
- * @state: atomic state being committed
+ * @old_state: old atomic state
  *
  * Calls _bridge_funcs.atomic_enable (falls back on
  * _bridge_funcs.enable) op for all the bridges in the 

[PATCH v3 17/21] dt-bindings: display: bridge: lvds-transmitter: Add new props

2019-10-23 Thread Boris Brezillon
Add the data-mapping property to describe the output bus format and
the bus-width property to describe the input bus format.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* New patch
---
 .../bindings/display/bridge/lvds-transmitter.txt| 13 +
 1 file changed, 13 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt 
b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index 60091db5dfa5..7b43b6f20279 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -36,6 +36,19 @@ graph bindings specified in 
Documentation/devicetree/bindings/graph.txt.
 - Video port 0 for parallel input
 - Video port 1 for LVDS output
 
+Optional port 0 node properties:
+
+- bus-width: number of data lines use to transmit the RGB data.
+Can be 18 or 24.
+
+Optional port 1 node properties:
+
+- data-mapping: see Documentation/devicetree/bindings/display/panel/lvds.yaml
+   for more details about this LVDS data-mapping property.
+   Supported values:
+   "jeida-18"
+   "jeida-24"
+   "vesa-24"
 
 Example
 ---
-- 
2.21.0

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

[PATCH v3 20/21] dt-bindings: display: panel: Add the LTA089AC29000 variant

2019-10-23 Thread Boris Brezillon
The LTA089AC29000 and LT089AC29000 are not exactly the same. Let's add
a new compatible for the LTA variant.

Signed-off-by: Boris Brezillon 
---
 .../bindings/display/panel/toshiba,lt089ac29000.txt  | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git 
a/Documentation/devicetree/bindings/display/panel/toshiba,lt089ac29000.txt 
b/Documentation/devicetree/bindings/display/panel/toshiba,lt089ac29000.txt
index 89826116628c..26ebfa098966 100644
--- a/Documentation/devicetree/bindings/display/panel/toshiba,lt089ac29000.txt
+++ b/Documentation/devicetree/bindings/display/panel/toshiba,lt089ac29000.txt
@@ -1,7 +1,10 @@
 Toshiba 8.9" WXGA (1280x768) TFT LCD panel
 
 Required properties:
-- compatible: should be "toshiba,lt089ac29000"
+- compatible: should be one of the following
+ "toshiba,lt089ac29000"
+ "toshiba,lta089ac29000"
+
 - power-supply: as specified in the base binding
 
 This binding is compatible with the simple-panel binding, which is specified
-- 
2.21.0

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

[PATCH v3 13/21] drm/bridge: Add the drm_bridge_chain_get_prev_bridge() helper

2019-10-23 Thread Boris Brezillon
Will be useful for bridge drivers that want to do bus format
negotiation with their neighbours.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* Inline drm_bridge_chain_get_prev_bridge()
* Fix the doc

Changes in v2:
* Fix the kerneldoc
* Drop the !bridge || !bridge->encoder check
---
 include/drm/drm_bridge.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 2beb1ef9a733..3fb518494640 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -526,6 +526,22 @@ drm_bridge_chain_get_next_bridge(struct drm_bridge *bridge)
return list_next_entry(bridge, chain_node);
 }
 
+/**
+ * drm_bridge_chain_get_prev_bridge() - Get the previous bridge in the chain
+ * @bridge: bridge object
+ *
+ * RETURNS:
+ * the previous bridge in the chain, or NULL if @bridge is the first.
+ */
+static inline struct drm_bridge *
+drm_bridge_chain_get_prev_bridge(struct drm_bridge *bridge)
+{
+   if (list_is_first(>chain_node, >encoder->bridge_chain))
+   return NULL;
+
+   return list_prev_entry(bridge, chain_node);
+}
+
 /**
  * drm_bridge_chain_get_first_bridge() - Get the first bridge in the chain
  * @encoder: encoder object
-- 
2.21.0

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

[PATCH v3 15/21] drm/imx: pd: Use bus format/flags provided by the bridge when available

2019-10-23 Thread Boris Brezillon
Now that bridges can expose the bus format/flags they expect, we can
use those instead of the relying on the display_info provided by the
connector (which is only valid if the encoder is directly connected
to bridge element driving the panel/display).

We also explicitly expose the bus formats supported by our encoder by
filling encoder->output_bus_caps with proper info.

Signed-off-by: Boris Brezillon 
---
Hi Philipp,

I think I addressed all your comments except the addition of
SYNC_DRIVE_POSEDGE/NEGEDGE flags, which would require changing
ipu_crtc_mode_set_nofb() to take those flags into account or
turning those flags into their PIXDATA counterpart. If you don't mind,
I'd like to leave that for later.

Regards,

Boris

Changes in v3 (all suggested by Philipp):
* Adjust to match core changes
* Propagate output format to input format
* Pick a default value when output_fmt = _FIXED
* Add missing BGR888 and GBR888 fmts to imx_pd_bus_fmts[]

Changes in v2:
* Adjust things to match the new bus-format negotiation infra
---
 drivers/gpu/drm/imx/parallel-display.c | 174 +
 1 file changed, 150 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/imx/parallel-display.c 
b/drivers/gpu/drm/imx/parallel-display.c
index 35518e5de356..28ae083df3ca 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -24,6 +24,7 @@
 struct imx_parallel_display {
struct drm_connector connector;
struct drm_encoder encoder;
+   struct drm_bridge bridge;
struct device *dev;
void *edid;
int edid_len;
@@ -31,7 +32,7 @@ struct imx_parallel_display {
u32 bus_flags;
struct drm_display_mode mode;
struct drm_panel *panel;
-   struct drm_bridge *bridge;
+   struct drm_bridge *next_bridge;
 };
 
 static inline struct imx_parallel_display *con_to_imxpd(struct drm_connector 
*c)
@@ -44,6 +45,11 @@ static inline struct imx_parallel_display 
*enc_to_imxpd(struct drm_encoder *e)
return container_of(e, struct imx_parallel_display, encoder);
 }
 
+static inline struct imx_parallel_display *bridge_to_imxpd(struct drm_bridge 
*b)
+{
+   return container_of(b, struct imx_parallel_display, bridge);
+}
+
 static int imx_pd_connector_get_modes(struct drm_connector *connector)
 {
struct imx_parallel_display *imxpd = con_to_imxpd(connector);
@@ -89,37 +95,151 @@ static struct drm_encoder *imx_pd_connector_best_encoder(
return >encoder;
 }
 
-static void imx_pd_encoder_enable(struct drm_encoder *encoder)
+static void imx_pd_bridge_enable(struct drm_bridge *bridge)
 {
-   struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
+   struct imx_parallel_display *imxpd = bridge_to_imxpd(bridge);
 
drm_panel_prepare(imxpd->panel);
drm_panel_enable(imxpd->panel);
 }
 
-static void imx_pd_encoder_disable(struct drm_encoder *encoder)
+static void imx_pd_bridge_disable(struct drm_bridge *bridge)
 {
-   struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
+   struct imx_parallel_display *imxpd = bridge_to_imxpd(bridge);
 
drm_panel_disable(imxpd->panel);
drm_panel_unprepare(imxpd->panel);
 }
 
-static int imx_pd_encoder_atomic_check(struct drm_encoder *encoder,
-  struct drm_crtc_state *crtc_state,
-  struct drm_connector_state *conn_state)
+static const u32 imx_pd_bus_fmts[] = {
+   MEDIA_BUS_FMT_RGB888_1X24,
+   MEDIA_BUS_FMT_BGR888_1X24,
+   MEDIA_BUS_FMT_GBR888_1X24,
+   MEDIA_BUS_FMT_RGB666_1X18,
+   MEDIA_BUS_FMT_RGB666_1X24_CPADHI,
+   MEDIA_BUS_FMT_RGB565_1X16,
+};
+
+static u32 *
+imx_pd_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
+struct drm_bridge_state *bridge_state,
+struct drm_crtc_state *crtc_state,
+struct drm_connector_state *conn_state,
+unsigned int *num_output_fmts)
+{
+   struct drm_display_info *di = _state->connector->display_info;
+   struct imx_parallel_display *imxpd = bridge_to_imxpd(bridge);
+   u32 *output_fmts;
+
+   if (!imxpd->bus_format && !di->num_bus_formats)
+   *num_output_fmts = ARRAY_SIZE(imx_pd_bus_fmts);
+   else
+   *num_output_fmts = 1;
+
+   output_fmts = kcalloc(*num_output_fmts, sizeof(*output_fmts),
+ GFP_KERNEL);
+   if (!output_fmts)
+   return NULL;
+
+   if (!imxpd->bus_format && di->num_bus_formats)
+   output_fmts[0] = di->bus_formats[0];
+   else if (!imxpd->bus_format)
+   memcpy(output_fmts, imx_pd_bus_fmts,
+  ARRAY_SIZE(imx_pd_bus_fmts));
+   else
+   output_fmts[0] = imxpd->bus_format;
+
+   return output_fmts;
+}
+
+static u32 *

[PATCH v3 16/21] drm/bridge: lvds-encoder: Implement basic bus format negotiation

2019-10-23 Thread Boris Brezillon
Some LVDS encoder might support several input/output bus formats. Add
a way to describe the one used on a specific design by adding optional
'data-mapping' properties to the input/output ports.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* Use bus-width for the rgb24/rgb18 distinction
* Adjust code to match core changes
* Get rid of of_get_data_mapping()
* Don't implement ->atomic_check() (the core now takes care of bus
  flags propagation)

Changes in v2:
* Make the bus-format negotiation logic more generic
---
 drivers/gpu/drm/bridge/lvds-encoder.c | 72 +++
 1 file changed, 72 insertions(+)

diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c 
b/drivers/gpu/drm/bridge/lvds-encoder.c
index e2132a8d5106..a2a8f7f4ac97 100644
--- a/drivers/gpu/drm/bridge/lvds-encoder.c
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -16,6 +17,8 @@ struct lvds_encoder {
struct drm_bridge bridge;
struct drm_bridge *panel_bridge;
struct gpio_desc *powerdown_gpio;
+   u32 output_fmt;
+   u32 input_fmt;
 };
 
 static int lvds_encoder_attach(struct drm_bridge *bridge)
@@ -48,10 +51,40 @@ static void lvds_encoder_disable(struct drm_bridge *bridge)
gpiod_set_value_cansleep(lvds_encoder->powerdown_gpio, 1);
 }
 
+static u32 *lvds_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+  struct drm_bridge_state 
*bridge_state,
+  struct drm_crtc_state *crtc_state,
+  struct drm_connector_state 
*conn_state,
+  u32 output_fmt,
+  unsigned int *num_input_fmts)
+{
+   struct lvds_encoder *lvds_encoder = container_of(bridge,
+struct lvds_encoder,
+bridge);
+   u32 *input_fmts;
+
+   if (output_fmt == MEDIA_BUS_FMT_FIXED ||
+   output_fmt == lvds_encoder->output_fmt)
+   *num_input_fmts = 1;
+   else
+   *num_input_fmts = 0;
+
+   if (!*num_input_fmts)
+   return NULL;
+
+   input_fmts = kcalloc(*num_input_fmts, sizeof(*input_fmts), GFP_KERNEL);
+   if (!input_fmts)
+   return NULL;
+
+   input_fmts[0] = lvds_encoder->input_fmt;
+   return input_fmts;
+}
+
 static struct drm_bridge_funcs funcs = {
.attach = lvds_encoder_attach,
.enable = lvds_encoder_enable,
.disable = lvds_encoder_disable,
+   .atomic_get_input_bus_fmts = lvds_atomic_get_input_bus_fmts,
 };
 
 static int lvds_encoder_probe(struct platform_device *pdev)
@@ -62,11 +95,16 @@ static int lvds_encoder_probe(struct platform_device *pdev)
struct device_node *panel_node;
struct drm_panel *panel;
struct lvds_encoder *lvds_encoder;
+   const char *output_data_mapping = NULL;
+   u32 input_bus_width = 0;
 
lvds_encoder = devm_kzalloc(dev, sizeof(*lvds_encoder), GFP_KERNEL);
if (!lvds_encoder)
return -ENOMEM;
 
+   lvds_encoder->input_fmt = MEDIA_BUS_FMT_FIXED;
+   lvds_encoder->output_fmt = MEDIA_BUS_FMT_FIXED;
+
lvds_encoder->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
   GPIOD_OUT_HIGH);
if (IS_ERR(lvds_encoder->powerdown_gpio)) {
@@ -77,6 +115,25 @@ static int lvds_encoder_probe(struct platform_device *pdev)
return err;
}
 
+   port = of_graph_get_port_by_id(dev->of_node, 0);
+   if (!port) {
+   dev_dbg(dev, "port 0 not found\n");
+   return -ENXIO;
+   }
+
+   of_node_put(port);
+
+   if (of_property_read_u32(port, "bus-width", _bus_width)) {
+   lvds_encoder->input_fmt = MEDIA_BUS_FMT_FIXED;
+   } else if (input_bus_width == 18) {
+   lvds_encoder->input_fmt = MEDIA_BUS_FMT_RGB666_1X18;
+   } else if (input_bus_width == 24) {
+   lvds_encoder->input_fmt = MEDIA_BUS_FMT_RGB888_1X24;
+   } else {
+   dev_dbg(dev, "unsupported bus-width)\n");
+   return -ENOTSUPP;
+   }
+
/* Locate the panel DT node. */
port = of_graph_get_port_by_id(dev->of_node, 1);
if (!port) {
@@ -84,6 +141,21 @@ static int lvds_encoder_probe(struct platform_device *pdev)
return -ENXIO;
}
 
+   of_property_read_string(port, "data-mapping", _data_mapping);
+   if (!output_data_mapping) {
+   lvds_encoder->output_fmt = MEDIA_BUS_FMT_FIXED;
+   } else if (!strcmp(output_data_mapping, "jeida-18")) {
+   lvds_encoder->output_fmt = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG;
+   } else if (!strcmp(output_data_mapping, "jeida-24")) {
+   

[PATCH v3 12/21] drm/bridge: Add an ->atomic_check() hook

2019-10-23 Thread Boris Brezillon
So that bridge drivers have a way to check/reject an atomic operation.
The drm_atomic_bridge_chain_check() (which is just a wrapper around
the ->atomic_check() hook) is called in place of
drm_bridge_chain_mode_fixup() (when ->atomic_check() is not implemented,
the core falls back on ->mode_fixup(), so the behavior should stay
the same for existing bridge drivers).

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* None

Changes in v2:
* Clarify the fact that ->atomic_check() is replacing ->mode_fixup()
---
 drivers/gpu/drm/drm_atomic_helper.c | 12 +++---
 drivers/gpu/drm/drm_bridge.c| 62 +
 include/drm/drm_bridge.h| 29 +-
 3 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index de985ba7ce2d..1d0a19511a0d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -437,12 +437,12 @@ mode_fixup(struct drm_atomic_state *state)
funcs = encoder->helper_private;
 
bridge = drm_bridge_chain_get_first_bridge(encoder);
-   ret = drm_bridge_chain_mode_fixup(bridge,
-   _crtc_state->mode,
-   _crtc_state->adjusted_mode);
-   if (!ret) {
-   DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
-   return -EINVAL;
+   ret = drm_atomic_bridge_chain_check(bridge,
+   new_crtc_state,
+   new_conn_state);
+   if (ret) {
+   DRM_DEBUG_ATOMIC("Bridge atomic check failed\n");
+   return ret;
}
 
if (funcs && funcs->atomic_check) {
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 377866e3214f..990e056296bd 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -615,6 +615,68 @@ void drm_atomic_bridge_chain_enable(struct drm_bridge 
*bridge,
 }
 EXPORT_SYMBOL(drm_atomic_bridge_chain_enable);
 
+static int drm_atomic_bridge_check(struct drm_bridge *bridge,
+  struct drm_crtc_state *crtc_state,
+  struct drm_connector_state *conn_state)
+{
+   if (bridge->funcs->atomic_check) {
+   struct drm_bridge_state *bridge_state;
+   int ret;
+
+   bridge_state = 
drm_atomic_get_new_bridge_state(crtc_state->state,
+  bridge);
+   if (WARN_ON(!bridge_state))
+   return -EINVAL;
+
+   ret = bridge->funcs->atomic_check(bridge, bridge_state,
+ crtc_state, conn_state);
+   if (ret)
+   return ret;
+   } else if (bridge->funcs->mode_fixup) {
+   if (!bridge->funcs->mode_fixup(bridge, _state->mode,
+  _state->adjusted_mode))
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+/**
+ * drm_atomic_bridge_chain_check() - Do an atomic check on the bridge chain
+ * @bridge: bridge control structure
+ * @crtc_state: new CRTC state
+ * @conn_state: new connector state
+ *
+ * Calls _bridge_funcs.atomic_check() (falls back on
+ * _bridge_funcs.mode_fixup()) op for all the bridges in the encoder chain,
+ * starting from the last bridge to the first. These are called before calling
+ * _encoder_helper_funcs.atomic_check()
+ *
+ * RETURNS:
+ * 0 on success, a negative error code on failure
+ */
+int drm_atomic_bridge_chain_check(struct drm_bridge *bridge,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+   struct drm_encoder *encoder = bridge->encoder;
+   struct drm_bridge *iter;
+
+   list_for_each_entry_reverse(iter, >bridge_chain, chain_node) {
+   int ret;
+
+   ret = drm_atomic_bridge_check(iter, crtc_state, conn_state);
+   if (ret)
+   return ret;
+
+   if (iter == bridge)
+   break;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_atomic_bridge_chain_check);
+
 /**
  * drm_atomic_helper_bridge_destroy_state() - Default destroy state helper
  * @bridge: the bridge this state refers to
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index b1f557d8dba9..2beb1ef9a733 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -127,7 +127,9 @@ struct drm_bridge_funcs {
 * this function passes all other callbacks must succeed for this
 * configuration.
 *
-* The @mode_fixup callback is optional.
+* The mode_fixup callback is optional. 

[PATCH v3 11/21] drm/bridge: Patch atomic hooks to take a drm_bridge_state

2019-10-23 Thread Boris Brezillon
This way the drm_bridge_funcs interface is consistent with the rest of
the subsystem.

The only driver implementing those hooks (analogix DP) is patched too.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* Old state clarification moved to a separate patch

Changes in v2:
* Pass the old bridge state
---
 .../drm/bridge/analogix/analogix_dp_core.c| 12 ++--
 drivers/gpu/drm/drm_bridge.c  | 61 +++
 include/drm/drm_bridge.h  | 24 
 3 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index bb411fe52ae8..e438e757f2ce 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1290,8 +1290,9 @@ struct drm_crtc *analogix_dp_get_new_crtc(struct 
analogix_dp_device *dp,
 }
 
 static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
-struct drm_atomic_state *state)
+struct drm_bridge_state 
*bstate)
 {
+   struct drm_atomic_state *state = bstate->base.state;
struct analogix_dp_device *dp = bridge->driver_private;
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state;
@@ -1367,8 +1368,9 @@ static int analogix_dp_set_bridge(struct 
analogix_dp_device *dp)
 }
 
 static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
-struct drm_atomic_state *state)
+struct drm_bridge_state *bstate)
 {
+   struct drm_atomic_state *state = bstate->base.state;
struct analogix_dp_device *dp = bridge->driver_private;
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state;
@@ -1441,8 +1443,9 @@ static void analogix_dp_bridge_disable(struct drm_bridge 
*bridge)
 }
 
 static void analogix_dp_bridge_atomic_disable(struct drm_bridge *bridge,
- struct drm_atomic_state *state)
+ struct drm_bridge_state *bstate)
 {
+   struct drm_atomic_state *state = bstate->base.state;
struct analogix_dp_device *dp = bridge->driver_private;
struct drm_crtc *crtc;
struct drm_crtc_state *new_crtc_state = NULL;
@@ -1465,8 +1468,9 @@ static void analogix_dp_bridge_atomic_disable(struct 
drm_bridge *bridge,
 
 static
 void analogix_dp_bridge_atomic_post_disable(struct drm_bridge *bridge,
-   struct drm_atomic_state *state)
+   struct drm_bridge_state *bstate)
 {
+   struct drm_atomic_state *state = bstate->base.state;
struct analogix_dp_device *dp = bridge->driver_private;
struct drm_crtc *crtc;
struct drm_crtc_state *new_crtc_state;
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index ca74bfe028c9..377866e3214f 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -468,10 +468,19 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge 
*bridge,
encoder = bridge->encoder;
list_for_each_entry_reverse(iter, >bridge_chain,
chain_node) {
-   if (iter->funcs->atomic_disable)
-   iter->funcs->atomic_disable(iter, old_state);
-   else if (iter->funcs->disable)
+   if (iter->funcs->atomic_disable) {
+   struct drm_bridge_state *old_bridge_state;
+
+   old_bridge_state =
+   drm_atomic_get_old_bridge_state(old_state,
+   iter);
+   if (WARN_ON(!old_bridge_state))
+   return;
+
+   iter->funcs->atomic_disable(iter, old_bridge_state);
+   } else if (iter->funcs->disable) {
iter->funcs->disable(iter);
+   }
 
if (iter == bridge)
break;
@@ -503,10 +512,20 @@ void drm_atomic_bridge_chain_post_disable(struct 
drm_bridge *bridge,
encoder = bridge->encoder;
list_for_each_entry_from(bridge, >bridge_chain,
 chain_node) {
-   if (bridge->funcs->atomic_post_disable)
-   bridge->funcs->atomic_post_disable(bridge, old_state);
-   else if (bridge->funcs->post_disable)
+   if (bridge->funcs->atomic_post_disable) {
+   struct drm_bridge_state *old_bridge_state;
+
+   old_bridge_state =
+   drm_atomic_get_old_bridge_state(old_state,
+   bridge);
+   if 

[PATCH v3 09/21] drm/bridge: Add a drm_bridge_state object

2019-10-23 Thread Boris Brezillon
One of the last remaining objects to not have its atomic state.

This is being motivated by our attempt to support runtime bus-format
negotiation between elements of the bridge chain.
This patch just paves the road for such a feature by adding a new
drm_bridge_state object inheriting from drm_private_obj so we can
re-use some of the existing state initialization/tracking logic.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* None

Changes in v2:
* Use drm_for_each_bridge_in_chain()
* Rename helpers to be more consistent with the rest of the DRM API
* Improve/fix the doc
---
 drivers/gpu/drm/drm_atomic.c|  39 +++
 drivers/gpu/drm/drm_atomic_helper.c |  20 
 drivers/gpu/drm/drm_bridge.c| 168 +++-
 include/drm/drm_atomic.h|   3 +
 include/drm/drm_bridge.h| 118 +++
 5 files changed, 343 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 419381abbdd1..6c249ce39380 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -30,6 +30,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1010,6 +1011,44 @@ static void drm_atomic_connector_print_state(struct 
drm_printer *p,
connector->funcs->atomic_print_state(p, state);
 }
 
+/**
+ * drm_atomic_add_encoder_bridges - add bridges attached to an encoder
+ * @state: atomic state
+ * @encoder: DRM encoder
+ *
+ * This function adds all bridges attached to @encoder. This is needed to add
+ * bridge states to @state and make them available when
+ * _funcs.atomic_{check,pre_enable,enable,disable_post_disable}() are
+ * called
+ *
+ * Returns:
+ * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is EDEADLK
+ * then the w/w mutex code has detected a deadlock and the entire atomic
+ * sequence must be restarted. All other errors are fatal.
+ */
+int
+drm_atomic_add_encoder_bridges(struct drm_atomic_state *state,
+  struct drm_encoder *encoder)
+{
+   struct drm_bridge_state *bridge_state;
+   struct drm_bridge *bridge;
+
+   if (!encoder)
+   return 0;
+
+   DRM_DEBUG_ATOMIC("Adding all bridges for [encoder:%d:%s] to %p\n",
+encoder->base.id, encoder->name, state);
+
+   drm_for_each_bridge_in_chain(encoder, bridge) {
+   bridge_state = drm_atomic_get_bridge_state(state, bridge);
+   if (IS_ERR(bridge_state))
+   return PTR_ERR(bridge_state);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_atomic_add_encoder_bridges);
+
 /**
  * drm_atomic_add_affected_connectors - add connectors for crtc
  * @state: atomic state
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index f02ddffd4960..de985ba7ce2d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -730,6 +730,26 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
return ret;
}
 
+   /*
+* Iterate over all connectors again, and add all affected bridges to
+* the state.
+*/
+   for_each_oldnew_connector_in_state(state, connector,
+  old_connector_state,
+  new_connector_state, i) {
+   struct drm_encoder *encoder;
+
+   encoder = old_connector_state->best_encoder;
+   ret = drm_atomic_add_encoder_bridges(state, encoder);
+   if (ret)
+   return ret;
+
+   encoder = new_connector_state->best_encoder;
+   ret = drm_atomic_add_encoder_bridges(state, encoder);
+   if (ret)
+   return ret;
+   }
+
ret = mode_valid(state);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index c5cf8a9c4237..c53966767782 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 
@@ -89,6 +90,38 @@ void drm_bridge_remove(struct drm_bridge *bridge)
 }
 EXPORT_SYMBOL(drm_bridge_remove);
 
+static struct drm_private_state *
+drm_bridge_atomic_duplicate_priv_state(struct drm_private_obj *obj)
+{
+   struct drm_bridge *bridge = drm_priv_to_bridge(obj);
+   struct drm_bridge_state *state;
+
+   if (bridge->funcs->atomic_duplicate_state)
+   state = bridge->funcs->atomic_duplicate_state(bridge);
+   else
+   state = drm_atomic_helper_bridge_duplicate_state(bridge);
+
+   return >base;
+}
+
+static void
+drm_bridge_atomic_destroy_priv_state(struct drm_private_obj *obj,
+struct drm_private_state *s)
+{
+   struct drm_bridge_state *state = drm_priv_to_bridge_state(s);
+   struct drm_bridge *bridge = 

[PATCH v3 06/21] drm: Stop accessing encoder->bridge directly

2019-10-23 Thread Boris Brezillon
We are about to replace the single-linked bridge list by a double-linked
one based on list.h, leading to the suppression of the encoder->bridge
field. But before we can do that we must provide a
drm_bridge_chain_get_first_bridge() bridge helper and patch all drivers
and core helpers to use it instead of directly accessing encoder->bridge.

Signed-off-by: Boris Brezillon 
---
 drivers/gpu/drm/drm_atomic_helper.c| 25 +
 drivers/gpu/drm/drm_encoder.c  |  3 ++-
 drivers/gpu/drm/drm_probe_helper.c |  4 +++-
 drivers/gpu/drm/msm/edp/edp_bridge.c   | 10 --
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 11 ---
 include/drm/drm_bridge.h   | 15 +++
 6 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index cf678be58fa4..f02ddffd4960 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -419,6 +419,7 @@ mode_fixup(struct drm_atomic_state *state)
for_each_new_connector_in_state(state, connector, new_conn_state, i) {
const struct drm_encoder_helper_funcs *funcs;
struct drm_encoder *encoder;
+   struct drm_bridge *bridge;
 
WARN_ON(!!new_conn_state->best_encoder != 
!!new_conn_state->crtc);
 
@@ -435,7 +436,8 @@ mode_fixup(struct drm_atomic_state *state)
encoder = new_conn_state->best_encoder;
funcs = encoder->helper_private;
 
-   ret = drm_bridge_chain_mode_fixup(encoder->bridge,
+   bridge = drm_bridge_chain_get_first_bridge(encoder);
+   ret = drm_bridge_chain_mode_fixup(bridge,
_crtc_state->mode,
_crtc_state->adjusted_mode);
if (!ret) {
@@ -493,6 +495,7 @@ static enum drm_mode_status mode_valid_path(struct 
drm_connector *connector,
struct drm_crtc *crtc,
const struct drm_display_mode *mode)
 {
+   struct drm_bridge *bridge;
enum drm_mode_status ret;
 
ret = drm_encoder_mode_valid(encoder, mode);
@@ -502,7 +505,8 @@ static enum drm_mode_status mode_valid_path(struct 
drm_connector *connector,
return ret;
}
 
-   ret = drm_bridge_chain_mode_valid(encoder->bridge, mode);
+   bridge = drm_bridge_chain_get_first_bridge(encoder);
+   ret = drm_bridge_chain_mode_valid(bridge, mode);
if (ret != MODE_OK) {
DRM_DEBUG_ATOMIC("[BRIDGE] mode_valid() failed\n");
return ret;
@@ -985,6 +989,7 @@ disable_outputs(struct drm_device *dev, struct 
drm_atomic_state *old_state)
for_each_oldnew_connector_in_state(old_state, connector, 
old_conn_state, new_conn_state, i) {
const struct drm_encoder_helper_funcs *funcs;
struct drm_encoder *encoder;
+   struct drm_bridge *bridge;
 
/* Shut down everything that's in the changeset and currently
 * still on. So need to check the old, saved state. */
@@ -1021,7 +1026,8 @@ disable_outputs(struct drm_device *dev, struct 
drm_atomic_state *old_state)
 * Each encoder has at most one connector (since we always steal
 * it away), so we won't call disable hooks twice.
 */
-   drm_atomic_bridge_chain_disable(encoder->bridge, old_state);
+   bridge = drm_bridge_chain_get_first_bridge(encoder);
+   drm_atomic_bridge_chain_disable(bridge, old_state);
 
/* Right function depends upon target state. */
if (funcs) {
@@ -1035,7 +1041,7 @@ disable_outputs(struct drm_device *dev, struct 
drm_atomic_state *old_state)
funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
}
 
-   drm_atomic_bridge_chain_post_disable(encoder->bridge,
+   drm_atomic_bridge_chain_post_disable(bridge,
 old_state);
}
 
@@ -1190,6 +1196,7 @@ crtc_set_mode(struct drm_device *dev, struct 
drm_atomic_state *old_state)
const struct drm_encoder_helper_funcs *funcs;
struct drm_encoder *encoder;
struct drm_display_mode *mode, *adjusted_mode;
+   struct drm_bridge *bridge;
 
if (!new_conn_state->best_encoder)
continue;
@@ -1217,8 +1224,8 @@ crtc_set_mode(struct drm_device *dev, struct 
drm_atomic_state *old_state)
funcs->mode_set(encoder, mode, adjusted_mode);
}
 
-   drm_bridge_chain_mode_set(encoder->bridge, mode,
- adjusted_mode);
+   bridge = drm_bridge_chain_get_first_bridge(encoder);
+   

[PATCH v3 00/21] drm: Add support for bus-format negotiation

2019-10-23 Thread Boris Brezillon
This patch series aims at adding support for runtime bus-format
negotiation between all elements of the
'encoder -> bridges -> connector/display' section of the pipeline.

In order to support that, we need drm bridges to fully take part in the
atomic state validation process, which requires adding a
drm_bridge_state and a new drm_bridge_funcs.atomic_check() hook.
Once those basic building blocks are in place, we can add new hooks to
allow bus format negotiation (those are called just before
->atomic_check()). The bus format selection is done at runtime by
testing all possible combinations across the whole bridge chain until
one is reported to work.

Major changes since v2:
* Get rid of the dummy bridge embedded in drm_encoder and let encoder
  drivers provide their own bridge element
* Clarify APIs and improve the doc
* Propagate bus flags by default

Major changes since the RFC:

* Add a dummy bridge to the drm_encoder object so that vc4 and exynos
  DSI drivers can implement the pre_enable/post_disable hooks instead
  of manually setting encoder->bridge to NULL to control the
  enable/disable sequence. This change is also a first step towards
  drm_bridge/drm_encoder unification. New encoder drivers should
  stop implementing drm_encoder_helper_funcs and switch to
  drm_bridge_funcs. Existing drivers can be converted progressively
  (already have a branch where I started converting some of them [1])
* rework the bus format negotiation to give more control to bridge
  drivers in the selection process (driver can select at runtime which
  input bus format they support for a specific output bus format based
  on any information available in the connector, crtc and bridge state.

A more detailed changelog is provided in each patch.

This patch series is also available here [2].

Thanks,

Boris

[1]https://github.com/bbrezillon/linux-0day/commits/drm-encoder-bridge
[2]https://github.com/bbrezillon/linux-0day/commits/drm-bridge-busfmt-v3

*** BLURB HERE ***

Boris Brezillon (21):
  drm/vc4: Declare the DSI encoder as a bridge element
  drm/exynos: Don't reset bridge->next
  drm/exynos: Declare the DSI encoder as a bridge element
  drm/bridge: Rename bridge helpers targeting a bridge chain
  drm/bridge: Introduce drm_bridge_chain_get_next_bridge()
  drm: Stop accessing encoder->bridge directly
  drm/bridge: Make the bridge chain a double-linked list
  drm/bridge: Add the drm_for_each_bridge_in_chain() helper
  drm/bridge: Add a drm_bridge_state object
  drm/bridge: Clarify the atomic enable/disable hooks semantics
  drm/bridge: Patch atomic hooks to take a drm_bridge_state
  drm/bridge: Add an ->atomic_check() hook
  drm/bridge: Add the drm_bridge_chain_get_prev_bridge() helper
  drm/bridge: Add the necessary bits to support bus format negotiation
  drm/imx: pd: Use bus format/flags provided by the bridge when
available
  drm/bridge: lvds-encoder: Implement basic bus format negotiation
  dt-bindings: display: bridge: lvds-transmitter: Add new props
  drm/bridge: panel: Propage bus format/flags
  drm/panel: simple: Add support for Toshiba LTA089AC29000 panel
  dt-bindings: display: panel: Add the LTA089AC29000 variant
  ARM: dts: imx: imx51-zii-rdu1: Fix the display pipeline definition

 .../display/bridge/lvds-transmitter.txt   |  13 +
 .../display/panel/toshiba,lt089ac29000.txt|   5 +-
 arch/arm/boot/dts/imx51-zii-rdu1.dts  |  24 +-
 .../drm/bridge/analogix/analogix_dp_core.c|  12 +-
 drivers/gpu/drm/bridge/lvds-encoder.c |  72 ++
 drivers/gpu/drm/bridge/panel.c|   1 +
 drivers/gpu/drm/drm_atomic.c  |  39 +
 drivers/gpu/drm/drm_atomic_helper.c   |  54 +-
 drivers/gpu/drm/drm_bridge.c  | 800 +++---
 drivers/gpu/drm/drm_encoder.c |  15 +-
 drivers/gpu/drm/drm_probe_helper.c|   4 +-
 drivers/gpu/drm/exynos/exynos_dp.c|   1 -
 drivers/gpu/drm/exynos/exynos_drm_dsi.c   |  90 +-
 drivers/gpu/drm/imx/parallel-display.c| 174 +++-
 drivers/gpu/drm/mediatek/mtk_hdmi.c   |   8 +-
 drivers/gpu/drm/msm/edp/edp_bridge.c  |  10 +-
 drivers/gpu/drm/omapdrm/omap_drv.c|   4 +-
 drivers/gpu/drm/omapdrm/omap_encoder.c|   3 +-
 drivers/gpu/drm/panel/panel-simple.c  |  36 +
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c|  11 +-
 drivers/gpu/drm/vc4/vc4_dsi.c |  90 +-
 include/drm/drm_atomic.h  |   3 +
 include/drm/drm_bridge.h  | 396 -
 include/drm/drm_encoder.h |   9 +-
 24 files changed, 1588 insertions(+), 286 deletions(-)

-- 
2.21.0

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

[PATCH v3 08/21] drm/bridge: Add the drm_for_each_bridge_in_chain() helper

2019-10-23 Thread Boris Brezillon
To iterate over all bridges attached to a specific encoder.

Suggested-by: Laurent Pinchart 
Signed-off-by: Boris Brezillon 
---
Changes in v3:
* None

Changes in v2:
* New patch
---
 include/drm/drm_bridge.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 3ab16c95e59e..238e84ab63a3 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -441,6 +441,17 @@ drm_bridge_chain_get_first_bridge(struct drm_encoder 
*encoder)
struct drm_bridge, chain_node);
 }
 
+/**
+ * for_each_bridge_in_chain() - Iterate over all bridges present in a chain
+ * @encoder: the encoder to iterate bridges on
+ * @bridge: a bridge pointer updated to point to the current bridge at each
+ * iteration
+ *
+ * Iterate over all bridges present in the bridge chain attached to @encoder.
+ */
+#define drm_for_each_bridge_in_chain(encoder, bridge)  \
+   list_for_each_entry(bridge, &(encoder)->bridge_chain, chain_node)
+
 bool drm_bridge_chain_mode_fixup(struct drm_bridge *bridge,
 const struct drm_display_mode *mode,
 struct drm_display_mode *adjusted_mode);
-- 
2.21.0

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

[PATCH v3 07/21] drm/bridge: Make the bridge chain a double-linked list

2019-10-23 Thread Boris Brezillon
So that each element in the chain can easily access its predecessor.
This will be needed to support bus format negotiation between elements
of the bridge chain.

Signed-off-by: Boris Brezillon 
---
Changes in v3:
* None

Changes in v2:
* Adjust things to the "dummy encoder bridge" change (patch 2 in this
  series)
---
 drivers/gpu/drm/drm_bridge.c  | 171 ++
 drivers/gpu/drm/drm_encoder.c |  16 +---
 include/drm/drm_bridge.h  |  12 ++-
 include/drm/drm_encoder.h |   9 +-
 4 files changed, 135 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 54c874493c57..c5cf8a9c4237 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -55,7 +55,7 @@
  * just provide additional hooks to get the desired output at the end of the
  * encoder chain.
  *
- * Bridges can also be chained up using the _bridge.next pointer.
+ * Bridges can also be chained up using the _bridge.chain_node field.
  *
  * Both legacy CRTC helpers and the new atomic modeset helpers support bridges.
  */
@@ -114,6 +114,7 @@ EXPORT_SYMBOL(drm_bridge_remove);
 int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
  struct drm_bridge *previous)
 {
+   LIST_HEAD(tmp_list);
int ret;
 
if (!encoder || !bridge)
@@ -127,6 +128,7 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
drm_bridge *bridge,
 
bridge->dev = encoder->dev;
bridge->encoder = encoder;
+   list_add_tail(>chain_node, _list);
 
if (bridge->funcs->attach) {
ret = bridge->funcs->attach(bridge);
@@ -138,9 +140,9 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
drm_bridge *bridge,
}
 
if (previous)
-   previous->next = bridge;
+   list_splice(_list, >chain_node);
else
-   encoder->bridge = bridge;
+   list_splice(_list, >bridge_chain);
 
return 0;
 }
@@ -157,6 +159,7 @@ void drm_bridge_detach(struct drm_bridge *bridge)
if (bridge->funcs->detach)
bridge->funcs->detach(bridge);
 
+   list_del(>chain_node);
bridge->dev = NULL;
 }
 
@@ -190,18 +193,22 @@ bool drm_bridge_chain_mode_fixup(struct drm_bridge 
*bridge,
 const struct drm_display_mode *mode,
 struct drm_display_mode *adjusted_mode)
 {
-   bool ret = true;
+   struct drm_encoder *encoder;
 
if (!bridge)
return true;
 
-   if (bridge->funcs->mode_fixup)
-   ret = bridge->funcs->mode_fixup(bridge, mode, adjusted_mode);
+   encoder = bridge->encoder;
+   list_for_each_entry_from(bridge, >bridge_chain,
+chain_node) {
+   if (!bridge->funcs->mode_fixup)
+   continue;
 
-   ret = ret && drm_bridge_chain_mode_fixup(bridge->next, mode,
-adjusted_mode);
+   if (!bridge->funcs->mode_fixup(bridge, mode, adjusted_mode))
+   return false;
+   }
 
-   return ret;
+   return true;
 }
 EXPORT_SYMBOL(drm_bridge_chain_mode_fixup);
 
@@ -224,18 +231,24 @@ enum drm_mode_status
 drm_bridge_chain_mode_valid(struct drm_bridge *bridge,
const struct drm_display_mode *mode)
 {
-   enum drm_mode_status ret = MODE_OK;
+   struct drm_encoder *encoder;
 
if (!bridge)
-   return ret;
+   return MODE_OK;
+
+   encoder = bridge->encoder;
+   list_for_each_entry_from(bridge, >bridge_chain, chain_node) {
+   enum drm_mode_status ret;
+
+   if (!bridge->funcs->mode_valid)
+   continue;
 
-   if (bridge->funcs->mode_valid)
ret = bridge->funcs->mode_valid(bridge, mode);
+   if (ret != MODE_OK)
+   return ret;
+   }
 
-   if (ret != MODE_OK)
-   return ret;
-
-   return drm_bridge_chain_mode_valid(bridge->next, mode);
+   return MODE_OK;
 }
 EXPORT_SYMBOL(drm_bridge_chain_mode_valid);
 
@@ -251,13 +264,20 @@ EXPORT_SYMBOL(drm_bridge_chain_mode_valid);
  */
 void drm_bridge_chain_disable(struct drm_bridge *bridge)
 {
+   struct drm_encoder *encoder;
+   struct drm_bridge *iter;
+
if (!bridge)
return;
 
-   drm_bridge_chain_disable(bridge->next);
+   encoder = bridge->encoder;
+   list_for_each_entry_reverse(iter, >bridge_chain, chain_node) {
+   if (iter->funcs->disable)
+   iter->funcs->disable(iter);
 
-   if (bridge->funcs->disable)
-   bridge->funcs->disable(bridge);
+   if (iter == bridge)
+   break;
+   }
 }
 EXPORT_SYMBOL(drm_bridge_chain_disable);
 
@@ -274,13 +294,16 @@ 

[PATCH v3 02/21] drm/exynos: Don't reset bridge->next

2019-10-23 Thread Boris Brezillon
bridge->next is only points to the new bridge if drm_bridge_attach()
succeeds. No need to reset it manually here.

Note that this change is part of the attempt to make the bridge chain
a double-linked list. In order to do that we must patch all drivers
manipulating the bridge->next field.

Signed-off-by: Boris Brezillon 
Reviewed-by: Laurent Pinchart 
---
Changes in v2:
* Add Laurent's R-b (I'd like to have a R-b from the DRM exynos
  maintainers before applying that one)
---
 drivers/gpu/drm/exynos/exynos_dp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp.c 
b/drivers/gpu/drm/exynos/exynos_dp.c
index 1e6aa24bf45e..4785885c0f4f 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -110,7 +110,6 @@ static int exynos_dp_bridge_attach(struct 
analogix_dp_plat_data *plat_data,
if (ret) {
DRM_DEV_ERROR(dp->dev,
  "Failed to attach bridge to drm\n");
-   bridge->next = NULL;
return ret;
}
}
-- 
2.21.0

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

Re: [PATCH V9 6/6] backlight: qcom-wled: Add auto string detection logic

2019-10-23 Thread Daniel Thompson
On Wed, Oct 23, 2019 at 12:37:03PM +0530, Kiran Gunda wrote:
> The auto string detection algorithm checks if the current WLED
> sink configuration is valid. It tries enabling every sink and
> checks if the OVP fault is observed. Based on this information
> it detects and enables the valid sink configuration.
> Auto calibration will be triggered when the OVP fault interrupts
> are seen frequently thereby it tries to fix the sink configuration.
> 
> The auto-detection also kicks in when the connected LED string
> of the display-backlight malfunctions (because of damage) and
> requires the damaged string to be turned off to prevent the
> complete panel and/or board from being damaged.
> 
> Signed-off-by: Kiran Gunda 

Reviewed-by: Daniel Thompson 

> ---
>  drivers/video/backlight/qcom-wled.c | 400 
> +++-
>  1 file changed, 394 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/backlight/qcom-wled.c 
> b/drivers/video/backlight/qcom-wled.c
> index 658b1e0..33b6007 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
> @@ -17,19 +17,29 @@
>  #define WLED_MAX_STRINGS 4
>  
>  #define WLED_DEFAULT_BRIGHTNESS  2048
> -
> +#define WLED_SOFT_START_DLY_US   1
>  #define WLED3_SINK_REG_BRIGHT_MAX0xFFF
>  
>  /* WLED3/WLED4 control registers */
> +#define WLED3_CTRL_REG_FAULT_STATUS  0x08
> +#define  WLED3_CTRL_REG_ILIM_FAULT_BIT   BIT(0)
> +#define  WLED3_CTRL_REG_OVP_FAULT_BITBIT(1)
> +#define  WLED4_CTRL_REG_SC_FAULT_BIT BIT(2)
> +
> +#define WLED3_CTRL_REG_INT_RT_STS0x10
> +#define  WLED3_CTRL_REG_OVP_FAULT_STATUS BIT(1)
> +
>  #define WLED3_CTRL_REG_MOD_EN0x46
>  #define  WLED3_CTRL_REG_MOD_EN_MASK  BIT(7)
>  #define  WLED3_CTRL_REG_MOD_EN_SHIFT 7
>  
> +#define WLED3_CTRL_REG_FEEDBACK_CONTROL  0x48
> +
>  #define WLED3_CTRL_REG_FREQ  0x4c
>  #define  WLED3_CTRL_REG_FREQ_MASKGENMASK(3, 0)
>  
>  #define WLED3_CTRL_REG_OVP   0x4d
> -#define  WLED3_CTRL_REG_OVP_MASK GENMASK(1, 0)
> +#define  WLED3_CTRL_REG_OVP_MASK GENMASK(1, 0)
>  
>  #define WLED3_CTRL_REG_ILIMIT0x4e
>  #define  WLED3_CTRL_REG_ILIMIT_MASK  GENMASK(2, 0)
> @@ -119,6 +129,7 @@ struct wled_config {
>   bool ext_gen;
>   bool cabc;
>   bool external_pfet;
> + bool auto_detection_enabled;
>  };
>  
>  struct wled {
> @@ -127,17 +138,22 @@ struct wled {
>   struct regmap *regmap;
>   struct mutex lock;  /* Lock to avoid race from thread irq handler */
>   ktime_t last_short_event;
> + ktime_t start_ovp_fault_time;
>   u16 ctrl_addr;
>   u16 sink_addr;
>   u16 max_string_count;
> + u16 auto_detection_ovp_count;
>   u32 brightness;
>   u32 max_brightness;
>   u32 short_count;
> + u32 auto_detect_count;
>   bool disabled_by_short;
>   bool has_short_detect;
>   int short_irq;
> + int ovp_irq;
>  
>   struct wled_config cfg;
> + struct delayed_work ovp_work;
>   int (*wled_set_brightness)(struct wled *wled, u16 brightness);
>  };
>  
> @@ -182,6 +198,13 @@ static int wled4_set_brightness(struct wled *wled, u16 
> brightness)
>   return 0;
>  }
>  
> +static void wled_ovp_work(struct work_struct *work)
> +{
> + struct wled *wled = container_of(work,
> +  struct wled, ovp_work.work);
> + enable_irq(wled->ovp_irq);
> +}
> +
>  static int wled_module_enable(struct wled *wled, int val)
>  {
>   int rc;
> @@ -193,7 +216,25 @@ static int wled_module_enable(struct wled *wled, int val)
>   WLED3_CTRL_REG_MOD_EN,
>   WLED3_CTRL_REG_MOD_EN_MASK,
>   val << WLED3_CTRL_REG_MOD_EN_SHIFT);
> - return rc;
> + if (rc < 0)
> + return rc;
> +
> + if (wled->ovp_irq > 0) {
> + if (val) {
> + /*
> +  * The hardware generates a storm of spurious OVP
> +  * interrupts during soft start operations. So defer
> +  * enabling the IRQ for 10ms to ensure that the
> +  * soft start is complete.
> +  */
> + schedule_delayed_work(>ovp_work, HZ / 100);
> + } else {
> + if (!cancel_delayed_work_sync(>ovp_work))
> + disable_irq(wled->ovp_irq);
> + }
> + }
> +
> + return 0;
>  }
>  
>  static int wled_sync_toggle(struct wled *wled)
> @@ -300,6 +341,304 @@ static irqreturn_t 

Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks

2019-10-23 Thread Linus Walleij
On Wed, Oct 23, 2019 at 12:13 PM Daniel Vetter  wrote:

> Passing the wrong type feels icky, everywhere else we use the pipe as
> the first parameter. Spotted while discussing patches with Thomas
> Zimmermann.
>
> v2: Make xen compile correctly
>
> Acked-By: Thomas Zimmermann  (v1)
> Cc: Thomas Zimmermann 
> Cc: Noralf Trønnes 
> Cc: Gerd Hoffmann 
> Cc: Eric Anholt 
> Cc: Emil Velikov 
> Cc: virtualizat...@lists.linux-foundation.org
> Cc: Linus Walleij 
> Signed-off-by: Daniel Vetter 

Makes perfect sense.
Reviewed-by: Linus Walleij 

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

Re: [PATCH] drm/amd/amdgpu: make undeclared variables static

2019-10-23 Thread Harry Wentland
On 2019-10-19 3:24 a.m., Wambui Karuga wrote:
> Make the `amdgpu_lockup_timeout` and `amdgpu_exp_hw_support` variables
> static to remove the following sparse warnings:
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:103:19: warning: symbol 
> 'amdgpu_lockup_timeout' was not declared. Should it be static?

This should be declared in amdgpu.h. amdgpu is maintained on the
amd-staging-drm-next branch from
https://cgit.freedesktop.org/~agd5f/linux/?h=amd-staging-drm-next. Can
you check there?

> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:117:18: warning: symbol 
> 'amdgpu_exp_hw_support' was not declared. Should it be static?
> 
> Signed-off-by: Wambui Karuga 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 3fae1007143e..c5b3c0c9193b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -100,7 +100,7 @@ int amdgpu_disp_priority = 0;
>  int amdgpu_hw_i2c = 0;
>  int amdgpu_pcie_gen2 = -1;
>  int amdgpu_msi = -1;
> -char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENTH];
> +static char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENTH];
>  int amdgpu_dpm = -1;
>  int amdgpu_fw_load_type = -1;
>  int amdgpu_aspm = -1;
> @@ -114,7 +114,7 @@ int amdgpu_vm_block_size = -1;
>  int amdgpu_vm_fault_stop = 0;
>  int amdgpu_vm_debug = 0;
>  int amdgpu_vm_update_mode = -1;
> -int amdgpu_exp_hw_support = 0;
> +static int amdgpu_exp_hw_support;

This is indeed only used in this file but for consistency's sake it's
probably better to also declare it in amdgpu.h rather than make it
static here.

Harry

>  int amdgpu_dc = -1;
>  int amdgpu_sched_jobs = 32;
>  int amdgpu_sched_hw_submission = 2;
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: remove assignment for return value

2019-10-23 Thread Harry Wentland
On 2019-10-19 3:32 a.m., Wambui Karuga wrote:
> Remove unnecessary assignment for return value and have the
> function return the required value directly.
> Issue found by coccinelle:
> @@
> local idexpression ret;
> expression e;
> @@
> 
> -ret =
> +return
>  e;
> -return ret;
> 
> Signed-off-by: Wambui Karuga 

Thanks for your patch.

Reviewed-by: Harry Wentland 

Harry


> ---
>  drivers/gpu/drm/radeon/cik.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 62eab82a64f9..daff9a2af3be 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -221,9 +221,7 @@ int ci_get_temp(struct radeon_device *rdev)
>   else
>   actual_temp = temp & 0x1ff;
>  
> - actual_temp = actual_temp * 1000;
> -
> - return actual_temp;
> + return actual_temp * 1000;
>  }
>  
>  /* get temperature in millidegrees */
> @@ -239,9 +237,7 @@ int kv_get_temp(struct radeon_device *rdev)
>   else
>   actual_temp = 0;
>  
> - actual_temp = actual_temp * 1000;
> -
> - return actual_temp;
> + return actual_temp * 1000;
>  }
>  
>  /*
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amd/amdgpu: correct length misspelling

2019-10-23 Thread Harry Wentland
On 2019-10-19 3:34 a.m., Wambui Karuga wrote:
> Correct the "_LENTH" mispelling in the AMDGPU_MAX_TIMEOUT_PARAM_LENGTH
> constant.
> 
> Signed-off-by: Wambui Karuga 

This patch would be better sent in a patch set with the "make undeclared
variables static" patch. You can do that by providing a range to "git
format-patch". I usually call git format-patch with the -o parameter to
put all my patches in a directory. Then I can send it with "git
send-email *" in that directory.

Reviewed-by: Harry Wentland 

This won't apply cleanly without "make undeclared variables static".
Please see my comments on that patch and send a v2 for this one.

Harry

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index c5b3c0c9193b..aaab37833659 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -86,7 +86,7 @@
>  #define KMS_DRIVER_MINOR 34
>  #define KMS_DRIVER_PATCHLEVEL0
>  
> -#define AMDGPU_MAX_TIMEOUT_PARAM_LENTH   256
> +#define AMDGPU_MAX_TIMEOUT_PARAM_LENGTH  256
>  
>  int amdgpu_vram_limit = 0;
>  int amdgpu_vis_vram_limit = 0;
> @@ -100,7 +100,7 @@ int amdgpu_disp_priority = 0;
>  int amdgpu_hw_i2c = 0;
>  int amdgpu_pcie_gen2 = -1;
>  int amdgpu_msi = -1;
> -static char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENTH];
> +static char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENGTH];
>  int amdgpu_dpm = -1;
>  int amdgpu_fw_load_type = -1;
>  int amdgpu_aspm = -1;
> @@ -1327,9 +1327,9 @@ int amdgpu_device_get_job_timeout_settings(struct 
> amdgpu_device *adev)
>   adev->sdma_timeout = adev->video_timeout = adev->gfx_timeout;
>   adev->compute_timeout = MAX_SCHEDULE_TIMEOUT;
>  
> - if (strnlen(input, AMDGPU_MAX_TIMEOUT_PARAM_LENTH)) {
> + if (strnlen(input, AMDGPU_MAX_TIMEOUT_PARAM_LENGTH)) {
>   while ((timeout_setting = strsep(, ",")) &&
> - strnlen(timeout_setting, 
> AMDGPU_MAX_TIMEOUT_PARAM_LENTH)) {
> + strnlen(timeout_setting, 
> AMDGPU_MAX_TIMEOUT_PARAM_LENGTH)) {
>   ret = kstrtol(timeout_setting, 0, );
>   if (ret)
>   return ret;
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: pwm_bl: configure pwm only once per backlight toggle

2019-10-23 Thread Uwe Kleine-König
On Wed, Oct 23, 2019 at 11:23:11AM -0300, Fabio Estevam wrote:
> On Wed, Oct 23, 2019 at 11:16 AM Adam Ford  wrote:
> 
> > What is the plan to address the regression for 5.4?  I wasn't sure if
> > we're going to apply the i.mx fixes or temporarily revert the
> > offending patch, or something else. (or maybe nothing at all)
> 
> Yes, I do see the regression on a imx53 board with 5.4-rc too and also
> interested on a fix.

Thierry already sent a revert of my change to the list. We only
discussed the wording shortly and I expect that this revert will make it
into 5.4.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109887] [Vega10][powerplay] P7 gets reset to max_vddc (1.2V/1.25V) after applying any custom settings via pp_od_clk_voltage and/or pp_table

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109887

Stefan Springer  changed:

   What|Removed |Added

Summary|vega56  |[Vega10][powerplay] P7 gets
   |undervolting/overclocking   |reset to max_vddc
   |voltage issues  |(1.2V/1.25V) after applying
   ||any custom settings via
   ||pp_od_clk_voltage and/or
   ||pp_table

-- 
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 110347] pp_od_clk_voltage mV cap ignored

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110347

Stefan Springer  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |DUPLICATE

--- Comment #7 from Stefan Springer  ---


*** This bug has been marked as a duplicate of bug 109887 ***

-- 
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 1/2] drm/property: Enforce more lifetime rules

2019-10-23 Thread Daniel Vetter
Properties can't be attached after registering, userspace would get
confused (no one bothers to reprobe really).

- Add kerneldoc
- Enforce this with some checks. This needs a somewhat ugly check
  since connectors can be added later on, but we still need to attach
  all properties before they go public.

Note that we already enforce that properties themselves are created
before the entire device is registered.

Cc: Jani Nikula 
Cc: Rajat Jain 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_mode_object.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/drm_mode_object.c 
b/drivers/gpu/drm/drm_mode_object.c
index 6a23e36ed4fe..35c2719407a8 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -224,12 +224,26 @@ EXPORT_SYMBOL(drm_mode_object_get);
  * This attaches the given property to the modeset object with the given 
initial
  * value. Currently this function cannot fail since the properties are stored 
in
  * a statically sized array.
+ *
+ * Note that all properties must be attached before the object itself is
+ * registered and accessible from userspace.
  */
 void drm_object_attach_property(struct drm_mode_object *obj,
struct drm_property *property,
uint64_t init_val)
 {
int count = obj->properties->count;
+   struct drm_device *dev = property->dev;
+
+
+   if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
+   struct drm_connector *connector = obj_to_connector(obj);
+
+   WARN_ON(!dev->driver->load &&
+   connector->registration_state == 
DRM_CONNECTOR_REGISTERED);
+   } else {
+   WARN_ON(!dev->driver->load && dev->registered);
+   }
 
if (count == DRM_OBJECT_MAX_PROPERTY) {
WARN(1, "Failed to attach object property (type: 0x%x). Please "
-- 
2.23.0

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

[PATCH 2/2] drm/todo: Add entry to remove load/unload hooks

2019-10-23 Thread Daniel Vetter
They're midlayer, broken, and because of the old gunk, we can't fix
them. For examples see the various checks in drm_mode_object.c against
dev->registered, which cannot be enforced if the driver still uses the
load hook.

Unfortunately our biggest driver still uses load/unload, so this would
be really great to get fixed.

Cc: Alex Deucher 
Cc: Harry Wentland 
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/todo.rst | 17 +
 1 file changed, 17 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 73c51b5a0997..5a44f46380c2 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -351,6 +351,23 @@ connector register/unregister fixes
 
 Level: Intermediate
 
+Remove load/unload callbacks from all non-DRIVER_LEGACY drivers
+---
+
+The load/unload callbacks in struct _driver are very much midlayers, plus
+for historical reasons they get the ordering wrong (and we can't fix that)
+between setting up the _driver structure and calling drm_dev_register().
+
+- Rework drivers to no longer use the load/unload callbacks, directly coding 
the
+  load/unload sequence into the driver's probe function.
+
+- Once all non-DRIVER_LEGACY drivers are converted, disallow the load/unload
+  callbacks for all modern drivers.
+
+Contact: Daniel Vetter
+
+Level: Intermediate
+
 Core refactorings
 =
 
-- 
2.23.0

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

[Bug 109887] vega56 undervolting/overclocking voltage issues

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109887

--- Comment #14 from Stefan Springer  ---
*** Bug 110347 has been marked as a duplicate of this bug. ***

-- 
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 109887] vega56 undervolting/overclocking voltage issues

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109887

Stefan Springer  changed:

   What|Removed |Added

 CC||wsla...@gmail.com

--- Comment #13 from Stefan Springer  ---
*** Bug 110113 has been marked as a duplicate of this bug. ***

-- 
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 110113] AMD Vega64 issue setting custom voltages

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110113

Stefan Springer  changed:

   What|Removed |Added

 Resolution|--- |DUPLICATE
 Status|NEW |RESOLVED

--- Comment #2 from Stefan Springer  ---


*** This bug has been marked as a duplicate of bug 109887 ***

-- 
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 2/6] drm: Introduce DRM_MODE_DUMB_KERNEL_MAP flag

2019-10-23 Thread Laurent Pinchart
Hi Rob,

On Tue, Oct 22, 2019 at 07:42:06AM -0500, Rob Herring wrote:
> On Tue, Oct 22, 2019 at 6:14 AM Laurent Pinchart wrote:
> > On Mon, Oct 21, 2019 at 04:45:46PM -0500, Rob Herring wrote:
> > > Introduce a new flag, DRM_MODE_DUMB_KERNEL_MAP, for struct
> > > drm_mode_create_dumb. This flag is for internal kernel use to indicate
> > > if dumb buffer allocation needs a kernel mapping. This is needed only for
> > > CMA where creating a kernel mapping or not has to be decided at allocation
> > > time because creating a mapping on demand (with vmap()) is not guaranteed
> > > to work. Several drivers are using CMA, but not the CMA helpers because
> > > they distinguish between kernel and userspace allocations to create a
> > > kernel mapping or not.
> > >
> > > Update the callers of drm_mode_dumb_create() to set
> > > drm_mode_dumb_create.flags to appropriate defaults. Currently, flags can
> > > be set to anything by userspace, but is unused within the kernel. Let's
> > > force flags to zero (no kernel mapping) for userspace callers by default.
> > > For in kernel clients, set DRM_MODE_DUMB_KERNEL_MAP by default. Drivers
> > > can override this as needed.
> > >
> > > Cc: David Airlie 
> > > Cc: Daniel Vetter 
> > > Cc: Maarten Lankhorst 
> > > Cc: Maxime Ripard 
> > > Cc: Sean Paul 
> > > Signed-off-by: Rob Herring 
> > > ---
> > >  drivers/gpu/drm/drm_client.c   | 1 +
> > >  drivers/gpu/drm/drm_dumb_buffers.c | 5 -
> > >  include/uapi/drm/drm_mode.h| 2 ++
> > >  3 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> > > index d9a2e3695525..dbfc8061b392 100644
> > > --- a/drivers/gpu/drm/drm_client.c
> > > +++ b/drivers/gpu/drm/drm_client.c
> > > @@ -264,6 +264,7 @@ drm_client_buffer_create(struct drm_client_dev 
> > > *client, u32 width, u32 height, u
> > >   dumb_args.width = width;
> > >   dumb_args.height = height;
> > >   dumb_args.bpp = info->cpp[0] * 8;
> > > + dumb_args.flags = DRM_MODE_DUMB_KERNEL_MAP;
> > >   ret = drm_mode_create_dumb(dev, _args, client->file);
> > >   if (ret)
> > >   goto err_delete;
> > > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c 
> > > b/drivers/gpu/drm/drm_dumb_buffers.c
> > > index d18a740fe0f1..74a13f14c173 100644
> > > --- a/drivers/gpu/drm/drm_dumb_buffers.c
> > > +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> > > @@ -97,7 +97,10 @@ int drm_mode_create_dumb(struct drm_device *dev,
> > >  int drm_mode_create_dumb_ioctl(struct drm_device *dev,
> > >  void *data, struct drm_file *file_priv)
> > >  {
> > > - return drm_mode_create_dumb(dev, data, file_priv);
> > > + struct drm_mode_create_dumb *args = data;
> > > +
> > > + args->flags = 0;
> >
> > I would prefer returning an error if flags is set to a non-zero value,
> > to catch userspace errors early, but I'm also worried it would break
> > existing userspace by uncovering existing bugs. Still, if we later add
> > flags that userspace can set, those existing bugs will be triggered, so
> > we'll have to deal with them anyway, and we could already give it a try.
> 
> I would like that too, but the comment just above this code tells me
> that's likely to break things:
> 
> /*
>  * handle, pitch and size are output parameters. Zero them out to
>  * prevent drivers from accidentally using uninitialized data. Since
>  * not all existing userspace is clearing these fields properly we
>  * cannot reject IOCTL with garbage in them.
>  */
> 
> Maybe userspace does correctly zero out input params.

But if that holds true, it means that we will never be able to add
userspace flags, as doing so could break applications for the same
reason. The flag field should thus be marked as deprecated for userspace
usage. I wonder how big the risk is.

-- 
Regards,

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

Re: [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag

2019-10-23 Thread Laurent Pinchart
Hi Rob,

On Tue, Oct 22, 2019 at 03:02:06PM -0500, Rob Herring wrote:
> On Tue, Oct 22, 2019 at 6:30 AM Laurent Pinchart wrote:
> > On Mon, Oct 21, 2019 at 04:45:48PM -0500, Rob Herring wrote:
> >> Add support in CMA helpers to handle callers specifying
> >> DRM_MODE_DUMB_KERNEL_MAP flag. Existing behavior is maintained with this
> >> change. drm_gem_cma_dumb_create() always creates a kernel mapping as
> >> before. drm_gem_cma_dumb_create_internal() lets the caller set the flags
> >> as desired. Therefore, update all the existing callers of
> >> drm_gem_cma_dumb_create_internal() to also set the
> >> DRM_MODE_DUMB_KERNEL_MAP flag.
> >>
> >> Cc: Maarten Lankhorst 
> >> Cc: Maxime Ripard 
> >> Cc: Sean Paul 
> >> Cc: David Airlie 
> >> Cc: Daniel Vetter 
> >> Cc: "James (Qian) Wang" 
> >> Cc: Liviu Dudau 
> >> Cc: Brian Starkey 
> >> Cc: Neil Armstrong 
> >> Cc: Kevin Hilman 
> >> Cc: Laurent Pinchart 
> >> Cc: Kieran Bingham 
> >> Cc: Sandy Huang 
> >> Cc: "Heiko Stübner" 
> >> Cc: Yannick Fertre 
> >> Cc: Philippe Cornu 
> >> Cc: Benjamin Gaignard 
> >> Cc: Vincent Abriou 
> >> Cc: Maxime Coquelin 
> >> Cc: Alexandre Torgue 
> >> Cc: Chen-Yu Tsai 
> >> Cc: linux-amlo...@lists.infradead.org
> >> Cc: linux-arm-ker...@lists.infradead.org
> >> Cc: linux-renesas-...@vger.kernel.org
> >> Cc: linux-rockc...@lists.infradead.org
> >> Cc: linux-st...@st-md-mailman.stormreply.com
> >> Signed-off-by: Rob Herring 
> >> ---
> >>  .../gpu/drm/arm/display/komeda/komeda_kms.c   |  1 +
> >>  drivers/gpu/drm/arm/malidp_drv.c  |  1 +
> >>  drivers/gpu/drm/drm_gem_cma_helper.c  | 48 +++
> >>  drivers/gpu/drm/meson/meson_drv.c |  1 +
> >>  drivers/gpu/drm/rcar-du/rcar_du_kms.c |  1 +
> >>  drivers/gpu/drm/rockchip/rockchip_drm_gem.c   |  1 +
> >>  drivers/gpu/drm/stm/drv.c |  1 +
> >>  drivers/gpu/drm/sun4i/sun4i_drv.c |  1 +
> >>  8 files changed, 36 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c 
> >> b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> >> index d49772de93e0..7cf0dc4cbfc1 100644
> >> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> >> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> >> @@ -31,6 +31,7 @@ static int komeda_gem_cma_dumb_create(struct drm_file 
> >> *file,
> >>   u32 pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> >>
> >>   args->pitch = ALIGN(pitch, mdev->chip.bus_width);
> >> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> >>
> >>   return drm_gem_cma_dumb_create_internal(file, dev, args);
> >>  }
> >> diff --git a/drivers/gpu/drm/arm/malidp_drv.c 
> >> b/drivers/gpu/drm/arm/malidp_drv.c
> >> index 8a76315aaa0f..aeb1a779ecc1 100644
> >> --- a/drivers/gpu/drm/arm/malidp_drv.c
> >> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> >> @@ -465,6 +465,7 @@ static int malidp_dumb_create(struct drm_file 
> >> *file_priv,
> >>   u8 alignment = malidp_hw_get_pitch_align(malidp->dev, 1);
> >>
> >>   args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), 
> >> alignment);
> >> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> >>
> >>   return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> >>  }
> >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c 
> >> b/drivers/gpu/drm/drm_gem_cma_helper.c
> >> index 4cebfe01e6ea..f91e9e8adeaf 100644
> >> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> >> @@ -78,21 +78,8 @@ __drm_gem_cma_create(struct drm_device *drm, size_t 
> >> size)
> >>   return ERR_PTR(ret);
> >>  }
> >>
> >> -/**
> >> - * drm_gem_cma_create - allocate an object with the given size
> >> - * @drm: DRM device
> >> - * @size: size of the object to allocate
> >> - *
> >> - * This function creates a CMA GEM object and allocates a contiguous 
> >> chunk of
> >> - * memory as backing store. The backing memory has the writecombine 
> >> attribute
> >> - * set.
> >> - *
> >> - * Returns:
> >> - * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded 
> >> negative
> >> - * error code on failure.
> >> - */
> >> -struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> >> -   size_t size)
> >> +static struct drm_gem_cma_object *
> >> +drm_gem_cma_create_flags(struct drm_device *drm, size_t size, u32 flags)
> >>  {
> >>   struct drm_gem_cma_object *cma_obj;
> >>   int ret;
> >> @@ -103,6 +90,9 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct 
> >> drm_device *drm,
> >>   if (IS_ERR(cma_obj))
> >>   return cma_obj;
> >>
> >> + if (!(flags & DRM_MODE_DUMB_KERNEL_MAP))
> >> + cma_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> >> +
> >>   cma_obj->vaddr = dma_alloc_attrs(drm->dev, size, _obj->paddr,
> >>GFP_KERNEL | __GFP_NOWARN,
> >>cma_obj->dma_attrs);
> >> @@ -119,6 +109,25 @@ 

Re: [PATCH] backlight: pwm_bl: configure pwm only once per backlight toggle

2019-10-23 Thread Fabio Estevam
On Wed, Oct 23, 2019 at 11:16 AM Adam Ford  wrote:

> What is the plan to address the regression for 5.4?  I wasn't sure if
> we're going to apply the i.mx fixes or temporarily revert the
> offending patch, or something else. (or maybe nothing at all)

Yes, I do see the regression on a imx53 board with 5.4-rc too and also
interested on a fix.

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

Re: [PATCH v2] drm/amdgpu: Add DC feature mask to disable fractional pwm

2019-10-23 Thread Lukáš Krejčí
On Mon, 2019-10-21 at 15:44 -0400, sunpeng...@amd.com wrote:
> From: Leo Li 
> 
> [Why]
> 
> Some LED panel drivers might not like fractional PWM. In such cases,
> backlight flickering may be observed.
> 
> [How]
> 
> Add a DC feature mask to disable fractional PWM, and associate it with
> the preexisting dc_config flag.
> 
> The flag is only plumbed through the dmcu firmware, so plumb it through
> the driver path as well.
> 
> To disable, add the following to the linux cmdline:
> amdgpu.dcfeaturemask=0x4
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204957
> Signed-off-by: Leo Li 
> ---
> 
> v2: Add bugzilla link
> 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
>  drivers/gpu/drm/amd/display/dc/dce/dce_abm.c  | 4 
>  drivers/gpu/drm/amd/include/amd_shared.h  | 1 +
>  3 files changed, 8 insertions(+)

Tested on Ubuntu 19.04, vanilla v5.3.7 kernel with the patch applied
and amdgpu.dcfeaturemask=0x4 added to the kernel command line, no
issues after 8 reboots. (The issue could be reproduced without
amdgpu.dcfeaturemask=0x4 on first boot.)

Tested-by: Lukáš Krejčí 

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

Re: [PATCH] backlight: pwm_bl: configure pwm only once per backlight toggle

2019-10-23 Thread Adam Ford
On Fri, Oct 18, 2019 at 4:36 AM Michal Vokáč  wrote:
>
> On 17. 10. 19 19:44, Adam Ford wrote:
> > On Thu, Oct 17, 2019 at 12:13 PM Thierry Reding
> >  wrote:
> >>
> >> On Thu, Oct 17, 2019 at 12:07:21PM -0500, Adam Ford wrote:
> >>> On Thu, Oct 17, 2019 at 10:14 AM Thierry Reding
> >>>  wrote:
> 
>  On Thu, Oct 17, 2019 at 03:58:25PM +0200, Michal Vokáč wrote:
> > On 17. 10. 19 14:59, Thierry Reding wrote:
> >> On Thu, Oct 17, 2019 at 02:09:17PM +0200, Uwe Kleine-König wrote:
> >>> On Thu, Oct 17, 2019 at 01:11:31PM +0200, Thierry Reding wrote:
>  On Thu, Oct 17, 2019 at 12:11:16PM +0200, Uwe Kleine-König wrote:
> > On Thu, Oct 17, 2019 at 11:48:08AM +0200, Michal Vokáč wrote:
> >> On 17. 10. 19 10:10, Uwe Kleine-König wrote:
> >>> A previous change in the pwm core (namely 01ccf903edd6 ("pwm: Let
> >>> pwm_get_state() return the last implemented state")) changed the
> >>> semantic of pwm_get_state() and disclosed an (as it seems) common
> >>> problem in lowlevel PWM drivers. By not relying on the period and 
> >>> duty
> >>> cycle being retrievable from a disabled PWM this type of problem 
> >>> is
> >>> worked around.
> >>>
> >>> Apart from this issue only calling the 
> >>> pwm_get_state/pwm_apply_state
> >>> combo once is also more effective.
> >>>
> >>> Signed-off-by: Uwe Kleine-König 
> >>> ---
> >>> Hello,
> >>>
> >>> There are now two reports about 01ccf903edd6 breaking a 
> >>> backlight. As
> >>> far as I understand the problem this is a combination of the 
> >>> backend pwm
> >>> driver yielding surprising results and the pwm-bl driver doing 
> >>> things
> >>> more complicated than necessary.
> >>>
> >>> So I guess this patch works around these problems. Still it would 
> >>> be
> >>> interesting to find out the details in the imx driver that 
> >>> triggers the
> >>> problem. So Adam, can you please instrument the pwm-imx27 driver 
> >>> to
> >>> print *state at the beginning of pwm_imx27_apply() and the end of
> >>> pwm_imx27_get_state() and provide the results?
> >>>
> >>> Note I only compile tested this change.
> >>
> >> Hi Uwe,
> >> I was just about to respond to the "pwm_bl on i.MX6Q broken on 
> >> 5.4-RC1+"
> >> thread that I have a similar problem when you submitted this patch.
> >>
> >> So here are my few cents:
> >>
> >> My setup is as follows:
> >>- imx6dl-yapp4-draco with i.MX6Solo
> >>- backlight is controlled with inverted PWM signal
> >>- max brightness level = 32, default brightness level set to 32 
> >> in DT.
> >>
> >> 1. Almost correct backlight behavior before 01ccf903edd6 ("pwm: Let
> >>  pwm_get_state() return the last implemented state):
> >>
> >>- System boots to userspace and backlight is enabled all the 
> >> time from
> >>  power up.
> >>
> >>  $ dmesg | grep state
> >>  [1.763381] get state end: -1811360608, enabled: 0
> >
> > What is -1811360608? When I wrote "print *state" above, I thought 
> > about
> > something like:
> >
> >pr_info("%s: period: %u, duty: %u, polarity: %d, enabled: 
> > %d",
> >__func__, state->period, state->duty_cycle, 
> > state->polarity, state->enabled);
> >
> > A quick look into drivers/pwm/pwm-imx27.c shows that this is another
> > driver that yields duty_cycle = 0 when the hardware is off.
> 
>  It seems to me like the best recourse to fix this for now would be to
>  patch up the drivers that return 0 when the hardware is off by 
>  caching
>  the currently configured duty cycle.
> 
>  How about the patch below?
> 
>  Thierry
> 
>  --- >8 ---
>    From 15a52a7f1b910804fabd74a5882befd3f9d6bb37 Mon Sep 17 00:00:00 
>  2001
>  From: Thierry Reding 
>  Date: Thu, 17 Oct 2019 12:56:00 +0200
>  Subject: [PATCH] pwm: imx27: Cache duty cycle register value
> 
>  The hardware register containing the duty cycle value cannot be 
>  accessed
>  when the PWM is disabled. This causes the ->get_state() callback to 
>  read
>  back a duty cycle value of 0, which can confuse consumer drivers.
> 
>  Signed-off-by: Thierry Reding 
>  ---
> drivers/pwm/pwm-imx27.c | 31 ---
> 1 file changed, 24 insertions(+), 7 deletions(-)
> 
>  diff --git 

Re: [PATCH v1] drm/tegra: plane: Remove format-modifier checking

2019-10-23 Thread Thierry Reding
On Sun, Feb 24, 2019 at 06:34:05PM +0300, Dmitry Osipenko wrote:
> Tiling modifier can't be applied to YV12 video overlay because all tiling
> modifiers are filtered out for multi-plane formats. AFAIK, all modifiers
> should work with all of formats, hence the checking is incorrect and
> simply not needed.
> 
> Fixes: e90124cb46bdb ("drm/tegra: plane: Support format modifiers")
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/gpu/drm/tegra/plane.c | 16 
>  1 file changed, 16 deletions(-)

I'm hesitant to apply this because we don't really have a good way to
test that this is actually the case. I vaguely recall that at least for
some of the block-linear formats supported on Tegra124 and later there
are additional restrictions on when they can be used.

There's also the problem that using these block-linear formats, and I
think this even applies to the TILED format on older Tegra SoCs, results
in higher bandwidth requirements.

Bandwidth requirements is something that we don't really concern
ourselves with, and that's bad enough as it is. I suspect that once we
blindly allow all format modifiers we could easily run into situations
where the display controllers underflow.

Now, regardless of which way you look at this, it boils down to testing.
We don't have a good way of testing various combinations of format
modifiers to verify that they work. You say yourself that "AFAIK, all
modifiers should work with all formats", but can you really know for
certain? Until we're able to properly test this, we really can't.

Given all of the above, I think it's better to be prudent and only allow
format/modifier combinations that we've actually tested. I'm not aware
of a good way to test planar formats, so we don't have a good way to get
the results that we need.

I'm all ears if you know of a good way to test this. It doesn't have to
be anything fully automated. Automated testing is especially difficult
to do for display because it usually needs visual inspection. But that's
okay, I'm willing to settle for something that we can roll into a script
and run manually after boot until we can find a way to automatically do
this type of test.

Thierry

> diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
> index d068e8aa3553..5a8a3387f5ee 100644
> --- a/drivers/gpu/drm/tegra/plane.c
> +++ b/drivers/gpu/drm/tegra/plane.c
> @@ -72,21 +72,6 @@ static void tegra_plane_atomic_destroy_state(struct 
> drm_plane *plane,
>   kfree(state);
>  }
>  
> -static bool tegra_plane_format_mod_supported(struct drm_plane *plane,
> -  uint32_t format,
> -  uint64_t modifier)
> -{
> - const struct drm_format_info *info = drm_format_info(format);
> -
> - if (modifier == DRM_FORMAT_MOD_LINEAR)
> - return true;
> -
> - if (info->num_planes == 1)
> - return true;
> -
> - return false;
> -}
> -
>  const struct drm_plane_funcs tegra_plane_funcs = {
>   .update_plane = drm_atomic_helper_update_plane,
>   .disable_plane = drm_atomic_helper_disable_plane,
> @@ -94,7 +79,6 @@ const struct drm_plane_funcs tegra_plane_funcs = {
>   .reset = tegra_plane_reset,
>   .atomic_duplicate_state = tegra_plane_atomic_duplicate_state,
>   .atomic_destroy_state = tegra_plane_atomic_destroy_state,
> - .format_mod_supported = tegra_plane_format_mod_supported,
>  };
>  
>  int tegra_plane_state_add(struct tegra_plane *plane,
> -- 
> 2.20.1
> 


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

[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

2019-10-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111481

--- Comment #127 from Shmerl  ---
powerplay is a different bug from the sdma one, but it was listed as part of
this report before, that's why I mentioned it above.

-- 
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 00/36] ARM: samsung platform cleanup

2019-10-23 Thread Arnd Bergmann
On Wed, Oct 23, 2019 at 3:11 PM Krzysztof Kozlowski  wrote:
> On Thu, Oct 10, 2019 at 10:28:02PM +0200, Arnd Bergmann wrote:
> > The contents are available for testing in
> >
> > git://kernel.org:/pub/scm/linux/kernel/git/arnd/playground.git 
> > s3c-multiplatform
>
> When sending v2, can you Cc:
>
> Paweł Chmiel 
> Lihua Yao 
> (or Lihua Yao  if outlook.com bounces)
> Sergio Prado 
> Sylwester Nawrocki 
>
> These are folks which to my knowledge had working S3C and S5P boards
> so maybe they could provide testing.

Ok, will do. I've uploaded the modified version based on your comments to
the above URL for now.

I'll probably give it a little more time before resending, but they
could already
start testing that version.

Thanks a lot for the review!

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

Re: [PATCH] dc.c:use kzalloc without test

2019-10-23 Thread Harry Wentland
On 2019-10-23 4:32 a.m., zhongshiqi wrote:
> dc.c:583:null check is needed after using kzalloc function
> 
> Signed-off-by: zhongshiqi 

Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/display/dc/core/dc.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 5d1aded..4b8819c 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -580,6 +580,10 @@ static bool construct(struct dc *dc,
>  #ifdef CONFIG_DRM_AMD_DC_DCN2_0
>   // Allocate memory for the vm_helper
>   dc->vm_helper = kzalloc(sizeof(struct vm_helper), GFP_KERNEL);
> + if (!dc->vm_helper) {
> + dm_error("%s: failed to create dc->vm_helper\n", __func__);
> + goto fail;
> + }
>  
>  #endif
>   memcpy(>bb_overrides, _params->bb_overrides, 
> sizeof(dc->bb_overrides));
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 28/36] fbdev: s3c2410fb: remove mach header dependency

2019-10-23 Thread Arnd Bergmann
On Wed, Oct 23, 2019 at 3:13 PM Krzysztof Kozlowski  wrote:
> On Thu, Oct 10, 2019 at 10:30:12PM +0200, Arnd Bergmann wrote:

> > @@ -321,6 +320,7 @@ static struct s3c2410fb_mach_info jive_lcd_config = {
> >* data. */
> >
> >   .gpcup  = (0xf << 1) | (0x3f << 10),
> > + .gpcup_reg =S3C2410_GPCUP,
>
> Nits: indentation before/after '=' looks wrong. Tab should be
> before '=', one space after.

Ok, fixed now for the four boards that had inconsistent indentation --
jive, mini2440, smdk2440, and rx1950. Unfortunately each board
seemed to have its own way of doing this.

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

Re: [PATCH] drm/virtio: print a single line with device features

2019-10-23 Thread Daniel Vetter
On Wed, Oct 23, 2019 at 3:18 PM Jani Nikula  wrote:
>
> On Tue, 22 Oct 2019, Daniel Vetter  wrote:
> > On Fri, Oct 18, 2019 at 01:38:32PM +0200, Gerd Hoffmann wrote:
> >> Signed-off-by: Gerd Hoffmann 
> >> ---
> >>  drivers/gpu/drm/virtio/virtgpu_kms.c | 9 -
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c 
> >> b/drivers/gpu/drm/virtio/virtgpu_kms.c
> >> index 0b3cdb0d83b0..2f5773e43557 100644
> >> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> >> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> >> @@ -155,16 +155,15 @@ int virtio_gpu_init(struct drm_device *dev)
> >>  #ifdef __LITTLE_ENDIAN
> >>  if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_VIRGL))
> >>  vgdev->has_virgl_3d = true;
> >> -DRM_INFO("virgl 3d acceleration %s\n",
> >> - vgdev->has_virgl_3d ? "enabled" : "not supported by host");
> >> -#else
> >> -DRM_INFO("virgl 3d acceleration not supported by guest\n");
> >>  #endif
> >>  if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_EDID)) {
> >>  vgdev->has_edid = true;
> >> -DRM_INFO("EDID support available.\n");
> >>  }
> >>
> >> +DRM_INFO("features: %cvirgl %cedid\n",
> >> + vgdev->has_virgl_3d ? '+' : '-',
> >> + vgdev->has_edid ? '+' : '-');
> >
> > Maybe we should move the various yesno/onoff/enableddisabled helpers from
> > i915_utils.h to drm_utils.h and use them more widely?
>
> I'm trying to take it one step further by adding them to
> include/linux/string-choice.h [1]. Maybe, uh, fourth time's the charm?
>
> BR,
> Jani.
>
> [1] http://lore.kernel.org/r/20191023131308.9420-1-jani.nik...@intel.com

Oh nice, r-b: me on that.

I think the rule for new headers like this is "just do it" once you
have enough senior kernel maintainers' approval. Maybe ask Dave Airlie
for ack and with Greg's r-b then just stuff it into drm-misc-next or
so?
-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

  1   2   >