date:20220808

[Bug 216119] 087451f372bf76d breaks hibernation on amdgpu Radeon R9 390

2022-08-08 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=216119

Mario Limonciello (AMD) (mario.limoncie...@amd.com) changed:

   What|Removed |Added

 CC||mario.limoncie...@amd.com

--- Comment #34 from Mario Limonciello (AMD) (mario.limoncie...@amd.com) ---
I recently became aware that the WX3200 in my workstation wasn't working
properly after suspend-to-ram.

61:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc.
[AMD/ATI] Lexa XT [Radeon PRO WX 3200] [1002:6981] (rev 10)

I ran a bisect, and unfortunately it came up to the commit that was created for
this bug
(https://github.com/torvalds/linux/commit/3a4b1cc28fbdc2325b3e3ed7d8024995a75f9216)

$ git bisect log
git bisect start '--term-new=broken' '--term-old=good'
# good: [4b0986a3613c92f4ec1bdc7f60ec66fea135991f] Linux 5.18
git bisect good 4b0986a3613c92f4ec1bdc7f60ec66fea135991f
# broken: [8843bf1f0737ecea456d2bbd19d4263d49f2d110] Linux 5.18.16
git bisect broken 8843bf1f0737ecea456d2bbd19d4263d49f2d110
# good: [ffd4c4d5293e4985092ea45ba21cad9326e2e434] drivers: staging: rtl8192e:
Fix deadlock in rtllib_beacons_stop()
git bisect good ffd4c4d5293e4985092ea45ba21cad9326e2e434
# good: [164f0714bae175e2f5737070d037d7475417228d] pinctrl: sunxi: a83t: Fix
NAND function name for some pins
git bisect good 164f0714bae175e2f5737070d037d7475417228d
# broken: [86fbd2844858c5aef57a28ebc3d53d298f37cc67] x86/retpoline: Use
-mfunction-return
git bisect broken 86fbd2844858c5aef57a28ebc3d53d298f37cc67
# broken: [7fc7c6d053cfca70bb81892f3f00937e5c459d5a] arm64: dts: broadcom:
bcm4908: Fix cpu node for smp boot
git bisect broken 7fc7c6d053cfca70bb81892f3f00937e5c459d5a
# good: [cd52154b924f2ea05069d4296045d9fd56a8da23] ALSA: hda - Add fixup for
Dell Latitidue E5430
git bisect good cd52154b924f2ea05069d4296045d9fd56a8da23
# good: [b8651049bdd77fa652bcf0f3157911a3a6fc4f2f] net/mlx5e: CT: Use own
workqueue instead of mlx5e priv
git bisect good b8651049bdd77fa652bcf0f3157911a3a6fc4f2f
# broken: [594cea2c09f7cd440d1ee1c4547d5bc6a646b0e4] netfilter: conntrack:
remove the percpu dying list
git bisect broken 594cea2c09f7cd440d1ee1c4547d5bc6a646b0e4
# broken: [cd486308d773d6d062a0140062458b48f8a0eb6b] ASoC: tas2764: Add post
reset delays
git bisect broken cd486308d773d6d062a0140062458b48f8a0eb6b
# broken: [4ffcacab7145080187330accafae69e87a481eec] drm/amdgpu/display:
disable prefer_shadow for generic fb helpers
git bisect broken 4ffcacab7145080187330accafae69e87a481eec
# good: [16427298f3dc02ec90bdfa31c8ef9b384ea5534a] net/mlx5e: Ring the TX
doorbell on DMA errors
git bisect good 16427298f3dc02ec90bdfa31c8ef9b384ea5534a
# good: [27dccf616a0a82f4d8004b7ee04560e7de419e63] drm/amdgpu: keep fbdev
buffers pinned during suspend
git bisect good 27dccf616a0a82f4d8004b7ee04560e7de419e63
# first broken commit: [4ffcacab7145080187330accafae69e87a481eec]
drm/amdgpu/display: disable prefer_shadow for generic fb helpers

It seems that a revert of that commit isn't the best solution as it's just
trading the S3 failure I see for your S4 failure.  But also it seems that from
your comment #32 there is still an underlying problem with using the fbdev
helper, albeit improved for you in S3.

Would you mind contrasting if S3 is working for you with/without that commit?

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

Re: imx8mm lcdif->dsi->adv7535 no video, no errors

2022-08-08 Thread Adam Ford

On Mon, Aug 8, 2022 at 5:13 AM Adam Ford  wrote:
>
> On Mon, Aug 8, 2022 at 3:54 AM Marco Felsch  wrote:
> >
> > On 22-08-07, Adam Ford wrote:
> > > On Fri, Aug 5, 2022 at 4:05 PM Adam Ford  wrote:
> > > >
> > > > On Fri, Aug 5, 2022 at 7:56 AM Adam Ford  wrote:
> > > > >
> > > > > On Fri, Aug 5, 2022 at 5:55 AM Adam Ford  wrote:
> > > > > >
> > > > > > On Fri, Aug 5, 2022 at 3:44 AM Biju Das 
> > > > > >  wrote:
> > > > > > >
> > > > > > > Hi Adam and all,
> > > > > > >
> > > > > > > > Subject: Re: imx8mm lcdif->dsi->adv7535 no video, no errors
> > > > > > > >
> > > > > > > > On Thu, Aug 4, 2022 at 9:52 AM Dave Stevenson
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > On Thu, 4 Aug 2022 at 13:51, Marco Felsch 
> > > > > > > > > 
> > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Dave,
> > > > > > > > > >
> > > > > > > > > > On 22-08-04, Dave Stevenson wrote:
> > > > > > > > > > > Hi Marco
> > > > > > > > > > >
> > > > > > > > > > > On Thu, 4 Aug 2022 at 10:38, Marco Felsch
> > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Dave, Adam,
> > > > > > > > > > > >
> > > > > > > > > > > > On 22-08-03, Dave Stevenson wrote:
> > > > > > > > > > > > > Hi Adam
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, 3 Aug 2022 at 12:03, Adam Ford 
> > > > > > > > > > > > > 
> > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > ...
> > > > > > > > > > > >
> > > > > > > > > > > > > > > Did managed to get access to the ADV7535 
> > > > > > > > > > > > > > > programming
> > > > > > > > > > > > > > > guide? This is the black box here. Let me check 
> > > > > > > > > > > > > > > if I can
> > > > > > > > > > > > > > > provide you a link with our repo so you can test 
> > > > > > > > > > > > > > > our
> > > > > > > > current DSIM state if you want.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I do have access to the programming guide, but it's 
> > > > > > > > > > > > > > under
> > > > > > > > > > > > > > NDA, but I'll try to answer questions if I can.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Not meaning to butt in, but I have datasheets for 
> > > > > > > > > > > > > ADV7533 and
> > > > > > > > > > > > > 7535 from previously looking at these chips.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for stepping into :)
> > > > > > > > > > > >
> > > > > > > > > > > > > Mine fairly plainly states:
> > > > > > > > > > > > > "The DSI receiver input supports DSI video mode 
> > > > > > > > > > > > > operation
> > > > > > > > > > > > > only, and specifically, only supports nonburst mode 
> > > > > > > > > > > > > with sync
> > > > > > > > pulses".
> > > > > > > > > > > >
> > > > > > > > > > > > I've read this also, and we are working in nonburst 
> > > > > > > > > > > > mode with
> > > > > > > > > > > > sync pulses. I have no access to an MIPI-DSI analyzer 
> > > > > > > > > > > > therefore
> > > > > > > > > > > > I can't verify it.
> > > > > > > > > > > >
> > > > > > > > > > > > > Non-burst mode meaning that the DSI pixel rate MUST 
> > > > > > > > > > > > > be the
> > > > > > > > > > > > > same as the HDMI pixel rate.
> > > > > > > > > > > >
> > > > > > > > > > > > On DSI side you don't have a pixel-clock instead there 
> > > > > > > > > > > > is bit-
> > > > > > > > clock.
> > > > > > > > > > >
> > > > > > > > > > > You have an effective pixel clock, with a fixed 
> > > > > > > > > > > conversion for the
> > > > > > > > > > > configuration.
> > > > > > > > > > >
> > > > > > > > > > > DSI bit-clock * number of lanes / bits_per_pixel = pixel 
> > > > > > > > > > > rate.
> > > > > > > > > > > 891Mbit/s * 4 lanes / 24bpp = 148.5 Mpixels/s
> > > > > > > > > >
> > > > > > > > > > Okay, I just checked the bandwidth which must equal.
> > > > > > > > > >
> > > > > > > > > > > As noted elsewhere, the DSI is DDR, so the clock lane 
> > > > > > > > > > > itself is
> > > > > > > > > > > only running at 891 / 2 = 445.5MHz.
> > > > > > > > > > >
> > > > > > > > > > > > > Section 6.1.1 "DSI Input Modes" of
> > > > > > > > > > > > > adv7533_hardware_user_s_guide is even more explicit 
> > > > > > > > > > > > > about the
> > > > > > > > > > > > > requirement of DSI timing matching
> > > > > > > > > > > >
> > > > > > > > > > > > Is it possible to share the key points of the 
> > > > > > > > > > > > requirements?
> > > > > > > > > > >
> > > > > > > > > > > "Specifically the ADV7533 supports the Non-Burst Mode 
> > > > > > > > > > > with syncs.
> > > > > > > > > > > This mode requires real time data generation as a pulse 
> > > > > > > > > > > packet
> > > > > > > > > > > received becomes a pulse generated. Therefore this mode 
> > > > > > > > > > > requires a
> > > > > > > > > > > continuous stream of data with correct video timing to 
> > > > > > > > > > > avoid any
> > > > > > > > > > > visual artifacts."
> > > > > > > > > > >
> > > > > > > > > > > LP mode is supported on data lanes. Clock lane must 
> > > > > > > > > > > remain in HS
> > > > > >

Re: [PATCH v2] drm/gem: Fix GEM handle release errors

2022-08-08 Thread Chen Jeffy


Hi Christian,

On 8/9 星期二 2:03, Christian König wrote:

Hi Jeffy,

Am 08.08.22 um 05:51 schrieb Chen Jeffy:

Hi Christian,

Thanks for your reply, and sorry i didn't make it clear.

On 8/8 星期一 0:52, Christian König wrote:

Am 03.08.22 um 10:32 schrieb Jeffy Chen:
Currently we are assuming a one to one mapping between dmabuf and 
handle

when releasing GEM handles.

But that is not always true, since we would create extra handles for 
the

GEM obj in cases like gem_open() and getfb{,2}().

A similar issue was reported at:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20211105083308.392156-1-jay.xu%40rock-chips.com%2Fdata=05%7C01%7Cchristian.koenig%40amd.com%7Cd7488e9f235041f7e84408da78f14882%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637955274964656400%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=9G2YbHcYUs1VQYyvjXwLzYawNw%2BP8i%2BjjPBSHx3r2yg%3Dreserved=0

Another problem is that the drm_gem_remove_prime_handles() now only
remove handle to the exported dmabuf (gem_obj->dma_buf), so the 
imported

ones would leak:
WARNING: CPU: 2 PID: 236 at drivers/gpu/drm/drm_prime.c:228 
drm_prime_destroy_file_private+0x18/0x24


Let's fix these by using handle to find the exact map to remove.


Well we are clearly something missing here. As far as I can see the 
current code is correct.


Creating multiple GEM handles for the same DMA-buf is possible, but 
illegal. >
In other words when a GEM handle is exported as DMA-buf and imported 
again you should intentionally always get the same handle.


These issue are not about having handles for importing an exported 
dma-buf case, but for having multiple handles to a GEM object(which 
means having multiple handles to a dma-buf).


I know the drm-prime is trying to make dma-buf and handle maps one to 
one, but the drm-gem is allowing to create extra handles for a GEM 
object, for example:

drm_gem_open_ioctl -> drm_gem_handle_create_tail
drm_mode_getfb2_ioctl -> drm_gem_handle_create
drm_mode_getfb -> fb->funcs->create_handle


Yes, so far that's correct.




So we are allowing GEM object to have multiple handles, and GEM object 
could have at most one dma-buf, doesn't that means that dma-buf could 
map to multiple handles?


No, at least not for the same GEM file private. That's the reason why 
the rb is indexed by the dma_buf object and not the handle.


In other words the rb is so that you have exactly one handle for each 
dma_buf in each file private.


I don't think so, because if user get multiple handles for the same GEM 
obj and use drm_gem_prime_handle_to_fd() for those handles, the current 
code would try to add multiple maps to rb:

drm_prime_add_buf_handle(buf_1, hdl_1)
drm_prime_add_buf_handle(buf_1, hdl_2)
...
drm_prime_add_buf_handle(buf_1, hdl_n)





Or should we rewrite the GEM framework to limit GEM object with uniq 
handle?


No, the extra handles are expected because when you call 
drm_mode_getfb*() and drm_gem_open_ioctl() the caller now owns the 
returned GEM handle.




The other issue is that we are leaking dma-buf <-> handle map for the 
imported dma-buf, since the drm_gem_remove_prime_handles doesn't take 
care of obj->import_attach->dmabuf.


No, that's correct as well. obj->dma_buf is set even for imported 
DMA-buf objects. See drm_gem_prime_fd_to_handle().


Well, that obj->dma_buf would be set in 
drm_gem_prime_fd_to_handle(create new handle), and cleared when 
releasing the latest handle(release handle).


So it doesn't cover other handle creating path.

For example, a imported dma buf:
drm_gem_prime_fd_to_handle <-- we got a handle and obj->dma_buf and 
obj->import_attach->dmabuf

drm_gem_handle_delete <-- we lost that handle and obj->dma_buf cleared
drm_gem_open_ioctl/or getfb* <-- we got a new handle and 
obj->import_attach->dmabuf
drm_gem_handle_delete <-- we lost that handle and obj->dma_buf is null, 
which means rb leaks.




Regards,
Christian.



But of cause this can be fixed in other way:
+++ b/drivers/gpu/drm/drm_gem.c
@@ -180,6 +180,9 @@ drm_gem_remove_prime_handles(struct drm_gem_object 
*obj, struct drm_file *filp)

drm_prime_remove_buf_handle_locked(>prime,
obj->dma_buf);
    }
+   if (obj->import_attach)
+ drm_prime_remove_buf_handle_locked(>prime,
+ obj->import_attach->dmabuf);
    mutex_unlock(>prime.lock);
 }


So this is pretty much a clear NAK to this patch since it shouldn't 
be necessary or something is seriously broken somewhere else.


Regards,
Christian.



Signed-off-by: Jeffy Chen 
---

Changes in v2:
Fix a typo of rbtree.

  drivers/gpu/drm/drm_gem.c  | 17 +
  drivers/gpu/drm/drm_internal.h |  4 ++--
  drivers/gpu/drm/drm_prime.c    | 20 
  3 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index eb0c2d041f13..ed39da383570 100644
--- a/drivers/gpu/drm/drm_gem.c
+++

Re: [PATCH v3 2/4] dt-bindings: display: add new bus-format property for panel-dpi

2022-08-08 Thread Marek Vasut

On 8/8/22 15:56, Max Krummenacher wrote:

Hi Marek

Hello Max,

[...]

+properties:
+ bus-format:
+$ref: /schemas/types.yaml#/definitions/uint32
+minimum: 0x1001
+maximum: 0x1fff
+description: |
+ Describes how the display panel is connected to the display
interface.
+ Valid values are defined in
.
+ The mapping between the color/significance of the panel lines to
the
+ parallel data lines are defined in:
+
https://www.kernel.org/doc/html/v5.17/userspace-api/media/v4l/subdev-formats.html#packed-rgb-formats

I am not sure whether I should re-open this discussion, but I still
wonder, wouldn't it be better to describe the DPI bus color channel
ordering (RGB, BGR, ...), width of each color channel in bits, pixel
format (RGB, YUV, ...) instead of using specific constants for the
entire format ?

From a system view it would be hard to define that structure which

will catch any and all future requirements. Assume that there will be
3D panels and they will need an additional depth field.

You can very much say you have panels which require Y/U/V color channels
instead of R/G/B , and then just add more color channels as needed. But
that -- color channel, their order, offset, bit width, can be described
rather easily, something like:

color-channel-names = "R", "G", "B";
color-channel-width = <8 8 8>;
color-channel-shift = <16 8 0>;

Or in
in addition to RGB data there will be a fourth color component. Or
whatever the panel manufacturers might come up with...
Or consider the Tegra 30 example I brought up in this thread. Tegras can
output RGB666 for 18bit panels, and then use the next 8 bits to extend
to 24bit (Maybe RGB666RGB222 ?).

I think there are two options here:

1) Look at 'data-lanes' property on DSI ? Both the DSI host and DSI
device define the 'data-lanes' property per endpoint and they might
not be the same.

But with DPI, the better option might be:

2) Implement something like LVDS codec, some sort of transparent DPI
bridge driver which can be defined in DT and represent the "glue"
wiring adapter between the mismatched DPI source and sink formats.

https://lore.kernel.org/all/71ef1b35301330d0bbb64844247b6c4c2237ad1c.ca...@gmail.com/
If such requirements pop up the enumeration can be extended with a new
value without changing the binding in any way, with a structured
approach this will require changed bindings, maybe even with issues
in backward compatibility.

If we have 2) which would describe how the DPI wires were connected,
like "channel R got shifted by two bits, bottom two bits got replicated,
etc.", then maybe we can avoid introducing new non-standard formats
altogether ?

From an implementation perspective for Linux the busformat in code is

currently an enumeration. So one would have to take the device tree
structured busformat and run it through a potentially complicated
function to get to the Linux busformat enumeration value. The final
consumer has no advantage over what is there today.

IMHO a change away from one enumeration value to a structured approach
creates some drawbacks without any obvious advantages.

Comments, other views on that?

See above.

Re: [PATCH] drm/gem: Fix GEM handle release errors

2022-08-08 Thread Chen Jeffy


Hi Christian,

Sorry, i've sent a v2 before, please check that.

On 8/9 星期二 2:05, Christian König wrote:



Am 02.08.22 um 13:33 schrieb Jeffy Chen:

Currently we are assuming a one to one mapping between dmabuf and handle
when releasing GEM handles.

But that is not always true, since we would create extra handles for the
GEM obj in cases like gem_open() and getfb{,2}().

A similar issue was reported at:
https://lore.kernel.org/all/20211105083308.392156-1-jay...@rock-chips.com/

Another problem is that the drm_gem_remove_prime_handles() now only
remove handle to the exported dmabuf (gem_obj->dma_buf), so the imported
ones would leak:
WARNING: CPU: 2 PID: 236 at drivers/gpu/drm/drm_prime.c:228 
drm_prime_destroy_file_private+0x18/0x24


Let's fix these by using handle to find the exact map to remove.

Signed-off-by: Jeffy Chen 
---

  drivers/gpu/drm/drm_gem.c  | 17 +
  drivers/gpu/drm/drm_internal.h |  4 ++--
  drivers/gpu/drm/drm_prime.c    | 16 ++--
  3 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index eb0c2d041f13..ed39da383570 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -168,21 +168,6 @@ void drm_gem_private_object_init(struct 
drm_device *dev,

  }
  EXPORT_SYMBOL(drm_gem_private_object_init);
-static void
-drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct 
drm_file *filp)

-{
-    /*
- * Note: obj->dma_buf can't disappear as long as we still hold a
- * handle reference in obj->handle_count.
- */
-    mutex_lock(>prime.lock);
-    if (obj->dma_buf) {
-    drm_prime_remove_buf_handle_locked(>prime,
-   obj->dma_buf);
-    }
-    mutex_unlock(>prime.lock);
-}
-
  /**
   * drm_gem_object_handle_free - release resources bound to userspace 
handles

   * @obj: GEM object to clean up.
@@ -253,7 +238,7 @@ drm_gem_object_release_handle(int id, void *ptr, 
void *data)

  if (obj->funcs->close)
  obj->funcs->close(obj, file_priv);
-    drm_gem_remove_prime_handles(obj, file_priv);
+    drm_prime_remove_buf_handle(_priv->prime, id);
  drm_vma_node_revoke(>vma_node, file_priv);
  drm_gem_object_handle_put_unlocked(obj);
diff --git a/drivers/gpu/drm/drm_internal.h 
b/drivers/gpu/drm/drm_internal.h

index 1fbbc19f1ac0..7bb98e6a446d 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -74,8 +74,8 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device 
*dev, void *data,
  void drm_prime_init_file_private(struct drm_prime_file_private 
*prime_fpriv);
  void drm_prime_destroy_file_private(struct drm_prime_file_private 
*prime_fpriv);
-void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private 
*prime_fpriv,

-    struct dma_buf *dma_buf);
+void drm_prime_remove_buf_handle(struct drm_prime_file_private 
*prime_fpriv,

+ uint32_t handle);
  /* drm_drv.c */
  struct drm_minor *drm_minor_acquire(unsigned int minor_id);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index e3f09f18110c..c28518ab62d0 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -190,29 +190,33 @@ static int drm_prime_lookup_buf_handle(struct 
drm_prime_file_private *prime_fpri

  return -ENOENT;
  }
-void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private 
*prime_fpriv,

-    struct dma_buf *dma_buf)
+void drm_prime_remove_buf_handle(struct drm_prime_file_private 
*prime_fpriv,

+ uint32_t handle)
  {
  struct rb_node *rb;
+    mutex_lock(_fpriv->lock);
+
  rb = prime_fpriv->dmabufs.rb_node;
  while (rb) {
  struct drm_prime_member *member;
  member = rb_entry(rb, struct drm_prime_member, dmabuf_rb);
-    if (member->dma_buf == dma_buf) {
+    if (member->handle == handle) {
  rb_erase(>handle_rb, _fpriv->handles);
  rb_erase(>dmabuf_rb, _fpriv->dmabufs);
-    dma_buf_put(dma_buf);
+    dma_buf_put(member->dma_buf);
  kfree(member);
-    return;
-    } else if (member->dma_buf < dma_buf) {
+    break;
+    } else if (member->handle < handle) {


Just to make it clear once more. That change here is completely broken.

The rb is indexed by the dma_buf object, not the handle.

Regards,
Christian.


  rb = rb->rb_right;
  } else {
  rb = rb->rb_left;
  }
  }
+
+    mutex_unlock(_fpriv->lock);
  }
  void drm_prime_init_file_private(struct drm_prime_file_private 
*prime_fpriv)

[RFC] drm/bridge: adv7533: remove dynamic lane switching from adv7533 bridge

2022-08-08 Thread Abhinav Kumar

adv7533 bridge tries to dynamically switch lanes based on the
mode by detaching and attaching the mipi dsi device.

This approach is incorrect because as per the DSI spec the
number of lanes is fixed at the time of system design or initial
configuration and may not change dynamically.

In addition this method of dynamic switch of detaching and
attaching the mipi dsi device also results in removing
and adding the component which is not necessary.

This approach is also prone to deadlocks. So for example, on the
db410c whenever this path is executed with lockdep enabled,
this results in a deadlock due to below ordering of locks.

-> #1 (crtc_ww_class_acquire){+.+.}-{0:0}:
lock_acquire+0x6c/0x90
drm_modeset_acquire_init+0xf4/0x150
drmm_mode_config_init+0x220/0x770
msm_drm_bind+0x13c/0x654
try_to_bring_up_aggregate_device+0x164/0x1d0
__component_add+0xa8/0x174
component_add+0x18/0x2c
dsi_dev_attach+0x24/0x30
dsi_host_attach+0x98/0x14c
devm_mipi_dsi_attach+0x38/0xb0
adv7533_attach_dsi+0x8c/0x110
adv7511_probe+0x5a0/0x930
i2c_device_probe+0x30c/0x350
really_probe.part.0+0x9c/0x2b0
__driver_probe_device+0x98/0x144
driver_probe_device+0xac/0x14c
__device_attach_driver+0xbc/0x124
bus_for_each_drv+0x78/0xd0
__device_attach+0xa8/0x1c0
device_initial_probe+0x18/0x24
bus_probe_device+0xa0/0xac
deferred_probe_work_func+0x90/0xd0
process_one_work+0x28c/0x6b0
worker_thread+0x240/0x444
kthread+0x110/0x114
ret_from_fork+0x10/0x20

-> #0 (component_mutex){+.+.}-{3:3}:
__lock_acquire+0x1280/0x20ac
lock_acquire.part.0+0xe0/0x230
lock_acquire+0x6c/0x90
__mutex_lock+0x84/0x400
mutex_lock_nested+0x3c/0x70
component_del+0x34/0x170
dsi_dev_detach+0x24/0x30
dsi_host_detach+0x20/0x64
mipi_dsi_detach+0x2c/0x40
adv7533_mode_set+0x64/0x90
adv7511_bridge_mode_set+0x210/0x214
drm_bridge_chain_mode_set+0x5c/0x84
crtc_set_mode+0x18c/0x1dc
drm_atomic_helper_commit_modeset_disables+0x40/0x50
msm_atomic_commit_tail+0x1d0/0x6e0
commit_tail+0xa4/0x180
drm_atomic_helper_commit+0x178/0x3b0
drm_atomic_commit+0xa4/0xe0
drm_client_modeset_commit_atomic+0x228/0x284
drm_client_modeset_commit_locked+0x64/0x1d0
drm_client_modeset_commit+0x34/0x60
drm_fb_helper_lastclose+0x74/0xcc
drm_lastclose+0x3c/0x80
drm_release+0xfc/0x114
__fput+0x70/0x224
fput+0x14/0x20
task_work_run+0x88/0x1a0
do_exit+0x350/0xa50
do_group_exit+0x38/0xa4
__wake_up_parent+0x0/0x34
invoke_syscall+0x48/0x114
el0_svc_common.constprop.0+0x60/0x11c
do_el0_svc+0x30/0xc0
el0_svc+0x58/0x100
el0t_64_sync_handler+0x1b0/0x1bc
el0t_64_sync+0x18c/0x190

Due to above reasons, remove the dynamic lane switching
code from adv7533 bridge chip and filter out the modes
which would need different number of lanes as compared
to the initialization time using the mode_valid callback.

Suggested-by: Dmitry Baryshkov 
Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/bridge/adv7511/adv7511.h |  3 ++-
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 18 ++
 drivers/gpu/drm/bridge/adv7511/adv7533.c | 25 +
 3 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 9e3bb8a8ee40..0a7cec80b75d 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -417,7 +417,8 @@ static inline int adv7511_cec_init(struct device *dev, 
struct adv7511 *adv7511)
 
 void adv7533_dsi_power_on(struct adv7511 *adv);
 void adv7533_dsi_power_off(struct adv7511 *adv);
-void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode 
*mode);
+enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
+   const struct drm_display_mode *mode);
 int adv7533_patch_registers(struct adv7511 *adv);
 int adv7533_patch_cec_registers(struct adv7511 *adv);
 int adv7533_attach_dsi(struct adv7511 *adv);
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 5bb9300040dd..1115ef9be83c 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -697,7 +697,7 @@ adv7511_detect(struct adv7511 *adv7511, struct 
drm_connector *connector)
 }
 
 static enum drm_mode_status adv7511_mode_valid(struct adv7511 *adv7511,
- struct drm_display_mode *mode)
+ const struct drm_display_mode *mode)
 {
if (mode->clock > 165000)
return MODE_CLOCK_HIGH;
@@ -791,9 +791,6 @@ static void

Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: pass a pointer for tlb seqno at vma_invalidate_tlb()

2022-08-08 Thread Vivi, Rodrigo

On Tue, 2022-08-09 at 01:09 +0200, Andi Shyti wrote:
> Hi Rodrigo,
> 
> On Mon, Aug 08, 2022 at 03:04:13PM -0400, Rodrigo Vivi wrote:
> > On Mon, Aug 08, 2022 at 06:37:58PM +0200, Andi Shyti wrote:
> > > Hi Mauro,
> > > 
> > > On Thu, Aug 04, 2022 at 09:37:22AM +0200, Mauro Carvalho Chehab
> > > wrote:
> > > > WRITE_ONCE() should happen at the original var, not on a local
> > > > copy of it.
> > > > 
> > > > Fixes: 5d36acb7198b ("drm/i915/gt: Batch TLB invalidations")
> > > > Signed-off-by: Mauro Carvalho Chehab 
> > > 
> > > Reviewed-by: Andi Shyti 
> > 
> > Thanks and pushed...
> 
> Thanks!
> 
> > > 
> > > Are you going to send it to the stable mailing list?
> > 
> > I added cc stable while pushing and the cherry-pick to drm-intel-
> > next-fixes
> > has the right sha, so I'd assume this would be automagically now.
> > But yeap, it would be good if Mauro can follow up whenever this
> > gets
> > to Linus tree and Greg's script start to pop up the heads-up
> > messages.
> 
> That's what I meant... does Mauro now need to send the e-mail
> again for the stable?
> 
> I thought there was some suspicion towards e-mails pushed without
> being first sent to both stable and upstream mailing lists
> because they can get lost or forgotten... maybe I'm wrong.

It doesn't help to send now to stable ml because it can only be merged
there after it reaches the Linus' master tree.
Right now with the right fixes and cc:stable it should be automatically
and he shouldn't worry.
But in case he notices that the first patch got in but the second
didn't then it is when we send the patch directly to the stable ml.


> 
> Andi
> 
> > Thanks,
> > Rodrigo.
> > 
> > > 
> > > Andi

1 2 >

1 - 100 of 131 matches

Mail list logo