Re: [PATCH v6 21/23] drm: rockchip: Add VOP2 driver

[Sascha Hauer]

On Sat, Feb 19, 2022 at 03:35:12PM +0800, Andy Yan wrote:
> Hi Sascha:
> 
> On 2/18/22 16:00, Sascha Hauer wrote:
> > On Fri, Feb 18, 2022 at 11:50:32AM +0800, Andy Yan wrote:
> > > Hi Sascha:
> > > 
> > > On 2/17/22 22:06, Heiko Stübner wrote:
> > > > Am Donnerstag, 17. Februar 2022, 14:58:23 CET schrieb Sascha Hauer:
> > > > > Hi Andy,
> > > > > 
> > > > > Please trim the context in your answers to the relevant parts, it 
> > > > > makes
> > > > > it easier to find the things you said.
> > > > > 
> > > > > On Thu, Feb 17, 2022 at 08:00:11PM +0800, Andy Yan wrote:
> > > > > > Hi Sascha:
> > > > > > 
> > > > > > > +
> > > > > > > + drm_for_each_encoder_mask(encoder, crtc->dev, 
> > > > > > > crtc_state->encoder_mask) {
> > > > > > > + struct rockchip_encoder *rkencoder = 
> > > > > > > to_rockchip_encoder(encoder);
> > > > > > > + struct device_node *node, *parent;
> > > > > > > +
> > > > > > > + parent = of_get_parent(rkencoder->port);
> > > > > > > +
> > > > > > > + for_each_endpoint_of_node(parent, node) {
> > > > > > Is there any hurt directly use our downstream vendor kernel method 
> > > > > > here: use
> > > > > > vcstate->output_if set by encoder driver to get which interface we 
> > > > > > should
> > > > > > enable here?
> > > > > There is no vcstate->output_if in mainline currently. Ok, we could add
> > > > > that. The other thing is that there are multiple HDMI interfaces and
> > > > > the id of the HDMI encoder is encoded into output_if. Downstream 
> > > > > kernel
> > > > > adds OF aliases to the HDMI ports. I didn't want to go that route
> > > > > because it doesn't seem to be very elegant to me.
> > > aliases is a very comm strategy in device tree world.
> > Yes, but not for retrieving bit offsets into registers. Normally aliases
> > can be changed at board level without confusing drivers.
> > 
> > > And your method also
> > > add need additional dt binds to define RK3568_VOP2_EP_xxx
> > > > > > You method is ok with device tree,  but it tied up this driver to 
> > > > > > device
> > > > > > tree, we are now tring to extend vop2 driver work with ACPI, so we 
> > > > > > hope this
> > > > > > driver can be much more flexible.
> > > > > The current rockchip drm driver seems to be pretty much tied to device
> > > > > tree. There are probably many other places that need parallel paths 
> > > > > for
> > > > > ACPI support, I think we can delay this particular part until we see 
> > > > > the
> > > > > whole picture. In the end we can still retrieve the output_if
> > > > > information differently with ACPI while still retrieving the 
> > > > > information
> > > > > from the device tree the way we are doing currently.
> > > The current driver only reference device thee at driver initial, we not 
> > > wrap
> > > 
> > > device tree related things in other parts, so if we extend it to support
> > > ACPI,
> > > 
> > > we just need modify the initial code, this make things easier.
> > The device tree parsing could be moved out of vop2_crtc_atomic_enable()
> > into some initialisation path. In the end it's static information,
> > there's no need to do it repeatedly in atomic_enable.
> 
> This could be one solution, the repeatedly parsing device tree in
> atomic_enable is also my concern.
> 
> In addition, there are 2 HDMI, 2 eDP, 2 MIPI on the coming rk3588, so it's
> better to consider give position
> 
> for HDMI1, EDP1, in  include/dt-bindings/soc/rockchip,vop2.h

The defines are rk3568 specific. rk3588 would use a set of rk3588
specific defines along with a rk3588_set_intf_mux().

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

Re: [PATCH v3 3/5] fbdev: Remove trailing whitespaces from cfbimgblt.c

[Javier Martinez Canillas]

On 2/23/22 20:38, Thomas Zimmermann wrote:
> Fix coding style. No functional changes.
> 
> Signed-off-by: Thomas Zimmermann 
> ---
Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v3 4/5] fbdev: Improve performance of cfb_imageblit()

[Javier Martinez Canillas]

On 2/23/22 20:38, Thomas Zimmermann wrote:
> Improve the performance of cfb_imageblit() by manually unrolling
> the inner blitting loop and moving some invariants out. The compiler
> failed to do this automatically. This change keeps cfb_imageblit()
> in sync with sys_imagebit().
> 
> A microbenchmark measures the average number of CPU cycles
> for cfb_imageblit() after a stabilizing period of a few minutes
> (i7-4790, FullHD, simpledrm, kernel with debugging).
> 
> cfb_imageblit(), new: 15724 cycles
> cfb_imageblit(): old: 30566 cycles
> 
> In the optimized case, cfb_imageblit() is now ~2x faster than before.
> 
> v3:
>   * fix commit description (Pekka)
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Makes sense, improves perf and makes the two more consistent as you mention.

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v3 5/5] drm: Add TODO item for optimizing format helpers

[Javier Martinez Canillas]

On 2/23/22 20:38, Thomas Zimmermann wrote:
> Add a TODO item for optimizing blitting and format-conversion helpers
> in DRM and fbdev. There's always demand for faster graphics output.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

After fixing the typos mentioned by Sam:

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v3 4/5] fbdev: Improve performance of cfb_imageblit()

[Javier Martinez Canillas]

Hello Sam,

On 2/23/22 21:25, Sam Ravnborg wrote:

[snip]

> 
> Question: What is cfb an abbreviation for anyway?
> Not related to the patch - but if I have known the memory is lost..
> 

I was curious so I dug on this. It seems CFB stands for Color Frame Buffer.
Doing a `git grep "(CFB)"` in the linux history repo [0], I get this:

  Documentation/isdn/README.diversion:   (CFB). 
  drivers/video/pmag-ba-fb.c: *   PMAG-BA TURBOchannel Color Frame Buffer (CFB) 
card support,
  include/video/pmag-ba-fb.h: *   TURBOchannel PMAG-BA Color Frame Buffer (CFB) 
card support,

Probably the helpers are called like this because they were for any fbdev
driver but assumed that the framebuffer was always in I/O memory. Later some
drivers were allocating the framebuffer in system memory and still using the
helpers, that were using I/O memory accessors and it's ilegal on some arches.

So the sys_* variants where introduced by commit 68648ed1f58d ("fbdev: add
drawing functions for framebuffers in system RAM") to fix this. The old
ones just kept their name, but probably it should had been renamed to io_*
for the naming to be consistent with the sys_* functions.

[0]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat

[PATCH 1/6] drm/i915/uapi: introduce drm_i915_gem_create_ext

[Pallavi Mishra]

From: Matthew Auld 

Same old gem_create but with now with extensions support. This is needed
to support various upcoming usecases.

v2:(Chris)
- Use separate ioctl number for gem_create_ext, instead of hijacking
  the existing gem_create ioctl, otherwise we run into the issue
  with being unable to detect if the kernel supports the new extension
  behaviour.
- We now have gem_create_ext.flags, which should be zeroed.
- I915_GEM_CREATE_EXT_SETPARAM value is now zero, since this is the
  index into our array of extensions.
- Setup a "vanilla" object which we can directly apply our extensions
  to.
v3:(Daniel & Jason)
- drop I915_GEM_CREATE_EXT_SETPARAM. Instead just have each extension
  do one thing only, instead of generic setparam which can cover
  various use cases.
- add some kernel-doc.

Signed-off-by: Matthew Auld 
Signed-off-by: CQ Tang 
Cc: Joonas Lahtinen 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-devel@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
Reviewed-by: Kenneth Graunke 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20210429103056.407067-5-matthew.a...@intel.com
(cherry picked from commit ebcb40298947bdb0622e53c69734e6b4fb64b348)
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c | 56 ++
 drivers/gpu/drm/i915/gem/i915_gem_ioctls.h |  2 +
 drivers/gpu/drm/i915/i915_driver.c |  1 +
 include/uapi/drm/i915_drm.h| 42 
 4 files changed, 101 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c 
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 409226df0dd2..1e82633a3898 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -8,6 +8,7 @@
 
 #include "i915_drv.h"
 #include "i915_trace.h"
+#include "i915_user_extensions.h"
 
 static int i915_gem_publish(struct drm_i915_gem_object *obj,
struct drm_file *file,
@@ -149,3 +150,58 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
i915_gem_object_free(obj);
return ret;
 }
+
+struct create_ext {
+   struct drm_i915_private *i915;
+   struct drm_i915_gem_object *vanilla_object;
+};
+
+static const i915_user_extension_fn create_extensions[] = {
+};
+
+/**
+ * Creates a new mm object and returns a handle to it.
+ * @dev: drm device pointer
+ * @data: ioctl data blob
+ * @file: drm file pointer
+ */
+int
+i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+   struct drm_i915_private *i915 = to_i915(dev);
+   struct drm_i915_gem_create_ext *args = data;
+   struct create_ext ext_data = { .i915 = i915 };
+   struct drm_i915_gem_object *obj;
+   int ret;
+
+   if (args->flags)
+   return -EINVAL;
+
+   i915_gem_flush_free_objects(i915);
+
+   obj = i915_gem_object_alloc();
+   if (!obj)
+   return -ENOMEM;
+
+   ext_data.vanilla_object = obj;
+   ret = i915_user_extensions(u64_to_user_ptr(args->extensions),
+  create_extensions,
+  ARRAY_SIZE(create_extensions),
+  &ext_data);
+   if (ret)
+   goto object_free;
+
+   ret = i915_gem_setup(obj,
+intel_memory_region_by_type(i915,
+INTEL_MEMORY_SYSTEM),
+args->size);
+   if (ret)
+   goto object_free;
+
+   return i915_gem_publish(obj, file, &args->size, &args->handle);
+
+object_free:
+   i915_gem_object_free(obj);
+   return ret;
+}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h 
b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
index 7fd22f3efbef..28d6526e32ab 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
@@ -14,6 +14,8 @@ int i915_gem_busy_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
 int i915_gem_create_ioctl(struct drm_device *dev, void *data,
  struct drm_file *file);
+int i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
 int i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file);
 int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_driver.c 
b/drivers/gpu/drm/i915/i915_driver.c
index 8b06e56f7d09..11c6d3318308 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1758,6 +1758,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, drm_noop, 
DRM_AUTH|DRM_MASTER|DRM_ROOT

[PATCH 2/6] drm/i915/uapi: implement object placement extension

[Pallavi Mishra]

From: Matthew Auld 

Add new extension to support setting an immutable-priority-list of
potential placements, at creation time.

If we use the normal gem_create or gem_create_ext without the
extensions/placements then we still get the old behaviour with only
placing the object in system memory.

v2(Daniel & Jason):
- Add a bunch of kernel-doc
- Simplify design for placements extension

Testcase: igt/gem_create/create-ext-placement-sanity-check
Testcase: igt/gem_create/create-ext-placement-each
Testcase: igt/gem_create/create-ext-placement-all
Signed-off-by: Matthew Auld 
Signed-off-by: CQ Tang 
Cc: Joonas Lahtinen 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-devel@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
Reviewed-by: Kenneth Graunke 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20210429103056.407067-6-matthew.a...@intel.com
(cherry picked from commit 2459e56fd8af0b47fcbfbdff2fdc02e4077680ec)
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c| 218 --
 drivers/gpu/drm/i915/gem/i915_gem_object.c|   3 +
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |   6 +
 .../drm/i915/gem/selftests/i915_gem_mman.c|  26 +++
 drivers/gpu/drm/i915/intel_memory_region.c|  16 ++
 drivers/gpu/drm/i915/intel_memory_region.h|   4 +
 include/uapi/drm/i915_drm.h   |  62 +
 7 files changed, 318 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c 
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 1e82633a3898..957f790c644b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -4,12 +4,51 @@
  */
 
 #include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_lmem.h"
 #include "gem/i915_gem_region.h"
 
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "i915_user_extensions.h"
 
+static u32 object_max_page_size(struct drm_i915_gem_object *obj)
+{
+   u32 max_page_size = 0;
+   int i;
+
+   for (i = 0; i < obj->mm.n_placements; i++) {
+   struct intel_memory_region *mr = obj->mm.placements[i];
+
+   GEM_BUG_ON(!is_power_of_2(mr->min_page_size));
+   max_page_size = max_t(u32, max_page_size, mr->min_page_size);
+   }
+
+   GEM_BUG_ON(!max_page_size);
+   return max_page_size;
+}
+
+static void object_set_placements(struct drm_i915_gem_object *obj,
+ struct intel_memory_region **placements,
+ unsigned int n_placements)
+{
+   GEM_BUG_ON(!n_placements);
+
+   /*
+* For the common case of one memory region, skip storing an
+* allocated array and just point at the region directly.
+*/
+   if (n_placements == 1) {
+   struct intel_memory_region *mr = placements[0];
+   struct drm_i915_private *i915 = mr->i915;
+
+   obj->mm.placements = &i915->mm.regions[mr->id];
+   obj->mm.n_placements = 1;
+   } else {
+   obj->mm.placements = placements;
+   obj->mm.n_placements = n_placements;
+   }
+}
+
 static int i915_gem_publish(struct drm_i915_gem_object *obj,
struct drm_file *file,
u64 *size_p,
@@ -29,14 +68,12 @@ static int i915_gem_publish(struct drm_i915_gem_object *obj,
 }
 
 static int
-i915_gem_setup(struct drm_i915_gem_object *obj,
-  struct intel_memory_region *mr,
-  u64 size)
+i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)
 {
+   struct intel_memory_region *mr = obj->mm.placements[0];
int ret;
 
-   GEM_BUG_ON(!is_power_of_2(mr->min_page_size));
-   size = round_up(size, mr->min_page_size);
+   size = round_up(size, object_max_page_size(obj));
if (size == 0)
return -EINVAL;
 
@@ -62,6 +99,7 @@ i915_gem_dumb_create(struct drm_file *file,
 struct drm_mode_create_dumb *args)
 {
struct drm_i915_gem_object *obj;
+   struct intel_memory_region *mr;
enum intel_memory_type mem_type;
int cpp = DIV_ROUND_UP(args->bpp, 8);
u32 format;
@@ -102,10 +140,10 @@ i915_gem_dumb_create(struct drm_file *file,
if (!obj)
return -ENOMEM;
 
-   ret = i915_gem_setup(obj,
-intel_memory_region_by_type(to_i915(dev),
-mem_type),
-args->size);
+   mr = intel_memory_region_by_type(to_i915(dev), mem_type);
+   object_set_placements(obj, &mr, 1);
+
+   ret = i915_gem_setup(obj, args->size);
if (ret)
goto object_free;
 
@@ -129,6 +167,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
struct drm_i915_private *i915 = to_i915(dev);
struct drm_i915_gem_create *args 

[PATCH 3/6] drm/i915/gem: clear userspace buffers for LMEM

[Pallavi Mishra]

From: Matthew Auld 

All userspace objects must be cleared when allocating the backing store,
before they are potentially visible to userspace.  For now use simple
CPU based clearing to do this for device local-memory objects, note that
in the near future this will instead use the blitter engine.

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-devel@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
Reviewed-by: Kenneth Graunke 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20210429103056.407067-8-matthew.a...@intel.com
(cherry picked from commit 0e997a36ecb61b161a22980ec9240ee7537f3f62)
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c 
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 957f790c644b..f6729feae582 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -71,6 +71,7 @@ static int
 i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)
 {
struct intel_memory_region *mr = obj->mm.placements[0];
+   unsigned int flags;
int ret;
 
size = round_up(size, object_max_page_size(obj));
@@ -83,7 +84,16 @@ i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)
if (i915_gem_object_size_2big(size))
return -E2BIG;
 
-   ret = mr->ops->init_object(mr, obj, size, 0);
+   /*
+* For now resort to CPU based clearing for device local-memory, in the
+* near future this will use the blitter engine for accelerated, GPU
+* based clearing.
+*/
+   flags = 0;
+   if (mr->type == INTEL_MEMORY_LOCAL)
+   flags = I915_BO_ALLOC_CPU_CLEAR;
+
+   ret = mr->ops->init_object(mr, obj, size, flags);
if (ret)
return ret;
 

[PATCH 4/6] drm/i915/gem: hide new uAPI behind CONFIG_BROKEN

[Pallavi Mishra]

From: Matthew Auld 

Treat it the same as the fake local-memory stuff, where it is disabled
for normal kernels, in case some random UMD is tempted to use this. Once
we have all the other bits and pieces in place, like the TTM conversion,
we can turn this on for real.

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Thomas Hellström 
Cc: Daniele Ceraolo Spurio 
Cc: Lionel Landwerlin 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Daniel Vetter 
Cc: Kenneth Graunke 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: dri-devel@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
Reviewed-by: Kenneth Graunke 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20210429103056.407067-9-matthew.a...@intel.com
(cherry picked from commit 0a46be95c282956a9d3229a46e33ba701c26594c)
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c | 3 +++
 drivers/gpu/drm/i915/i915_query.c  | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c 
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index f6729feae582..548ddf39d853 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -335,6 +335,9 @@ static int ext_set_placements(struct i915_user_extension 
__user *base,
 {
struct drm_i915_gem_create_ext_memory_regions ext;
 
+   if (!IS_ENABLED(CONFIG_DRM_I915_UNSTABLE_FAKE_LMEM))
+   return -ENODEV;
+
if (copy_from_user(&ext, base, sizeof(ext)))
return -EFAULT;
 
diff --git a/drivers/gpu/drm/i915/i915_query.c 
b/drivers/gpu/drm/i915/i915_query.c
index 51b368be0fc4..8a72923fbdba 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -434,6 +434,9 @@ static int query_memregion_info(struct drm_i915_private 
*i915,
u32 total_length;
int ret, id, i;
 
+   if (!IS_ENABLED(CONFIG_DRM_I915_UNSTABLE_FAKE_LMEM))
+   return -ENODEV;
+
if (query_item->flags != 0)
return -EINVAL;
 

Re: [PATCH 3/3] drm: bridge: Switch to devm_drm_of_get_bridge

[Jagan Teki]

Hi Linus,

On Sat, Dec 11, 2021 at 11:59 AM Jagan Teki  wrote:
>
> Hi Linus,
>
> On Sat, Dec 11, 2021 at 5:37 AM Linus Walleij  
> wrote:
> >
> > On Fri, Dec 10, 2021 at 6:49 PM Jagan Teki  
> > wrote:
> > >
> > > devm_drm_of_get_bridge is capable of looking up the downstream
> > > bridge and panel and trying to add a panel bridge if the panel
> > > is found.
> > >
> > > Replace explicit finding calls with devm_drm_of_get_bridge.
> > >
> > > Cc: Philipp Zabel 
> > > Cc: Chun-Kuang Hu 
> > > Cc: Linus Walleij 
> > > Signed-off-by: Jagan Teki 
> >
> > Nice overall!
> >
> > > -   /* Look for a panel as a child to this node */
> > > -   for_each_available_child_of_node(dev->of_node, child) {
> > > -   panel = of_drm_find_panel(child);
> > > -   if (IS_ERR(panel)) {
> > > -   dev_err(dev, "failed to find panel try bridge 
> > > (%ld)\n",
> > > -   PTR_ERR(panel));
> > > -   panel = NULL;
> > > -
> > > -   bridge = of_drm_find_bridge(child);
> > > -   if (!bridge) {
> > > -   dev_err(dev, "failed to find bridge\n");
> > > -   return -EINVAL;
> > > -   }
> > > -   }
> > > -   }
> > > -   if (panel) {
> > > -   bridge = drm_panel_bridge_add_typed(panel,
> > > -   
> > > DRM_MODE_CONNECTOR_DSI);
> >
> > And we are guaranteed that the right type of connector will be
> > used here? (Just checking.)
>
> Yes. Each panel driver initialised the connector_type via
> drm_panel_init and it will check during devm_drm_of_get_bridge.
>
> >
> > > -   if (IS_ERR(bridge)) {
> > > -   dev_err(dev, "error adding panel bridge\n");
> > > -   return PTR_ERR(bridge);
> > > -   }
> > > -   dev_info(dev, "connected to panel\n");
> > > -   d->panel = panel;
> >
> > How does this assignment happen after your patch?
> > I'm using that...
> >
> > devm_drm_of_get_bridge() needs some more argument right?
>
> Yes, but I think we don't need to preserve the panel here. Yes I
> didn't add that change, will take care in v2.
>
> >
> > > -   } else if (bridge) {
> > > -   /* TODO: AV8100 HDMI encoder goes here for example */
> > > -   dev_info(dev, "connected to non-panel bridge 
> > > (unsupported)\n");
> > > -   return -ENODEV;
> > > -   } else {
> > > -   dev_err(dev, "no panel or bridge\n");
> > > -   return -ENODEV;
> > > +   bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
> > > +   if (IS_ERR(bridge)) {
> > > +   dev_err(dev, "error to get bridge\n");
> > > +   return PTR_ERR(bridge);
> >
> > I'm gonna want to test this somehow on the hardware. But the TODO comment
> > there wasn't supposed to be deleted if I will still need to take some 
> > special
> > action whether this is a panel bridge or some other bridge.
>
> Agreed. Even I do like to add this prints, since
> devm_drm_of_get_bridge is not able to return differentiated bridge so
> it it not possible now. May be we can discuss this point.

Any comments on this? I will try to send the next version based on it.

Thanks,
Jagan.

Re: [Intel-gfx] [PATCH 1/3] drm/i915/guc: Limit scheduling properties to avoid overflow

[Tvrtko Ursulin]

On 23/02/2022 19:03, John Harrison wrote:

On 2/23/2022 04:13, Tvrtko Ursulin wrote:

On 23/02/2022 02:11, John Harrison wrote:

On 2/22/2022 01:52, Tvrtko Ursulin wrote:

On 18/02/2022 21:33, john.c.harri...@intel.com wrote:

From: John Harrison 

GuC converts the pre-emption timeout and timeslice quantum values into
clock ticks internally. That significantly reduces the point of 32bit
overflow. On current platforms, worst case scenario is approximately


Where does 32-bit come from, the GuC side? We already use 64-bits so 
that something to fix to start with. Yep...
Yes, the GuC API is defined as 32bits only and then does a straight 
multiply by the clock speed with no range checking. We have requested 
64bit support but there was push back on the grounds that it is not 
something the GuC timer hardware supports and such long timeouts are 
not real world usable anyway.


As long as compute are happy with 100 seconds, then it "should be 
enough for everbody". :D

Compute disable all forms of reset and rely on manual kill. So yes.

But even if they aren't. That's all we can do at the moment. If there is 
a genuine customer requirement for more then we can push for full 64bit 
software implemented timers in the GuC but until that happens, we don't 
have much choice.


Yeah.







./gt/uc/intel_guc_fwif.h:   u32 execution_quantum;

./gt/uc/intel_guc_submission.c: desc->execution_quantum = 
engine->props.timeslice_duration_ms * 1000;


./gt/intel_engine_types.h:  unsigned long 
timeslice_duration_ms;


timeslice_store/preempt_timeout_store:
err = kstrtoull(buf, 0, &duration);

So both kconfig and sysfs can already overflow GuC, not only because 
of tick conversion internally but because at backend level nothing 
was done for assigning 64-bit into 32-bit. Or I failed to find where 
it is handled.
That's why I'm adding this range check to make sure we don't allow 
overflows.


Yes and no, this fixes it, but the first bug was not only due GuC 
internal tick conversion. It was present ever since the u64 from i915 
was shoved into u32 sent to GuC. So even if GuC used the value without 
additional multiplication, bug was be there. My point being when GuC 
backend was added timeout_ms values should have been limited/clamped 
to U32_MAX. The tick discovery is additional limit on top.
I'm not disagreeing. I'm just saying that the truncation wasn't noticed 
until I actually tried using very long timeouts to debug a particular 
problem. Now that it is noticed, we need some method of range checking 
and this simple clamp solves all the truncation problems.


Agreed in principle, just please mention in the commit message all aspects of 
the problem.

I think we can get away without a Fixes: tag since it requires user fiddling to 
break things in unexpected ways.

I would though put in a code a clamping which expresses both, something like min(u32, ..GUC 
LIMIT..). So the full story is documented forever. Or "if > u32 || > ..GUC LIMIT..) 
return -EINVAL". Just in case GuC limit one day changes but u32 stays. Perhaps internal 
ticks go away or anything and we are left with plain 1:1 millisecond relationship.


110 seconds. Rather than allowing the user to set higher values and
then get confused by early timeouts, add limits when setting these
values.


Btw who is reviewing GuC patches these days - things have somehow 
gotten pretty quiet in activity and I don't think that's due absence 
of stuff to improve or fix? Asking since I think I noticed a few 
already which you posted and then crickets on the mailing list.

Too much work to do and not enough engineers to do it all :(.





Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/gt/intel_engine_cs.c   | 15 +++
  drivers/gpu/drm/i915/gt/sysfs_engines.c | 14 ++
  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h |  9 +
  3 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c

index e53008b4dd05..2a1e9f36e6f5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -389,6 +389,21 @@ static int intel_engine_setup(struct intel_gt 
*gt, enum intel_engine_id id,

  if (GRAPHICS_VER(i915) == 12 && engine->class == RENDER_CLASS)
  engine->props.preempt_timeout_ms = 0;
  +    /* Cap timeouts to prevent overflow inside GuC */
+    if (intel_guc_submission_is_wanted(>->uc.guc)) {
+    if (engine->props.timeslice_duration_ms > 
GUC_POLICY_MAX_EXEC_QUANTUM_MS) {


Hm "wanted".. There's been too much back and forth on the GuC load 
options over the years to keep track.. intel_engine_uses_guc work 
sounds like would work and read nicer.
I'm not adding a new feature check here. I'm just using the existing 
one. If we want to rename it yet again then that would be a different 
patch set.


$ grep intel_engine_uses_guc . -rl
./i915_perf.c
./i915_request.c
./selftests/intel_scheduler_helpers.c

Re: Report 2 in ext4 and journal based on v5.17-rc1

[Jan Kara]

On Thu 24-02-22 10:11:02, Byungchul Park wrote:
> On Wed, Feb 23, 2022 at 03:48:59PM +0100, Jan Kara wrote:
> > > KJOURNALD2(kthread)   TASK1(ksys_write)   TASK2(ksys_write)
> > > 
> > > wait A
> > > --- stuck
> > >   wait B
> > >   --- stuck
> > >   wait C
> > >   --- stuck
> > > 
> > > wake up B wake up C   wake up A
> > > 
> > > where:
> > > A is a wait_queue, j_wait_commit
> > > B is a wait_queue, j_wait_transaction_locked
> > > C is a rwsem, mapping.invalidate_lock
> > 
> > I see. But a situation like this is not necessarily a guarantee of a
> > deadlock, is it? I mean there can be task D that will eventually call say
> > 'wake up B' and unblock everything and this is how things were designed to
> > work? Multiple sources of wakeups are quite common I'd say... What does
> 
> Yes. At the very beginning when I desgined Dept, I was thinking whether
> to support multiple wakeup sources or not for a quite long time.
> Supporting it would be a better option to aovid non-critical reports.
> However, I thought anyway we'd better fix it - not urgent tho - if
> there's any single circle dependency. That's why I decided not to
> support it for now and wanted to gather the kernel guys' opinions. Thing
> is which policy we should go with.

I see. So supporting only a single wakeup source is fine for locks I guess.
But for general wait queues or other synchronization mechanisms, I'm afraid
it will lead to quite some false positive reports. Just my 2c.

> > Dept do to prevent false reports in cases like this?
> > 
> > > The above is the simplest form. And it's worth noting that Dept focuses
> > > on wait and event itself rather than grabing and releasing things like
> > > lock. The following is the more descriptive form of it.
> > > 
> > > KJOURNALD2(kthread)   TASK1(ksys_write)   TASK2(ksys_write)
> > > 
> > > wait @j_wait_commit
> > >   ext4_truncate_failed_write()
> > >  down_write(mapping.invalidate_lock)
> > > 
> > >  ext4_truncate()
> > > ...
> > > wait @j_wait_transaction_locked
> > > 
> > >   ext_truncate_failed_write()
> > >  
> > > down_write(mapping.invalidate_lock)
> > > 
> > >   ext4_should_retry_alloc()
> > >  ...
> > >  __jbd2_log_start_commit()
> > > wake_up(j_wait_commit)
> > > jbd2_journal_commit_transaction()
> > >wake_up(j_wait_transaction_locked)
> > >  up_write(mapping.invalidate_lock)
> > > 
> > > I hope this would help you understand the report.
> > 
> > I see, thanks for explanation! So the above scenario is impossible because
> 
> My pleasure.
> 
> > for anyone to block on @j_wait_transaction_locked the transaction must be
> > committing, which is done only by kjournald2 kthread and so that thread
> > cannot be waiting at @j_wait_commit. Essentially blocking on
> > @j_wait_transaction_locked means @j_wait_commit wakeup was already done.
> 
> kjournal2 repeatedly does the wait and the wake_up so the above scenario
> looks possible to me even based on what you explained. Maybe I should
> understand how the journal things work more for furhter discussion. Your
> explanation is so helpful. Thank you really.

OK, let me provide you with more details for better understanding :) In
jbd2 we have an object called 'transaction'. This object can go through
many states but for our case is important that transaction is moved to
T_LOCKED state and out of it only while jbd2_journal_commit_transaction()
function is executing and waiting on j_wait_transaction_locked waitqueue is
exactly waiting for a transaction to get out of T_LOCKED state. Function
jbd2_journal_commit_transaction() is executed only by kjournald. Hence
anyone can see transaction in T_LOCKED state only if kjournald is running
inside jbd2_journal_commit_transaction() and thus kjournald cannot be
sleeping on j_wait_commit at the same time. Does this explain things?

Honza
-- 
Jan Kara 
SUSE Labs, CR

Re: [PATCH v3 4/5] fbdev: Improve performance of cfb_imageblit()

[Sam Ravnborg]

Hi Javier,
On Thu, Feb 24, 2022 at 10:02:59AM +0100, Javier Martinez Canillas wrote:
> Hello Sam,
> 
> On 2/23/22 21:25, Sam Ravnborg wrote:
> 
> [snip]
> 
> > 
> > Question: What is cfb an abbreviation for anyway?
> > Not related to the patch - but if I have known the memory is lost..
> > 
> 
> I was curious so I dug on this. It seems CFB stands for Color Frame Buffer.
> Doing a `git grep "(CFB)"` in the linux history repo [0], I get this:
> 
>   Documentation/isdn/README.diversion:   (CFB). 
>   drivers/video/pmag-ba-fb.c: *   PMAG-BA TURBOchannel Color Frame Buffer 
> (CFB) card support,
>   include/video/pmag-ba-fb.h: *   TURBOchannel PMAG-BA Color Frame Buffer 
> (CFB) card support,
> 
> Probably the helpers are called like this because they were for any fbdev
> driver but assumed that the framebuffer was always in I/O memory. Later some
> drivers were allocating the framebuffer in system memory and still using the
> helpers, that were using I/O memory accessors and it's ilegal on some arches.
> 
> So the sys_* variants where introduced by commit 68648ed1f58d ("fbdev: add
> drawing functions for framebuffers in system RAM") to fix this. The old
> ones just kept their name, but probably it should had been renamed to io_*
> for the naming to be consistent with the sys_* functions.
> 
> [0]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/

Interesting - thanks for the history lesson and thanks for taking your
time to share your findings too.

Sam

Re: [PATCH v3 4/5] fbdev: Improve performance of cfb_imageblit()

[Geert Uytterhoeven]

Hi Javier,

On Thu, Feb 24, 2022 at 10:03 AM Javier Martinez Canillas
 wrote:
> On 2/23/22 21:25, Sam Ravnborg wrote:
> > Question: What is cfb an abbreviation for anyway?
> > Not related to the patch - but if I have known the memory is lost..
>
> I was curious so I dug on this. It seems CFB stands for Color Frame Buffer.
> Doing a `git grep "(CFB)"` in the linux history repo [0], I get this:

The naming actually comes from X11.
"mfb" is a monochrome frame buffer (bpp = 1).
"cfb" is a color frame buffer (bpp > 1), which uses a chunky format.

> Probably the helpers are called like this because they were for any fbdev
> driver but assumed that the framebuffer was always in I/O memory. Later some
> drivers were allocating the framebuffer in system memory and still using the
> helpers, that were using I/O memory accessors and it's ilegal on some arches.

Yep.  Graphics memory used to be on a graphics card.
On systems (usually non-x86) where it was part of main memory, usually
it didn't matter at all whether you used I/O memory or plain memory
accessors anyway.

Then x86 got unified memory...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v6 21/23] drm: rockchip: Add VOP2 driver

[Andy Yan]

Hi Sascha:

On 2/24/22 16:19, Sascha Hauer wrote:

On Sat, Feb 19, 2022 at 03:35:12PM +0800, Andy Yan wrote:

Hi Sascha:

On 2/18/22 16:00, Sascha Hauer wrote:

On Fri, Feb 18, 2022 at 11:50:32AM +0800, Andy Yan wrote:

Hi Sascha:

On 2/17/22 22:06, Heiko Stübner wrote:

Am Donnerstag, 17. Februar 2022, 14:58:23 CET schrieb Sascha Hauer:

Hi Andy,

Please trim the context in your answers to the relevant parts, it makes
it easier to find the things you said.

On Thu, Feb 17, 2022 at 08:00:11PM +0800, Andy Yan wrote:

Hi Sascha:


+
+   drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) 
{
+   struct rockchip_encoder *rkencoder = 
to_rockchip_encoder(encoder);
+   struct device_node *node, *parent;
+
+   parent = of_get_parent(rkencoder->port);
+
+   for_each_endpoint_of_node(parent, node) {

Is there any hurt directly use our downstream vendor kernel method here: use
vcstate->output_if set by encoder driver to get which interface we should
enable here?

There is no vcstate->output_if in mainline currently. Ok, we could add
that. The other thing is that there are multiple HDMI interfaces and
the id of the HDMI encoder is encoded into output_if. Downstream kernel
adds OF aliases to the HDMI ports. I didn't want to go that route
because it doesn't seem to be very elegant to me.

aliases is a very comm strategy in device tree world.

Yes, but not for retrieving bit offsets into registers. Normally aliases
can be changed at board level without confusing drivers.


And your method also
add need additional dt binds to define RK3568_VOP2_EP_xxx

You method is ok with device tree,  but it tied up this driver to device
tree, we are now tring to extend vop2 driver work with ACPI, so we hope this
driver can be much more flexible.

The current rockchip drm driver seems to be pretty much tied to device
tree. There are probably many other places that need parallel paths for
ACPI support, I think we can delay this particular part until we see the
whole picture. In the end we can still retrieve the output_if
information differently with ACPI while still retrieving the information
from the device tree the way we are doing currently.

The current driver only reference device thee at driver initial, we not wrap

device tree related things in other parts, so if we extend it to support
ACPI,

we just need modify the initial code, this make things easier.

The device tree parsing could be moved out of vop2_crtc_atomic_enable()
into some initialisation path. In the end it's static information,
there's no need to do it repeatedly in atomic_enable.

This could be one solution, the repeatedly parsing device tree in
atomic_enable is also my concern.

In addition, there are 2 HDMI, 2 eDP, 2 MIPI on the coming rk3588, so it's
better to consider give position

for HDMI1, EDP1, in  include/dt-bindings/soc/rockchip,vop2.h

The defines are rk3568 specific. rk3588 would use a set of rk3588
specific defines along with a rk3588_set_intf_mux().



Why not try to share these RK3568_VOP2_EP_XXX across all vop2 even vop 
based rockchip socs?


If make these definition RK3568 specific, we need copy all of it and 
change 3568 to 3588 than add HDMI1, HDMI0, EDP1,EDP0


when rk3588 coming, if there is another rk35xx, we need to the same 
thing again but they share same code logic and number,


the only difference is the definition name.


Please take a look at the current upstream vop driver,  it support 13 
socs, when we add support for a new vop , most of


the work is just add registers definition in rockchip_vop_reg.c, we 
don't need to duplicate soc specific code in rockchip_drm_vop.c,


these make the upstream process much easier, and keep the vop driver 
tiny and clean.



Sascha

Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: Make the heartbeat play nice with long pre-emption timeouts

[Tvrtko Ursulin]

On 23/02/2022 20:00, John Harrison wrote:

On 2/23/2022 05:58, Tvrtko Ursulin wrote:

On 23/02/2022 02:45, John Harrison wrote:

On 2/22/2022 03:19, Tvrtko Ursulin wrote:

On 18/02/2022 21:33, john.c.harri...@intel.com wrote:

From: John Harrison 

Compute workloads are inherantly not pre-emptible for long periods on
current hardware. As a workaround for this, the pre-emption timeout
for compute capable engines was disabled. This is undesirable with GuC
submission as it prevents per engine reset of hung contexts. Hence the
next patch will re-enable the timeout but bumped up by an order of
magnititude.


(Some typos above.)

I'm spotting 'inherently' but not anything else.


Magnititude! O;)

Doh!

[snip]

Whereas, bumping all heartbeat periods to be greater than the 
pre-emption timeout is wasteful and unnecessary. That leads to a 
total heartbeat time of about a minute. Which is a very long time to 
wait for a hang to be detected and recovered. Especially when the 
official limit on a context responding to an 'are you dead' query is 
only 7.5 seconds.


Not sure how did you get one minute?
7.5 * 2 (to be safe) = 15. 15 * 5 (number of heartbeat periods) = 75 => 
1 minute 15 seconds


Even ignoring any safety factor and just going with 7.5 * 5 still gets 
you to 37.5 seconds which is over a half a minute and likely to race.


Ah because my starting point is there should be no preempt timeout = heartbeat 
* 3, I just think that's too ugly.

Regardless, crux of argument was to avoid GuC engine reset and 
heartbeat reset racing with each other, and to do that by considering 
the preempt timeout with the heartbeat interval. I was thinking about 
this scenario in this series:


[Please use fixed width font and no line wrap to view.]

A)

tP = preempt timeout
tH = hearbeat interval

tP = 3 * tH

1) Background load = I915_PRIORITY_DISPLAY

<-- [tH] --> Pulse1 <-- [tH] --> Pulse2 <-- [tH] --> Pulse3 < [2 * 
tH] > FULL RESET

   |
   \- preemption 
triggered, tP = 3 * tH --\

\-> preempt timeout would hit here

Here we have collateral damage due full reset, since we can't tell GuC 
to reset just one engine and we fudged tP just to "account" for 
heartbeats.
You are missing the whole point of the patch series which is that the 
last heartbeat period is '2 * tP' not '2 * tH'.

+        longer = READ_ONCE(engine->props.preempt_timeout_ms) * 2;

By making the last period double the pre-emption timeout, it is 
guaranteed that the FULL RESET stage cannot be hit before the hardware 
has attempted and timed-out on at least one pre-emption.


Oh well :) that probably means the overall scheme is too odd for me. tp = 3tH 
and last pulse after 2tP I mean.


[snip]


<-- [tH] --> Pulse1 <-- [tH] --> Pulse2 <-- [tH] --> Pulse3 < [2 * 
tH] > full reset would be here

   |
   \- preemption triggered, tP = 3 * tH \
\-> Preempt timeout reset

Here is is kind of least worse, but question is why we fudged tP when 
it gives us nothing good in this case.


The point of fudging tP(RCS) is to give compute workloads longer to 
reach a pre-emptible point (given that EU walkers are basically not 
pre-emptible). The reason for doing the fudge is not connected to the 
heartbeat at all. The fact that it causes problems for the heartbeat is 
an undesired side effect.


Note that the use of 'tP(RCS) = tH * 3' was just an arbitrary 
calculation that gave us something that all interested parties were 
vaguely happy with. It could just as easily be a fixed, hard coded value 
of 7.5s but having it based on something configurable seemed more 
sensible. The other option was 'tP(RCS) = tP * 12' but that felt more 
arbitrary than basing it on the average heartbeat timeout. As in, three 
heartbeat periods is about what a normal prio task gets before it gets 
pre-empted by the heartbeat. So using that for general purpose 
pre-emptions (e.g. time slicing between multiple user apps) seems 
reasonable.


I think the fact you say tP fudge is not related to heartbeats and then go to mention 
heartbeat even in the "formula" which uses no tH is saying something (at least 
that's how I read the 7.5s option). :)


B)

Instead, my idea to account for preempt timeout when calculating when 
to schedule next hearbeat would look like this:


First of all tP can be left at a large value unrelated to tH. Lets say 
tP = 640ms. tH stays 2.5s.
640ms is not 'large'. The requirement is either zero (disabled) or 
region of 7.5s. The 640ms figure is the default for non-compute engines. 
Anything that can run EUs needs to be 'huge'.





1) Background load = I915_PRIORITY_DISPLAY

<-- [tH + tP] --> Pulse1 <-- [tH + tP] --> Pulse2 <-- [tH + tP] --> 
Pulse3 <-- [tH + tP] --> full reset would be here
Sure, this works but each period is now 2.5 + 7.5 = 10s. The full five 
periods is therefore 50s, which is practically a 

[PULL] drm-intel-fixes

[Tvrtko Ursulin]

Hi Dave,  Daniel,

An assortment of display fixes for -rc6.

Regards,

Tvrtko

drm-intel-fixes-2022-02-24:
- Fix QGV handling on ADL-P+ (Ville Syrjälä)
- Fix bw atomic check when switching between SAGV vs. no SAGV (Ville Syrjälä)
- Disconnect PHYs left connected by BIOS on disabled ports (Imre Deak)
- Fix SAVG to no SAGV transitions on TGL+ (Ville Syrjälä)
- Print PHY name properly on calibration error (DG2) (Matt Roper)
The following changes since commit cfb92440ee71adcc2105b0890bb01ac3cddb8507:

  Linux 5.17-rc5 (2022-02-20 13:07:20 -0800)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2022-02-24

for you to fetch changes up to 28adef861233c6fce47372ebd2070b55eaa8e899:

  drm/i915/dg2: Print PHY name properly on calibration error (2022-02-21 
09:43:11 +)


- Fix QGV handling on ADL-P+ (Ville Syrjälä)
- Fix bw atomic check when switching between SAGV vs. no SAGV (Ville Syrjälä)
- Disconnect PHYs left connected by BIOS on disabled ports (Imre Deak)
- Fix SAVG to no SAGV transitions on TGL+ (Ville Syrjälä)
- Print PHY name properly on calibration error (DG2) (Matt Roper)


Imre Deak (1):
  drm/i915: Disconnect PHYs left connected by BIOS on disabled ports

Matt Roper (1):
  drm/i915/dg2: Print PHY name properly on calibration error

Ville Syrjälä (3):
  drm/i915: Widen the QGV point mask
  drm/i915: Correctly populate use_sagv_wm for all pipes
  drm/i915: Fix bw atomic check when switching between SAGV vs. no SAGV

 drivers/gpu/drm/i915/display/intel_bw.c   | 18 --
 drivers/gpu/drm/i915/display/intel_bw.h   |  8 
 drivers/gpu/drm/i915/display/intel_snps_phy.c |  2 +-
 drivers/gpu/drm/i915/display/intel_tc.c   | 26 --
 drivers/gpu/drm/i915/intel_pm.c   | 22 +++---
 5 files changed, 52 insertions(+), 24 deletions(-)

Re: [PATCH v6 21/23] drm: rockchip: Add VOP2 driver

[Sascha Hauer]

On Thu, Feb 24, 2022 at 06:54:35PM +0800, Andy Yan wrote:
> Hi Sascha:
> 
> On 2/24/22 16:19, Sascha Hauer wrote:
> > On Sat, Feb 19, 2022 at 03:35:12PM +0800, Andy Yan wrote:
> > > Hi Sascha:
> > > 
> > > On 2/18/22 16:00, Sascha Hauer wrote:
> > > > On Fri, Feb 18, 2022 at 11:50:32AM +0800, Andy Yan wrote:
> > > > > Hi Sascha:
> > > > > 
> > > > > On 2/17/22 22:06, Heiko Stübner wrote:
> > > > > > Am Donnerstag, 17. Februar 2022, 14:58:23 CET schrieb Sascha Hauer:
> > > > > > > Hi Andy,
> > > > > > > 
> > > > > > > Please trim the context in your answers to the relevant parts, it 
> > > > > > > makes
> > > > > > > it easier to find the things you said.
> > > > > > > 
> > > > > > > On Thu, Feb 17, 2022 at 08:00:11PM +0800, Andy Yan wrote:
> > > > > > > > Hi Sascha:
> > > > > > > > 
> > > > > > > > > +
> > > > > > > > > + drm_for_each_encoder_mask(encoder, crtc->dev, 
> > > > > > > > > crtc_state->encoder_mask) {
> > > > > > > > > + struct rockchip_encoder *rkencoder = 
> > > > > > > > > to_rockchip_encoder(encoder);
> > > > > > > > > + struct device_node *node, *parent;
> > > > > > > > > +
> > > > > > > > > + parent = of_get_parent(rkencoder->port);
> > > > > > > > > +
> > > > > > > > > + for_each_endpoint_of_node(parent, node) {
> > > > > > > > Is there any hurt directly use our downstream vendor kernel 
> > > > > > > > method here: use
> > > > > > > > vcstate->output_if set by encoder driver to get which interface 
> > > > > > > > we should
> > > > > > > > enable here?
> > > > > > > There is no vcstate->output_if in mainline currently. Ok, we 
> > > > > > > could add
> > > > > > > that. The other thing is that there are multiple HDMI interfaces 
> > > > > > > and
> > > > > > > the id of the HDMI encoder is encoded into output_if. Downstream 
> > > > > > > kernel
> > > > > > > adds OF aliases to the HDMI ports. I didn't want to go that route
> > > > > > > because it doesn't seem to be very elegant to me.
> > > > > aliases is a very comm strategy in device tree world.
> > > > Yes, but not for retrieving bit offsets into registers. Normally aliases
> > > > can be changed at board level without confusing drivers.
> > > > 
> > > > > And your method also
> > > > > add need additional dt binds to define RK3568_VOP2_EP_xxx
> > > > > > > > You method is ok with device tree,  but it tied up this driver 
> > > > > > > > to device
> > > > > > > > tree, we are now tring to extend vop2 driver work with ACPI, so 
> > > > > > > > we hope this
> > > > > > > > driver can be much more flexible.
> > > > > > > The current rockchip drm driver seems to be pretty much tied to 
> > > > > > > device
> > > > > > > tree. There are probably many other places that need parallel 
> > > > > > > paths for
> > > > > > > ACPI support, I think we can delay this particular part until we 
> > > > > > > see the
> > > > > > > whole picture. In the end we can still retrieve the output_if
> > > > > > > information differently with ACPI while still retrieving the 
> > > > > > > information
> > > > > > > from the device tree the way we are doing currently.
> > > > > The current driver only reference device thee at driver initial, we 
> > > > > not wrap
> > > > > 
> > > > > device tree related things in other parts, so if we extend it to 
> > > > > support
> > > > > ACPI,
> > > > > 
> > > > > we just need modify the initial code, this make things easier.
> > > > The device tree parsing could be moved out of vop2_crtc_atomic_enable()
> > > > into some initialisation path. In the end it's static information,
> > > > there's no need to do it repeatedly in atomic_enable.
> > > This could be one solution, the repeatedly parsing device tree in
> > > atomic_enable is also my concern.
> > > 
> > > In addition, there are 2 HDMI, 2 eDP, 2 MIPI on the coming rk3588, so it's
> > > better to consider give position
> > > 
> > > for HDMI1, EDP1, in  include/dt-bindings/soc/rockchip,vop2.h
> > The defines are rk3568 specific. rk3588 would use a set of rk3588
> > specific defines along with a rk3588_set_intf_mux().
> 
> 
> Why not try to share these RK3568_VOP2_EP_XXX across all vop2 even vop based
> rockchip socs?
> 
> If make these definition RK3568 specific, we need copy all of it and change
> 3568 to 3588 than add HDMI1, HDMI0, EDP1,EDP0
> 
> when rk3588 coming, if there is another rk35xx, we need to the same thing
> again but they share same code logic and number,

I can make the defines RK3568 agnostic and use ROCKCHIP_ as prefix. The
actual numbers don't matter much, so we can add new interfaces or
instances thereof at the end with the next free number.

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

[PATCH] drm/todo: Update panic handling todo

[Daniel Vetter]

Some things changed, and add two useful links.

Cc: Javier Martinez Canillas 
Cc: Pekka Paalanen 
Cc: gpicc...@igalia.com
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/todo.rst | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 7bf7f2111696..283d35a500bd 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -475,8 +475,12 @@ This is a really varied tasks with lots of little bits and 
pieces:
   achieved by using an IPI to the local processor.
 
 * There's a massive confusion of different panic handlers. DRM fbdev emulation
-  helpers have one, but on top of that the fbcon code itself also has one. We
-  need to make sure that they stop fighting over each another.
+  helpers had their own (long removed), but on top of that the fbcon code 
itself
+  also has one. We need to make sure that they stop fighting over each another.
+  This is worked around by checking ``oops_in_progress`` at various entry 
points
+  into the DRM fbdev emulation helpers. A much cleaner approach here would be 
to
+  switch fbcon to the `threaded printk support
+  `_.
 
 * ``drm_can_sleep()`` is a mess. It hides real bugs in normal operations and
   isn't a full solution for panic paths. We need to make sure that it only
@@ -488,16 +492,9 @@ This is a really varied tasks with lots of little bits and 
pieces:
   even spinlocks (because NMI and hardirq can panic too). We need to either
   make sure to not call such paths, or trylock everything. Really tricky.
 
-* For the above locking troubles reasons it's pretty much impossible to
-  attempt a synchronous modeset from panic handlers. The only thing we could
-  try to achive is an atomic ``set_base`` of the primary plane, and hope that
-  it shows up. Everything else probably needs to be delayed to some worker or
-  something else which happens later on. Otherwise it just kills the box
-  harder, prevent the panic from going out on e.g. netconsole.
-
-* There's also proposal for a simplied DRM console instead of the full-blown
-  fbcon and DRM fbdev emulation. Any kind of panic handling tricks should
-  obviously work for both console, in case we ever get kmslog merged.
+* A clean solution would be an entirely separate panic output support in KMS,
+  bypassing the current fbcon support. See `[PATCH v2 0/3] drm: Add panic 
handling
+  
`_.
 
 Contact: Daniel Vetter
 
-- 
2.34.1

[PATCH] drm/i915: Be more gentle when exiting non-persistent contexts

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

When a non-persistent context exits we currently mark it as banned in
order to trigger fast termination of any outstanding GPU jobs it may have
left running.

In doing so we apply a very strict 1ms limit in which the left over job
has to preempt before we issues an engine resets.

Some workloads are not able to cleanly preempt in that time window and it
can be argued that it would instead be better to give them a bit more
grace since avoiding engine resets is generally preferrable.

To achieve this the patch splits handling of banned contexts from simply
closed non-persistent ones and then applies different timeouts for both
and also extends the criteria which determines if a request should be
scheduled back in after preemption or not.

20ms preempt timeout grace is given to exited non-persistent contexts
which have been empirically tested to satisfy customers requirements
and still provides reasonably quick cleanup post exit.

v2:
 * Streamline fast path checks.

v3:
 * Simplify by using only schedulable status.
 * Increase timeout to 20ms.

v4:
 * Fix live_execlists selftest.

v5:
 * Fix logic in kill_engines.

v6:
 * Rebase.

v7:
 * Add GuC support.

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: Zhen Han 
Cc: Matthew Brost 
Cc: John Harrison 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 22 +++-
 drivers/gpu/drm/i915/gt/intel_context.c   | 25 ++
 drivers/gpu/drm/i915/gt/intel_context.h   | 26 ++-
 drivers/gpu/drm/i915/gt/intel_context_types.h |  4 ++-
 .../drm/i915/gt/intel_execlists_submission.c  | 13 +++---
 .../gpu/drm/i915/gt/intel_ring_submission.c   |  7 ++---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 ++-
 drivers/gpu/drm/i915/i915_request.c   |  2 +-
 8 files changed, 86 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index bc6d59df064d..3a61ec753894 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1332,7 +1332,8 @@ static struct intel_engine_cs *active_engine(struct 
intel_context *ce)
return engine;
 }
 
-static void kill_engines(struct i915_gem_engines *engines, bool ban)
+static void
+kill_engines(struct i915_gem_engines *engines, bool ban, bool persistent)
 {
struct i915_gem_engines_iter it;
struct intel_context *ce;
@@ -1346,8 +1347,15 @@ static void kill_engines(struct i915_gem_engines 
*engines, bool ban)
 */
for_each_gem_engine(ce, engines, it) {
struct intel_engine_cs *engine;
+   bool skip = false;
 
-   if (ban && intel_context_ban(ce, NULL))
+   if (ban)
+   skip = intel_context_ban(ce, NULL);
+   else if (!persistent)
+   skip = intel_context_exit_nonpersistent(ce, NULL);
+
+   /* Already banned or non-persistent closed. */
+   if (skip)
continue;
 
/*
@@ -1360,7 +1368,7 @@ static void kill_engines(struct i915_gem_engines 
*engines, bool ban)
engine = active_engine(ce);
 
/* First attempt to gracefully cancel the context */
-   if (engine && !__cancel_engine(engine) && ban)
+   if (engine && !__cancel_engine(engine) && (ban || !persistent))
/*
 * If we are unable to send a preemptive pulse to bump
 * the context from the GPU, we have to resort to a full
@@ -1372,8 +1380,6 @@ static void kill_engines(struct i915_gem_engines 
*engines, bool ban)
 
 static void kill_context(struct i915_gem_context *ctx)
 {
-   bool ban = (!i915_gem_context_is_persistent(ctx) ||
-   !ctx->i915->params.enable_hangcheck);
struct i915_gem_engines *pos, *next;
 
spin_lock_irq(&ctx->stale.lock);
@@ -1386,7 +1392,8 @@ static void kill_context(struct i915_gem_context *ctx)
 
spin_unlock_irq(&ctx->stale.lock);
 
-   kill_engines(pos, ban);
+   kill_engines(pos, !ctx->i915->params.enable_hangcheck,
+i915_gem_context_is_persistent(ctx));
 
spin_lock_irq(&ctx->stale.lock);
GEM_BUG_ON(i915_sw_fence_signaled(&pos->fence));
@@ -1432,7 +1439,8 @@ static void engines_idle_release(struct i915_gem_context 
*ctx,
 
 kill:
if (list_empty(&engines->link)) /* raced, already closed */
-   kill_engines(engines, true);
+   kill_engines(engines, true,
+i915_gem_context_is_persistent(ctx));
 
i915_sw_fence_commit(&engines->fence);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index 5d0ec7c49b6a..27cd71c13097 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i9

Re: [PATCH] drm/todo: Update panic handling todo

[Javier Martinez Canillas]

Hello Daniel,

On 2/24/22 13:59, Daniel Vetter wrote:
> Some things changed, and add two useful links.
> 
> Cc: Javier Martinez Canillas 
> Cc: Pekka Paalanen 
> Cc: gpicc...@igalia.com
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/gpu/todo.rst | 21 +
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 7bf7f2111696..283d35a500bd 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -475,8 +475,12 @@ This is a really varied tasks with lots of little bits 
> and pieces:
>achieved by using an IPI to the local processor.
>  
>  * There's a massive confusion of different panic handlers. DRM fbdev 
> emulation
> -  helpers have one, but on top of that the fbcon code itself also has one. We
> -  need to make sure that they stop fighting over each another.
> +  helpers had their own (long removed), but on top of that the fbcon code 
> itself
> +  also has one. We need to make sure that they stop fighting over each 
> another.

The "over each another" sounds a little bit off, shouldn't be "over each other" 
?

> +  This is worked around by checking ``oops_in_progress`` at various entry 
> points
> +  into the DRM fbdev emulation helpers. A much cleaner approach here would 
> be to
> +  switch fbcon to the `threaded printk support
> +  `_.
>  
>  * ``drm_can_sleep()`` is a mess. It hides real bugs in normal operations and
>isn't a full solution for panic paths. We need to make sure that it only
> @@ -488,16 +492,9 @@ This is a really varied tasks with lots of little bits 
> and pieces:
>even spinlocks (because NMI and hardirq can panic too). We need to either
>make sure to not call such paths, or trylock everything. Really tricky.
>  
> -* For the above locking troubles reasons it's pretty much impossible to
> -  attempt a synchronous modeset from panic handlers. The only thing we could
> -  try to achive is an atomic ``set_base`` of the primary plane, and hope that
> -  it shows up. Everything else probably needs to be delayed to some worker or
> -  something else which happens later on. Otherwise it just kills the box
> -  harder, prevent the panic from going out on e.g. netconsole.
> -
> -* There's also proposal for a simplied DRM console instead of the full-blown
> -  fbcon and DRM fbdev emulation. Any kind of panic handling tricks should
> -  obviously work for both console, in case we ever get kmslog merged.
> +* A clean solution would be an entirely separate panic output support in KMS,
> +  bypassing the current fbcon support. See `[PATCH v2 0/3] drm: Add panic 
> handling
> +  
> `_.
>  

Having these two links in the docs is very useful. Thanks a lot for adding that.

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat

[PATCH] drm/todo: Update panic handling todo

[Daniel Vetter]

Some things changed, and add two useful links.

v2: Also include a link to the QR encoding work. Plus review from
Javier.

Reviewed-by: Javier Martinez Canillas 
Cc: Javier Martinez Canillas 
Cc: Pekka Paalanen 
Cc: gpicc...@igalia.com
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/todo.rst | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 7bf7f2111696..2b1e7fa45603 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -475,8 +475,12 @@ This is a really varied tasks with lots of little bits and 
pieces:
   achieved by using an IPI to the local processor.
 
 * There's a massive confusion of different panic handlers. DRM fbdev emulation
-  helpers have one, but on top of that the fbcon code itself also has one. We
-  need to make sure that they stop fighting over each another.
+  helpers had their own (long removed), but on top of that the fbcon code 
itself
+  also has one. We need to make sure that they stop fighting over each other.
+  This is worked around by checking ``oops_in_progress`` at various entry 
points
+  into the DRM fbdev emulation helpers. A much cleaner approach here would be 
to
+  switch fbcon to the `threaded printk support
+  `_.
 
 * ``drm_can_sleep()`` is a mess. It hides real bugs in normal operations and
   isn't a full solution for panic paths. We need to make sure that it only
@@ -488,16 +492,15 @@ This is a really varied tasks with lots of little bits 
and pieces:
   even spinlocks (because NMI and hardirq can panic too). We need to either
   make sure to not call such paths, or trylock everything. Really tricky.
 
-* For the above locking troubles reasons it's pretty much impossible to
-  attempt a synchronous modeset from panic handlers. The only thing we could
-  try to achive is an atomic ``set_base`` of the primary plane, and hope that
-  it shows up. Everything else probably needs to be delayed to some worker or
-  something else which happens later on. Otherwise it just kills the box
-  harder, prevent the panic from going out on e.g. netconsole.
+* A clean solution would be an entirely separate panic output support in KMS,
+  bypassing the current fbcon support. See `[PATCH v2 0/3] drm: Add panic 
handling
+  
`_.
 
-* There's also proposal for a simplied DRM console instead of the full-blown
-  fbcon and DRM fbdev emulation. Any kind of panic handling tricks should
-  obviously work for both console, in case we ever get kmslog merged.
+* Encoding the actual oops and preceeding dmesg in a QR might help with the
+  dread "important stuff scrolled away" problem. See `[RFC][PATCH] Oops 
messages
+  transfer using QR codes
+  
`_
+  for some example code that could be reused.
 
 Contact: Daniel Vetter
 
-- 
2.34.1

Re: [PATCH] drm/todo: Update panic handling todo

[Guilherme G. Piccoli]

On 24/02/2022 10:24, Daniel Vetter wrote:
> Some things changed, and add two useful links.
> 
> v2: Also include a link to the QR encoding work. Plus review from
> Javier.
> 
> Reviewed-by: Javier Martinez Canillas 
> Cc: Javier Martinez Canillas 
> Cc: Pekka Paalanen 
> Cc: gpicc...@igalia.com
> Signed-off-by: Daniel Vetter 
> ---

Hi Daniel, thanks for the improvement - much appreciated!

Below a comment inline; other than that, feel free to add my:
Reviewed-by: Guilherme G. Piccoli 

Cheers,


Guilherme

> [...]
>  
> -* There's also proposal for a simplied DRM console instead of the full-blown
> -  fbcon and DRM fbdev emulation. Any kind of panic handling tricks should
> -  obviously work for both console, in case we ever get kmslog merged.
> +* Encoding the actual oops and preceeding dmesg in a QR might help with the

Email client complains about "preceeding" word - misspelled maybe?

> +  dread "important stuff scrolled away" problem. See `[RFC][PATCH] Oops 
> messages
> +  transfer using QR codes
> +  
> `_
> +  for some example code that could be reused.
>  
>  Contact: Daniel Vetter
>  

Re: [PATCH v8 15/19] drm/mediatek: Add mt8195 Embedded DisplayPort driver

[AngeloGioacchino Del Regno]

Il 18/02/22 15:54, Guillaume Ranquet ha scritto:

From: Markus Schneider-Pargmann 

This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.

It supports the mt8195, the embedded DisplayPort units. It offers
hot-plug-detection and DisplayPort 1.4 with up to 4 lanes.

The driver creates a child device for the phy. The child device will
never exist without the parent being active. As they are sharing a
register range, the parent passes a regmap pointer to the child so that
both can work with the same register range. The phy driver sets device
data that is read by the parent to get the phy device that can be used
to control the phy properties.

This driver is based on an initial version by
Jason-JH.Lin .

Signed-off-by: Markus Schneider-Pargmann 
Signed-off-by: Guillaume Ranquet 
Reported-by: kernel test robot 
---
  drivers/gpu/drm/mediatek/Kconfig   |7 +
  drivers/gpu/drm/mediatek/Makefile  |2 +
  drivers/gpu/drm/mediatek/mtk_dp.c  | 2358 
  drivers/gpu/drm/mediatek/mtk_dp_reg.h  |  568 ++
  drivers/gpu/drm/mediatek/mtk_drm_drv.c |1 +
  drivers/gpu/drm/mediatek/mtk_drm_drv.h |1 +
  6 files changed, 2937 insertions(+)
  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
  create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h




Sorry for the double review, but I've just noticed something critical:

the new version of mtk_dp_train_handler is severely misbehaving, producing

non-functional display.

I also went on with some effort to give you a solution for this,
which implies to also tick the DP_STATE with the DP_TRAIN_STATE;

This is my take on it:

static int mtk_dp_train_handler(struct mtk_dp *mtk_dp)

{

bool training_done = false;

short max_retry = 50;

int ret = 0;



do {

switch (mtk_dp->train_state) {

case MTK_DP_TRAIN_STATE_STARTUP:

mtk_dp_state_handler(mtk_dp);

mtk_dp->train_state = MTK_DP_TRAIN_STATE_CHECKCAP;

break;



case MTK_DP_TRAIN_STATE_CHECKCAP:

if (mtk_dp_parse_capabilities(mtk_dp)) {

mtk_dp->train_info.check_cap_count = 0;

mtk_dp->train_state = 
MTK_DP_TRAIN_STATE_CHECKEDID;

} else {

mtk_dp->train_info.check_cap_count++;



if (mtk_dp->train_info.check_cap_count >

MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT) {

mtk_dp->train_info.check_cap_count = 0;

mtk_dp->train_state = 
MTK_DP_TRAIN_STATE_DPIDLE;

ret = -ETIMEDOUT;

}

}

break;



case MTK_DP_TRAIN_STATE_CHECKEDID:

mtk_dp->audio_enable =

!mtk_dp_edid_parse_audio_capabilities(

mtk_dp, 
&mtk_dp->info.audio_caps);



if (!mtk_dp->audio_enable)

memset(&mtk_dp->info.audio_caps, 0,

   sizeof(mtk_dp->info.audio_caps));



mtk_dp->train_state = MTK_DP_TRAIN_STATE_TRAINING_PRE;

break;



case MTK_DP_TRAIN_STATE_TRAINING_PRE:

mtk_dp_state_handler(mtk_dp);

mtk_dp->train_state = MTK_DP_TRAIN_STATE_TRAINING;

break;



case MTK_DP_TRAIN_STATE_TRAINING:

ret = mtk_dp_train_start(mtk_dp);

if (ret == 0) {

mtk_dp_video_mute(mtk_dp, true);

mtk_dp_audio_mute(mtk_dp, true);

mtk_dp->train_state = MTK_DP_TRAIN_STATE_NORMAL;

mtk_dp_fec_enable(mtk_dp, mtk_dp->has_fec);

} else if (ret != -EAGAIN) {

mtk_dp->train_state = MTK_DP_TRAIN_STATE_DPIDLE;

}

break;

case MTK_DP_TRAIN_STATE_NORMAL:

mtk_dp_state_handler(mtk_dp);

training_done = true;

break;

case MTK_DP_TRAIN_STATE_DPIDLE:

break;

default:

break;

}



if (ret) {

if (ret == -EAGAIN)

continue;

/*

 * If we get any other error number, it doesn't

 * make any sense to keep iterating.

Re: [PATCH v5 1/4] drm/i915/guc: Add fetch of hwconfig table

[Michal Wajdeczko]

On 22.02.2022 11:36, Jordan Justen wrote:
> From: John Harrison 
> 
> Implement support for fetching the hardware description table from the
> GuC. The call is made twice - once without a destination buffer to
> query the size and then a second time to fill in the buffer.
> 
> Note that the table is only available on ADL-P and later platforms.
> 
> v5 (of Jordan's posting):
>  * Various changes made by Jordan and recommended by Michal
>- Makefile ordering
>- Adjust "struct intel_guc_hwconfig hwconfig" comment
>- Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
>- Drop inline from hwconfig_to_guc()
>- Replace hwconfig param with guc in __guc_action_get_hwconfig()
>- Move zero size check into guc_hwconfig_discover_size()
>- Change comment to say zero size offset/size is needed to get size
>- Add has_guc_hwconfig to devinfo and drop has_table()
>- Change drm_err to notice in __uc_init_hw() and use %pe
> 
> Cc: Michal Wajdeczko 
> Signed-off-by: Rodrigo Vivi 
> Signed-off-by: John Harrison 
> Reviewed-by: Matthew Brost 
> Acked-by: Jon Bloomfield 
> Signed-off-by: Jordan Justen 
> ---
>  drivers/gpu/drm/i915/Makefile |   1 +
>  .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   1 +
>  .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h   |   4 +
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h|   3 +
>  .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   | 145 ++
>  .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.h   |  19 +++
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c |   7 +
>  drivers/gpu/drm/i915/i915_pci.c   |   1 +
>  drivers/gpu/drm/i915/intel_device_info.h  |   1 +
>  9 files changed, 182 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e9ce09620eb5..661f1afb51d7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -188,6 +188,7 @@ i915-y += gt/uc/intel_uc.o \
> gt/uc/intel_guc_ct.o \
> gt/uc/intel_guc_debugfs.o \
> gt/uc/intel_guc_fw.o \
> +   gt/uc/intel_guc_hwconfig.o \
> gt/uc/intel_guc_log.o \
> gt/uc/intel_guc_log_debugfs.o \
> gt/uc/intel_guc_rc.o \
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h 
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> index fe5d7d261797..4a61c819f32b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> @@ -137,6 +137,7 @@ enum intel_guc_action {
>   INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
>   INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
>   INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
> + INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100,
>   INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
>   INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
>   INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505,
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h 
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
> index 488b6061ee89..f9e2a6aaef4a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
> @@ -8,6 +8,10 @@
>  
>  enum intel_guc_response_status {
>   INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0,
> + INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20,
> + INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201,
> + INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202,
> + INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204,
>   INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
>  };
>  
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index f9240d4baa69..2058eb8c3d0c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -13,6 +13,7 @@
>  #include "intel_guc_fw.h"
>  #include "intel_guc_fwif.h"
>  #include "intel_guc_ct.h"
> +#include "intel_guc_hwconfig.h"
>  #include "intel_guc_log.h"
>  #include "intel_guc_reg.h"
>  #include "intel_guc_slpc_types.h"
> @@ -37,6 +38,8 @@ struct intel_guc {
>   struct intel_guc_ct ct;
>   /** @slpc: sub-structure containing SLPC related data and objects */
>   struct intel_guc_slpc slpc;
> + /** @hwconfig: data related to hardware configuration KLV blob */
> + struct intel_guc_hwconfig hwconfig;
>  
>   /** @sched_engine: Global engine used to submit requests to GuC */
>   struct i915_sched_engine *sched_engine;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> new file mode 100644
> index ..ad289603460c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include "gt/in

Re: [PATCH] drm/repaper: Use format helper for xrgb8888 to monochrome conversion

[Noralf Trønnes]

Den 23.02.2022 20.37, skrev Javier Martinez Canillas:
> There is now a drm_fb_xrgb_to_mono_reversed() helper function to do
> format conversion from XRGB to reversed monochrome.
> 
> Use that helper and remove the open coded version in the repaper driver.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---

Tested-by: Noralf Trønnes 
Reviewed-by: Noralf Trønnes 

Do you have commit rights and will apply this patch?

Noralf.

[PATCH 0/3] enhanced edid driver compatibility

[Lee Shawn C]

Support to parse multiple CEA extension blocks and HF-EEODB to
extend drm edid driver's capability.

Lee Shawn C (3):
  drm/edid: parse multiple CEA extension block
  drm/edid: read HF-EEODB ext block
  drm/edid: parse HF-EEODB CEA extension block

 drivers/gpu/drm/drm_connector.c |   5 +-
 drivers/gpu/drm/drm_displayid.c |   5 +-
 drivers/gpu/drm/drm_edid.c  | 200 ++--
 include/drm/drm_edid.h  |   4 +-
 4 files changed, 151 insertions(+), 63 deletions(-)

-- 
2.17.1

[PATCH 2/3] drm/edid: read HF-EEODB ext block

[Lee Shawn C]

Support to read HF_EEODB block that request by HDMI 2.1 specification.

Cc: Jani Nikula 
Cc: Ville Syrjala 
Cc: Ankit Nautiyal 
Signed-off-by: Lee Shawn C 
---
 drivers/gpu/drm/drm_connector.c |  5 ++-
 drivers/gpu/drm/drm_edid.c  | 76 ++---
 include/drm/drm_edid.h  |  2 +
 3 files changed, 77 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index a50c82bc2b2f..0f9e3ef00be7 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2137,8 +2137,11 @@ int drm_connector_update_edid_property(struct 
drm_connector *connector,
if (connector->override_edid)
return 0;
 
-   if (edid)
+   if (edid) {
size = EDID_LENGTH * (1 + edid->extensions);
+   if (drm_edid_is_hf_eeodb_blk_available(edid))
+   size = EDID_LENGTH * (1 + 
drm_edid_read_hf_eeodb_blk_size(edid));
+   }
 
/* Set the display info, using edid if available, otherwise
 * resetting the values to defaults. This duplicates the work
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 19426f28b411..056e735ef932 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1991,7 +1991,7 @@ struct edid *drm_do_get_edid(struct drm_connector 
*connector,
void *data)
 {
int i, j = 0, valid_extensions = 0;
-   u8 *edid, *new;
+   u8 *edid, *new, ext_eeodb_blk_size;
struct edid *override;
 
override = drm_get_override_edid(connector);
@@ -2051,7 +2051,40 @@ struct edid *drm_do_get_edid(struct drm_connector 
*connector,
}
 
kfree(edid);
+   return (struct edid *)new;
+   }
+
+   if (drm_edid_is_hf_eeodb_blk_available((struct edid *)edid)) {
+   ext_eeodb_blk_size = drm_edid_read_hf_eeodb_blk_size((struct 
edid *)edid);
+
+   // no more ext blk wait for read
+   if (ext_eeodb_blk_size <= 1)
+   return (struct edid *)edid;
+
+   new = krealloc(edid, (ext_eeodb_blk_size + 1) * EDID_LENGTH, 
GFP_KERNEL);
+   if (!new)
+   goto out;
edid = new;
+
+   valid_extensions = ext_eeodb_blk_size - 1;
+   for (j = 2; j <= ext_eeodb_blk_size; j++) {
+   u8 *block = edid + j * EDID_LENGTH;
+
+   for (i = 0; i < 4; i++) {
+   if (get_edid_block(data, block, j, EDID_LENGTH))
+   goto out;
+   if (drm_edid_block_valid(block, j, false, NULL))
+   break;
+   }
+
+   if (i == 4)
+   valid_extensions--;
+   }
+
+   if (valid_extensions != ext_eeodb_blk_size - 1) {
+   DRM_ERROR("Not able to retrieve proper EDID contain 
HF-EEODB data.\n");
+   goto out;
+   }
}
 
return (struct edid *)edid;
@@ -3315,15 +3348,17 @@ add_detailed_modes(struct drm_connector *connector, 
struct edid *edid,
 #define VIDEO_BLOCK 0x02
 #define VENDOR_BLOCK0x03
 #define SPEAKER_BLOCK  0x04
-#define HDR_STATIC_METADATA_BLOCK  0x6
-#define USE_EXTENDED_TAG 0x07
-#define EXT_VIDEO_CAPABILITY_BLOCK 0x00
+#define EXT_VIDEO_CAPABILITY_BLOCK 0x00
+#define HDR_STATIC_METADATA_BLOCK  0x06
+#define USE_EXTENDED_TAG   0x07
 #define EXT_VIDEO_DATA_BLOCK_420   0x0E
-#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
+#define EXT_VIDEO_CAP_BLOCK_Y420CMDB   0x0F
+#define EXT_VIDEO_HF_EEODB_DATA_BLOCK  0x78
 #define EDID_BASIC_AUDIO   (1 << 6)
 #define EDID_CEA_YCRCB444  (1 << 5)
 #define EDID_CEA_YCRCB422  (1 << 4)
 #define EDID_CEA_VCDB_QS   (1 << 6)
+#define HF_EEODB_LENGTH2
 
 /*
  * Search EDID for CEA extension block.
@@ -4273,6 +4308,37 @@ static bool cea_db_is_y420vdb(const u8 *db)
return true;
 }
 
+static bool cea_db_is_hdmi_forum_eeodb(const u8 *db)
+{
+   if (cea_db_tag(db) != USE_EXTENDED_TAG)
+   return false;
+
+   if (cea_db_payload_len(db) != HF_EEODB_LENGTH)
+   return false;
+
+   if (cea_db_extended_tag(db) != EXT_VIDEO_HF_EEODB_DATA_BLOCK)
+   return false;
+
+   return true;
+}
+
+bool drm_edid_is_hf_eeodb_blk_available(const struct edid *edid)
+{
+   const u8 *eeodb_header = (u8 *)edid + EDID_LENGTH + 4;
+
+   if (!edid->extensions)
+   return false;
+
+   return cea_db_is_hdmi_forum_eeodb(eeodb_header);
+}
+EXPORT_SYMBOL_GPL(drm_edid_is_hf_eeodb_blk_available);
+
+u8 drm_edid_read_hf_eeodb_blk_size(const struct edid *edid)
+{
+   return ((u8 *)edid)[EDID_LENGTH + 6];
+}
+EXPORT_SYMBOL_GPL(drm_edid_read_hf_eeodb_blk_size);
+
 #define for_each_cea_db(

[PATCH 1/3] drm/edid: parse multiple CEA extension block

[Lee Shawn C]

Try to find and parse more CEA ext blocks if edid->extensions
is greater than one.

Cc: Jani Nikula 
Cc: Ville Syrjala 
Cc: Ankit Nautiyal 
Signed-off-by: Lee Shawn C 
---
 drivers/gpu/drm/drm_edid.c | 110 -
 1 file changed, 60 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a504542238ed..19426f28b411 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3353,16 +3353,14 @@ const u8 *drm_find_edid_extension(const struct edid 
*edid,
return edid_ext;
 }
 
-static const u8 *drm_find_cea_extension(const struct edid *edid)
+static const u8 *drm_find_cea_extension(const struct edid *edid, int 
*ext_index)
 {
const struct displayid_block *block;
struct displayid_iter iter;
const u8 *cea;
-   int ext_index = 0;
 
-   /* Look for a top level CEA extension block */
-   /* FIXME: make callers iterate through multiple CEA ext blocks? */
-   cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index);
+   /* Look for a CEA extension block from ext_index */
+   cea = drm_find_edid_extension(edid, CEA_EXT, ext_index);
if (cea)
return cea;
 
@@ -3643,10 +3641,10 @@ add_alternate_cea_modes(struct drm_connector 
*connector, struct edid *edid)
struct drm_device *dev = connector->dev;
struct drm_display_mode *mode, *tmp;
LIST_HEAD(list);
-   int modes = 0;
+   int modes = 0, ext_index = 0;
 
/* Don't add CEA modes if the CEA extension block is missing */
-   if (!drm_find_cea_extension(edid))
+   if (!drm_find_cea_extension(edid, &ext_index))
return 0;
 
/*
@@ -4321,46 +4319,58 @@ static void drm_parse_y420cmdb_bitmap(struct 
drm_connector *connector,
 static int
 add_cea_modes(struct drm_connector *connector, struct edid *edid)
 {
-   const u8 *cea = drm_find_cea_extension(edid);
+   const u8 *cea = NULL;
const u8 *db, *hdmi = NULL, *video = NULL;
u8 dbl, hdmi_len, video_len = 0;
-   int modes = 0;
+   int modes = 0, ext_index = 0;
 
-   if (cea && cea_revision(cea) >= 3) {
-   int i, start, end;
+   for (;;) {
+   cea = drm_find_cea_extension(edid, &ext_index);
 
-   if (cea_db_offsets(cea, &start, &end))
-   return 0;
+   if (!cea)
+   break;
 
-   for_each_cea_db(cea, i, start, end) {
-   db = &cea[i];
-   dbl = cea_db_payload_len(db);
+   if (cea && cea_revision(cea) >= 3) {
+   int i, start, end;
+
+   if (cea_db_offsets(cea, &start, &end))
+   continue;
 
-   if (cea_db_tag(db) == VIDEO_BLOCK) {
-   video = db + 1;
-   video_len = dbl;
-   modes += do_cea_modes(connector, video, dbl);
-   } else if (cea_db_is_hdmi_vsdb(db)) {
-   hdmi = db;
-   hdmi_len = dbl;
-   } else if (cea_db_is_y420vdb(db)) {
-   const u8 *vdb420 = &db[2];
-
-   /* Add 4:2:0(only) modes present in EDID */
-   modes += do_y420vdb_modes(connector,
- vdb420,
- dbl - 1);
+   for_each_cea_db(cea, i, start, end) {
+   db = &cea[i];
+   dbl = cea_db_payload_len(db);
+
+   if (cea_db_tag(db) == VIDEO_BLOCK) {
+   video = db + 1;
+   video_len = dbl;
+   modes += do_cea_modes(connector, video, 
dbl);
+   } else if (cea_db_is_hdmi_vsdb(db)) {
+   hdmi = db;
+   hdmi_len = dbl;
+   } else if (cea_db_is_y420vdb(db)) {
+   const u8 *vdb420 = &db[2];
+
+   /* Add 4:2:0(only) modes present in 
EDID */
+   modes += do_y420vdb_modes(connector,
+ vdb420,
+ dbl - 1);
+   }
}
}
-   }
 
-   /*
-* We parse the HDMI VSDB after having added the cea modes as we will
-* be patching their flags when the sink supports stereo 3D.
-*/
-   if (hdmi)
-   modes += do_hdmi_vsdb_modes(connector,

[PATCH 3/3] drm/edid: parse HF-EEODB CEA extension block

[Lee Shawn C]

While adding CEA modes, try to get available EEODB block
number. Then based on it to parse numbers of ext blocks,
retrieve CEA information and add more CEA modes.

Cc: Jani Nikula 
Cc: Ville Syrjala 
Cc: Ankit Nautiyal 
Signed-off-by: Lee Shawn C 
---
 drivers/gpu/drm/drm_displayid.c |  5 -
 drivers/gpu/drm/drm_edid.c  | 34 ++---
 include/drm/drm_edid.h  |  2 +-
 3 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_displayid.c b/drivers/gpu/drm/drm_displayid.c
index 32da557b960f..dc649a9efaa2 100644
--- a/drivers/gpu/drm/drm_displayid.c
+++ b/drivers/gpu/drm/drm_displayid.c
@@ -37,7 +37,10 @@ static const u8 *drm_find_displayid_extension(const struct 
edid *edid,
  int *length, int *idx,
  int *ext_index)
 {
-   const u8 *displayid = drm_find_edid_extension(edid, DISPLAYID_EXT, 
ext_index);
+   const u8 *displayid = drm_find_edid_extension(edid,
+ DISPLAYID_EXT,
+ ext_index,
+ edid->extensions);
const struct displayid_header *base;
int ret;
 
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 056e735ef932..5e43d5ac0a5e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3364,23 +3364,23 @@ add_detailed_modes(struct drm_connector *connector, 
struct edid *edid,
  * Search EDID for CEA extension block.
  */
 const u8 *drm_find_edid_extension(const struct edid *edid,
- int ext_id, int *ext_index)
+ int ext_id, int *ext_index, int num_ext_blk)
 {
const u8 *edid_ext = NULL;
int i;
 
/* No EDID or EDID extensions */
-   if (edid == NULL || edid->extensions == 0)
+   if (edid == NULL || edid->extensions == 0 || *ext_index >= num_ext_blk)
return NULL;
 
/* Find CEA extension */
-   for (i = *ext_index; i < edid->extensions; i++) {
+   for (i = *ext_index; i < num_ext_blk; i++) {
edid_ext = (const u8 *)edid + EDID_LENGTH * (i + 1);
if (edid_ext[0] == ext_id)
break;
}
 
-   if (i >= edid->extensions)
+   if (i >= num_ext_blk)
return NULL;
 
*ext_index = i + 1;
@@ -3388,14 +3388,15 @@ const u8 *drm_find_edid_extension(const struct edid 
*edid,
return edid_ext;
 }
 
-static const u8 *drm_find_cea_extension(const struct edid *edid, int 
*ext_index)
+static const u8 *drm_find_cea_extension(const struct edid *edid,
+   int *ext_index, int num_ext_blk)
 {
const struct displayid_block *block;
struct displayid_iter iter;
const u8 *cea;
 
/* Look for a CEA extension block from ext_index */
-   cea = drm_find_edid_extension(edid, CEA_EXT, ext_index);
+   cea = drm_find_edid_extension(edid, CEA_EXT, ext_index, num_ext_blk);
if (cea)
return cea;
 
@@ -3679,7 +3680,7 @@ add_alternate_cea_modes(struct drm_connector *connector, 
struct edid *edid)
int modes = 0, ext_index = 0;
 
/* Don't add CEA modes if the CEA extension block is missing */
-   if (!drm_find_cea_extension(edid, &ext_index))
+   if (!drm_find_cea_extension(edid, &ext_index, edid->extensions))
return 0;
 
/*
@@ -4387,11 +4388,14 @@ add_cea_modes(struct drm_connector *connector, struct 
edid *edid)
 {
const u8 *cea = NULL;
const u8 *db, *hdmi = NULL, *video = NULL;
-   u8 dbl, hdmi_len, video_len = 0;
+   u8 dbl, hdmi_len, video_len = 0, num_ext_blk = edid->extensions;
int modes = 0, ext_index = 0;
 
+   if (num_ext_blk && drm_edid_is_hf_eeodb_blk_available(edid))
+   num_ext_blk = drm_edid_read_hf_eeodb_blk_size(edid);
+
for (;;) {
-   cea = drm_find_cea_extension(edid, &ext_index);
+   cea = drm_find_cea_extension(edid, &ext_index, num_ext_blk);
 
if (!cea)
break;
@@ -4647,7 +4651,7 @@ static void drm_edid_to_eld(struct drm_connector 
*connector, struct edid *edid)
if (!edid)
return;
 
-   cea = drm_find_cea_extension(edid, &ext_index);
+   cea = drm_find_cea_extension(edid, &ext_index, edid->extensions);
if (!cea) {
DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
return;
@@ -4735,7 +4739,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad 
**sads)
int i, start, end, dbl;
const u8 *cea;
 
-   cea = drm_find_cea_extension(edid, &ext_index);
+   cea = drm_find_cea_extension(edid, &ext_index, edid->extensions);
if (!cea) {
DRM_DEBUG_KMS("SAD: no CEA Extension found\n

Re: [PATCH] drm/repaper: Use format helper for xrgb8888 to monochrome conversion

[Javier Martinez Canillas]

Hello Noralf,

On 2/24/22 15:04, Noralf Trønnes wrote:
> 
> 
> Den 23.02.2022 20.37, skrev Javier Martinez Canillas:
>> There is now a drm_fb_xrgb_to_mono_reversed() helper function to do
>> format conversion from XRGB to reversed monochrome.
>>
>> Use that helper and remove the open coded version in the repaper driver.
>>
>> Signed-off-by: Javier Martinez Canillas 
>> ---
> 
> Tested-by: Noralf Trønnes 
> Reviewed-by: Noralf Trønnes 
>

Thanks a lot for testing and for your review.
 
> Do you have commit rights and will apply this patch?
>

Yes, I do. Can apply this later today.

Best regards, -- 
Javier Martinez Canillas
Linux Engineering
Red Hat

gpu/drm/dp/drm_dp.c:59:27: warning: array subscript 10 is outside array bounds of 'const u8[6]'

[Naresh Kamboju]

[Please ignore this email if it is already reported]

Linux next-20220223 arch riscv build warnings noticed.
Build configs:
  - riscv-gcc-9-defconfig
  - riscv-gcc-9-defconfig
  - riscv-gcc-10-defconfig
  - riscv-gcc-11-defconfig

metadata:
  git_ref: master
  git_repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next
  git_sha: d4a0ae62a277377de396850ed4b709b6bd9b7326
  git_describe: next-20220223
  arch: riscv
  toolchain: gcc-11

Build warnings:
---
drivers/gpu/drm/dp/drm_dp.c: In function
'drm_dp_get_adjust_request_post_cursor':
drivers/gpu/drm/dp/drm_dp.c:59:27: warning: array subscript 10 is
outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
[-Warray-bounds]
   59 | return link_status[r - DP_LANE0_1_STATUS];
  |~~~^~~
drivers/gpu/drm/dp/drm_dp.c:210:51: note: while referencing 'link_status'
  210 | u8 drm_dp_get_adjust_request_post_cursor(const u8
link_status[DP_LINK_STATUS_SIZE],
  |
~^~~~
make[1]: Target '__all' not remade because of errors.

Reported-by: Linux Kernel Functional Testing 

Steps to reproduce:
--
# To install tuxmake on your system globally:
# sudo pip3 install -U tuxmake

tuxmake --runtime podman --target-arch riscv --toolchain gcc-11
--kconfig defconfig

--
Linaro LKFT
https://lkft.linaro.org

[1] https://builds.tuxbuild.com/25XO99nwfQgKrWKz4yfBPYw0wyU/

Re: [PATCH v6 21/23] drm: rockchip: Add VOP2 driver

[Sascha Hauer]

On Thu, Feb 24, 2022 at 05:36:29PM +0300, Dmitry Osipenko wrote:
> On 2/24/22 10:47, Sascha Hauer wrote:
> > On Thu, Feb 17, 2022 at 04:24:29PM +0300, Dmitry Osipenko wrote:
> >> 17.02.2022 11:29, Sascha Hauer пишет:
> >>> @@ -28,6 +28,12 @@ config ROCKCHIP_VOP
> >>> This selects support for the VOP driver. You should enable it
> >>> on all older SoCs up to RK3399.
> >>>  
> >>> +config ROCKCHIP_VOP2
> >>> + bool "Rockchip VOP2 driver"
> >>> + help
> >>> +   This selects support for the VOP2 driver. You should enable it
> >>> +   on all newer SoCs beginning form RK3568.
> >>
> >> s/form/from/
> >>
> >> The ROCKCHIP_VOP option is "default y". Do you really want "default n"
> >> for the VOP2?
> > 
> > ROCKCHIP_VOP is only default y to keep the VOP driver enabled for
> > existing defconfig that were generated before the introduction of
> > that symbol.
> > We don't have this problem for VOP2, so no need to make it default y.
> 
> To me it will be more consistent of you'll have both defaulting to y,
> since both options are behind DRM_ROCKCHIP.

New drivers should not be enabled by default, at least that's what I
have been told before. The VOP driver is enabled by default for the
reasons explained. But yes, you are right, it's more consistent to have
the same default on both drivers. Personally I don't care much, for now
I just follow what Heiko suggests as he is the one who hopefully merges
these patches ;)

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

Re: [PATCH 3/7] drm/bridge: Extend struct drm_bus_cfg with clock field

[Maxime Ripard]

Hi,

On Sat, Feb 19, 2022 at 01:28:40AM +0100, Marek Vasut wrote:
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 1701c2128a5cb..32455cf28f0bc 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -1077,6 +1077,11 @@ struct drm_bus_cfg {
>* @flags: DRM_BUS_* flags used on this bus
>*/
>   u32 flags;
> +
> + /**
> +  * @clock: Clock frequency in kHz used on this bus
> +  */
> + u32 clock;
>  };

This is fairly vague. You were mentioning DSI: is it the pixel clock?
The HS clock rate? With or without counting the lanes? What about the
burst mode: would it be the lane or pixel rate?

It would be just as confusing for HDMI: is it the the TMDS character
rate? The TMDS bit rate ? TMDS Clock rate?

Maxime


signature.asc
Description: PGP signature

[PATCH v5 1/5] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Noralf Trønnes]

Add binding for MIPI DBI compatible SPI panels.

v5:
- Add sainsmart18 to compatible items (Rob)
- Expand write-only description (Sam)

v4:
- There should only be two compatible (Maxime)
- s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible

v3:
- Move properties to Device Tree (Maxime)
- Use contains for compatible (Maxime)
- Add backlight property to example
- Flesh out description

v2:
- Fix path for panel-common.yaml
- Use unevaluatedProperties
- Drop properties which are in the allOf section
- Drop model property (Rob)

Acked-by: Maxime Ripard 
Acked-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---
 .../display/panel/panel-mipi-dbi-spi.yaml | 127 ++
 1 file changed, 127 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml

diff --git 
a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
new file mode 100644
index ..a054f65435ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MIPI DBI SPI Panel
+
+maintainers:
+  - Noralf Trønnes 
+
+description: |
+  This binding is for display panels using a MIPI DBI compatible controller
+  in SPI mode.
+
+  The MIPI Alliance Standard for Display Bus Interface defines the electrical
+  and logical interfaces for display controllers historically used in mobile
+  phones. The standard defines 4 display architecture types and this binding is
+  for type 1 which has full frame memory. There are 3 interface types in the
+  standard and type C is the serial interface.
+
+  The standard defines the following interface signals for type C:
+  - Power:
+- Vdd: Power supply for display module
+- Vddi: Logic level supply for interface signals
+Combined into one in this binding called: power-supply
+  - Interface:
+- CSx: Chip select
+- SCL: Serial clock
+- Dout: Serial out
+- Din: Serial in
+- SDA: Bidrectional in/out
+- D/CX: Data/command selection, high=data, low=command
+  Called dc-gpios in this binding.
+- RESX: Reset when low
+  Called reset-gpios in this binding.
+
+  The type C interface has 3 options:
+
+- Option 1: 9-bit mode and D/CX as the 9th bit
+  |  Command  |  the next command or following 
data  |
+  
|<0>||
+
+- Option 2: 16-bit mode and D/CX as a 9th bit
+  |  Command or data  |
+  ||
+
+- Option 3: 8-bit mode and D/CX as a separate interface line
+  |Command or data |
+  ||
+
+  The panel resolution is specified using the panel-timing node properties
+  hactive (width) and vactive (height). The other mandatory panel-timing
+  properties should be set to zero except clock-frequency which can be
+  optionally set to inform about the actual pixel clock frequency.
+
+  If the panel is wired to the controller at an offset specify this using
+  hback-porch (x-offset) and vback-porch (y-offset).
+
+allOf:
+  - $ref: panel-common.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+items:
+  - enum:
+- sainsmart18
+  - const: panel-mipi-dbi-spi
+
+  write-only:
+type: boolean
+description:
+  Controller is not readable (ie. Din (MISO on the SPI interface) is not
+  wired up).
+
+  dc-gpios:
+maxItems: 1
+description: |
+  Controller data/command selection (D/CX) in 4-line SPI mode.
+  If not set, the controller is in 3-line SPI mode.
+
+required:
+  - compatible
+  - reg
+  - panel-timing
+
+unevaluatedProperties: false
+
+examples:
+  - |
+#include 
+
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+display@0{
+compatible = "sainsmart18", "panel-mipi-dbi-spi";
+reg = <0>;
+spi-max-frequency = <4000>;
+
+dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
+reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
+write-only;
+
+backlight = <&backlight>;
+
+width-mm = <35>;
+height-mm = <28>;
+
+panel-timing {
+hactive = <160>;
+vactive = <128>;
+hback-porch = <0>;
+vback-porch = <0>;
+
+clock-frequency = <0>;
+hfront-porch = <0>;
+hsync-len = <0>;
+vfront-porch = <0>;
+vsync-len = <0>;
+};
+};
+};

[PATCH v5 4/5] drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev

[Noralf Trønnes]

devm_drm_dev_alloc() can't allocate structures that embed a structure
which then again embeds drm_device. Workaround this by adding a
driver_private pointer to struct mipi_dbi_dev which the driver can use for
its additional state.

v3:
- Add documentation

Acked-by: Maxime Ripard 
Acked-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---
 include/drm/drm_mipi_dbi.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 6fe13cce2670..dad2f187b64b 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -130,6 +130,14 @@ struct mipi_dbi_dev {
 * @dbi: MIPI DBI interface
 */
struct mipi_dbi dbi;
+
+   /**
+* @driver_private: Driver private data.
+*  Necessary for drivers with private data since 
devm_drm_dev_alloc()
+*  can't allocate structures that embed a structure 
which then again
+*  embeds drm_device.
+*/
+   void *driver_private;
 };
 
 static inline struct mipi_dbi_dev *drm_to_mipi_dbi_dev(struct drm_device *drm)
-- 
2.33.0

[PATCH v5 2/5] drm/modes: Remove trailing whitespace

[Noralf Trønnes]

Remove trailing whitespace from a comment.

Signed-off-by: Noralf Trønnes 
---
 drivers/gpu/drm/drm_modes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 96b13e36293c..77a4c8dd0bb8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -127,7 +127,7 @@ EXPORT_SYMBOL(drm_mode_probed_add);
  * according to the hdisplay, vdisplay, vrefresh.
  * It is based from the VESA(TM) Coordinated Video Timing Generator by
  * Graham Loveridge April 9, 2003 available at
- * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls 
+ * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
  *
  * And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c.
  * What I have done is to translate it by using integer calculation.
-- 
2.33.0

[PATCH v5 0/5] drm/tiny: Add MIPI DBI compatible SPI driver

[Noralf Trønnes]

Hi,

This patchset adds a driver that will work with most MIPI DBI compatible
SPI panels out there.

It can replace the SPI interface support in these drivers:

$ grep -lr MIPI_DCS drivers/staging/fbtft/ | grep -v "-" | uniq | sort
drivers/staging/fbtft/fb_hx8340bn.c
drivers/staging/fbtft/fb_hx8353d.c
drivers/staging/fbtft/fb_hx8357d.c
drivers/staging/fbtft/fb_ili9163.c
drivers/staging/fbtft/fb_ili9340.c
drivers/staging/fbtft/fb_ili9341.c
drivers/staging/fbtft/fb_ili9481.c
drivers/staging/fbtft/fb_ili9486.c
drivers/staging/fbtft/fb_s6d02a1.c
drivers/staging/fbtft/fb_st7735r.c
drivers/staging/fbtft/fb_st7789v.c
drivers/staging/fbtft/fb_tinylcd.c

Note that the MIPI DBI parallel interface supported by fbtft does not
yet exist in DRM.

Maxime gave[1] a good overview of the situation with these displays and
proposed to make a driver that works with all MIPI DBI compatible
controllers and use a firmware file to provide the controller setup for
a particular panel.

Changes since version 4:
- Add sainsmart18 to compatible items (Rob)
- Expand write-only description (Sam)
- kconfig: s/DRM_KMS_CMA_HELPER/DRM_GEM_CMA_HELPER/ (Sam)
- kconfig: Add select VIDEOMODE_HELPERS (Sam)
- kconfig: Add wiki url in the description (Sam)
- Split out and use of_get_drm_panel_display_mode()(Sam)
- Only use the first compatible to look for a firmware file since the
  binding mandates 2 compatibles.
- Make having a firmware file mandatory so we can print an error
  message if it's missing to improve the user experience. It's very
  unlikely that a controller doesn't need to be initialized and if
  it doesn't, it's possible to have a firmware file containing only
  a DCS NOP.

See wiki[2] for a script to make command firmware files.

Noralf.

[1] https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
[2] https://github.com/notro/panel-mipi-dbi/wiki


Noralf Trønnes (5):
  dt-bindings: display: add bindings for MIPI DBI compatible SPI panels
  drm/modes: Remove trailing whitespace
  drm/modes: Add of_get_drm_panel_display_mode()
  drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev
  drm/tiny: Add MIPI DBI compatible SPI driver

 .../display/panel/panel-mipi-dbi-spi.yaml | 127 ++
 MAINTAINERS   |   8 +
 drivers/gpu/drm/drm_modes.c   |  51 ++-
 drivers/gpu/drm/tiny/Kconfig  |  15 +
 drivers/gpu/drm/tiny/Makefile |   1 +
 drivers/gpu/drm/tiny/panel-mipi-dbi.c | 398 ++
 include/drm/drm_mipi_dbi.h|   8 +
 include/drm/drm_modes.h   |   8 +
 8 files changed, 615 insertions(+), 1 deletion(-)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
 create mode 100644 drivers/gpu/drm/tiny/panel-mipi-dbi.c

-- 
2.33.0

[PATCH v5 3/5] drm/modes: Add of_get_drm_panel_display_mode()

[Noralf Trønnes]

Add a function to get a drm_display_mode from a panel-timing
device tree subnode.

Suggested-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---
 drivers/gpu/drm/drm_modes.c | 49 +
 include/drm/drm_modes.h |  8 ++
 2 files changed, 57 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 77a4c8dd0bb8..3f819c7a021b 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 
@@ -727,6 +728,54 @@ int of_get_drm_display_mode(struct device_node *np,
return 0;
 }
 EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+
+/**
+ * of_get_drm_panel_display_mode - get a panel-timing drm_display_mode from 
devicetree
+ * @np: device_node with the panel-timing specification
+ * @dmode: will be set to the return value
+ * @bus_flags: information about pixelclk, sync and DE polarity
+ *
+ * The Device Tree properties width-mm and height-mm will be read and set on
+ * the display mode if they are present.
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+int of_get_drm_panel_display_mode(struct device_node *np,
+ struct drm_display_mode *dmode, u32 
*bus_flags)
+{
+   u32 width_mm = 0, height_mm = 0;
+   struct display_timing timing;
+   struct videomode vm;
+   int ret;
+
+   ret = of_get_display_timing(np, "panel-timing", &timing);
+   if (ret)
+   return ret;
+
+   videomode_from_timing(&timing, &vm);
+
+   memset(dmode, 0, sizeof(*dmode));
+   drm_display_mode_from_videomode(&vm, dmode);
+   if (bus_flags)
+   drm_bus_flags_from_videomode(&vm, bus_flags);
+
+   ret = of_property_read_u32(np, "width-mm", &width_mm);
+   if (ret && ret != -EINVAL)
+   return ret;
+
+   ret = of_property_read_u32(np, "height-mm", &height_mm);
+   if (ret && ret != -EINVAL)
+   return ret;
+
+   dmode->width_mm = width_mm;
+   dmode->height_mm = height_mm;
+
+   drm_mode_debug_printmodeline(dmode);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_drm_panel_display_mode);
 #endif /* CONFIG_OF */
 #endif /* CONFIG_VIDEOMODE_HELPERS */
 
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 29ba4adf0c53..2fa6b2c33b3f 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -466,6 +466,8 @@ void drm_bus_flags_from_videomode(const struct videomode 
*vm, u32 *bus_flags);
 int of_get_drm_display_mode(struct device_node *np,
struct drm_display_mode *dmode, u32 *bus_flags,
int index);
+int of_get_drm_panel_display_mode(struct device_node *np,
+ struct drm_display_mode *dmode, u32 
*bus_flags);
 #else
 static inline int of_get_drm_display_mode(struct device_node *np,
  struct drm_display_mode *dmode,
@@ -473,6 +475,12 @@ static inline int of_get_drm_display_mode(struct 
device_node *np,
 {
return -EINVAL;
 }
+
+static inline int of_get_drm_panel_display_mode(struct device_node *np,
+   struct drm_display_mode *dmode, 
u32 *bus_flags)
+{
+   return -EINVAL;
+}
 #endif
 
 void drm_mode_set_name(struct drm_display_mode *mode);
-- 
2.33.0

[PATCH v5 5/5] drm/tiny: Add MIPI DBI compatible SPI driver

[Noralf Trønnes]

Add a driver that will work with most MIPI DBI compatible SPI panels.
This avoids adding a driver for every new MIPI DBI compatible controller
that is to be used by Linux. The 'compatible' Device Tree property with
a '.bin' suffix will be used to load a firmware file that contains the
controller configuration.

Example (driver will load sainsmart18.bin):

display@0 {
compatible = "sainsmart18", "panel-mipi-dbi-spi";
...
};

v5:
- kconfig: s/DRM_KMS_CMA_HELPER/DRM_GEM_CMA_HELPER/ (Sam)
- kconfig: Add select VIDEOMODE_HELPERS (Sam)
- kconfig: Add wiki url in the description (Sam)
- Split out and use of_get_drm_panel_display_mode()(Sam)
- Only use the first compatible to look for a firmware file since the
  binding mandates 2 compatibles.
- Make having a firmware file mandatory so we can print an error
  message if it's missing to improve the user experience. It's very
  unlikely that a controller doesn't need to be initialized and if
  it doesn't, it's possible to have a firmware file containing only
  a DCS NOP.

v4:
- Move driver to drm/tiny where the other drivers of its kind are located.
  The driver module will not be shared with a future DPI driver after all.

v3:
- Move properties to DT (Maxime)
- The MIPI DPI spec has optional support for DPI where the controller is
  configured over DBI. Rework the command functions so they can be moved
  to drm_mipi_dbi and shared with a future panel-mipi-dpi-spi driver

v2:
- Drop model property and use compatible instead (Rob)
- Add wiki entry in MAINTAINERS

Acked-by: Maxime Ripard 
Signed-off-by: Noralf Trønnes 
---
 MAINTAINERS   |   8 +
 drivers/gpu/drm/tiny/Kconfig  |  15 +
 drivers/gpu/drm/tiny/Makefile |   1 +
 drivers/gpu/drm/tiny/panel-mipi-dbi.c | 398 ++
 4 files changed, 422 insertions(+)
 create mode 100644 drivers/gpu/drm/tiny/panel-mipi-dbi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e6e892f99f0..3a0e57f23ad0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6107,6 +6107,14 @@ T:   git git://anongit.freedesktop.org/drm/drm-misc
 F: Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt
 F: drivers/gpu/drm/tiny/mi0283qt.c
 
+DRM DRIVER FOR MIPI DBI compatible panels
+M: Noralf Trønnes 
+S: Maintained
+W: https://github.com/notro/panel-mipi-dbi/wiki
+T: git git://anongit.freedesktop.org/drm/drm-misc
+F: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
+F: drivers/gpu/drm/tiny/panel-mipi-dbi.c
+
 DRM DRIVER FOR MSM ADRENO GPU
 M: Rob Clark 
 M: Sean Paul 
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 712e0004e96e..627d637a1e7e 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -51,6 +51,21 @@ config DRM_GM12U320
 This is a KMS driver for projectors which use the GM12U320 chipset
 for video transfer over USB2/3, such as the Acer C120 mini projector.
 
+config DRM_PANEL_MIPI_DBI
+   tristate "DRM support for MIPI DBI compatible panels"
+   depends on DRM && SPI
+   select DRM_KMS_HELPER
+   select DRM_GEM_CMA_HELPER
+   select DRM_MIPI_DBI
+   select BACKLIGHT_CLASS_DEVICE
+   select VIDEOMODE_HELPERS
+   help
+ Say Y here if you want to enable support for MIPI DBI compatible
+ panels. The controller command setup can be provided using a
+ firmware file. For more information see
+ https://github.com/notro/panel-mipi-dbi/wiki.
+ To compile this driver as a module, choose M here.
+
 config DRM_SIMPLEDRM
tristate "Simple framebuffer driver"
depends on DRM && MMU
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 5d5505d40e7b..1d9d6227e7ab 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU)+= arcpgu.o
 obj-$(CONFIG_DRM_BOCHS)+= bochs.o
 obj-$(CONFIG_DRM_CIRRUS_QEMU)  += cirrus.o
 obj-$(CONFIG_DRM_GM12U320) += gm12u320.o
+obj-$(CONFIG_DRM_PANEL_MIPI_DBI)   += panel-mipi-dbi.o
 obj-$(CONFIG_DRM_SIMPLEDRM)+= simpledrm.o
 obj-$(CONFIG_TINYDRM_HX8357D)  += hx8357d.o
 obj-$(CONFIG_TINYDRM_ILI9163)  += ili9163.o
diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c 
b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
new file mode 100644
index ..7f8c6c51387f
--- /dev/null
+++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
@@ -0,0 +1,398 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DRM driver for MIPI DBI compatible display panels
+ *
+ * Copyright 2022 Noralf Trønnes
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+static const u8 panel_mipi_dbi_magic[15] = { 'M', 'I', 'P', 'I', ' ', 'D', 
'B', 'I',
+ 

Re: [RFC][PATCH 0/7] drm/bridge: Add support for selecting DSI host HS clock from DSI bridge

[Maxime Ripard]

Hi,

On Sat, Feb 19, 2022 at 01:28:37AM +0100, Marek Vasut wrote:
> This patch series attempts to address a problem of missing support for DSI
> bridge-to-bridge or panel-to-bridge clock frequency negotiation. The problem
> has two variants.
> 
> First, a DSI->to->x bridge derives its own internal clock from DSI HS clock,
> but the DSI HS clock cannot be set to arbitrary values. TS358767 is one such
> bridge in case it operates without Xtal. In that case, the TC358767 driver
> must be able to negotiate the specific suitable DSI HS clock frequency for
> the chip.
> 
> Second, both DSI->to->x bridges and DSI hosts currently calculate, or rather
> guess and hope they both guess the same number as their neighbor, the DSI HS
> clock frequency from form of PLL=(width * height * bpp / lanes / 2). This is
> dangerous, since the PLL capabilities on both ends of the DSI bus might differ
> and the DSI host could easily end up generating wildly different clock than
> what the DSI bridge/panel expects to receive.
> 
> This series attempts to address these negotiation problems by extending the
> existing .atomic_get_input_bus_fmts callback into .atomic_get_input_bus_cfgs
> callback in struct drm_bridge_funcs {}. The extended version returns not only
> a list of a list of bus formats supported by a bridge, but the entire list of
> struct drm_bus_cfg, which currently contains format and bus flags, but can be
> extended with other members, like desired clock frequency, as required.
> 
> This series demonstrates such extension by adding the support for negotiating
> the DSI clock and by implementing such support in DW DSI Host and TC358767 DSI
> bridge.

We discussed it a bit on IRC as well but there's another issue with
this, let's imagine this setup:

encoder -> DSI-to-DPI bridge -> DPI-to-HDMI bridge -> HDMI Monitor

HDMI is fairly favorable, and it would probably use pixel clocks of
either 148.5, 297 or 594MHz. Let's simplify this a bit and assume your
DSI-to-DPI bridge can only operate at a frequency equivalent to 297MHz.

594Mhz is going to be used by those new fancy monitors, and thus the
preferred mode is likely to be using 594MHz.

With your solution, it effectively means that when the system will boot
up, the preferred mode will be reported to the userspace (and the fbdev
emulation), whatever is coming next is going to use it, and you're just
going to... refuse it because it never worked in the first place. You'll
leave a blank display, and that's it. That's not a great behavior,
really.

And since you don't get a state until you start a commit, this would
need to be able to work without one. Of course, some state parameters
will affect the clock (like the bpc count) so it won't be perfect, but
we can at least try.

Another thing is that the clock that needs to be negociated is likely to
be device specific. It's probably going to be fairly similar across
similar devices (like all the DSI bridges you mentioned are using the HS
clock), but I'm not sure we can make that assumption.

I think we could make something that work by asking the previous bridge
in the chain for a given clock rate with a given mode, and then filter
out / adjust anything we don't like. It would then be able to first
check if it can provide that clock in the first place, and then the rate
it has, and would be free to forward the query up to the encoder. And
since it's tied to the mode, it would work with mode_valid too.

Maxime


signature.asc
Description: PGP signature

Re: [PATCH v1] drm/tegra: Use dev_err_probe()

[Thierry Reding]

On Tue, Feb 08, 2022 at 12:29:23AM +0300, Dmitry Osipenko wrote:
> Replace dev_printk() with a generic dev_err_probe() helper which silences
> noisy error messages about deferred probe and makes easy to debug failing
> deferred probe by printing notification about the failure to KMSG in the
> end of kernel booting process and by adding failing device and the reason
> of deferred probe to devices_deferred of debugfs. This was proven to be
> useful in the case of eDP driver regression by immediately showing why
> display driver was failing when user asked for help, otherwise it would've
> been much more difficult to debug such problems on a third party device
> that doesn't have developer setup.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/gpu/drm/tegra/dc.c   | 13 +++--
>  drivers/gpu/drm/tegra/hdmi.c | 34 +-
>  2 files changed, 12 insertions(+), 35 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature

Re: [RFC v4 02/11] drm/amdgpu: Move scheduler init to after XGMI is ready

[Andrey Grodzovsky]

No because all the patch-set including this patch was landed into 
drm-misc-next and will reach amd-staging-drm-next on the next upstream 
rebase i guess.


Andrey

On 2022-02-24 01:47, JingWen Chen wrote:

Hi Andrey,

Will you port this patch into amd-staging-drm-next?

on 2/10/22 2:06 AM, Andrey Grodzovsky wrote:

All comments are fixed and code pushed. Thanks for everyone
who helped reviewing.

Andrey

On 2022-02-09 02:53, Christian König wrote:

Am 09.02.22 um 01:23 schrieb Andrey Grodzovsky:

Before we initialize schedulers we must know which reset
domain are we in - for single device there iis a single
domain per device and so single wq per device. For XGMI
the reset domain spans the entire XGMI hive and so the
reset wq is per hive.

Signed-off-by: Andrey Grodzovsky 

One more comment below, with that fixed Reviewed-by: Christian König 
.


---
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 45 ++
   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c  | 34 ++--
   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h   |  2 +
   3 files changed, 51 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 9704b0e1fd82..00123b0013d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2287,6 +2287,47 @@ static int amdgpu_device_fw_loading(struct amdgpu_device 
*adev)
   return r;
   }
   +static int amdgpu_device_init_schedulers(struct amdgpu_device *adev)
+{
+    long timeout;
+    int r, i;
+
+    for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
+    struct amdgpu_ring *ring = adev->rings[i];
+
+    /* No need to setup the GPU scheduler for rings that don't need it */
+    if (!ring || ring->no_scheduler)
+    continue;
+
+    switch (ring->funcs->type) {
+    case AMDGPU_RING_TYPE_GFX:
+    timeout = adev->gfx_timeout;
+    break;
+    case AMDGPU_RING_TYPE_COMPUTE:
+    timeout = adev->compute_timeout;
+    break;
+    case AMDGPU_RING_TYPE_SDMA:
+    timeout = adev->sdma_timeout;
+    break;
+    default:
+    timeout = adev->video_timeout;
+    break;
+    }
+
+    r = drm_sched_init(&ring->sched, &amdgpu_sched_ops,
+   ring->num_hw_submission, amdgpu_job_hang_limit,
+   timeout, adev->reset_domain.wq, ring->sched_score, 
ring->name);
+    if (r) {
+    DRM_ERROR("Failed to create scheduler on ring %s.\n",
+  ring->name);
+    return r;
+    }
+    }
+
+    return 0;
+}
+
+
   /**
    * amdgpu_device_ip_init - run init for hardware IPs
    *
@@ -2419,6 +2460,10 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
   }
   }
   +    r = amdgpu_device_init_schedulers(adev);
+    if (r)
+    goto init_failed;
+
   /* Don't init kfd if whole hive need to be reset during init */
   if (!adev->gmc.xgmi.pending_reset)
   amdgpu_amdkfd_device_init(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 45977a72b5dd..fa302540c69a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -457,8 +457,6 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring,
     atomic_t *sched_score)
   {
   struct amdgpu_device *adev = ring->adev;
-    long timeout;
-    int r;
     if (!adev)
   return -EINVAL;
@@ -478,36 +476,12 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring 
*ring,
   spin_lock_init(&ring->fence_drv.lock);
   ring->fence_drv.fences = kcalloc(num_hw_submission * 2, sizeof(void *),
    GFP_KERNEL);
-    if (!ring->fence_drv.fences)
-    return -ENOMEM;
   -    /* No need to setup the GPU scheduler for rings that don't need it */
-    if (ring->no_scheduler)
-    return 0;
+    ring->num_hw_submission = num_hw_submission;
+    ring->sched_score = sched_score;

Let's move this into the caller and then use ring->num_hw_submission in the 
fence code as well.

The maximum number of jobs on the ring is not really fence specific.

Regards,
Christian.


   -    switch (ring->funcs->type) {
-    case AMDGPU_RING_TYPE_GFX:
-    timeout = adev->gfx_timeout;
-    break;
-    case AMDGPU_RING_TYPE_COMPUTE:
-    timeout = adev->compute_timeout;
-    break;
-    case AMDGPU_RING_TYPE_SDMA:
-    timeout = adev->sdma_timeout;
-    break;
-    default:
-    timeout = adev->video_timeout;
-    break;
-    }
-
-    r = drm_sched_init(&ring->sched, &amdgpu_sched_ops,
-   num_hw_submission, amdgpu_job_hang_limit,
-   timeout, NULL, sched_score, ring->name);
-    if (r) {
-    DRM_ERROR("Failed to create scheduler on ring %s.\n",
-  ring->name);
-    return r;
-    }
+    if (!ring->fence_drv.fences)
+

linux-next: manual merge of the drm tree with the drm-fixes tree

[broonie]

Hi all,

Today's linux-next merge of the drm tree got a conflict in:

  drivers/gpu/drm/amd/display/dc/core/dc_resource.c

between commit:

  3743e7f6fcb93 ("drm/amd/display: Fix stream->link_enc unassigned during 
stream removal")

from the drm-fixes tree and commits:

  6d33f0e820bfb ("drm/amd/display: Fix stream->link_enc unassigned during 
stream removal")
  d9eb8fea6862e ("drm/amd/display: Drop DCN for DP2.x logic")

from the drm tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

diff --cc drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index 18757c1585232,19b56f9acf84e..0
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c

[Used drm version]

Re: [PATCH 1/2] kernel/hung_task: Exporting sysctl_hung_task_timeout_secs

[Lucas De Marchi]

On Thu, Feb 24, 2022 at 01:22:02AM +0530, Ramalingam C wrote:

Exporting sysctl_hung_task_timeout_secs, to make it available for other
kernel modules.


I guess this should only be done if second patch is accepted by sound
subsystem maintainers. If it is, then I'd do some changes in the commit
message.

Please use imperative style in commit messages. Here we also need to
give more details on why we are exporting this and Cc the maintainers.
Proposed new message:

Subject: kernel/hung_task: Export sysctl_hung_task_timeout_secs

Kernel modules may want to read sysctl_hung_task_timeout_secs so they
can do long waits by multiples of that value, avoiding the hung task
detector to trigger. This is already done in other places in the kernel
that are builtin-only, like:

block/bio.c:submit_bio_wait()
block/blk-mq.c:blk_execute_rq()
mm/kfence/core.c:toggle_allocation_gate()

Export it so it can also be used by modules.

Cc: Andrew Morton 
Cc: Luis Chamberlain 
Cc: Kai Vehmanen 
Cc: Jaroslav Kysela  
Cc: Takashi Iwai 




Signed-off-by: Ramalingam C 
cc: Lucas De Marchi 


Acked-by: Lucas De Marchi 

Lucas De Marchi


---
kernel/hung_task.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index db59b6d4f0e7..01120265395d 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -43,6 +43,7 @@ int __read_mostly sysctl_hung_task_check_count = 
PID_MAX_LIMIT;
 * Zero means infinite timeout - no checking done:
 */
unsigned long __read_mostly sysctl_hung_task_timeout_secs = 
CONFIG_DEFAULT_HUNG_TASK_TIMEOUT;
+EXPORT_SYMBOL(sysctl_hung_task_timeout_secs);

/*
 * Zero (default value) means use sysctl_hung_task_timeout_secs:
--
2.20.1

Re: [Intel-gfx] [PATCH 2/2] hda/i915: split the wait for the component binding

[Kai Vehmanen]

Hi,

On Thu, 24 Feb 2022, Ramalingam C wrote:

> Split the wait for component binding from i915 in multiples of
> sysctl_hung_task_timeout_secs. This helps to avoid the possible kworker
> thread hung detection given below.

while I understand the problem, I'm not sure whether a simpler option
should be chosen. Maybe just split the wait_for_completion_timeout()
into small 5sec iterations, without consulting value of hung_task_timeout.
This would seem unligned with more mainstream use of 
wait_for_completion_timeout() in kernel and still do the job.

I'll loop in Takashi here as well. Basicly the 60 sec timeout in 
hda/hdac_i915.c is getting caught by hung_task_detection logic in builds
where the hung_task_timeout is below 60secs.

I have a patch that tries to avoid hitting the timeout in some of the more 
common cases:
"ALSA: hda/i915 - skip acomp init if no matching display"
https://lists.freedesktop.org/archives/intel-gfx-trybot/2022-February/128278.html
... but we'll still be stuck with some configurations where the timeout 
will be hit. And above needs careful testing.

One logic comment below as well, but I'll quote the whole patch to give
context to Takashi.
 
> <3>[   60.946316] INFO: task kworker/11:1:104 blocked for more than 30
> seconds.
> <3>[   60.946479]   Tainted: GW
> 5.17.0-rc5-CI-CI_DRM_11265+ #1
> <3>[   60.946580] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
> disables this message.
> <6>[   60.946688] task:kworker/11:1state:D stack:14192 pid:  104
> ppid: 2 flags:0x4000
> <6>[   60.946713] Workqueue: events azx_probe_work [snd_hda_intel]
> <6>[   60.946740] Call Trace:
> <6>[   60.946745]  
> <6>[   60.946763]  __schedule+0x42c/0xa80
> <6>[   60.946797]  schedule+0x3f/0xc0
> <6>[   60.946811]  schedule_timeout+0x1be/0x2e0
> <6>[   60.946829]  ? del_timer_sync+0xb0/0xb0
> <6>[   60.946849]  ? 0x8100
> <6>[   60.946864]  ? wait_for_completion_timeout+0x79/0x120
> <6>[   60.946879]  wait_for_completion_timeout+0xab/0x120
> <6>[   60.946906]  snd_hdac_i915_init+0xa5/0xb0 [snd_hda_core]
> <6>[   60.946943]  azx_probe_work+0x71/0x84c [snd_hda_intel]
> <6>[   60.946974]  process_one_work+0x275/0x5c0
> <6>[   60.947010]  worker_thread+0x37/0x370
> <6>[   60.947028]  ? process_one_work+0x5c0/0x5c0
> <6>[   60.947038]  kthread+0xef/0x120
> <6>[   60.947047]  ? kthread_complete_and_exit+0x20/0x20
> <6>[   60.947065]  ret_from_fork+0x22/0x30
> <6>[   60.947110]  
> 
> Signed-off-by: Ramalingam C 
> cc: Kai Vehmanen 
> cc: Lucas De Marchi 
> ---
>  sound/hda/hdac_i915.c | 17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
> index d20a450a9a15..daaeebc5099e 100644
> --- a/sound/hda/hdac_i915.c
> +++ b/sound/hda/hdac_i915.c
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -163,7 +164,8 @@ static bool dg1_gfx_present(void)
>  int snd_hdac_i915_init(struct hdac_bus *bus)
>  {
>   struct drm_audio_component *acomp;
> - int err;
> + unsigned long timeout, ret = 0;
> + int err, i, itr_cnt;
>  
>   if (!i915_gfx_present())
>   return -ENODEV;
> @@ -182,9 +184,18 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
>   if (!acomp->ops) {
>   if (!IS_ENABLED(CONFIG_MODULES) ||
>   !request_module("i915")) {
5~> +   if (!sysctl_hung_task_timeout_secs) {
> + itr_cnt = 1;
> + timeout = msecs_to_jiffies(60 * 1000);
> + } else {
> + itr_cnt = DIV_ROUND_UP(60, 
> sysctl_hung_task_timeout_secs);
> + timeout = 
> msecs_to_jiffies(sysctl_hung_task_timeout_secs * 1000);
> + }
> +
>   /* 60s timeout */
> - 
> wait_for_completion_timeout(&acomp->master_bind_complete,
> - msecs_to_jiffies(30 * 
> 1000));
> + for (i = 0; i < itr_cnt || !ret; i++)
> + ret = 
> wait_for_completion_timeout(&acomp->master_bind_complete,
> +   timeout);

I think that should be 'i < itr_cnt && !ret'. If wait_for_completion
returns with a positive value, we should stop waiting as the completion
has been signalled.

>   }
>   }
>   if (!acomp->ops) {
> -- 

Br, Kai

Re: [PATCH libdrm v2 02/25] tegra: Remove unused IOCTL implementations

[Dmitry Osipenko]

17.02.2022 22:16, Thierry Reding пишет:
> From: Thierry Reding 
> 
> The DRM_TEGRA_GEM_{GET,SET}_FLAGS and DRM_TEGRA_GEM_{GET,SET}_TILING
> IOCTLs were badly designed and have since been obsoleted by framebuffer
> modifiers. Remove these implementations to make it clear their usage is
> discouraged.
> 
> Signed-off-by: Thierry Reding 

To me it's not a good idea to remove any function, you're breaking ABI.

I foresee that get/set flags should become useful.

Instead of the removal, you may mark functions deprecated to let
compiler produce a compile-time warning and add clarifying comments to
the code.

Re: [PATCH libdrm v2 07/25] tegra: Make API more consistent

[Dmitry Osipenko]

17.02.2022 22:19, Thierry Reding пишет:
> From: Thierry Reding 
> 
> Most functions in libdrm_tegra take as first parameter the object that
> they operate on. Make the device and buffer object creation functions
> follow the same scheme.
> 
> Signed-off-by: Thierry Reding 
> ---
>  tegra/tegra.c   | 13 +++--
>  tegra/tegra.h   | 10 +-
>  tests/tegra/openclose.c |  2 +-
>  3 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/tegra/tegra.c b/tegra/tegra.c
> index cf091c1d758f..6a51c43110e5 100644
> --- a/tegra/tegra.c
> +++ b/tegra/tegra.c
> @@ -66,7 +66,7 @@ static int drm_tegra_wrap(struct drm_tegra **drmp, int fd, 
> bool close)
>  return 0;
>  }
>  
> -drm_public int drm_tegra_new(struct drm_tegra **drmp, int fd)
> +drm_public int drm_tegra_new(int fd, struct drm_tegra **drmp)

Does libdrm allow to break ABI?

Re: [RFC v4 02/11] drm/amdgpu: Move scheduler init to after XGMI is ready

[Deucher, Alexander]

[Public]

If it applies cleanly, feel free to drop it in.  I'll drop those patches for 
drm-next since they are already in drm-misc.

Alex


From: amd-gfx  on behalf of Andrey 
Grodzovsky 
Sent: Thursday, February 24, 2022 11:24 AM
To: Chen, JingWen ; Christian König 
; dri-devel@lists.freedesktop.org 
; amd-...@lists.freedesktop.org 

Cc: Liu, Monk ; Chen, Horace ; Lazar, 
Lijo ; Koenig, Christian ; 
dan...@ffwll.ch 
Subject: Re: [RFC v4 02/11] drm/amdgpu: Move scheduler init to after XGMI is 
ready

No because all the patch-set including this patch was landed into
drm-misc-next and will reach amd-staging-drm-next on the next upstream
rebase i guess.

Andrey

On 2022-02-24 01:47, JingWen Chen wrote:
> Hi Andrey,
>
> Will you port this patch into amd-staging-drm-next?
>
> on 2/10/22 2:06 AM, Andrey Grodzovsky wrote:
>> All comments are fixed and code pushed. Thanks for everyone
>> who helped reviewing.
>>
>> Andrey
>>
>> On 2022-02-09 02:53, Christian König wrote:
>>> Am 09.02.22 um 01:23 schrieb Andrey Grodzovsky:
 Before we initialize schedulers we must know which reset
 domain are we in - for single device there iis a single
 domain per device and so single wq per device. For XGMI
 the reset domain spans the entire XGMI hive and so the
 reset wq is per hive.

 Signed-off-by: Andrey Grodzovsky 
>>> One more comment below, with that fixed Reviewed-by: Christian König 
>>> .
>>>
 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 45 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c  | 34 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h   |  2 +
3 files changed, 51 insertions(+), 30 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 index 9704b0e1fd82..00123b0013d3 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 @@ -2287,6 +2287,47 @@ static int amdgpu_device_fw_loading(struct 
 amdgpu_device *adev)
return r;
}
+static int amdgpu_device_init_schedulers(struct amdgpu_device *adev)
 +{
 +long timeout;
 +int r, i;
 +
 +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 +struct amdgpu_ring *ring = adev->rings[i];
 +
 +/* No need to setup the GPU scheduler for rings that don't need 
 it */
 +if (!ring || ring->no_scheduler)
 +continue;
 +
 +switch (ring->funcs->type) {
 +case AMDGPU_RING_TYPE_GFX:
 +timeout = adev->gfx_timeout;
 +break;
 +case AMDGPU_RING_TYPE_COMPUTE:
 +timeout = adev->compute_timeout;
 +break;
 +case AMDGPU_RING_TYPE_SDMA:
 +timeout = adev->sdma_timeout;
 +break;
 +default:
 +timeout = adev->video_timeout;
 +break;
 +}
 +
 +r = drm_sched_init(&ring->sched, &amdgpu_sched_ops,
 +   ring->num_hw_submission, amdgpu_job_hang_limit,
 +   timeout, adev->reset_domain.wq, ring->sched_score, 
 ring->name);
 +if (r) {
 +DRM_ERROR("Failed to create scheduler on ring %s.\n",
 +  ring->name);
 +return r;
 +}
 +}
 +
 +return 0;
 +}
 +
 +
/**
 * amdgpu_device_ip_init - run init for hardware IPs
 *
 @@ -2419,6 +2460,10 @@ static int amdgpu_device_ip_init(struct 
 amdgpu_device *adev)
}
}
+r = amdgpu_device_init_schedulers(adev);
 +if (r)
 +goto init_failed;
 +
/* Don't init kfd if whole hive need to be reset during init */
if (!adev->gmc.xgmi.pending_reset)
amdgpu_amdkfd_device_init(adev);
 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 index 45977a72b5dd..fa302540c69a 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 @@ -457,8 +457,6 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring 
 *ring,
  atomic_t *sched_score)
{
struct amdgpu_device *adev = ring->adev;
 -long timeout;
 -int r;
  if (!adev)
return -EINVAL;
 @@ -478,36 +476,12 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring 
 *ring,
spin_lock_init(&ring->fence_drv.lock);
ring->fence_drv.fences = kcalloc(num_hw_submission * 2, sizeof(void 
 *),
 GFP_KERNEL);
 -if (!ring->fence_drv.fences)
 -return -ENOMEM;
-/* No need to setup the GPU scheduler f

Re: [PATCH 2/2] hda/i915: split the wait for the component binding

[Lucas De Marchi]

On Thu, Feb 24, 2022 at 01:22:03AM +0530, Ramalingam C wrote:

Split the wait for component binding from i915 in multiples of
sysctl_hung_task_timeout_secs. This helps to avoid the possible kworker
thread hung detection given below.

<3>[   60.946316] INFO: task kworker/11:1:104 blocked for more than 30
seconds.
<3>[   60.946479]   Tainted: GW
5.17.0-rc5-CI-CI_DRM_11265+ #1
<3>[   60.946580] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
<6>[   60.946688] task:kworker/11:1state:D stack:14192 pid:  104
ppid: 2 flags:0x4000
<6>[   60.946713] Workqueue: events azx_probe_work [snd_hda_intel]
<6>[   60.946740] Call Trace:
<6>[   60.946745]  
<6>[   60.946763]  __schedule+0x42c/0xa80
<6>[   60.946797]  schedule+0x3f/0xc0
<6>[   60.946811]  schedule_timeout+0x1be/0x2e0
<6>[   60.946829]  ? del_timer_sync+0xb0/0xb0
<6>[   60.946849]  ? 0x8100
<6>[   60.946864]  ? wait_for_completion_timeout+0x79/0x120
<6>[   60.946879]  wait_for_completion_timeout+0xab/0x120
<6>[   60.946906]  snd_hdac_i915_init+0xa5/0xb0 [snd_hda_core]
<6>[   60.946943]  azx_probe_work+0x71/0x84c [snd_hda_intel]
<6>[   60.946974]  process_one_work+0x275/0x5c0
<6>[   60.947010]  worker_thread+0x37/0x370
<6>[   60.947028]  ? process_one_work+0x5c0/0x5c0
<6>[   60.947038]  kthread+0xef/0x120
<6>[   60.947047]  ? kthread_complete_and_exit+0x20/0x20
<6>[   60.947065]  ret_from_fork+0x22/0x30
<6>[   60.947110]  

Signed-off-by: Ramalingam C 
cc: Kai Vehmanen 
cc: Lucas De Marchi 


some more Cc needed?

$ ./scripts/get_maintainer.pl sound/hda/hdac_i915.c
Jaroslav Kysela  (maintainer:SOUND)
Takashi Iwai  (maintainer:SOUND)
Lucas De Marchi  (commit_signer:2/2=100%)
Rodrigo Vivi  (commit_signer:1/2=50%)
Ramalingam C  
(commit_signer:1/2=50%,authored:1/2=50%,removed_lines:1/1=100%)
Chris Wilson  (authored:1/2=50%,added_lines:23/24=96%)
alsa-de...@alsa-project.org (moderated list:SOUND)
linux-ker...@vger.kernel.org (open list)



---
sound/hda/hdac_i915.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index d20a450a9a15..daaeebc5099e 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -6,6 +6,7 @@
#include 
#include 
#include 
+#include 
#include 
#include 
#include 
@@ -163,7 +164,8 @@ static bool dg1_gfx_present(void)
int snd_hdac_i915_init(struct hdac_bus *bus)
{
struct drm_audio_component *acomp;
-   int err;
+   unsigned long timeout, ret = 0;
+   int err, i, itr_cnt;

if (!i915_gfx_present())
return -ENODEV;
@@ -182,9 +184,18 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
if (!acomp->ops) {
if (!IS_ENABLED(CONFIG_MODULES) ||
!request_module("i915")) {
+   if (!sysctl_hung_task_timeout_secs) {
+   itr_cnt = 1;
+   timeout = msecs_to_jiffies(60 * 1000);
+   } else {
+   itr_cnt = DIV_ROUND_UP(60, 
sysctl_hung_task_timeout_secs);
+   timeout = 
msecs_to_jiffies(sysctl_hung_task_timeout_secs * 1000);
+   }
+
/* 60s timeout */
-   
wait_for_completion_timeout(&acomp->master_bind_complete,
-   msecs_to_jiffies(30 * 
1000));
+   for (i = 0; i < itr_cnt || !ret; i++)


it will return 0 if a timeout occurs so it's trapped forever here? Did
you mean &&?

Also maybe:

itr_cnt = DIV_ROUND_UP(60, sysctl_hung_task_timeout_secs ?: 60);

Lucas De Marchi

} while ();


+   ret = 
wait_for_completion_timeout(&acomp->master_bind_complete,
+ timeout);
}
}
if (!acomp->ops) {
--
2.20.1

Re: [PATCH libdrm v2 05/25] tegra: Add flink helpers

[Dmitry Osipenko]

17.02.2022 22:16, Thierry Reding пишет:
> +int drm_tegra_bo_get_name(struct drm_tegra_bo *bo, uint32_t *name);
> +int drm_tegra_bo_open(struct drm_tegra *drm, uint32_t name, uint32_t flags,
> +  struct drm_tegra_bo **bop);

drm_tegra_bo_open() isn't a very good name for a function. How will you
name dmabuf and handle variants?

In grate-drive we're using these names:

drm_tegra_bo_from_name
drm_tegra_bo_from_dmabuf
drm_tegra_bo_from_handle

I suggest to use more meaningful function names before will be too late,
especially given that this is the upstream libdrm.

Re: [RFC v4 02/11] drm/amdgpu: Move scheduler init to after XGMI is ready

[Andrey Grodzovsky]

No problem if so but before I do,


JingWen - why you think this patch is needed as a standalone now ? It 
has no use without the
entire feature together with it. Is it some changes you want to do on 
top of that code ?



Andrey


On 2022-02-24 12:12, Deucher, Alexander wrote:


[Public]


If it applies cleanly, feel free to drop it in.  I'll drop those 
patches for drm-next since they are already in drm-misc.


Alex


*From:* amd-gfx  on behalf of 
Andrey Grodzovsky 

*Sent:* Thursday, February 24, 2022 11:24 AM
*To:* Chen, JingWen ; Christian König 
; dri-devel@lists.freedesktop.org 
; amd-...@lists.freedesktop.org 

*Cc:* Liu, Monk ; Chen, Horace 
; Lazar, Lijo ; Koenig, 
Christian ; dan...@ffwll.ch 
*Subject:* Re: [RFC v4 02/11] drm/amdgpu: Move scheduler init to after 
XGMI is ready

No because all the patch-set including this patch was landed into
drm-misc-next and will reach amd-staging-drm-next on the next upstream
rebase i guess.

Andrey

On 2022-02-24 01:47, JingWen Chen wrote:
> Hi Andrey,
>
> Will you port this patch into amd-staging-drm-next?
>
> on 2/10/22 2:06 AM, Andrey Grodzovsky wrote:
>> All comments are fixed and code pushed. Thanks for everyone
>> who helped reviewing.
>>
>> Andrey
>>
>> On 2022-02-09 02:53, Christian König wrote:
>>> Am 09.02.22 um 01:23 schrieb Andrey Grodzovsky:
 Before we initialize schedulers we must know which reset
 domain are we in - for single device there iis a single
 domain per device and so single wq per device. For XGMI
 the reset domain spans the entire XGMI hive and so the
 reset wq is per hive.

 Signed-off-by: Andrey Grodzovsky 
>>> One more comment below, with that fixed Reviewed-by: Christian 
König .

>>>
 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 45 
++

 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c  | 34 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h   |  2 +
    3 files changed, 51 insertions(+), 30 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

 index 9704b0e1fd82..00123b0013d3 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 @@ -2287,6 +2287,47 @@ static int amdgpu_device_fw_loading(struct 
amdgpu_device *adev)

    return r;
    }
    +static int amdgpu_device_init_schedulers(struct amdgpu_device 
*adev)

 +{
 +    long timeout;
 +    int r, i;
 +
 +    for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 +    struct amdgpu_ring *ring = adev->rings[i];
 +
 +    /* No need to setup the GPU scheduler for rings that 
don't need it */

 +    if (!ring || ring->no_scheduler)
 +    continue;
 +
 +    switch (ring->funcs->type) {
 +    case AMDGPU_RING_TYPE_GFX:
 +    timeout = adev->gfx_timeout;
 +    break;
 +    case AMDGPU_RING_TYPE_COMPUTE:
 +    timeout = adev->compute_timeout;
 +    break;
 +    case AMDGPU_RING_TYPE_SDMA:
 +    timeout = adev->sdma_timeout;
 +    break;
 +    default:
 +    timeout = adev->video_timeout;
 +    break;
 +    }
 +
 +    r = drm_sched_init(&ring->sched, &amdgpu_sched_ops,
 + ring->num_hw_submission, amdgpu_job_hang_limit,
 +   timeout, adev->reset_domain.wq, 
ring->sched_score, ring->name);

 +    if (r) {
 +    DRM_ERROR("Failed to create scheduler on ring %s.\n",
 +  ring->name);
 +    return r;
 +    }
 +    }
 +
 +    return 0;
 +}
 +
 +
    /**
     * amdgpu_device_ip_init - run init for hardware IPs
     *
 @@ -2419,6 +2460,10 @@ static int amdgpu_device_ip_init(struct 
amdgpu_device *adev)

    }
    }
    +    r = amdgpu_device_init_schedulers(adev);
 +    if (r)
 +    goto init_failed;
 +
    /* Don't init kfd if whole hive need to be reset during 
init */

    if (!adev->gmc.xgmi.pending_reset)
 amdgpu_amdkfd_device_init(adev);
 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c

 index 45977a72b5dd..fa302540c69a 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 @@ -457,8 +457,6 @@ int amdgpu_fence_driver_init_ring(struct 
amdgpu_ring *ring,

      atomic_t *sched_score)
    {
    struct amdgpu_device *adev = ring->adev;
 -    long timeout;
 -    int r;
      if (!adev)
    return -EINVAL;
 @@ -478,36 +476,12 @@ int amdgpu_fence_driver_init_ring(struct 
amdgpu_ring *ring,

 spin_lock_init(&ring->fence_drv

Re: [PATCH v5 1/5] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

On Thu, 24 Feb 2022 16:27:04 +0100, Noralf Trønnes wrote:
> Add binding for MIPI DBI compatible SPI panels.
> 
> v5:
> - Add sainsmart18 to compatible items (Rob)
> - Expand write-only description (Sam)
> 
> v4:
> - There should only be two compatible (Maxime)
> - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible
> 
> v3:
> - Move properties to Device Tree (Maxime)
> - Use contains for compatible (Maxime)
> - Add backlight property to example
> - Flesh out description
> 
> v2:
> - Fix path for panel-common.yaml
> - Use unevaluatedProperties
> - Drop properties which are in the allOf section
> - Drop model property (Rob)
> 
> Acked-by: Maxime Ripard 
> Acked-by: Sam Ravnborg 
> Signed-off-by: Noralf Trønnes 
> ---
>  .../display/panel/panel-mipi-dbi-spi.yaml | 127 ++
>  1 file changed, 127 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml:68:9: 
[warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1597203

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

linux-next: manual merge of the drm-intel tree with the drm-intel-gt tree

[broonie]

Hi all,

Today's linux-next merge of the drm-intel tree got a conflict in:

  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c

between commit:

  721fd84ea1fe9 ("drm/i915/pmu: Use PM timestamp instead of RING TIMESTAMP for 
reference")

from the drm-intel-gt tree and commit:

  b3f74938d6566 ("drm/i915/pmu: Use PM timestamp instead of RING TIMESTAMP for 
reference")

from the drm-intel tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

diff --cc drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 04b8321fc7587,b3a429a92c0da..0
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c

[Used drm-next version]

Re: [PATCH v4 3/3] drm/msm/dp: replace DRM_DEBUG_DP marco with drm_dbg_dp

[Kuogee Hsieh]

Hi Stephen,

Are you still has concern on this patch?

Thanks,


On 2/17/2022 12:01 PM, Kuogee Hsieh wrote:


On 2/17/2022 11:36 AM, Stephen Boyd wrote:

Quoting Kuogee Hsieh (2022-02-17 10:35:30)

Since DRM_DEBUG_DP is deprecated in favor of drm_dbg_dp(NULL, ...),
this patch replace all DRM_DEBUG_DP with drm_dbg_dp().

Changes in v4:
-- replace (strucr drm_dev *)NULL with drm_dev

Why can't the platform device be used?

#define drm_dbg_dp(drm, fmt, ...)   \

    drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DP, fmt, 
##__VA_ARGS__)


it looks for (drm)->dev (pointer)

struct platform_device {
    const char  *name;
    int id;
    bool    id_auto;
    struct device   dev  <== not an pointer here

Re: [Freedreno] [RFC PATCH v2 1/5] drm/msm/dp: fix panel bridge attachment

[Abhinav Kumar]

On 2/18/2022 6:26 PM, Dmitry Baryshkov wrote:

On 19/02/2022 02:56, Stephen Boyd wrote:

Quoting Dmitry Baryshkov (2022-02-11 14:40:02)

In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
enable and disable") the DP driver received a drm_bridge instance, which
is always attached to the encoder as a root bridge. However it conflicts
with the panel_bridge support for eDP panels. The panel bridge attaches
to the encoder before the "dp" bridge has a chace to do so. Change


s/chace/chance/


panel_bridge attachment to come after dp_bridge attachment.


s/panel_bridge/panel bridge/ possibly? And maybe clarify that dp_bridge
is the "DP driver's drm_bridge instance created in
msm_dp_bridge_init()"?

My understanding is that we want to pass the bridge created in
msm_dp_bridge_init() as the 'previous' bridge so that it attaches the
panel bridge to the output of the DP bridge that's attached to the
encoder.


Thanks! I'll update the commit log when pushing the patches.


Please correct if i am missing something here.

You are right that the eDP panel's panel bridge attaches to the encoder 
in dp_drm_connector_init() which happens before msm_dp_bridge_init() and 
hence it will attach directly to the encoder.


But we are talking about different encoders here. DP's dp_display has a 
different encoder compared to the EDP's dp_display.


So DP's bridge will still be attached to its encoder's root.

So what are we achieving with this change?







Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display 
enable and disable")

Cc: Kuogee Hsieh 
Signed-off-by: Dmitry Baryshkov 
---


Reviewed-by: Stephen Boyd 


  drivers/gpu/drm/msm/dp/dp_drm.c | 21 +++--
  1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c 
b/drivers/gpu/drm/msm/dp/dp_drm.c

index d4d360d19eba..26ef41a4c1b6 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -169,16 +169,6 @@ struct drm_connector 
*dp_drm_connector_init(struct msm_dp *dp_display)


 drm_connector_attach_encoder(connector, dp_display->encoder);

-   if (dp_display->panel_bridge) {
-   ret = drm_bridge_attach(dp_display->encoder,
-   dp_display->panel_bridge, NULL,
-   DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-   if (ret < 0) {
-   DRM_ERROR("failed to attach panel bridge: 
%d\n", ret);

-   return ERR_PTR(ret);
-   }
-   }
-
 return connector;
  }

@@ -246,5 +236,16 @@ struct drm_bridge *msm_dp_bridge_init(struct 
msm_dp *dp_display, struct drm_devi

 return ERR_PTR(rc);
 }

+   if (dp_display->panel_bridge) {
+   rc = drm_bridge_attach(dp_display->encoder,
+   dp_display->panel_bridge, 
bridge,

+   DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+   if (rc < 0) {
+   DRM_ERROR("failed to attach panel bridge: 
%d\n", rc);

+   drm_bridge_remove(bridge);
+   return ERR_PTR(rc);
+   }
+   }
+
 return bridge;


Not a problem with this patch, but what is this pointer used for? I see
it's assigned to priv->bridges and num_bridges is incremented but nobody
seems to look at that.



That's on my todo list. to remove connectors array and to destroy 
created bridges during drm device destruction.

Re: [PATCH v4 3/3] drm/msm/dp: replace DRM_DEBUG_DP marco with drm_dbg_dp

[Stephen Boyd]

Quoting Kuogee Hsieh (2022-02-17 10:35:30)
> Since DRM_DEBUG_DP is deprecated in favor of drm_dbg_dp(NULL, ...),
> this patch replace all DRM_DEBUG_DP with drm_dbg_dp().
>
> Changes in v4:
> -- replace (strucr drm_dev *)NULL with drm_dev
>
> Signed-off-by: Kuogee Hsieh 
> ---

Reviewed-by: Stephen Boyd 

Re: [PATCH v4 3/3] drm/msm/dp: replace DRM_DEBUG_DP marco with drm_dbg_dp

[Stephen Boyd]

Quoting Kuogee Hsieh (2022-02-24 10:20:31)
> Hi Stephen,
>
> Are you still has concern on this patch?
>

No more concerns.

Re: [PATCH v5 1/5] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring]

On Thu, Feb 24, 2022 at 04:27:04PM +0100, Noralf Trønnes wrote:
> Add binding for MIPI DBI compatible SPI panels.
> 
> v5:
> - Add sainsmart18 to compatible items (Rob)
> - Expand write-only description (Sam)
> 
> v4:
> - There should only be two compatible (Maxime)
> - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible
> 
> v3:
> - Move properties to Device Tree (Maxime)
> - Use contains for compatible (Maxime)
> - Add backlight property to example
> - Flesh out description
> 
> v2:
> - Fix path for panel-common.yaml
> - Use unevaluatedProperties
> - Drop properties which are in the allOf section
> - Drop model property (Rob)
> 
> Acked-by: Maxime Ripard 
> Acked-by: Sam Ravnborg 
> Signed-off-by: Noralf Trønnes 
> ---
>  .../display/panel/panel-mipi-dbi-spi.yaml | 127 ++
>  1 file changed, 127 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml 
> b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> new file mode 100644
> index ..a054f65435ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> @@ -0,0 +1,127 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MIPI DBI SPI Panel
> +
> +maintainers:
> +  - Noralf Trønnes 
> +
> +description: |
> +  This binding is for display panels using a MIPI DBI compatible controller
> +  in SPI mode.
> +
> +  The MIPI Alliance Standard for Display Bus Interface defines the electrical
> +  and logical interfaces for display controllers historically used in mobile
> +  phones. The standard defines 4 display architecture types and this binding 
> is
> +  for type 1 which has full frame memory. There are 3 interface types in the
> +  standard and type C is the serial interface.
> +
> +  The standard defines the following interface signals for type C:
> +  - Power:
> +- Vdd: Power supply for display module
> +- Vddi: Logic level supply for interface signals
> +Combined into one in this binding called: power-supply
> +  - Interface:
> +- CSx: Chip select
> +- SCL: Serial clock
> +- Dout: Serial out
> +- Din: Serial in
> +- SDA: Bidrectional in/out
> +- D/CX: Data/command selection, high=data, low=command
> +  Called dc-gpios in this binding.
> +- RESX: Reset when low
> +  Called reset-gpios in this binding.
> +
> +  The type C interface has 3 options:
> +
> +- Option 1: 9-bit mode and D/CX as the 9th bit
> +  |  Command  |  the next command or following 
> data  |
> +  
> |<0>||
> +
> +- Option 2: 16-bit mode and D/CX as a 9th bit
> +  |  Command or data  |
> +  ||
> +
> +- Option 3: 8-bit mode and D/CX as a separate interface line
> +  |Command or data |
> +  ||
> +
> +  The panel resolution is specified using the panel-timing node properties
> +  hactive (width) and vactive (height). The other mandatory panel-timing
> +  properties should be set to zero except clock-frequency which can be
> +  optionally set to inform about the actual pixel clock frequency.
> +
> +  If the panel is wired to the controller at an offset specify this using
> +  hback-porch (x-offset) and vback-porch (y-offset).
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +items:
> +  - enum:
> +- sainsmart18
> +  - const: panel-mipi-dbi-spi
> +
> +  write-only:
> +type: boolean
> +description:
> +  Controller is not readable (ie. Din (MISO on the SPI interface) is not
> +  wired up).
> +
> +  dc-gpios:
> +maxItems: 1
> +description: |
> +  Controller data/command selection (D/CX) in 4-line SPI mode.
> +  If not set, the controller is in 3-line SPI mode.
> +
> +required:
> +  - compatible
> +  - reg
> +  - panel-timing
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +#include 
> +
> +spi {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +display@0{
> +compatible = "sainsmart18", "panel-mipi-dbi-spi";
> +reg = <0>;
> +spi-max-frequency = <4000>;
> +
> +dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> +reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
> +write-only;
> +
> +backlight = <&backlight>;
> +
> +width-mm = <35>;
> +height-mm = <28>;
> +
> +panel-timing {
> +hactive = <160>;

The indentation is inconsi

Re: [PATCH V2 04/11] drm/bridge: tc358767: Implement atomic_check callback

[Marek Vasut]

On 2/21/22 10:01, Maxime Ripard wrote:

On Sat, Feb 19, 2022 at 03:26:40AM +0100, Marek Vasut wrote:

On 2/18/22 18:34, Lucas Stach wrote:

Hi

[...]


   drivers/gpu/drm/bridge/tc358767.c | 26 ++
   1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index 522c2c4d8514f..01d11adee6c74 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1289,6 +1289,31 @@ static bool tc_bridge_mode_fixup(struct drm_bridge 
*bridge,
return true;
   }
+static int tc_edp_common_atomic_check(struct drm_bridge *bridge,


Drop the edp in the name here? Later in the series you call this
function from the DPI code, so this breaks the nice clean naming
separation from patch 1.


+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ const unsigned int max_khz)
+{
+   tc_bridge_mode_fixup(bridge, &crtc_state->mode,
+&crtc_state->adjusted_mode);
+
+   if (crtc_state->adjusted_mode.clock > max_khz)
+   crtc_state->adjusted_mode.clock = max_khz;


I don't think this is correct. The adjusted most is just for minor
adjustments if the bridge can not fully match the mode. If userspace
supplies a invalid high modeclock I think it would be better to fail
the atomic check -> return -EINVAL


Maxime was telling me that returning -EINVAL from atomic_check is weird, so
maybe we should also wait for his opinion on this part.


That was in a completely different context?

Our discussion was about how you would propagate clock constraints
across a pipeline, and I was telling you that it would be weird to
return -EINVAL for a mode that was reported on a connector as supported
(or even preferred).

My argument was for mode_valid to filter them out.

If your clock is way above what you can support on your device, then
returning an error in atomic_check is the right thing to do.


Ah OK

Re: [PATCH V2 05/11] drm/bridge: tc358767: Move hardware init to enable callback

[Marek Vasut]

On 2/21/22 10:12, Lucas Stach wrote:

Am Samstag, dem 19.02.2022 um 03:39 +0100 schrieb Marek Vasut:

On 2/18/22 18:49, Lucas Stach wrote:

Am Freitag, dem 18.02.2022 um 02:00 +0100 schrieb Marek Vasut:

The TC358767/TC358867/TC9595 are all capable of operating either from
attached Xtal or from DSI clock lane clock. In case the later is used,
all I2C accesses will fail until the DSI clock lane is running and
supplying clock to the chip.

Move all hardware initialization to enable callback to guarantee the
DSI clock lane is running before accessing the hardware. In case Xtal
is attached to the chip, this change has no effect.


I'm not sure if that last statement is correct. When the chip is
bridging to eDP, the aux channel and HPD handling is needed to be
functional way before the atomic enable happen. I have no idea how this
would interact with the clock supplied from the DSI lanes. Maybe it
doesn't work at all and proper eDP support always needs a external
reference clock?


The driver currently assumes the TC358767 always gets RefClk from Xtal.

There is subsequent series which adds support for deriving clock which
drive the TC358767 PLLs from DSI HS clock instead of Xtal in case the
bridge operates in DSI-to-DPI mode. That needs additional plumbing, as
the TC358767 must be able to select specific clock frequency on the DSI
HS clock lane, because its PLLs need specific frequencies, see:

[RFC][PATCH 0/7] drm/bridge: Add support for selecting DSI host HS clock
from DSI bridge

If someone needs to implement DSI-to-(e)DP mode without Xtal, ugh, that
would likely need to have a way to figure out the DSI HS clock frequency
already in probe and then enable those DSI HS clock very early on too ?


Probably, but that was really just something I wondered about while
passing by.

The real issue is that I think _this_ patch breaks eDP operation, as
now the chip is initialized way too late, as the AUX channel
functionality needs to be available long before the atomic bridge
enable callback is called.


I don't think that's correct -- look at the tc_hardware_init() function, 
all it does is it reads the chip ID, optionally does software reset if 
there is no reset GPIO connected, and then sets up hotplug detect IRQ. 
There is nothing specific to bringing up the AUX channel in that 
function, so the AUX channel should work even before tc_hardware_init() 
is called.



The AUX channel is used to fetch the display
EDID, so before you can even set a mode it needs to be functional.
Unconditionally moving the chip init is probably (I haven't tested it
yet) going to break this.


If you have such a setup with eDP, please test it, I don't think this is 
going to break it.

[PATCH 0/4] Address a few compilation warnings

[Magali Lemes]

This patchset addresses a few warnings reported by the Kernel Test Robot and
sparse.

Magali Lemes (4):
  drm/amd/display: Adjust functions documentation
  drm/amd/display: Add conditional around function
  drm/amd/display: Use NULL instead of 0
  drm/amd/display: Turn functions into static

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c   | 4 +++-
 .../gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c  | 2 +-
 drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_smu.c  | 4 ++--
 drivers/gpu/drm/amd/display/dc/core/dc.c| 6 +++---
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c   | 2 +-
 drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c | 4 ++--
 drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 4 +---
 .../gpu/drm/amd/display/dc/irq/dcn315/irq_service_dcn315.c  | 2 +-
 8 files changed, 14 insertions(+), 14 deletions(-)

-- 
2.25.1

Re: [PATCH v4,1/5] dt-bindings: arm: mediatek: mmsys: add support for MT8186

[Rob Herring]

On Tue, 22 Feb 2022 13:27:59 +0800, Rex-BC Chen wrote:
> Add "mediatek,mt8186-mmsys" to binding document.
> 
> Signed-off-by: Rex-BC Chen 
> ---
>  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring 

[PATCH 1/4] drm/amd/display: Adjust functions documentation

[Magali Lemes]

Part of the documentation of the 'dc_process_dmub_aux_transfer_async'
function was misplaced, being put together with the
‘dc_enable_dmub_notifications’ documentation. This caused the following
warning:

drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:3757: warning:
expecting prototype for dc_process_dmub_aux_transfer_async(). Prototype
was for dc_enable_dmub_notifications() instead

This commit fixes the warning by placing the function documentations in
their right place.

Reported-by: kernel test robot 
Signed-off-by: Magali Lemes 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index b1ce3c0cf477..61e3bb99375f 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -3737,8 +3737,8 @@ bool dc_is_dmub_outbox_supported(struct dc *dc)
return dc->debug.enable_dmub_aux_for_legacy_ddc;
 }
 
-/**
- * dc_process_dmub_aux_transfer_async - Submits aux command to dmub via inbox 
message
+/*
+ *
  *  Function: dc_enable_dmub_notifications
  *
  *  @brief
@@ -3780,7 +3780,7 @@ void dc_enable_dmub_outbox(struct dc *dc)
 }
 
 /**
- *
+ * dc_process_dmub_aux_transfer_async - Submits aux command to dmub via inbox 
message
  *  Sets port index appropriately for 
legacy DDC
  * @dc: dc structure
  * @link_index: link index
-- 
2.25.1

[PATCH 2/4] drm/amd/display: Add conditional around function

[Magali Lemes]

When CONFIG_DRM_AMD_DC_DCN is not set, the function
'dm_helpers_enable_periodic_detection' doesn't have its prototype defined,
causing the following warning:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:805:6:
warning: no previous prototype for function 
'dm_helpers_enable_periodic_detection' [-Wmissing-prototypes]
   void dm_helpers_enable_periodic_detection(struct dc_context *ctx, bool 
enable)
^
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:805:1:
note: declare 'static' if the function is not intended to be used outside
of this translation unit
   void dm_helpers_enable_periodic_detection(struct dc_context *ctx, bool 
enable)
   ^
   static
   1 warning generated.

This commit silences this warning by adding a conditional directive
around the mentioned function, and also corrects a small spelling error.

Reported-by: kernel test robot 
Signed-off-by: Magali Lemes 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 010498ff5911..f5f39984702f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -977,7 +977,9 @@ void dm_set_phyd32clk(struct dc_context *ctx, int freq_khz)
// TODO
 }
 
+#if defined(CONFIG_DRM_AMD_DC_DCN)
 void dm_helpers_enable_periodic_detection(struct dc_context *ctx, bool enable)
 {
-   /* TODO: add peridic detection implementation */
+   /* TODO: add periodic detection implementation */
 }
+#endif
-- 
2.25.1

[PATCH 3/4] drm/amd/display: Use NULL instead of 0

[Magali Lemes]

Silence the following sparse warnings:

../drivers/gpu/drm/amd/amdgpu/../display/dc/dce112/dce112_resource.c:865:16:
sparse: warning: Using plain integer as NULL pointer

../drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_hw_sequencer.c:1588:84:
sparse: warning: Using plain integer as NULL pointer

../drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_hw_sequencer.c:2725:84:
sparse: warning: Using plain integer as NULL pointer

../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:1919:16:
sparse: warning: Using plain integer as NULL pointer

Signed-off-by: Magali Lemes 
---
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c   | 2 +-
 drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c | 4 ++--
 drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 4 +---
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index 4367a6e0c224..cc8e60ec35c6 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -1924,7 +1924,7 @@ static struct audio *find_first_free_audio(
return pool->audios[i];
}
}
-   return 0;
+   return NULL;
 }
 
 /*
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index ace04e2ed34e..8378b80e8517 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -1585,7 +1585,7 @@ static enum dc_status apply_single_controller_ctx_to_hw(
hws->funcs.enable_stream_timing(pipe_ctx, context, dc);
}
 
-   pipe_ctx->plane_res.scl_data.lb_params.alpha_en = pipe_ctx->bottom_pipe 
!= 0;
+   pipe_ctx->plane_res.scl_data.lb_params.alpha_en = pipe_ctx->bottom_pipe 
!= NULL;
 
pipe_ctx->stream->link->psr_settings.psr_feature_enabled = false;
 
@@ -2722,7 +2722,7 @@ static void dce110_program_front_end_for_pipe(
 

pipe_ctx->plane_res.xfm->funcs->transform_set_gamut_remap(pipe_ctx->plane_res.xfm,
 &adjust);
 
-   pipe_ctx->plane_res.scl_data.lb_params.alpha_en = pipe_ctx->bottom_pipe 
!= 0;
+   pipe_ctx->plane_res.scl_data.lb_params.alpha_en = pipe_ctx->bottom_pipe 
!= NULL;
 
program_scaler(dc, pipe_ctx);
 
diff --git a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c 
b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
index ee55cda854bf..8104c0c67f78 100644
--- a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
@@ -858,11 +858,9 @@ static struct clock_source *find_matching_pll(
return pool->clock_sources[DCE112_CLK_SRC_PLL4];
case TRANSMITTER_UNIPHY_F:
return pool->clock_sources[DCE112_CLK_SRC_PLL5];
-   default:
-   return NULL;
}
 
-   return 0;
+   return NULL;
 }
 
 static enum dc_status build_mapped_resource(
-- 
2.25.1

[PATCH 4/4] drm/amd/display: Turn functions into static

[Magali Lemes]

Silence [-Wmissing-prototypes] sparse warnings from the display folder
such as:

../drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn315/dcn315_smu.c:126:5: 
warning: no previous prototype for ‘dcn315_smu_send_msg_with_param’ 
[-Wmissing-prototypes]
  126 | int dcn315_smu_send_msg_with_param(
  | ^~

Cc: Qingqing Zhuo (Lillian) 
Reported-by: kernel test robot 
Signed-off-by: Magali Lemes 
---
 .../gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c| 2 +-
 drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_smu.c| 4 ++--
 .../gpu/drm/amd/display/dc/irq/dcn315/irq_service_dcn315.c| 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
index d66633bef2b3..357f89b4a8e0 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
@@ -430,7 +430,7 @@ static unsigned int find_clk_for_voltage(
return 0;
 }
 
-void dcn315_clk_mgr_helper_populate_bw_params(
+static void dcn315_clk_mgr_helper_populate_bw_params(
struct clk_mgr_internal *clk_mgr,
struct integrated_info *bios_info,
const DpmClocks_315_t *clock_table)
diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_smu.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_smu.c
index a60543258a5b..831fd1494d60 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_smu.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_smu.c
@@ -123,7 +123,7 @@ static uint32_t dcn315_smu_wait_for_response(struct 
clk_mgr_internal *clk_mgr, u
return res_val;
 }
 
-int dcn315_smu_send_msg_with_param(
+static int dcn315_smu_send_msg_with_param(
struct clk_mgr_internal *clk_mgr,
unsigned int msg_id, unsigned int param)
 {
@@ -181,7 +181,7 @@ int dcn315_smu_set_dispclk(struct clk_mgr_internal 
*clk_mgr, int requested_dispc
 
return actual_dispclk_set_mhz * 1000;
 }
-int dcn315_smu_set_voltage_via_phyclk(struct clk_mgr_internal *clk_mgr, int 
requested_phyclk_khz)
+static int dcn315_smu_set_voltage_via_phyclk(struct clk_mgr_internal *clk_mgr, 
int requested_phyclk_khz)
 {
int actual_phypclk_set_mhz = -1;
 
diff --git a/drivers/gpu/drm/amd/display/dc/irq/dcn315/irq_service_dcn315.c 
b/drivers/gpu/drm/amd/display/dc/irq/dcn315/irq_service_dcn315.c
index 33367773699b..e722171f0d2d 100644
--- a/drivers/gpu/drm/amd/display/dc/irq/dcn315/irq_service_dcn315.c
+++ b/drivers/gpu/drm/amd/display/dc/irq/dcn315/irq_service_dcn315.c
@@ -42,7 +42,7 @@
 #define DCN_BASE__INST0_SEG4   0x02403C00
 #define DCN_BASE__INST0_SEG5   0
 
-enum dc_irq_source to_dal_irq_source_dcn315(
+static enum dc_irq_source to_dal_irq_source_dcn315(
struct irq_service *irq_service,
uint32_t src_id,
uint32_t ext_id)
-- 
2.25.1

Re: [PATCH v4,2/5] dt-bindings: display: mediatek: add MT8186 SoC binding

[Rob Herring]

On Tue, Feb 22, 2022 at 01:28:00PM +0800, Rex-BC Chen wrote:
> Add MT8186 SoC binding to AAL, CCORR, COLOR, DITHER, GAMMA, MUTEX,
> OVL, POSTMASK and RDMA.
> 
> Signed-off-by: Rex-BC Chen 
> ---
>  .../devicetree/bindings/display/mediatek/mediatek,aal.yaml | 7 +++
>  .../bindings/display/mediatek/mediatek,ccorr.yaml  | 5 +
>  .../bindings/display/mediatek/mediatek,color.yaml  | 1 +
>  .../bindings/display/mediatek/mediatek,dither.yaml | 1 +
>  .../bindings/display/mediatek/mediatek,gamma.yaml  | 1 +
>  .../bindings/display/mediatek/mediatek,mutex.yaml  | 2 ++
>  .../bindings/display/mediatek/mediatek,ovl-2l.yaml | 5 +
>  .../devicetree/bindings/display/mediatek/mediatek,ovl.yaml | 5 +
>  .../bindings/display/mediatek/mediatek,postmask.yaml   | 5 +
>  .../bindings/display/mediatek/mediatek,rdma.yaml   | 1 +
>  10 files changed, 33 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml 
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> index 225f9dd726d2..3a5416937293 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> @@ -23,6 +23,8 @@ properties:
>  oneOf:
>- items:
>- const: mediatek,mt8173-disp-aal
> +  - items:
> +  - const: mediatek,mt8183-disp-aal

This patch is for 8186...

>- items:
>- enum:
>- mediatek,mt2712-disp-aal
> @@ -31,6 +33,11 @@ properties:
>- mediatek,mt8195-disp-aal
>- enum:
>- mediatek,mt8173-disp-aal
> +  - items:
> +  - enum:
> +  - mediatek,mt8186-disp-aal

> +  - enum:
> +  - mediatek,mt8183-disp-aal

There won't be more than 1 fallback, so use const rather than enum.

>  
>reg:
>  maxItems: 1
> diff --git 
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml 
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml
> index 6894b6999412..8ac87b5896ac 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml
> @@ -30,6 +30,11 @@ properties:
>- mediatek,mt8195-disp-ccorr
>- enum:
>- mediatek,mt8192-disp-ccorr
> +  - items:
> +  - enum:
> +  - mediatek,mt8186-disp-ccorr
> +  - enum:
> +  - mediatek,mt8183-disp-ccorr

Same here.

>  
>reg:
>  maxItems: 1
> diff --git 
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml 
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> index bc83155b3b4c..d0a4b9eb71fd 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> @@ -37,6 +37,7 @@ properties:
>- items:
>- enum:
>- mediatek,mt8183-disp-color
> +  - mediatek,mt8186-disp-color
>- mediatek,mt8192-disp-color
>- mediatek,mt8195-disp-color
>- enum:
> diff --git 
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml 
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml
> index 9d89297f5f1d..9a08514ed909 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml
> @@ -26,6 +26,7 @@ properties:
>- const: mediatek,mt8183-disp-dither
>- items:
>- enum:
> +  - mediatek,mt8186-disp-dither
>- mediatek,mt8192-disp-dither
>- mediatek,mt8195-disp-dither
>- enum:
> diff --git 
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml 
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> index 247baad147b3..6d96f6736d91 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> @@ -27,6 +27,7 @@ properties:
>- const: mediatek,mt8183-disp-gamma
>- items:
>- enum:
> +  - mediatek,mt8186-disp-gamma
>- mediatek,mt8192-disp-gamma
>- mediatek,mt8195-disp-gamma
>- enum:
> diff --git 
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,mutex.yaml 
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mutex.yaml
> index 6eca525eced0..55391b5c08c4 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,mutex.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,mutex.yaml
> @@ -34,6 

Re: [Intel-gfx] [PATCH 1/3] drm/i915/guc: Limit scheduling properties to avoid overflow

[John Harrison]

On 2/24/2022 01:59, Tvrtko Ursulin wrote:

On 23/02/2022 19:03, John Harrison wrote:

On 2/23/2022 04:13, Tvrtko Ursulin wrote:

On 23/02/2022 02:11, John Harrison wrote:

On 2/22/2022 01:52, Tvrtko Ursulin wrote:

On 18/02/2022 21:33, john.c.harri...@intel.com wrote:

From: John Harrison 

GuC converts the pre-emption timeout and timeslice quantum values 
into
clock ticks internally. That significantly reduces the point of 
32bit

overflow. On current platforms, worst case scenario is approximately


Where does 32-bit come from, the GuC side? We already use 64-bits 
so that something to fix to start with. Yep...
Yes, the GuC API is defined as 32bits only and then does a straight 
multiply by the clock speed with no range checking. We have 
requested 64bit support but there was push back on the grounds that 
it is not something the GuC timer hardware supports and such long 
timeouts are not real world usable anyway.


As long as compute are happy with 100 seconds, then it "should be 
enough for everbody". :D

Compute disable all forms of reset and rely on manual kill. So yes.

But even if they aren't. That's all we can do at the moment. If there 
is a genuine customer requirement for more then we can push for full 
64bit software implemented timers in the GuC but until that happens, 
we don't have much choice.


Yeah.







./gt/uc/intel_guc_fwif.h:   u32 execution_quantum;

./gt/uc/intel_guc_submission.c: desc->execution_quantum = 
engine->props.timeslice_duration_ms * 1000;


./gt/intel_engine_types.h:  unsigned long 
timeslice_duration_ms;


timeslice_store/preempt_timeout_store:
err = kstrtoull(buf, 0, &duration);

So both kconfig and sysfs can already overflow GuC, not only 
because of tick conversion internally but because at backend level 
nothing was done for assigning 64-bit into 32-bit. Or I failed to 
find where it is handled.
That's why I'm adding this range check to make sure we don't allow 
overflows.


Yes and no, this fixes it, but the first bug was not only due GuC 
internal tick conversion. It was present ever since the u64 from 
i915 was shoved into u32 sent to GuC. So even if GuC used the value 
without additional multiplication, bug was be there. My point being 
when GuC backend was added timeout_ms values should have been 
limited/clamped to U32_MAX. The tick discovery is additional limit 
on top.
I'm not disagreeing. I'm just saying that the truncation wasn't 
noticed until I actually tried using very long timeouts to debug a 
particular problem. Now that it is noticed, we need some method of 
range checking and this simple clamp solves all the truncation problems.


Agreed in principle, just please mention in the commit message all 
aspects of the problem.


I think we can get away without a Fixes: tag since it requires user 
fiddling to break things in unexpected ways.


I would though put in a code a clamping which expresses both, 
something like min(u32, ..GUC LIMIT..). So the full story is 
documented forever. Or "if > u32 || > ..GUC LIMIT..) return -EINVAL". 
Just in case GuC limit one day changes but u32 stays. Perhaps internal 
ticks go away or anything and we are left with plain 1:1 millisecond 
relationship.
Can certainly add a comment along the lines of "GuC API only takes a 
32bit field but that is further reduced to GUC_LIMIT due to internal 
calculations which would otherwise overflow".


But if the GuC limit is > u32 then, by definition, that means the GuC 
API has changed to take a u64 instead of a u32. So there will no u32 
truncation any more. So I'm not seeing a need to explicitly test the 
integer size when the value check covers that.





110 seconds. Rather than allowing the user to set higher values and
then get confused by early timeouts, add limits when setting these
values.


Btw who is reviewing GuC patches these days - things have somehow 
gotten pretty quiet in activity and I don't think that's due 
absence of stuff to improve or fix? Asking since I think I noticed 
a few already which you posted and then crickets on the mailing list.

Too much work to do and not enough engineers to do it all :(.





Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/gt/intel_engine_cs.c   | 15 +++
  drivers/gpu/drm/i915/gt/sysfs_engines.c | 14 ++
  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h |  9 +
  3 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c

index e53008b4dd05..2a1e9f36e6f5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -389,6 +389,21 @@ static int intel_engine_setup(struct 
intel_gt *gt, enum intel_engine_id id,

  if (GRAPHICS_VER(i915) == 12 && engine->class == RENDER_CLASS)
  engine->props.preempt_timeout_ms = 0;
  +    /* Cap timeouts to prevent overflow inside GuC */
+    if (intel_guc_submission_is_wanted(>->uc.guc)) {
+    if (engine->p

Re: [PATCH v5 2/5] drm/modes: Remove trailing whitespace

[Sam Ravnborg]

On Thu, Feb 24, 2022 at 04:27:05PM +0100, Noralf Trønnes wrote:
> Remove trailing whitespace from a comment.
> 
> Signed-off-by: Noralf Trønnes 
Acked-by: Sam Ravnborg 
> ---
>  drivers/gpu/drm/drm_modes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 96b13e36293c..77a4c8dd0bb8 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -127,7 +127,7 @@ EXPORT_SYMBOL(drm_mode_probed_add);
>   * according to the hdisplay, vdisplay, vrefresh.
>   * It is based from the VESA(TM) Coordinated Video Timing Generator by
>   * Graham Loveridge April 9, 2003 available at
> - * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls 
> + * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
>   *
>   * And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c.
>   * What I have done is to translate it by using integer calculation.
> -- 
> 2.33.0

Re: [PATCH v5 3/5] drm/modes: Add of_get_drm_panel_display_mode()

[Sam Ravnborg]

On Thu, Feb 24, 2022 at 04:27:06PM +0100, Noralf Trønnes wrote:
> Add a function to get a drm_display_mode from a panel-timing
> device tree subnode.

Thanks for implementing this!

> 
> Suggested-by: Sam Ravnborg 
> Signed-off-by: Noralf Trønnes 
Reviewed-by: Sam Ravnborg 

Re: [PATCH v5 5/5] drm/tiny: Add MIPI DBI compatible SPI driver

[Sam Ravnborg]

Hi Noralf,

On Thu, Feb 24, 2022 at 04:27:08PM +0100, Noralf Trønnes wrote:
> Add a driver that will work with most MIPI DBI compatible SPI panels.
> This avoids adding a driver for every new MIPI DBI compatible controller
> that is to be used by Linux. The 'compatible' Device Tree property with
> a '.bin' suffix will be used to load a firmware file that contains the
> controller configuration.
> 
> Example (driver will load sainsmart18.bin):
> 
> display@0 {
>   compatible = "sainsmart18", "panel-mipi-dbi-spi";
> ...
> };
> 
> v5:
> - kconfig: s/DRM_KMS_CMA_HELPER/DRM_GEM_CMA_HELPER/ (Sam)
> - kconfig: Add select VIDEOMODE_HELPERS (Sam)
> - kconfig: Add wiki url in the description (Sam)
> - Split out and use of_get_drm_panel_display_mode()(Sam)
> - Only use the first compatible to look for a firmware file since the
>   binding mandates 2 compatibles.
> - Make having a firmware file mandatory so we can print an error
>   message if it's missing to improve the user experience. It's very
>   unlikely that a controller doesn't need to be initialized and if
>   it doesn't, it's possible to have a firmware file containing only
>   a DCS NOP.
> 
> v4:
> - Move driver to drm/tiny where the other drivers of its kind are located.
>   The driver module will not be shared with a future DPI driver after all.
> 
> v3:
> - Move properties to DT (Maxime)
> - The MIPI DPI spec has optional support for DPI where the controller is
>   configured over DBI. Rework the command functions so they can be moved
>   to drm_mipi_dbi and shared with a future panel-mipi-dpi-spi driver
> 
> v2:
> - Drop model property and use compatible instead (Rob)
> - Add wiki entry in MAINTAINERS
> 
> Acked-by: Maxime Ripard 
> Signed-off-by: Noralf Trønnes 

Looks good.

Reviewed-by: Sam Ravnborg 

Re: [PATCH 0/4] Address a few compilation warnings

[Harry Wentland]

Series is
Reviewed-by: Harry Wentland 

Harry

On 2022-02-24 14:15, Magali Lemes wrote:
> This patchset addresses a few warnings reported by the Kernel Test Robot and
> sparse.
> 
> Magali Lemes (4):
>   drm/amd/display: Adjust functions documentation
>   drm/amd/display: Add conditional around function
>   drm/amd/display: Use NULL instead of 0
>   drm/amd/display: Turn functions into static
> 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c   | 4 +++-
>  .../gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c  | 2 +-
>  drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_smu.c  | 4 ++--
>  drivers/gpu/drm/amd/display/dc/core/dc.c| 6 +++---
>  drivers/gpu/drm/amd/display/dc/core/dc_resource.c   | 2 +-
>  drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c | 4 ++--
>  drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 4 +---
>  .../gpu/drm/amd/display/dc/irq/dcn315/irq_service_dcn315.c  | 2 +-
>  8 files changed, 14 insertions(+), 14 deletions(-)
> 

Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: Make the heartbeat play nice with long pre-emption timeouts

[John Harrison]

On 2/24/2022 03:41, Tvrtko Ursulin wrote:

On 23/02/2022 20:00, John Harrison wrote:

On 2/23/2022 05:58, Tvrtko Ursulin wrote:

On 23/02/2022 02:45, John Harrison wrote:

On 2/22/2022 03:19, Tvrtko Ursulin wrote:

On 18/02/2022 21:33, john.c.harri...@intel.com wrote:

From: John Harrison 

Compute workloads are inherantly not pre-emptible for long 
periods on

current hardware. As a workaround for this, the pre-emption timeout
for compute capable engines was disabled. This is undesirable 
with GuC
submission as it prevents per engine reset of hung contexts. 
Hence the

next patch will re-enable the timeout but bumped up by an order of
magnititude.


(Some typos above.)

I'm spotting 'inherently' but not anything else.


Magnititude! O;)

Doh!

[snip]

Whereas, bumping all heartbeat periods to be greater than the 
pre-emption timeout is wasteful and unnecessary. That leads to a 
total heartbeat time of about a minute. Which is a very long time 
to wait for a hang to be detected and recovered. Especially when 
the official limit on a context responding to an 'are you dead' 
query is only 7.5 seconds.


Not sure how did you get one minute?
7.5 * 2 (to be safe) = 15. 15 * 5 (number of heartbeat periods) = 75 
=> 1 minute 15 seconds


Even ignoring any safety factor and just going with 7.5 * 5 still 
gets you to 37.5 seconds which is over a half a minute and likely to 
race.


Ah because my starting point is there should be no preempt timeout = 
heartbeat * 3, I just think that's too ugly.
Then complain at the hardware designers to give us mid-thread 
pre-emption back. The heartbeat is only one source of pre-emption 
events. For example, a user can be running multiple contexts in parallel 
and expecting them to time slice on a single engine. Or maybe a user is 
just running one compute task in the background but is doing render work 
in the foreground. Etc.


There was a reason the original hack was to disable pre-emption rather 
than increase the heartbeat. This is simply a slightly less ugly version 
of the same hack. And unfortunately, the basic idea of the hack is 
non-negotiable.


As per other comments, 'tP(RCS) = tH *3' or 'tP(RCS) = tP(default) * 12' 
or 'tP(RCS) = 7500' are the available options. Given that the heartbeat 
is the ever present hard limit, it seems most plausible to base the hack 
on that. Any of the others works, though. Although I think a explicit 
hardcoded value is the most ugly. I guess the other option is to add 
CONFIG_DRM_I915_PREEMPT_TIMEOUT_COMPUTE and default that to 7500.


Take your pick. But 640ms is not allowed.



Regardless, crux of argument was to avoid GuC engine reset and 
heartbeat reset racing with each other, and to do that by 
considering the preempt timeout with the heartbeat interval. I was 
thinking about this scenario in this series:


[Please use fixed width font and no line wrap to view.]

A)

tP = preempt timeout
tH = hearbeat interval

tP = 3 * tH

1) Background load = I915_PRIORITY_DISPLAY

<-- [tH] --> Pulse1 <-- [tH] --> Pulse2 <-- [tH] --> Pulse3 < [2 
* tH] > FULL RESET

   |
   \- preemption 
triggered, tP = 3 * tH --\

\-> preempt timeout would hit here

Here we have collateral damage due full reset, since we can't tell 
GuC to reset just one engine and we fudged tP just to "account" for 
heartbeats.
You are missing the whole point of the patch series which is that the 
last heartbeat period is '2 * tP' not '2 * tH'.

+        longer = READ_ONCE(engine->props.preempt_timeout_ms) * 2;

By making the last period double the pre-emption timeout, it is 
guaranteed that the FULL RESET stage cannot be hit before the 
hardware has attempted and timed-out on at least one pre-emption.


Oh well :) that probably means the overall scheme is too odd for me. 
tp = 3tH and last pulse after 2tP I mean.
To be accurate, it is 'tP(RCS) = 3 * tH(default); tH(final) = 
tP(current) * 2;'. Seems fairly straight forward to me. It's not a 
recursive definition or anything like that. It gives us a total 
heartbeat timeout that is close to the original version but still allows 
at least one pre-emption event.





[snip]


<-- [tH] --> Pulse1 <-- [tH] --> Pulse2 <-- [tH] --> Pulse3 < [2 
* tH] > full reset would be here

   |
   \- preemption triggered, tP = 3 * tH \
\-> Preempt timeout reset

Here is is kind of least worse, but question is why we fudged tP 
when it gives us nothing good in this case.


The point of fudging tP(RCS) is to give compute workloads longer to 
reach a pre-emptible point (given that EU walkers are basically not 
pre-emptible). The reason for doing the fudge is not connected to the 
heartbeat at all. The fact that it causes problems for the heartbeat 
is an undesired side effect.


Note that the use of 'tP(RCS) = tH * 3' was just an arbitrary 
calculation that gave us something that all i

Re: [Intel-gfx] [PATCH 1/3] drm/i915/guc: Limit scheduling properties to avoid overflow

[John Harrison]

On 2/24/2022 11:19, John Harrison wrote:

[snip]

I'll change it to _uses_ and repost, then.


[    7.683149] kernel BUG at drivers/gpu/drm/i915/gt/uc/intel_guc.h:367!

Told you that one went bang.

John.

Re: [RFC PATCH v2 2/5] drm/msm/dp: support attaching bridges to the DP encoder

[Abhinav Kumar]

On 2/11/2022 2:40 PM, Dmitry Baryshkov wrote:

Currently DP driver will allocate panel bridge for eDP panels. This
supports only the following topology:

- eDP encoder ⇒ eDP panel (wrapped using panel-bridge)

Simplify this code to just check if there is any next bridge in the
chain (be it a panel bridge or regular bridge). Rename panel_bridge
field to next_bridge accordingly.

This allows one to use e.g. one of the following display topologies:

- eDP encoder ⇒ ptn3460 ⇒ fixed LVDS panel
- eDP encoder ⇒ ptn3460 ⇒ LVDS connector with EDID lines for panel autodetect
- eDP encoder ⇒ ptn3460 ⇒ THC63LVD1024 ⇒ DPI panel.
- eDP encoder ⇒ LT8912 ⇒ DSI panel

Signed-off-by: Dmitry Baryshkov 


Reviewed-by: Abhinav Kumar 


---
  drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
  drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
  drivers/gpu/drm/msm/dp/dp_drm.c |  4 ++--
  drivers/gpu/drm/msm/dp/dp_parser.c  | 31 +++--
  drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
  5 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index 44d42c76c2a3..45f9a912ecc5 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -266,7 +266,7 @@ static int dp_display_bind(struct device *dev, struct 
device *master,
goto end;
}
  
-	dp->dp_display.panel_bridge = dp->parser->panel_bridge;

+   dp->dp_display.next_bridge = dp->parser->next_bridge;
  
  	dp->aux->drm_dev = drm;

rc = dp_aux_register(dp->aux);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
b/drivers/gpu/drm/msm/dp/dp_display.h
index e3adcd578a90..7af2b186d2d9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -16,7 +16,7 @@ struct msm_dp {
struct drm_bridge *bridge;
struct drm_connector *connector;
struct drm_encoder *encoder;
-   struct drm_bridge *panel_bridge;
+   struct drm_bridge *next_bridge;
bool is_connected;
bool audio_enabled;
bool power_on;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 26ef41a4c1b6..80f59cf99089 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -236,9 +236,9 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp 
*dp_display, struct drm_devi
return ERR_PTR(rc);
}
  
-	if (dp_display->panel_bridge) {

+   if (dp_display->next_bridge) {
rc = drm_bridge_attach(dp_display->encoder,
-   dp_display->panel_bridge, bridge,
+   dp_display->next_bridge, bridge,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (rc < 0) {
DRM_ERROR("failed to attach panel bridge: %d\n", rc);
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
b/drivers/gpu/drm/msm/dp/dp_parser.c
index a7acc23f742b..901d7967370f 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -265,23 +265,16 @@ static int dp_parser_clock(struct dp_parser *parser)
return 0;
  }
  
-static int dp_parser_find_panel(struct dp_parser *parser)

+static int dp_parser_find_next_bridge(struct dp_parser *parser)
  {
struct device *dev = &parser->pdev->dev;
-   struct drm_panel *panel;
-   int rc;
+   struct drm_bridge *bridge;
  
-	rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);

-   if (rc) {
-   DRM_ERROR("failed to acquire DRM panel: %d\n", rc);
-   return rc;
-   }
+   bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+   if (IS_ERR(bridge))
+   return PTR_ERR(bridge);
  
-	parser->panel_bridge = devm_drm_panel_bridge_add(dev, panel);

-   if (IS_ERR(parser->panel_bridge)) {
-   DRM_ERROR("failed to create panel bridge\n");
-   return PTR_ERR(parser->panel_bridge);
-   }
+   parser->next_bridge = bridge;
  
  	return 0;

  }
@@ -307,10 +300,18 @@ static int dp_parser_parse(struct dp_parser *parser, int 
connector_type)
if (rc)
return rc;
  
+	/*

+* Currently we support external bridges only for eDP connectors.
+*
+* No external bridges are expected for the DisplayPort connector,
+* it is physically present in a form of a DP or USB-C connector.
+*/
if (connector_type == DRM_MODE_CONNECTOR_eDP) {
-   rc = dp_parser_find_panel(parser);
-   if (rc)
+   rc = dp_parser_find_next_bridge(parser);
+   if (rc) {
+   DRM_ERROR("DP: failed to find next bridge\n");
return rc;
+   }
}
  
  	/* Map the corresponding regulator information according to

diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h 
b/drivers/gpu/drm/msm/dp/dp_parser.h
i

[PATCH V3 00/12] drm/bridge: tc358767: Add DSI-to-DPI mode support

[Marek Vasut]

The TC358767/TC358867/TC9595 are all capable of operating in multiple
modes, DPI-to-(e)DP, DSI-to-(e)DP, DSI-to-DPI. Clean up the driver,
switch to atomic ops, and add support for the DSI-to-DPI mode in
addition to already supported DPI-to-(e)DP mode.

Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
To: dri-devel@lists.freedesktop.org

Marek Vasut (12):
  dt-bindings: display: bridge: tc358867: Document DPI output support
  dt-bindings: display: bridge: tc358867: Document DSI data-lanes
property
  drm/bridge: tc358767: Change tc_ prefix to tc_edp_ for (e)DP specific
functions
  drm/bridge: tc358767: Convert to atomic ops
  drm/bridge: tc358767: Implement atomic_check callback
  drm/bridge: tc358767: Move hardware init to enable callback
  drm/bridge: tc358767: Move (e)DP bridge endpoint parsing into
dedicated function
  drm/bridge: tc358767: Wrap (e)DP aux I2C registration into
tc_aux_link_setup()
  drm/bridge: tc358767: Move bridge ops setup into
tc_probe_edp_bridge_endpoint()
  drm/bridge: tc358767: Detect bridge mode from connected endpoints in
DT
  drm/bridge: tc358767: Split tc_set_video_mode() into common and (e)DP
part
  drm/bridge: tc358767: Add DSI-to-DPI mode support

 .../display/bridge/toshiba,tc358767.yaml  |  22 +-
 drivers/gpu/drm/bridge/tc358767.c | 681 +++---
 2 files changed, 596 insertions(+), 107 deletions(-)

-- 
2.34.1

[PATCH V3 08/12] drm/bridge: tc358767: Wrap (e)DP aux I2C registration into tc_aux_link_setup()

[Marek Vasut]

This bit of code is (e)DP and aux I2C link specific, move it into
tc_aux_link_setup() to permit cleaner addition of DSI-to-DPI mode.
No functional change.

Reviewed-by: Lucas Stach 
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - New patch
V3: - Add RB from Lucas
---
 drivers/gpu/drm/bridge/tc358767.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index e9cec6f8e0e9d..b4ae4dd5b89aa 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -656,6 +656,12 @@ static int tc_aux_link_setup(struct tc_data *tc)
if (ret)
goto err;
 
+   /* Register DP AUX channel */
+   tc->aux.name = "TC358767 AUX i2c adapter";
+   tc->aux.dev = tc->dev;
+   tc->aux.transfer = tc_aux_transfer;
+   drm_dp_aux_init(&tc->aux);
+
return 0;
 err:
dev_err(tc->dev, "tc_aux_link_setup failed: %d\n", ret);
@@ -1751,12 +1757,6 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
if (ret)
return ret;
 
-   /* Register DP AUX channel */
-   tc->aux.name = "TC358767 AUX i2c adapter";
-   tc->aux.dev = tc->dev;
-   tc->aux.transfer = tc_aux_transfer;
-   drm_dp_aux_init(&tc->aux);
-
tc->bridge.funcs = &tc_edp_bridge_funcs;
if (tc->hpd_pin >= 0)
tc->bridge.ops |= DRM_BRIDGE_OP_DETECT;
-- 
2.34.1

[PATCH V3 09/12] drm/bridge: tc358767: Move bridge ops setup into tc_probe_edp_bridge_endpoint()

[Marek Vasut]

The bridge ops are specific to the bridge configuration, move them
into tc_probe_edp_bridge_endpoint() to permit cleaner addition of
DSI-to-DPI mode. No functional change.

Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - New patch
V3: - No change
---
 drivers/gpu/drm/bridge/tc358767.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index b4ae4dd5b89aa..07da6142d5cf2 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1676,6 +1676,11 @@ static int tc_probe_edp_bridge_endpoint(struct tc_data 
*tc)
tc->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
}
 
+   tc->bridge.funcs = &tc_edp_bridge_funcs;
+   if (tc->hpd_pin >= 0)
+   tc->bridge.ops |= DRM_BRIDGE_OP_DETECT;
+   tc->bridge.ops |= DRM_BRIDGE_OP_EDID;
+
return ret;
 }
 
@@ -1757,11 +1762,6 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
if (ret)
return ret;
 
-   tc->bridge.funcs = &tc_edp_bridge_funcs;
-   if (tc->hpd_pin >= 0)
-   tc->bridge.ops |= DRM_BRIDGE_OP_DETECT;
-   tc->bridge.ops |= DRM_BRIDGE_OP_EDID;
-
tc->bridge.of_node = dev->of_node;
drm_bridge_add(&tc->bridge);
 
-- 
2.34.1

[PATCH V3 02/12] dt-bindings: display: bridge: tc358867: Document DSI data-lanes property

[Marek Vasut]

It is necessary to specify the number of connected/used DSI data lanes when
using the DSI input port of this bridge. Document the 'data-lanes' property
of the DSI input port.

Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Rob Herring 
Cc: Sam Ravnborg 
Cc: devicet...@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
V3: - New patch
---
 .../display/bridge/toshiba,tc358767.yaml   | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git 
a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml 
b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
index 5cfda6f2ba69c..ed280053ec62b 100644
--- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
@@ -53,11 +53,27 @@ properties:
 
 properties:
   port@0:
-$ref: /schemas/graph.yaml#/properties/port
+$ref: /schemas/graph.yaml#/$defs/port-base
+unevaluatedProperties: false
 description: |
 DSI input port. The remote endpoint phandle should be a
 reference to a valid DSI output endpoint node
 
+properties:
+  endpoint:
+$ref: /schemas/media/video-interfaces.yaml#
+unevaluatedProperties: false
+
+properties:
+  data-lanes:
+description: array of physical DSI data lane indexes.
+minItems: 1
+items:
+  - const: 1
+  - const: 2
+  - const: 3
+  - const: 4
+
   port@1:
 $ref: /schemas/graph.yaml#/properties/port
 description: |
-- 
2.34.1

[PATCH V3 01/12] dt-bindings: display: bridge: tc358867: Document DPI output support

[Marek Vasut]

The TC358767/TC358867/TC9595 are all capable of operating in multiple
modes, DPI-to-(e)DP, DSI-to-(e)DP, DSI-to-DPI. Document support for the
DPI output port, which can now be connected both as input and output.

Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Rob Herring 
Cc: Sam Ravnborg 
Cc: devicet...@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
V2: - Rebase on next-20220217
V3: - No change
---
 .../devicetree/bindings/display/bridge/toshiba,tc358767.yaml  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml 
b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
index f1541cc052977..5cfda6f2ba69c 100644
--- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
@@ -61,8 +61,8 @@ properties:
   port@1:
 $ref: /schemas/graph.yaml#/properties/port
 description: |
-DPI input port. The remote endpoint phandle should be a
-reference to a valid DPI output endpoint node
+DPI input/output port. The remote endpoint phandle should be a
+reference to a valid DPI output or input endpoint node.
 
   port@2:
 $ref: /schemas/graph.yaml#/properties/port
-- 
2.34.1

[PATCH V3 11/12] drm/bridge: tc358767: Split tc_set_video_mode() into common and (e)DP part

[Marek Vasut]

The tc_set_video_mode() sets up both common and (e)DP video mode settings of
the bridge chip. Split the function into tc_set_common_video_mode() to set
the common settings and tc_set_edp_video_mode() to set the (e)DP specific
settings. No functional change.

Reviewed-by: Lucas Stach 
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - New patch
V3: - Add RB from Lucas
---
 drivers/gpu/drm/bridge/tc358767.c | 48 ---
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index 1ccb95704a4c9..b1b02de4bbb3d 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -734,11 +734,10 @@ static int tc_get_display_props(struct tc_data *tc)
return ret;
 }
 
-static int tc_set_video_mode(struct tc_data *tc,
-const struct drm_display_mode *mode)
+static int tc_set_common_video_mode(struct tc_data *tc,
+   const struct drm_display_mode *mode)
 {
int ret;
-   int vid_sync_dly;
int max_tu_symbol;
 
int left_margin = mode->htotal - mode->hsync_end;
@@ -747,7 +746,6 @@ static int tc_set_video_mode(struct tc_data *tc,
int upper_margin = mode->vtotal - mode->vsync_end;
int lower_margin = mode->vsync_start - mode->vdisplay;
int vsync_len = mode->vsync_end - mode->vsync_start;
-   u32 dp0_syncval;
u32 bits_per_pixel = 24;
u32 in_bw, out_bw;
 
@@ -818,8 +816,35 @@ static int tc_set_video_mode(struct tc_data *tc,
   FIELD_PREP(COLOR_B, 99) |
   ENI2CFILTER |
   FIELD_PREP(COLOR_BAR_MODE, COLOR_BAR_MODE_BARS));
-   if (ret)
-   return ret;
+
+   return ret;
+}
+
+static int tc_set_edp_video_mode(struct tc_data *tc,
+const struct drm_display_mode *mode)
+{
+   int ret;
+   int vid_sync_dly;
+   int max_tu_symbol;
+
+   int left_margin = mode->htotal - mode->hsync_end;
+   int hsync_len = mode->hsync_end - mode->hsync_start;
+   int upper_margin = mode->vtotal - mode->vsync_end;
+   int vsync_len = mode->vsync_end - mode->vsync_start;
+   u32 dp0_syncval;
+   u32 bits_per_pixel = 24;
+   u32 in_bw, out_bw;
+
+   /*
+* Recommended maximum number of symbols transferred in a transfer unit:
+* DIV_ROUND_UP((input active video bandwidth in bytes) * tu_size,
+*  (output active video bandwidth in bytes))
+* Must be less than tu_size.
+*/
+
+   in_bw = mode->clock * bits_per_pixel / 8;
+   out_bw = tc->link.num_lanes * tc->link.rate;
+   max_tu_symbol = DIV_ROUND_UP(in_bw * TU_SIZE_RECOMMENDED, out_bw);
 
/* DP Main Stream Attributes */
vid_sync_dly = hsync_len + left_margin + mode->hdisplay;
@@ -869,10 +894,7 @@ static int tc_set_video_mode(struct tc_data *tc,
   FIELD_PREP(MAX_TU_SYMBOL, max_tu_symbol) |
   FIELD_PREP(TU_SIZE, TU_SIZE_RECOMMENDED) |
   BPC_8);
-   if (ret)
-   return ret;
-
-   return 0;
+   return ret;
 }
 
 static int tc_wait_link_training(struct tc_data *tc)
@@ -1185,7 +1207,11 @@ static int tc_edp_stream_enable(struct tc_data *tc)
return ret;
}
 
-   ret = tc_set_video_mode(tc, &tc->mode);
+   ret = tc_set_common_video_mode(tc, &tc->mode);
+   if (ret)
+   return ret;
+
+   ret = tc_set_edp_video_mode(tc, &tc->mode);
if (ret)
return ret;
 
-- 
2.34.1

[PATCH V3 10/12] drm/bridge: tc358767: Detect bridge mode from connected endpoints in DT

[Marek Vasut]

The TC358767/TC358867/TC9595 are all capable of operating in multiple
modes, DPI-to-(e)DP, DSI-to-(e)DP, DSI-to-DPI. Only the first mode is
currently supported. It is possible to find out the mode in which the
bridge should be operated by testing connected endpoints in DT.

Port allocation:
port@0 - DSI input
port@1 - DPI input/output
port@2 - eDP output

Possible connections:
DPI -> port@1 -> port@2 -> eDP :: [port@0 is not connected]
DSI -> port@0 -> port@2 -> eDP :: [port@1 is not connected]
DSI -> port@0 -> port@1 -> DPI :: [port@2 is not connected]

Add function to determine the bridge mode based on connected endpoints.

Reviewed-by: Lucas Stach 
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - New patch
V3: - Add RB from Lucas
---
 drivers/gpu/drm/bridge/tc358767.c | 46 ++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index 07da6142d5cf2..1ccb95704a4c9 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1684,6 +1684,50 @@ static int tc_probe_edp_bridge_endpoint(struct tc_data 
*tc)
return ret;
 }
 
+static int tc_probe_bridge_endpoint(struct tc_data *tc)
+{
+   struct device *dev = tc->dev;
+   struct of_endpoint endpoint;
+   struct device_node *node = NULL;
+   const u8 mode_dpi_to_edp = BIT(1) | BIT(2);
+   const u8 mode_dsi_to_edp = BIT(0) | BIT(2);
+   const u8 mode_dsi_to_dpi = BIT(0) | BIT(1);
+   u8 mode = 0;
+
+   /*
+* Determine bridge configuration.
+*
+* Port allocation:
+* port@0 - DSI input
+* port@1 - DPI input/output
+* port@2 - eDP output
+*
+* Possible connections:
+* DPI -> port@1 -> port@2 -> eDP :: [port@0 is not connected]
+* DSI -> port@0 -> port@2 -> eDP :: [port@1 is not connected]
+* DSI -> port@0 -> port@1 -> DPI :: [port@2 is not connected]
+*/
+
+   for_each_endpoint_of_node(dev->of_node, node) {
+   of_graph_parse_endpoint(node, &endpoint);
+   if (endpoint.port > 2)
+   return -EINVAL;
+
+   mode |= BIT(endpoint.port);
+   }
+
+   if (mode == mode_dpi_to_edp)
+   return tc_probe_edp_bridge_endpoint(tc);
+   else if (mode == mode_dsi_to_dpi)
+   dev_warn(dev, "The mode DSI-to-DPI is not supported!\n");
+   else if (mode == mode_dsi_to_edp)
+   dev_warn(dev, "The mode DSI-to-(e)DP is not supported!\n");
+   else
+   dev_warn(dev, "Invalid mode (0x%x) is not supported!\n", mode);
+
+   return -EINVAL;
+}
+
 static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
struct device *dev = &client->dev;
@@ -1696,7 +1740,7 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
 
tc->dev = dev;
 
-   ret = tc_probe_edp_bridge_endpoint(tc);
+   ret = tc_probe_bridge_endpoint(tc);
if (ret)
return ret;
 
-- 
2.34.1

[PATCH V3 04/12] drm/bridge: tc358767: Convert to atomic ops

[Marek Vasut]

Use the atomic version of the enable/disable operations to continue the
transition to the atomic API. This will be needed to access the mode
from the atomic state.

Reviewed-by: Lucas Stach 
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - New patch
V3: - Add RB from Lucas
---
 drivers/gpu/drm/bridge/tc358767.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index 23e0280f484de..f88d8e616f7f8 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1234,7 +1234,9 @@ static int tc_edp_stream_disable(struct tc_data *tc)
return 0;
 }
 
-static void tc_edp_bridge_enable(struct drm_bridge *bridge)
+static void
+tc_edp_bridge_atomic_enable(struct drm_bridge *bridge,
+   struct drm_bridge_state *old_bridge_state)
 {
struct tc_data *tc = bridge_to_tc(bridge);
int ret;
@@ -1259,7 +1261,9 @@ static void tc_edp_bridge_enable(struct drm_bridge 
*bridge)
}
 }
 
-static void tc_edp_bridge_disable(struct drm_bridge *bridge)
+static void
+tc_edp_bridge_atomic_disable(struct drm_bridge *bridge,
+struct drm_bridge_state *old_bridge_state)
 {
struct tc_data *tc = bridge_to_tc(bridge);
int ret;
@@ -1459,11 +1463,14 @@ static const struct drm_bridge_funcs 
tc_edp_bridge_funcs = {
.detach = tc_edp_bridge_detach,
.mode_valid = tc_edp_mode_valid,
.mode_set = tc_bridge_mode_set,
-   .enable = tc_edp_bridge_enable,
-   .disable = tc_edp_bridge_disable,
+   .atomic_enable = tc_edp_bridge_atomic_enable,
+   .atomic_disable = tc_edp_bridge_atomic_disable,
.mode_fixup = tc_bridge_mode_fixup,
.detect = tc_bridge_detect,
.get_edid = tc_get_edid,
+   .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+   .atomic_reset = drm_atomic_helper_bridge_reset,
 };
 
 static bool tc_readable_reg(struct device *dev, unsigned int reg)
-- 
2.34.1

[PATCH V3 06/12] drm/bridge: tc358767: Move hardware init to enable callback

[Marek Vasut]

The TC358767/TC358867/TC9595 are all capable of operating either from
attached Xtal or from DSI clock lane clock. In case the later is used,
all I2C accesses will fail until the DSI clock lane is running and
supplying clock to the chip.

Move all hardware initialization to enable callback to guarantee the
DSI clock lane is running before accessing the hardware. In case Xtal
is attached to the chip, this change has no effect. Operation without
Xtal is currently not supported. The DSI-to-(e)DP mode is currently
not supported and it might be difficult to implement without Xtal.

Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - Rebase on next-20220217
V3: - Adjust commit message, point out operation without Xtal and
  DSI-to-(e)DP modes are not supported yet.
---
 drivers/gpu/drm/bridge/tc358767.c | 111 +-
 1 file changed, 63 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index e95153d9c1499..ea0d4467878f0 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1234,6 +1234,63 @@ static int tc_edp_stream_disable(struct tc_data *tc)
return 0;
 }
 
+static int tc_hardware_init(struct tc_data *tc)
+{
+   int ret;
+
+   ret = regmap_read(tc->regmap, TC_IDREG, &tc->rev);
+   if (ret) {
+   dev_err(tc->dev, "can not read device ID: %d\n", ret);
+   return ret;
+   }
+
+   if ((tc->rev != 0x6601) && (tc->rev != 0x6603)) {
+   dev_err(tc->dev, "invalid device ID: 0x%08x\n", tc->rev);
+   return -EINVAL;
+   }
+
+   tc->assr = (tc->rev == 0x6601); /* Enable ASSR for eDP panels */
+
+   if (!tc->reset_gpio) {
+   /*
+* If the reset pin isn't present, do a software reset. It isn't
+* as thorough as the hardware reset, as we can't reset the I2C
+* communication block for obvious reasons, but it's getting the
+* chip into a defined state.
+*/
+   regmap_update_bits(tc->regmap, SYSRSTENB,
+   ENBLCD0 | ENBBM | ENBDSIRX | ENBREG | ENBHDCP,
+   0);
+   regmap_update_bits(tc->regmap, SYSRSTENB,
+   ENBLCD0 | ENBBM | ENBDSIRX | ENBREG | ENBHDCP,
+   ENBLCD0 | ENBBM | ENBDSIRX | ENBREG | ENBHDCP);
+   usleep_range(5000, 1);
+   }
+
+   if (tc->hpd_pin >= 0) {
+   u32 lcnt_reg = tc->hpd_pin == 0 ? INT_GP0_LCNT : INT_GP1_LCNT;
+   u32 h_lc = INT_GPIO_H(tc->hpd_pin) | INT_GPIO_LC(tc->hpd_pin);
+
+   /* Set LCNT to 2ms */
+   regmap_write(tc->regmap, lcnt_reg,
+clk_get_rate(tc->refclk) * 2 / 1000);
+   /* We need the "alternate" mode for HPD */
+   regmap_write(tc->regmap, GPIOM, BIT(tc->hpd_pin));
+
+   if (tc->have_irq) {
+   /* enable H & LC */
+   regmap_update_bits(tc->regmap, INTCTL_G, h_lc, h_lc);
+   }
+   }
+
+   if (tc->have_irq) {
+   /* enable SysErr */
+   regmap_write(tc->regmap, INTCTL_G, INT_SYSERR);
+   }
+
+   return 0;
+}
+
 static void
 tc_edp_bridge_atomic_enable(struct drm_bridge *bridge,
struct drm_bridge_state *old_bridge_state)
@@ -1241,6 +1298,12 @@ tc_edp_bridge_atomic_enable(struct drm_bridge *bridge,
struct tc_data *tc = bridge_to_tc(bridge);
int ret;
 
+   ret = tc_hardware_init(tc);
+   if (ret < 0) {
+   dev_err(tc->dev, "failed to initialize bridge: %d\n", ret);
+   return;
+   }
+
ret = tc_get_display_props(tc);
if (ret < 0) {
dev_err(tc->dev, "failed to read display props: %d\n", ret);
@@ -1660,9 +1723,6 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
}
 
if (client->irq > 0) {
-   /* enable SysErr */
-   regmap_write(tc->regmap, INTCTL_G, INT_SYSERR);
-
ret = devm_request_threaded_irq(dev, client->irq,
NULL, tc_irq_handler,
IRQF_ONESHOT,
@@ -1675,51 +1735,6 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
tc->have_irq = true;
}
 
-   ret = regmap_read(tc->regmap, TC_IDREG, &tc->rev);
-   if (ret) {
-   dev_err(tc->dev, "can not read device ID: %d\n", ret);
-   return ret;
-   }
-
-   if ((tc->rev != 0x6601) && (tc->rev != 0x6603)) {
-   dev_err(tc->dev, "invalid device ID: 0x%08x\n", tc->rev);
-   return -EINVAL;
-   }
-
-  

[PATCH V3 03/12] drm/bridge: tc358767: Change tc_ prefix to tc_edp_ for (e)DP specific functions

[Marek Vasut]

These functions are specific to (e)DP output initialization and
operation, add specific tc_edp_ prefix to those functions to
discern them from DPI output functions that will be added later
in this series. No functional change.

Reviewed-by: Lucas Stach 
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - New patch
V3: - Add RB from Lucas
- Rename tc_bridge_funcs to tc_edp_bridge_funcs
---
 drivers/gpu/drm/bridge/tc358767.c | 39 ---
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index c23e0abc65e8f..23e0280f484de 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1164,7 +1164,7 @@ static int tc_main_link_disable(struct tc_data *tc)
return regmap_write(tc->regmap, DP0CTL, 0);
 }
 
-static int tc_stream_enable(struct tc_data *tc)
+static int tc_edp_stream_enable(struct tc_data *tc)
 {
int ret;
u32 value;
@@ -1219,7 +1219,7 @@ static int tc_stream_enable(struct tc_data *tc)
return 0;
 }
 
-static int tc_stream_disable(struct tc_data *tc)
+static int tc_edp_stream_disable(struct tc_data *tc)
 {
int ret;
 
@@ -1234,7 +1234,7 @@ static int tc_stream_disable(struct tc_data *tc)
return 0;
 }
 
-static void tc_bridge_enable(struct drm_bridge *bridge)
+static void tc_edp_bridge_enable(struct drm_bridge *bridge)
 {
struct tc_data *tc = bridge_to_tc(bridge);
int ret;
@@ -1251,7 +1251,7 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
return;
}
 
-   ret = tc_stream_enable(tc);
+   ret = tc_edp_stream_enable(tc);
if (ret < 0) {
dev_err(tc->dev, "main link stream start error: %d\n", ret);
tc_main_link_disable(tc);
@@ -1259,12 +1259,12 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
}
 }
 
-static void tc_bridge_disable(struct drm_bridge *bridge)
+static void tc_edp_bridge_disable(struct drm_bridge *bridge)
 {
struct tc_data *tc = bridge_to_tc(bridge);
int ret;
 
-   ret = tc_stream_disable(tc);
+   ret = tc_edp_stream_disable(tc);
if (ret < 0)
dev_err(tc->dev, "main link stream stop error: %d\n", ret);
 
@@ -1285,9 +1285,10 @@ static bool tc_bridge_mode_fixup(struct drm_bridge 
*bridge,
return true;
 }
 
-static enum drm_mode_status tc_mode_valid(struct drm_bridge *bridge,
- const struct drm_display_info *info,
- const struct drm_display_mode *mode)
+static enum drm_mode_status
+tc_edp_mode_valid(struct drm_bridge *bridge,
+ const struct drm_display_info *info,
+ const struct drm_display_mode *mode)
 {
struct tc_data *tc = bridge_to_tc(bridge);
u32 req, avail;
@@ -1395,8 +1396,8 @@ static const struct drm_connector_funcs 
tc_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static int tc_bridge_attach(struct drm_bridge *bridge,
-   enum drm_bridge_attach_flags flags)
+static int tc_edp_bridge_attach(struct drm_bridge *bridge,
+   enum drm_bridge_attach_flags flags)
 {
u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
struct tc_data *tc = bridge_to_tc(bridge);
@@ -1448,18 +1449,18 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
return ret;
 }
 
-static void tc_bridge_detach(struct drm_bridge *bridge)
+static void tc_edp_bridge_detach(struct drm_bridge *bridge)
 {
drm_dp_aux_unregister(&bridge_to_tc(bridge)->aux);
 }
 
-static const struct drm_bridge_funcs tc_bridge_funcs = {
-   .attach = tc_bridge_attach,
-   .detach = tc_bridge_detach,
-   .mode_valid = tc_mode_valid,
+static const struct drm_bridge_funcs tc_edp_bridge_funcs = {
+   .attach = tc_edp_bridge_attach,
+   .detach = tc_edp_bridge_detach,
+   .mode_valid = tc_edp_mode_valid,
.mode_set = tc_bridge_mode_set,
-   .enable = tc_bridge_enable,
-   .disable = tc_bridge_disable,
+   .enable = tc_edp_bridge_enable,
+   .disable = tc_edp_bridge_disable,
.mode_fixup = tc_bridge_mode_fixup,
.detect = tc_bridge_detect,
.get_edid = tc_get_edid,
@@ -1696,7 +1697,7 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
tc->aux.transfer = tc_aux_transfer;
drm_dp_aux_init(&tc->aux);
 
-   tc->bridge.funcs = &tc_bridge_funcs;
+   tc->bridge.funcs = &tc_edp_bridge_funcs;
if (tc->hpd_pin >= 0)
tc->bridge.ops |= DRM_BRIDGE_OP_DETECT;
tc->bridge.ops |= DRM_BRIDGE_OP_EDID;
-- 
2.34.1

[PATCH V3 12/12] drm/bridge: tc358767: Add DSI-to-DPI mode support

[Marek Vasut]

The TC358767/TC358867/TC9595 are all capable of operating in multiple
modes, DPI-to-(e)DP, DSI-to-(e)DP, DSI-to-DPI. Add support for the
DSI-to-DPI mode.

This requires skipping most of the (e)DP initialization code, which is
currently a large part of this driver, hence it is better to have far
simpler separate tc_dpi_bridge_funcs and their implementation.

The configuration of DPI output is also much simpler. The configuration
of the DSI input is rather similar to the other TC bridge chips.

The Pixel PLL in DPI output mode does not have the 65..150 MHz limitation
imposed on the (e)DP output mode, so this limitation is skipped to permit
operating panels with far slower pixel clock, even below 9 MHz. This mode
of operation of the PLL is valid and tested.

The detection of bridge mode is now added into tc_probe_bridge_mode(),
where in case a DPI panel is found on port@1 endpoint@1, the mode is
assumed to be DSI-to-DPI. If (e)DP is detected on port@2, the mode is
assumed to be DPI-to-(e)DP.

The DSI-to-(e)DP mode is not supported due to lack of proper hardware,
but this would be some sort of mix between the two aforementioned modes.

Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - Rebase on next-20220217 and new patches in this series
V3: - Drop edp from tc_edp_common_atomic_check,
  s@\@tc_common_atomic_check@g
- Limit Pixel PLL output to 0-100 MHz for DPI and 150-650 MHz for eDP
- Drop VID_EN write from tc_dpi_stream_disable()
- Reduce PLL stabilization delay to 120..150us in tc_dpi_stream_enable()
- Call drm_bridge_remove() in case tc_mipi_dsi_host_attach() fails
- Check of_property_count_u32_elems() return code as int instead of u8
- Enable DP0/DP1 PLL for DSI-to-DPI mode too, they clock the internal
  framebuffer and it is too slow if those PLLs are in bypass
---
 drivers/gpu/drm/bridge/tc358767.c | 364 +-
 1 file changed, 353 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index b1b02de4bbb3d..16c15aaab1b47 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1,6 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * tc358767 eDP bridge driver
+ * TC358767/TC358867/TC9595 DSI/DPI-to-DPI/(e)DP bridge driver
+ *
+ * The TC358767/TC358867/TC9595 can operate in multiple modes.
+ * The following modes are supported:
+ *   DPI->(e)DP -- supported
+ *   DSI->DPI  supported
+ *   DSI->(e)DP .. NOT supported
  *
  * Copyright (C) 2016 CogentEmbedded Inc
  * Author: Andrey Gusakov 
@@ -29,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -36,7 +43,35 @@
 
 /* Registers */
 
-/* Display Parallel Interface */
+/* PPI layer registers */
+#define PPI_STARTPPI   0x0104 /* START control bit */
+#define PPI_LPTXTIMECNT0x0114 /* LPTX timing signal */
+#define LPX_PERIOD 3
+#define PPI_LANEENABLE 0x0134
+#define PPI_TX_RX_TA   0x013c
+#define TTA_GET0x4
+#define TTA_SURE   6
+#define PPI_D0S_ATMR   0x0144
+#define PPI_D1S_ATMR   0x0148
+#define PPI_D0S_CLRSIPOCOUNT   0x0164 /* Assertion timer for Lane 0 */
+#define PPI_D1S_CLRSIPOCOUNT   0x0168 /* Assertion timer for Lane 1 */
+#define PPI_D2S_CLRSIPOCOUNT   0x016c /* Assertion timer for Lane 2 */
+#define PPI_D3S_CLRSIPOCOUNT   0x0170 /* Assertion timer for Lane 3 */
+#define PPI_START_FUNCTION BIT(0)
+
+/* DSI layer registers */
+#define DSI_STARTDSI   0x0204 /* START control bit of DSI-TX */
+#define DSI_LANEENABLE 0x0210 /* Enables each lane */
+#define DSI_RX_START   BIT(0)
+
+/* Lane enable PPI and DSI register bits */
+#define LANEENABLE_CLENBIT(0)
+#define LANEENABLE_L0ENBIT(1)
+#define LANEENABLE_L1ENBIT(2)
+#define LANEENABLE_L2ENBIT(1)
+#define LANEENABLE_L3ENBIT(2)
+
+/* Display Parallel Input Interface */
 #define DPIPXLFMT  0x0440
 #define VS_POL_ACTIVE_LOW  (1 << 10)
 #define HS_POL_ACTIVE_LOW  (1 << 9)
@@ -48,6 +83,14 @@
 #define DPI_BPP_RGB666 (1 << 0)
 #define DPI_BPP_RGB565 (2 << 0)
 
+/* Display Parallel Output Interface */
+#define POCTRL 0x0448
+#define POCTRL_S2P BIT(7)
+#define POCTRL_PCLK_POLBIT(3)
+#define POCTRL_VS_POL  BIT(2)
+#define POCTRL_HS_POL  BIT(1)
+#define POCTRL_DE_POL  BIT(0)
+
 /* Video Path */
 #define VPCTRL00x0450
 #define VSDELAYGENMASK(31, 20)
@@ -247,6 +290,9 @@ struct tc_data {
struct drm_bridge   *panel_bridge;
struct drm_connector 

[PATCH V3 07/12] drm/bridge: tc358767: Move (e)DP bridge endpoint parsing into dedicated function

[Marek Vasut]

The TC358767/TC358867/TC9595 are all capable of operating in multiple
modes, DPI-to-(e)DP, DSI-to-(e)DP, DSI-to-DPI. Only the first mode is
currently supported. In order to support the rest of the modes without
making the tc_probe() overly long, split the bridge endpoint parsing
into dedicated function, where the necessary logic to detect the bridge
mode based on which endpoints are connected, can be implemented.

Reviewed-by: Lucas Stach 
Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - Rename tc_probe_bridge_mode() to tc_probe_edp_bridge_endpoint()
  to better reflect that it parses the (e)DP output endpoint
V3: - Add RB from Lucas
---
 drivers/gpu/drm/bridge/tc358767.c | 30 +-
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index ea0d4467878f0..e9cec6f8e0e9d 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1646,19 +1646,12 @@ static irqreturn_t tc_irq_handler(int irq, void *arg)
return IRQ_HANDLED;
 }
 
-static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
+static int tc_probe_edp_bridge_endpoint(struct tc_data *tc)
 {
-   struct device *dev = &client->dev;
+   struct device *dev = tc->dev;
struct drm_panel *panel;
-   struct tc_data *tc;
int ret;
 
-   tc = devm_kzalloc(dev, sizeof(*tc), GFP_KERNEL);
-   if (!tc)
-   return -ENOMEM;
-
-   tc->dev = dev;
-
/* port@2 is the output port */
ret = drm_of_find_panel_or_bridge(dev->of_node, 2, 0, &panel, NULL);
if (ret && ret != -ENODEV)
@@ -1677,6 +1670,25 @@ static int tc_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
tc->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
}
 
+   return ret;
+}
+
+static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+   struct device *dev = &client->dev;
+   struct tc_data *tc;
+   int ret;
+
+   tc = devm_kzalloc(dev, sizeof(*tc), GFP_KERNEL);
+   if (!tc)
+   return -ENOMEM;
+
+   tc->dev = dev;
+
+   ret = tc_probe_edp_bridge_endpoint(tc);
+   if (ret)
+   return ret;
+
/* Shut down GPIO is optional */
tc->sd_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
if (IS_ERR(tc->sd_gpio))
-- 
2.34.1

[PATCH V3 05/12] drm/bridge: tc358767: Implement atomic_check callback

[Marek Vasut]

Implement .atomic_check callback which prevents user space from setting
unsupported mode. The tc_edp_common_atomic_check() variant is already
prepared for DSI-to-DPI mode addition, which has different frequency
limits.

Signed-off-by: Marek Vasut 
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
---
V2: - New patch
V3: - Drop edp from tc_edp_common_atomic_check,
  s@\@tc_common_atomic_check@g
- Return -EINVAL in case clock frequency is too high
---
 drivers/gpu/drm/bridge/tc358767.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index f88d8e616f7f8..e95153d9c1499 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1289,6 +1289,31 @@ static bool tc_bridge_mode_fixup(struct drm_bridge 
*bridge,
return true;
 }
 
+static int tc_common_atomic_check(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ const unsigned int max_khz)
+{
+   tc_bridge_mode_fixup(bridge, &crtc_state->mode,
+&crtc_state->adjusted_mode);
+
+   if (crtc_state->adjusted_mode.clock > max_khz)
+   return -EINVAL;
+
+   return 0;
+}
+
+static int tc_edp_atomic_check(struct drm_bridge *bridge,
+  struct drm_bridge_state *bridge_state,
+  struct drm_crtc_state *crtc_state,
+  struct drm_connector_state *conn_state)
+{
+   /* DPI->(e)DP interface clock limitation: upto 154 MHz */
+   return tc_common_atomic_check(bridge, bridge_state, crtc_state,
+ conn_state, 154000);
+}
+
 static enum drm_mode_status
 tc_edp_mode_valid(struct drm_bridge *bridge,
  const struct drm_display_info *info,
@@ -1463,6 +1488,7 @@ static const struct drm_bridge_funcs tc_edp_bridge_funcs 
= {
.detach = tc_edp_bridge_detach,
.mode_valid = tc_edp_mode_valid,
.mode_set = tc_bridge_mode_set,
+   .atomic_check = tc_edp_atomic_check,
.atomic_enable = tc_edp_bridge_atomic_enable,
.atomic_disable = tc_edp_bridge_atomic_disable,
.mode_fixup = tc_bridge_mode_fixup,
-- 
2.34.1

Re: [Intel-gfx] [PATCH 0/3] Improve anti-pre-emption w/a for compute workloads

[John Harrison]

On 2/23/2022 04:00, Tvrtko Ursulin wrote:

On 23/02/2022 02:22, John Harrison wrote:

On 2/22/2022 01:53, Tvrtko Ursulin wrote:

On 18/02/2022 21:33, john.c.harri...@intel.com wrote:

From: John Harrison 

Compute workloads are inherently not pre-emptible on current hardware.
Thus the pre-emption timeout was disabled as a workaround to prevent
unwanted resets. Instead, the hang detection was left to the heartbeat
and its (longer) timeout. This is undesirable with GuC submission as
the heartbeat is a full GT reset rather than a per engine reset and so
is much more destructive. Instead, just bump the pre-emption timeout


Can we have a feature request to allow asking GuC for an engine reset?

For what purpose?


To allow "stopped heartbeat" to reset the engine, however..

GuC manages the scheduling of contexts across engines. With virtual 
engines, the KMD has no knowledge of which engine a context might be 
executing on. Even without virtual engines, the KMD still has no 
knowledge of which context is currently executing on any given engine 
at any given time.


There is a reason why hang detection should be left to the entity 
that is doing the scheduling. Any other entity is second guessing at 
best.


The reason for keeping the heartbeat around even when GuC submission 
is enabled is for the case where the KMD/GuC have got out of sync 
with either other somehow or GuC itself has just crashed. I.e. when 
no submission at all is working and we need to reset the GuC itself 
and start over.


.. I wasn't really up to speed to know/remember heartbeats are nerfed 
already in GuC mode.
Not sure what you mean by that claim. Engine resets are handled by GuC 
because GuC handles the scheduling. You can't do the former if you 
aren't doing the latter. However, the heartbeat is still present and is 
still the watchdog by which engine resets are triggered. As per the rest 
of the submission process, the hang detection and recovery is split 
between i915 and GuC.





I am not sure it was the best way since full reset penalizes everyone 
for one hanging engine. Better question would be why leave heartbeats 
around to start with with GuC? If you want to use it to health check 
GuC, as you say, maybe just send a CT message and expect replies? Then 
full reset would make sense. It also achieves the goal of not 
seconding guessing the submission backend you raise.
Adding yet another hang detection mechanism is yet more complication and 
is unnecessary when we already have one that works well enough. As 
above, the heartbeat is still required for sending the pulses that cause 
pre-emptions and so let GuC detect hangs. It also provides a fallback 
against a dead GuC by default. So why invent yet another wheel?




Like it is now, and the need for this series demonstrates it, the 
whole thing has a pretty poor "impedance" match. Not even sure what 
intel_guc_find_hung_context is doing in intel_engine_hearbeat.c - why 
is that not in intel_gt_handle_error at least? Why is hearbeat code 
special and other callers of intel_gt_handle_error don't need it?
There is no guilty context if the reset was triggered via debugfs or 
similar. And as stated ad nauseam, i915 is no longer handling the 
scheduling and so cannot make assumptions about what is or is not 
running on the hardware at any given time. And obviously, if the reset 
initiated by GuC itself then i915 should not be second guessing the 
guilty context as the GuC notification has already told us who was 
responsible.


And to be clear, the 'poor impedance match' is purely because we don't 
have mid-thread pre-emption and so need a stupidly huge timeout on 
compute capable engines. Whereas, we don't want a total heatbeat timeout 
of a minute or more. That is the impedance mis-match. If the 640ms was 
acceptable for RCS then none of this hacky timeout algorithm mush would 
be needed.


John.




Regards,

Tvrtko

Re: [PATCH 3/7] drm/bridge: Extend struct drm_bus_cfg with clock field

[Marek Vasut]

On 2/24/22 16:19, Maxime Ripard wrote:

Hi,


Hi,


On Sat, Feb 19, 2022 at 01:28:40AM +0100, Marek Vasut wrote:

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 1701c2128a5cb..32455cf28f0bc 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -1077,6 +1077,11 @@ struct drm_bus_cfg {
 * @flags: DRM_BUS_* flags used on this bus
 */
u32 flags;
+
+   /**
+* @clock: Clock frequency in kHz used on this bus
+*/
+   u32 clock;
  };


This is fairly vague. You were mentioning DSI: is it the pixel clock?


DSI HS clock is the one I need.

I hope we can flesh out what exactly should be in here.


The HS clock rate?


Yes


With or without counting the lanes? What about the


Without


burst mode: would it be the lane or pixel rate?


Still the HS clock rate.


It would be just as confusing for HDMI: is it the the TMDS character
rate? The TMDS bit rate ? TMDS Clock rate?


For HDMI I would expect 148.5 MHz here , and if HDMI needs additional 
extras, they might have to be added to struct drm_bus_cfg as extra fields ?

Re: [RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces

[Abhinav Kumar]

On 2/11/2022 2:40 PM, Dmitry Baryshkov wrote:

It is possible to supply display-connector (bridge) to the DP interface,
add support for parsing it too.

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/dp/dp_parser.c | 19 ---
  1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
b/drivers/gpu/drm/msm/dp/dp_parser.c
index 901d7967370f..1056b8d5755b 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -301,17 +301,22 @@ static int dp_parser_parse(struct dp_parser *parser, int 
connector_type)
return rc;
  
  	/*

-* Currently we support external bridges only for eDP connectors.
+* External bridges are mandatory for eDP interfaces: one has to
+* provide at least an eDP panel (which gets wrapped into panel-bridge).
 *
-* No external bridges are expected for the DisplayPort connector,
-* it is physically present in a form of a DP or USB-C connector.
+* For DisplayPort interfaces external bridges are optional, so
+* silently ignore an error if one is not present (-ENODEV).
 */
-   if (connector_type == DRM_MODE_CONNECTOR_eDP) {
-   rc = dp_parser_find_next_bridge(parser);
-   if (rc) {
-   DRM_ERROR("DP: failed to find next bridge\n");
+   rc = dp_parser_find_next_bridge(parser);
+   if (rc == -ENODEV) {
+   if (connector_type == DRM_MODE_CONNECTOR_eDP) {
+   DRM_ERROR("eDP: next bridge is not present\n");
return rc;
}
+   } else if (rc) {
+   if (rc != -EPROBE_DEFER)
+   DRM_ERROR("DP: error parsing next bridge: %d\n", rc);
+   return rc;
}


How is this silently ignoring?

static int dp_display_bind(struct device *dev, struct device *master,
   void *data)
{
int rc = 0;
struct dp_display_private *dp = dev_get_dp_display_private(dev);
struct msm_drm_private *priv = dev_get_drvdata(master);
struct drm_device *drm = priv->dev;

dp->dp_display.drm_dev = drm;
priv->dp[dp->id] = &dp->dp_display;

rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
if (rc) {
DRM_ERROR("device tree parsing failed\n");
goto end;
}

dp_display_bind will still fail if a bridge is not found.

If supplying a bridge is optional even this should succeed right?

  
  	/* Map the corresponding regulator information according to

Re: [PATCH 0/4] Address a few compilation warnings

[Alex Deucher]

Applied with some minor modifications to patches 3 and 4 to avoid
adding new warnings.

Alex

On Thu, Feb 24, 2022 at 2:44 PM Harry Wentland  wrote:
>
> Series is
> Reviewed-by: Harry Wentland 
>
> Harry
>
> On 2022-02-24 14:15, Magali Lemes wrote:
> > This patchset addresses a few warnings reported by the Kernel Test Robot and
> > sparse.
> >
> > Magali Lemes (4):
> >   drm/amd/display: Adjust functions documentation
> >   drm/amd/display: Add conditional around function
> >   drm/amd/display: Use NULL instead of 0
> >   drm/amd/display: Turn functions into static
> >
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c   | 4 +++-
> >  .../gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c  | 2 +-
> >  drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_smu.c  | 4 ++--
> >  drivers/gpu/drm/amd/display/dc/core/dc.c| 6 +++---
> >  drivers/gpu/drm/amd/display/dc/core/dc_resource.c   | 2 +-
> >  drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c | 4 ++--
> >  drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 4 +---
> >  .../gpu/drm/amd/display/dc/irq/dcn315/irq_service_dcn315.c  | 2 +-
> >  8 files changed, 14 insertions(+), 14 deletions(-)
> >
>

Re: [RFC][PATCH 0/7] drm/bridge: Add support for selecting DSI host HS clock from DSI bridge

[Marek Vasut]

On 2/24/22 16:40, Maxime Ripard wrote:

Hi,


Hi,


On Sat, Feb 19, 2022 at 01:28:37AM +0100, Marek Vasut wrote:

This patch series attempts to address a problem of missing support for DSI
bridge-to-bridge or panel-to-bridge clock frequency negotiation. The problem
has two variants.

First, a DSI->to->x bridge derives its own internal clock from DSI HS clock,
but the DSI HS clock cannot be set to arbitrary values. TS358767 is one such
bridge in case it operates without Xtal. In that case, the TC358767 driver
must be able to negotiate the specific suitable DSI HS clock frequency for
the chip.

Second, both DSI->to->x bridges and DSI hosts currently calculate, or rather
guess and hope they both guess the same number as their neighbor, the DSI HS
clock frequency from form of PLL=(width * height * bpp / lanes / 2). This is
dangerous, since the PLL capabilities on both ends of the DSI bus might differ
and the DSI host could easily end up generating wildly different clock than
what the DSI bridge/panel expects to receive.

This series attempts to address these negotiation problems by extending the
existing .atomic_get_input_bus_fmts callback into .atomic_get_input_bus_cfgs
callback in struct drm_bridge_funcs {}. The extended version returns not only
a list of a list of bus formats supported by a bridge, but the entire list of
struct drm_bus_cfg, which currently contains format and bus flags, but can be
extended with other members, like desired clock frequency, as required.

This series demonstrates such extension by adding the support for negotiating
the DSI clock and by implementing such support in DW DSI Host and TC358767 DSI
bridge.


We discussed it a bit on IRC as well but there's another issue with
this, let's imagine this setup:

encoder -> DSI-to-DPI bridge -> DPI-to-HDMI bridge -> HDMI Monitor

HDMI is fairly favorable, and it would probably use pixel clocks of
either 148.5, 297 or 594MHz. Let's simplify this a bit and assume your
DSI-to-DPI bridge can only operate at a frequency equivalent to 297MHz.

594Mhz is going to be used by those new fancy monitors, and thus the
preferred mode is likely to be using 594MHz.

With your solution, it effectively means that when the system will boot
up, the preferred mode will be reported to the userspace (and the fbdev
emulation), whatever is coming next is going to use it, and you're just
going to... refuse it because it never worked in the first place. You'll
leave a blank display, and that's it. That's not a great behavior,
really.


If you cannot support such a panel with this kind of scanout engine, 
what else would you do than blank screen ?



And since you don't get a state until you start a commit, this would
need to be able to work without one. Of course, some state parameters
will affect the clock (like the bpc count) so it won't be perfect, but
we can at least try.

Another thing is that the clock that needs to be negociated is likely to
be device specific. It's probably going to be fairly similar across
similar devices (like all the DSI bridges you mentioned are using the HS
clock), but I'm not sure we can make that assumption.


The bridge (data sink) should be able to figure out what kind of clock 
it needs from the source and then request those, yes. With DSI you can 
make an assumption about what kind of clock frequencies each link mode 
would require, but in general, you cannot assume much.



I think we could make something that work by asking the previous bridge
in the chain for a given clock rate with a given mode, and then filter
out / adjust anything we don't like. It would then be able to first
check if it can provide that clock in the first place, and then the rate
it has, and would be free to forward the query up to the encoder. And
since it's tied to the mode, it would work with mode_valid too.


It seems to me this is similar to this solution, except it must happen 
when the mode is available ? But then the question also comes to mind, 
should select_bus_fmt_recursive() be called only after mode is available 
too ?

Re: [PATCH] drm/nouveau: Remove the unused header file nvif/list.h

[Lyude Paul]

Thanks for the ping!

Reviewed-by: Lyude Paul 

I will push this to drm-misc-next in a bit

On Wed, 2022-02-23 at 10:18 +0800, Cai Huoqing wrote:
> On 09 2月 22 14:53:19, Cai Huoqing wrote:
> > The nouveau driver depends on include/linux/list.h instead of
> > nvif/list.h, so remove the obstacle-nvif/list.h.
> > 
> > Signed-off-by: Cai Huoqing 
> > ---
> Ping :)
> >  drivers/gpu/drm/nouveau/include/nvif/list.h | 353 
> >  1 file changed, 353 deletions(-)
> >  delete mode 100644 drivers/gpu/drm/nouveau/include/nvif/list.h
> > 
> > diff --git a/drivers/gpu/drm/nouveau/include/nvif/list.h
> > b/drivers/gpu/drm/nouveau/include/nvif/list.h
> > deleted file mode 100644
> > index 8af5d144ecb0..
> > --- a/drivers/gpu/drm/nouveau/include/nvif/list.h
> > +++ /dev/null
> > @@ -1,353 +0,0 @@
> > -/*
> > - * Copyright © 2010 Intel Corporation
> > - * Copyright © 2010 Francisco Jerez 
> > - *
> > - * Permission is hereby granted, free of charge, to any person obtaining
> > a
> > - * copy of this software and associated documentation files (the
> > "Software"),
> > - * to deal in the Software without restriction, including without
> > limitation
> > - * the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > - * and/or sell copies of the Software, and to permit persons to whom the
> > - * Software is furnished to do so, subject to the following conditions:
> > - *
> > - * The above copyright notice and this permission notice (including the
> > next
> > - * paragraph) shall be included in all copies or substantial portions of
> > the
> > - * Software.
> > - *
> > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
> > SHALL
> > - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING
> > - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > - * IN THE SOFTWARE.
> > - *
> > - */
> > -
> > -/* Modified by Ben Skeggs  to match kernel list APIs
> > */
> > -
> > -#ifndef _XORG_LIST_H_
> > -#define _XORG_LIST_H_
> > -
> > -/**
> > - * @file Classic doubly-link circular list implementation.
> > - * For real usage examples of the linked list, see the file test/list.c
> > - *
> > - * Example:
> > - * We need to keep a list of struct foo in the parent struct bar, i.e.
> > what
> > - * we want is something like this.
> > - *
> > - * struct bar {
> > - *  ...
> > - *  struct foo *list_of_foos; -> struct foo {}, struct foo
> > {}, struct foo{}
> > - *  ...
> > - * }
> > - *
> > - * We need one list head in bar and a list element in all list_of_foos
> > (both are of
> > - * data type 'struct list_head').
> > - *
> > - * struct bar {
> > - *  ...
> > - *  struct list_head list_of_foos;
> > - *  ...
> > - * }
> > - *
> > - * struct foo {
> > - *  ...
> > - *  struct list_head entry;
> > - *  ...
> > - * }
> > - *
> > - * Now we initialize the list head:
> > - *
> > - * struct bar bar;
> > - * ...
> > - * INIT_LIST_HEAD(&bar.list_of_foos);
> > - *
> > - * Then we create the first element and add it to this list:
> > - *
> > - * struct foo *foo = malloc(...);
> > - * 
> > - * list_add(&foo->entry, &bar.list_of_foos);
> > - *
> > - * Repeat the above for each element you want to add to the list.
> > Deleting
> > - * works with the element itself.
> > - *  list_del(&foo->entry);
> > - *  free(foo);
> > - *
> > - * Note: calling list_del(&bar.list_of_foos) will set bar.list_of_foos to
> > an empty
> > - * list again.
> > - *
> > - * Looping through the list requires a 'struct foo' as iterator and the
> > - * name of the field the subnodes use.
> > - *
> > - * struct foo *iterator;
> > - * list_for_each_entry(iterator, &bar.list_of_foos, entry) {
> > - *  if (iterator->something == ...)
> > - * ...
> > - * }
> > - *
> > - * Note: You must not call list_del() on the iterator if you continue the
> > - * loop. You need to run the safe for-each loop instead:
> > - *
> > - * struct foo *iterator, *next;
> > - * list_for_each_entry_safe(iterator, next, &bar.list_of_foos, entry) {
> > - *  if (...)
> > - *  list_del(&iterator->entry);
> > - * }
> > - *
> > - */
> > -
> > -/**
> > - * The linkage struct for list nodes. This struct must be part of your
> > - * to-be-linked struct. struct list_head is required for both the head of
> > the
> > - * list and for each list node.
> > - *
> > - * Position and name of the struct list_head field is irrelevant.
> > - * There are no requirements that elements of a list are of the same
> > type.
> > - * There are no requirements for a list head, any struct list_head

Re: [PATCH 0/3] drm/helpers: Make the suballocation manager drm generic.

[Maarten Lankhorst]

Op 23-02-2022 om 16:11 schreef Christian König:
> Am 23.02.22 um 14:51 schrieb Maarten Lankhorst:
>> Second version of the patch. I didn't fix the copyright (which ame up
>> in the previous version), as I feel the original author should send a
>> patch for that.
>>
>> I've made the suballocator into its own module, and did a cleanup pass on it.
>> The suballocator is generic enough to be useful for any resource that can be
>> subdivided and is guarded by a completion fence.
>
> Well the main issue is still that you removed the per allocation alignment.
>
> For amdgpu that is not much of a problem, but for radeon that could cause 
> massive issues with UVD semaphore synchronization. 

Hey,

Is this really a problem? I made the per allocation alignment fixed and set it 
to the higher of the 2 places it's used in by radeon. This just means that 
slightly more memory might be allocated to each suballocation. This shouldn't 
cause any problem. It would reserve 256 bytes for a semaphore allocation, 
instead of 8.

~Maarten

Re: [Freedreno] [RFC PATCH v2 1/5] drm/msm/dp: fix panel bridge attachment

[Dmitry Baryshkov]

On Thu, 24 Feb 2022 at 21:25, Abhinav Kumar  wrote:
>
>
>
> On 2/18/2022 6:26 PM, Dmitry Baryshkov wrote:
> > On 19/02/2022 02:56, Stephen Boyd wrote:
> >> Quoting Dmitry Baryshkov (2022-02-11 14:40:02)
> >>> In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> >>> enable and disable") the DP driver received a drm_bridge instance, which
> >>> is always attached to the encoder as a root bridge. However it conflicts
> >>> with the panel_bridge support for eDP panels. The panel bridge attaches
> >>> to the encoder before the "dp" bridge has a chace to do so. Change
> >>
> >> s/chace/chance/
> >>
> >>> panel_bridge attachment to come after dp_bridge attachment.
> >>
> >> s/panel_bridge/panel bridge/ possibly? And maybe clarify that dp_bridge
> >> is the "DP driver's drm_bridge instance created in
> >> msm_dp_bridge_init()"?
> >>
> >> My understanding is that we want to pass the bridge created in
> >> msm_dp_bridge_init() as the 'previous' bridge so that it attaches the
> >> panel bridge to the output of the DP bridge that's attached to the
> >> encoder.
> >
> > Thanks! I'll update the commit log when pushing the patches.
>
> Please correct if i am missing something here.
>
> You are right that the eDP panel's panel bridge attaches to the encoder
> in dp_drm_connector_init() which happens before msm_dp_bridge_init() and
> hence it will attach directly to the encoder.
>
> But we are talking about different encoders here. DP's dp_display has a
> different encoder compared to the EDP's dp_display.

The encoders are different. However both encoders use the same
codepath, which includes dp_bridge. It controls dp_display by calling
msm_dp_display_foo() functions.

> So DP's bridge will still be attached to its encoder's root.

There is one dp_bridge per each encoder. Consider sc8180x which has 3
DP controllers (and thus 3 dp_bridges).

>
> So what are we achieving with this change?
>
> >
> >>
> >>>
> >>> Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> >>> enable and disable")
> >>> Cc: Kuogee Hsieh 
> >>> Signed-off-by: Dmitry Baryshkov 
> >>> ---
> >>
> >> Reviewed-by: Stephen Boyd 
> >>
> >>>   drivers/gpu/drm/msm/dp/dp_drm.c | 21 +++--
> >>>   1 file changed, 11 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c
> >>> b/drivers/gpu/drm/msm/dp/dp_drm.c
> >>> index d4d360d19eba..26ef41a4c1b6 100644
> >>> --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> >>> +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> >>> @@ -169,16 +169,6 @@ struct drm_connector
> >>> *dp_drm_connector_init(struct msm_dp *dp_display)
> >>>
> >>>  drm_connector_attach_encoder(connector, dp_display->encoder);
> >>>
> >>> -   if (dp_display->panel_bridge) {
> >>> -   ret = drm_bridge_attach(dp_display->encoder,
> >>> -   dp_display->panel_bridge, NULL,
> >>> -   DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> >>> -   if (ret < 0) {
> >>> -   DRM_ERROR("failed to attach panel bridge:
> >>> %d\n", ret);
> >>> -   return ERR_PTR(ret);
> >>> -   }
> >>> -   }
> >>> -
> >>>  return connector;
> >>>   }
> >>>
> >>> @@ -246,5 +236,16 @@ struct drm_bridge *msm_dp_bridge_init(struct
> >>> msm_dp *dp_display, struct drm_devi
> >>>  return ERR_PTR(rc);
> >>>  }
> >>>
> >>> +   if (dp_display->panel_bridge) {
> >>> +   rc = drm_bridge_attach(dp_display->encoder,
> >>> +   dp_display->panel_bridge,
> >>> bridge,
> >>> +   DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> >>> +   if (rc < 0) {
> >>> +   DRM_ERROR("failed to attach panel bridge:
> >>> %d\n", rc);
> >>> +   drm_bridge_remove(bridge);
> >>> +   return ERR_PTR(rc);
> >>> +   }
> >>> +   }
> >>> +
> >>>  return bridge;
> >>
> >> Not a problem with this patch, but what is this pointer used for? I see
> >> it's assigned to priv->bridges and num_bridges is incremented but nobody
> >> seems to look at that.
> >
> >
> > That's on my todo list. to remove connectors array and to destroy
> > created bridges during drm device destruction.
> >



-- 
With best wishes
Dmitry

Re: [RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces

[Dmitry Baryshkov]

On Thu, 24 Feb 2022 at 23:13, Abhinav Kumar  wrote:
>
>
>
> On 2/11/2022 2:40 PM, Dmitry Baryshkov wrote:
> > It is possible to supply display-connector (bridge) to the DP interface,
> > add support for parsing it too.
> >
> > Signed-off-by: Dmitry Baryshkov 
> > ---
> >   drivers/gpu/drm/msm/dp/dp_parser.c | 19 ---
> >   1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
> > b/drivers/gpu/drm/msm/dp/dp_parser.c
> > index 901d7967370f..1056b8d5755b 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> > @@ -301,17 +301,22 @@ static int dp_parser_parse(struct dp_parser *parser, 
> > int connector_type)
> >   return rc;
> >
> >   /*
> > -  * Currently we support external bridges only for eDP connectors.
> > +  * External bridges are mandatory for eDP interfaces: one has to
> > +  * provide at least an eDP panel (which gets wrapped into 
> > panel-bridge).
> >*
> > -  * No external bridges are expected for the DisplayPort connector,
> > -  * it is physically present in a form of a DP or USB-C connector.
> > +  * For DisplayPort interfaces external bridges are optional, so
> > +  * silently ignore an error if one is not present (-ENODEV).
> >*/
> > - if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> > - rc = dp_parser_find_next_bridge(parser);
> > - if (rc) {
> > - DRM_ERROR("DP: failed to find next bridge\n");
> > + rc = dp_parser_find_next_bridge(parser);
> > + if (rc == -ENODEV) {
> > + if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> > + DRM_ERROR("eDP: next bridge is not present\n");
> >   return rc;
> >   }
> > + } else if (rc) {
> > + if (rc != -EPROBE_DEFER)
> > + DRM_ERROR("DP: error parsing next bridge: %d\n", rc);
> > + return rc;
> >   }
>
> How is this silently ignoring?
>
> static int dp_display_bind(struct device *dev, struct device *master,
> void *data)
> {
>  int rc = 0;
>  struct dp_display_private *dp = dev_get_dp_display_private(dev);
>  struct msm_drm_private *priv = dev_get_drvdata(master);
>  struct drm_device *drm = priv->dev;
>
>  dp->dp_display.drm_dev = drm;
>  priv->dp[dp->id] = &dp->dp_display;
>
>  rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
>  if (rc) {
>  DRM_ERROR("device tree parsing failed\n");
>  goto end;
>  }
>
> dp_display_bind will still fail if a bridge is not found.
>
> If supplying a bridge is optional even this should succeed right?

It will succeed as dp_parser_parse() will not return -ENODEV if the
connector is not eDP.
To rephrase the comment:
For the dp_parser_find_next_bridge() result:
- for eDP the driver passes all errors to the calling function.
- for DP the driver ignores -ENODEV (no external bridge is supplied),
but passes all other errors (which can mean e.g. that the bridge is
not properly declared or that it did hasn't been probed yet).

-- 
With best wishes
Dmitry

Re: [RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces

[Abhinav Kumar]

On 2/24/2022 12:49 PM, Dmitry Baryshkov wrote:

On Thu, 24 Feb 2022 at 23:13, Abhinav Kumar  wrote:




On 2/11/2022 2:40 PM, Dmitry Baryshkov wrote:

It is possible to supply display-connector (bridge) to the DP interface,
add support for parsing it too.

Signed-off-by: Dmitry Baryshkov 
---
   drivers/gpu/drm/msm/dp/dp_parser.c | 19 ---
   1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
b/drivers/gpu/drm/msm/dp/dp_parser.c
index 901d7967370f..1056b8d5755b 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -301,17 +301,22 @@ static int dp_parser_parse(struct dp_parser *parser, int 
connector_type)
   return rc;

   /*
-  * Currently we support external bridges only for eDP connectors.
+  * External bridges are mandatory for eDP interfaces: one has to
+  * provide at least an eDP panel (which gets wrapped into panel-bridge).
*
-  * No external bridges are expected for the DisplayPort connector,
-  * it is physically present in a form of a DP or USB-C connector.
+  * For DisplayPort interfaces external bridges are optional, so
+  * silently ignore an error if one is not present (-ENODEV).
*/
- if (connector_type == DRM_MODE_CONNECTOR_eDP) {
- rc = dp_parser_find_next_bridge(parser);
- if (rc) {
- DRM_ERROR("DP: failed to find next bridge\n");
+ rc = dp_parser_find_next_bridge(parser);
+ if (rc == -ENODEV) {
+ if (connector_type == DRM_MODE_CONNECTOR_eDP) {
+ DRM_ERROR("eDP: next bridge is not present\n");
   return rc;
   }
+ } else if (rc) {
+ if (rc != -EPROBE_DEFER)
+ DRM_ERROR("DP: error parsing next bridge: %d\n", rc);
+ return rc;
   }


How is this silently ignoring?

static int dp_display_bind(struct device *dev, struct device *master,
 void *data)
{
  int rc = 0;
  struct dp_display_private *dp = dev_get_dp_display_private(dev);
  struct msm_drm_private *priv = dev_get_drvdata(master);
  struct drm_device *drm = priv->dev;

  dp->dp_display.drm_dev = drm;
  priv->dp[dp->id] = &dp->dp_display;

  rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
  if (rc) {
  DRM_ERROR("device tree parsing failed\n");
  goto end;
  }

dp_display_bind will still fail if a bridge is not found.

If supplying a bridge is optional even this should succeed right?


It will succeed as dp_parser_parse() will not return -ENODEV if the
connector is not eDP.
To rephrase the comment:
For the dp_parser_find_next_bridge() result:
- for eDP the driver passes all errors to the calling function.
- for DP the driver ignores -ENODEV (no external bridge is supplied),
but passes all other errors (which can mean e.g. that the bridge is
not properly declared or that it did hasn't been probed yet).



Ah okay, I just noticed that dp_parser_parse() returns 0 by default and 
not rc.


So in this case it will still return 0.

Hence this change LGTM,

Reviewed-by: Abhinav Kumar 

Re: [Intel-gfx] [PATCH 5/8] drm/i915/guc: Move lrc desc setup to where it is needed

[John Harrison]

On 2/23/2022 18:03, Ceraolo Spurio, Daniele wrote:

On 2/23/2022 12:23 PM, John Harrison wrote:

On 2/22/2022 17:12, Ceraolo Spurio, Daniele wrote:

On 2/17/2022 3:52 PM, john.c.harri...@intel.com wrote:

From: John Harrison 

The LRC descriptor was being initialised early on in the context
registration sequence. It could then be determined that the actual
registration needs to be delayed and the descriptor would be wiped
out. This is inefficient, so move the setup to later in the process
after the point of no return.

Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c

index 0ab2d1a24bf6..aa74ec74194a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -2153,6 +2153,8 @@ static int 
__guc_action_register_context(struct intel_guc *guc,

   0, loop);
  }
  +static void prepare_context_registration_info(struct 
intel_context *ce);

+
  static int register_context(struct intel_context *ce, bool loop)
  {
  struct intel_guc *guc = ce_to_guc(ce);
@@ -2163,6 +2165,8 @@ static int register_context(struct 
intel_context *ce, bool loop)

  GEM_BUG_ON(intel_context_is_child(ce));
  trace_intel_context_register(ce);
  +    prepare_context_registration_info(ce);
+
  if (intel_context_is_parent(ce))
  ret = __guc_action_register_multi_lrc(guc, ce, 
ce->guc_id.id,

    offset, loop);
@@ -2246,7 +2250,6 @@ static void 
prepare_context_registration_info(struct intel_context *ce)

  struct intel_context *child;
    GEM_BUG_ON(!engine->mask);
-    GEM_BUG_ON(!sched_state_is_init(ce));
    /*
   * Ensure LRC + CT vmas are is same region as write barrier 
is done
@@ -2314,9 +2317,13 @@ static int try_context_registration(struct 
intel_context *ce, bool loop)

  bool context_registered;
  int ret = 0;
  +    GEM_BUG_ON(!sched_state_is_init(ce));
+
  context_registered = ctx_id_mapped(guc, desc_idx);
  -    prepare_context_registration_info(ce);
+    if (context_registered)
+    clr_ctx_id_mapping(guc, desc_idx);
+    set_ctx_id_mapping(guc, desc_idx, ce);


I think we can do the clr unconditionally. Also, should we drop the 
clr/set pair in prepare_context_registration_info? it shouldn't be 
needed, unless I'm missing a path where we don;t pass through here.


Daniele

I don't believe so.

The point is that the context id might have changed (it got stolen, 
re-used, etc. - all the state machine code below can cause aborts and 
retries and such like if something is pending and the register needs 
to be delayed). So we need to clear out the old mapping and add a new 
one to be safe. Also, I'm not sure if it is safe to do a xa_store to 
an already used entry as an update or if you are supposed to clear it 
first? But that's what the code did before and I'm trying to not 
change any actual behaviour here.


I was comparing with previous behavior. before this patch, we only do 
the setting of the ctx_id here (inside 
prepare_context_registration_info) and you're not changing any of the 
abort/retry behavior, so if it was enough before it should be enough now.
Hmm, I think I must have confused myself with the intermediate steps 
along the way. Yes, it looks like the clr/set in prepare is redundant by 
the end.




Regarding the xa ops, we did an unconditional clear before, so it 
should be ok to just do the same and have the clear and set back to 
back without checking if the context ID was already in use or not.
Actually, I was thinking you meant to drop the clr completely rather 
than just drop the condition. Yeah, that sounds fine.


Will post an update.

John.



Daniele



John.




    /*
   * The context_lookup xarray is used to determine if the 
hardware