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

2019-11-24 Thread Laurent Pinchart
Hi Boris,

Thank you for the patch.

On Wed, Oct 23, 2019 at 05:44:58PM +0200, Boris Brezillon wrote:
> So that each element in the chain can easily access its predecessor.
> This will be needed to support bus format negotiation between elements
> of the bridge chain.
> 
> Signed-off-by: Boris Brezillon 
> ---
> Changes in v3:
> * None
> 
> Changes in v2:
> * Adjust things to the "dummy encoder bridge" change (patch 2 in this
>   series)
> ---
>  drivers/gpu/drm/drm_bridge.c  | 171 ++
>  drivers/gpu/drm/drm_encoder.c |  16 +---
>  include/drm/drm_bridge.h  |  12 ++-
>  include/drm/drm_encoder.h |   9 +-
>  4 files changed, 135 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 54c874493c57..c5cf8a9c4237 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -55,7 +55,7 @@
>   * just provide additional hooks to get the desired output at the end of the
>   * encoder chain.
>   *
> - * Bridges can also be chained up using the _bridge.next pointer.
> + * Bridges can also be chained up using the _bridge.chain_node field.
>   *
>   * Both legacy CRTC helpers and the new atomic modeset helpers support 
> bridges.
>   */
> @@ -114,6 +114,7 @@ EXPORT_SYMBOL(drm_bridge_remove);
>  int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> struct drm_bridge *previous)
>  {
> + LIST_HEAD(tmp_list);
>   int ret;
>  
>   if (!encoder || !bridge)
> @@ -127,6 +128,7 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
> drm_bridge *bridge,
>  
>   bridge->dev = encoder->dev;
>   bridge->encoder = encoder;
> + list_add_tail(>chain_node, _list);

Couldn't we simplify this by adding the bridge to the list here ? We
would need to remove it in the error path of the attach operation
though, but wouldn't it make the code easier to read, and more efficient
in the most likely path ?

>  
>   if (bridge->funcs->attach) {
>   ret = bridge->funcs->attach(bridge);
> @@ -138,9 +140,9 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
> drm_bridge *bridge,
>   }
>  
>   if (previous)
> - previous->next = bridge;
> + list_splice(_list, >chain_node);
>   else
> - encoder->bridge = bridge;
> + list_splice(_list, >bridge_chain);
>  
>   return 0;
>  }
> @@ -157,6 +159,7 @@ void drm_bridge_detach(struct drm_bridge *bridge)
>   if (bridge->funcs->detach)
>   bridge->funcs->detach(bridge);
>  
> + list_del(>chain_node);
>   bridge->dev = NULL;
>  }
>  
> @@ -190,18 +193,22 @@ bool drm_bridge_chain_mode_fixup(struct drm_bridge 
> *bridge,
>const struct drm_display_mode *mode,
>struct drm_display_mode *adjusted_mode)
>  {
> - bool ret = true;
> + struct drm_encoder *encoder;
>  
>   if (!bridge)
>   return true;
>  
> - if (bridge->funcs->mode_fixup)
> - ret = bridge->funcs->mode_fixup(bridge, mode, adjusted_mode);
> + encoder = bridge->encoder;
> + list_for_each_entry_from(bridge, >bridge_chain,
> +  chain_node) {
> + if (!bridge->funcs->mode_fixup)
> + continue;
>  
> - ret = ret && drm_bridge_chain_mode_fixup(bridge->next, mode,
> -  adjusted_mode);
> + if (!bridge->funcs->mode_fixup(bridge, mode, adjusted_mode))
> + return false;
> + }
>  
> - return ret;
> + return true;
>  }
>  EXPORT_SYMBOL(drm_bridge_chain_mode_fixup);
>  
> @@ -224,18 +231,24 @@ enum drm_mode_status
>  drm_bridge_chain_mode_valid(struct drm_bridge *bridge,
>   const struct drm_display_mode *mode)
>  {
> - enum drm_mode_status ret = MODE_OK;
> + struct drm_encoder *encoder;
>  
>   if (!bridge)
> - return ret;
> + return MODE_OK;
> +
> + encoder = bridge->encoder;
> + list_for_each_entry_from(bridge, >bridge_chain, chain_node) {
> + enum drm_mode_status ret;
> +
> + if (!bridge->funcs->mode_valid)
> + continue;
>  
> - if (bridge->funcs->mode_valid)
>   ret = bridge->funcs->mode_valid(bridge, mode);
> + if (ret != MODE_OK)
> + return ret;
> + }
>  
> - if (ret != MODE_OK)
> - return ret;
> -
> - return drm_bridge_chain_mode_valid(bridge->next, mode);
> + return MODE_OK;
>  }
>  EXPORT_SYMBOL(drm_bridge_chain_mode_valid);
>  
> @@ -251,13 +264,20 @@ EXPORT_SYMBOL(drm_bridge_chain_mode_valid);
>   */
>  void drm_bridge_chain_disable(struct drm_bridge *bridge)
>  {
> + struct drm_encoder *encoder;
> + struct drm_bridge *iter;
> +
>   if (!bridge)
>   return;
>  
> - 

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

2019-11-23 Thread Boris Brezillon
Hi Neil,

Sorry for the late reply.

On Tue, 5 Nov 2019 17:02:30 +0100
Neil Armstrong  wrote:

> Hi,
> 
> 
> On 25/10/2019 15:29, Neil Armstrong wrote:
> > On 23/10/2019 17:44, Boris Brezillon wrote:  
> >> So that each element in the chain can easily access its predecessor.
> >> This will be needed to support bus format negotiation between elements
> >> of the bridge chain.
> >>
> >> Signed-off-by: Boris Brezillon 
> >> ---
> >> Changes in v3:
> >> * None
> >>
> >> Changes in v2:
> >> * Adjust things to the "dummy encoder bridge" change (patch 2 in this
> >>   series)
> >> ---
> >>  drivers/gpu/drm/drm_bridge.c  | 171 ++
> >>  drivers/gpu/drm/drm_encoder.c |  16 +---
> >>  include/drm/drm_bridge.h  |  12 ++-
> >>  include/drm/drm_encoder.h |   9 +-
> >>  4 files changed, 135 insertions(+), 73 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> >> index 54c874493c57..c5cf8a9c4237 100644
> >> --- a/drivers/gpu/drm/drm_bridge.c
> >> +++ b/drivers/gpu/drm/drm_bridge.c  
> 
> [...]
> 
> >>  
> >> @@ -426,15 +471,23 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable);
> >>  void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge,
> >>struct drm_atomic_state *state)
> >>  {
> >> +  struct drm_encoder *encoder;
> >> +  struct drm_bridge *iter;
> >> +
> >>if (!bridge)
> >>return;
> >>  
> >> -  drm_atomic_bridge_chain_pre_enable(bridge->next, state);
> >> +  encoder = bridge->encoder;
> >> +  list_for_each_entry_reverse(iter, >encoder->bridge_chain,
> >> +  chain_node) {  
> 
> This should use the encoder local variable in list_for_each_entry_reverse()
> 
> >> +  if (iter->funcs->atomic_pre_enable)
> >> +  iter->funcs->atomic_pre_enable(iter, state);
> >> +  else if (iter->funcs->pre_enable)
> >> +  iter->funcs->pre_enable(iter);
> >>  
> >> -  if (bridge->funcs->atomic_pre_enable)
> >> -  bridge->funcs->atomic_pre_enable(bridge, state);
> >> -  else if (bridge->funcs->pre_enable)
> >> -  bridge->funcs->pre_enable(bridge);
> >> +  if (iter == bridge)
> >> +  break;
> >> +  }
> >>  }
> >>  EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable);
> >>  
> >> @@ -453,15 +506,19 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable);
> >>  void drm_atomic_bridge_chain_enable(struct drm_bridge *bridge,
> >>struct drm_atomic_state *state)
> >>  {
> >> +  struct drm_encoder *encoder;
> >> +
> >>if (!bridge)
> >>return;
> >>  
> >> -  if (bridge->funcs->atomic_enable)
> >> -  bridge->funcs->atomic_enable(bridge, state);
> >> -  else if (bridge->funcs->enable)
> >> -  bridge->funcs->enable(bridge);
> >> -
> >> -  drm_atomic_bridge_chain_enable(bridge->next, state);
> >> +  encoder = bridge->encoder;
> >> +  list_for_each_entry_from(bridge, >encoder->bridge_chain,
> >> +   chain_node) {  
> 
> This should use encoder instead of bridge->encoder otherwise bridge will
> change and bridge->encoder->bridge_chain won't be valid during the for_each 
> and
> cause the following :

Oops, indeed. I fixed the very same problem in another helper but
somehow missed those 2. Thanks for testing/reporting the bug.

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

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

2019-11-05 Thread Neil Armstrong
Hi,


On 25/10/2019 15:29, Neil Armstrong wrote:
> On 23/10/2019 17:44, Boris Brezillon wrote:
>> So that each element in the chain can easily access its predecessor.
>> This will be needed to support bus format negotiation between elements
>> of the bridge chain.
>>
>> Signed-off-by: Boris Brezillon 
>> ---
>> Changes in v3:
>> * None
>>
>> Changes in v2:
>> * Adjust things to the "dummy encoder bridge" change (patch 2 in this
>>   series)
>> ---
>>  drivers/gpu/drm/drm_bridge.c  | 171 ++
>>  drivers/gpu/drm/drm_encoder.c |  16 +---
>>  include/drm/drm_bridge.h  |  12 ++-
>>  include/drm/drm_encoder.h |   9 +-
>>  4 files changed, 135 insertions(+), 73 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>> index 54c874493c57..c5cf8a9c4237 100644
>> --- a/drivers/gpu/drm/drm_bridge.c
>> +++ b/drivers/gpu/drm/drm_bridge.c

[...]

>>  
>> @@ -426,15 +471,23 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable);
>>  void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge,
>>  struct drm_atomic_state *state)
>>  {
>> +struct drm_encoder *encoder;
>> +struct drm_bridge *iter;
>> +
>>  if (!bridge)
>>  return;
>>  
>> -drm_atomic_bridge_chain_pre_enable(bridge->next, state);
>> +encoder = bridge->encoder;
>> +list_for_each_entry_reverse(iter, >encoder->bridge_chain,
>> +chain_node) {

This should use the encoder local variable in list_for_each_entry_reverse()

>> +if (iter->funcs->atomic_pre_enable)
>> +iter->funcs->atomic_pre_enable(iter, state);
>> +else if (iter->funcs->pre_enable)
>> +iter->funcs->pre_enable(iter);
>>  
>> -if (bridge->funcs->atomic_pre_enable)
>> -bridge->funcs->atomic_pre_enable(bridge, state);
>> -else if (bridge->funcs->pre_enable)
>> -bridge->funcs->pre_enable(bridge);
>> +if (iter == bridge)
>> +break;
>> +}
>>  }
>>  EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable);
>>  
>> @@ -453,15 +506,19 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable);
>>  void drm_atomic_bridge_chain_enable(struct drm_bridge *bridge,
>>  struct drm_atomic_state *state)
>>  {
>> +struct drm_encoder *encoder;
>> +
>>  if (!bridge)
>>  return;
>>  
>> -if (bridge->funcs->atomic_enable)
>> -bridge->funcs->atomic_enable(bridge, state);
>> -else if (bridge->funcs->enable)
>> -bridge->funcs->enable(bridge);
>> -
>> -drm_atomic_bridge_chain_enable(bridge->next, state);
>> +encoder = bridge->encoder;
>> +list_for_each_entry_from(bridge, >encoder->bridge_chain,
>> + chain_node) {

This should use encoder instead of bridge->encoder otherwise bridge will
change and bridge->encoder->bridge_chain won't be valid during the for_each and
cause the following :

[   79.082861] WARNING: CPU: 2 PID: 1999 at drivers/gpu/drm/drm_bridge.c:607 
drm_atomic_bridge_chain_enable+0xac/0xc0
...
[   79.210153]  drm_atomic_bridge_chain_enable+0xac/0xc0
[   79.215156]  drm_atomic_helper_commit_modeset_enables+0x138/0x248
[   79.221190]  drm_atomic_helper_commit_tail_rpm+0x2c/0x78
[   79.226452]  commit_tail+0x50/0xc0
[   79.229815]  drm_atomic_helper_commit+0xe8/0x168
[   79.234386]  drm_atomic_commit+0x48/0x58
[   79.238269]  drm_client_modeset_commit_atomic.isra.15+0x184/0x220
[   79.244305]  drm_client_modeset_commit_force+0x64/0x1a0
[   79.249482]  drm_fb_helper_restore_fbdev_mode_unlocked+0x70/0xe8
[   79.255432]  drm_fbdev_client_restore+0x14/0x20
[   79.259920]  drm_client_dev_restore+0x80/0xd8
[   79.264231]  drm_lastclose+0x4c/0x58
[   79.267765]  drm_release+0xa8/0x178

>> +if (bridge->funcs->atomic_enable)
>> +bridge->funcs->atomic_enable(bridge, state);
>> +else if (bridge->funcs->enable)
>> +bridge->funcs->enable(bridge);
>> +}
>>  }
>>  EXPORT_SYMBOL(drm_atomic_bridge_chain_enable);
>>  
>> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
>> index 4fe9e723e227..e555281f43d4 100644
>> --- a/drivers/gpu/drm/drm_encoder.c
>> +++ b/drivers/gpu/drm/drm_encoder.c
>> @@ -140,6 +140,7 @@ int drm_encoder_init(struct drm_device *dev,
>>  goto out_put;
>>  }
>>  
>> +INIT_LIST_HEAD(>bridge_chain);
>>  list_add_tail(>head, >mode_config.encoder_list);
>>  encoder->index = dev->mode_config.num_encoder++;
>>  
>> @@ -160,23 +161,16 @@ EXPORT_SYMBOL(drm_encoder_init);
>>  void drm_encoder_cleanup(struct drm_encoder *encoder)
>>  {
>>  struct drm_device *dev = encoder->dev;
>> +struct drm_bridge *bridge, *next;
>>  
>>  /* Note that the encoder_list is considered to be static; should we
>>   * remove the drm_encoder at runtime we would have to decrement all
>> 

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

2019-10-25 Thread Neil Armstrong
On 23/10/2019 17:44, Boris Brezillon wrote:
> So that each element in the chain can easily access its predecessor.
> This will be needed to support bus format negotiation between elements
> of the bridge chain.
> 
> Signed-off-by: Boris Brezillon 
> ---
> Changes in v3:
> * None
> 
> Changes in v2:
> * Adjust things to the "dummy encoder bridge" change (patch 2 in this
>   series)
> ---
>  drivers/gpu/drm/drm_bridge.c  | 171 ++
>  drivers/gpu/drm/drm_encoder.c |  16 +---
>  include/drm/drm_bridge.h  |  12 ++-
>  include/drm/drm_encoder.h |   9 +-
>  4 files changed, 135 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 54c874493c57..c5cf8a9c4237 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -55,7 +55,7 @@
>   * just provide additional hooks to get the desired output at the end of the
>   * encoder chain.
>   *
> - * Bridges can also be chained up using the _bridge.next pointer.
> + * Bridges can also be chained up using the _bridge.chain_node field.
>   *
>   * Both legacy CRTC helpers and the new atomic modeset helpers support 
> bridges.
>   */
> @@ -114,6 +114,7 @@ EXPORT_SYMBOL(drm_bridge_remove);
>  int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> struct drm_bridge *previous)
>  {
> + LIST_HEAD(tmp_list);
>   int ret;
>  
>   if (!encoder || !bridge)
> @@ -127,6 +128,7 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
> drm_bridge *bridge,
>  
>   bridge->dev = encoder->dev;
>   bridge->encoder = encoder;
> + list_add_tail(>chain_node, _list);
>  
>   if (bridge->funcs->attach) {
>   ret = bridge->funcs->attach(bridge);
> @@ -138,9 +140,9 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
> drm_bridge *bridge,
>   }
>  
>   if (previous)
> - previous->next = bridge;
> + list_splice(_list, >chain_node);
>   else
> - encoder->bridge = bridge;
> + list_splice(_list, >bridge_chain);
>  
>   return 0;
>  }
> @@ -157,6 +159,7 @@ void drm_bridge_detach(struct drm_bridge *bridge)
>   if (bridge->funcs->detach)
>   bridge->funcs->detach(bridge);
>  
> + list_del(>chain_node);
>   bridge->dev = NULL;
>  }
>  
> @@ -190,18 +193,22 @@ bool drm_bridge_chain_mode_fixup(struct drm_bridge 
> *bridge,
>const struct drm_display_mode *mode,
>struct drm_display_mode *adjusted_mode)
>  {
> - bool ret = true;
> + struct drm_encoder *encoder;
>  
>   if (!bridge)
>   return true;
>  
> - if (bridge->funcs->mode_fixup)
> - ret = bridge->funcs->mode_fixup(bridge, mode, adjusted_mode);
> + encoder = bridge->encoder;
> + list_for_each_entry_from(bridge, >bridge_chain,
> +  chain_node) {
> + if (!bridge->funcs->mode_fixup)
> + continue;
>  
> - ret = ret && drm_bridge_chain_mode_fixup(bridge->next, mode,
> -  adjusted_mode);
> + if (!bridge->funcs->mode_fixup(bridge, mode, adjusted_mode))
> + return false;
> + }
>  
> - return ret;
> + return true;
>  }
>  EXPORT_SYMBOL(drm_bridge_chain_mode_fixup);
>  
> @@ -224,18 +231,24 @@ enum drm_mode_status
>  drm_bridge_chain_mode_valid(struct drm_bridge *bridge,
>   const struct drm_display_mode *mode)
>  {
> - enum drm_mode_status ret = MODE_OK;
> + struct drm_encoder *encoder;
>  
>   if (!bridge)
> - return ret;
> + return MODE_OK;
> +
> + encoder = bridge->encoder;
> + list_for_each_entry_from(bridge, >bridge_chain, chain_node) {
> + enum drm_mode_status ret;
> +
> + if (!bridge->funcs->mode_valid)
> + continue;
>  
> - if (bridge->funcs->mode_valid)
>   ret = bridge->funcs->mode_valid(bridge, mode);
> + if (ret != MODE_OK)
> + return ret;
> + }
>  
> - if (ret != MODE_OK)
> - return ret;
> -
> - return drm_bridge_chain_mode_valid(bridge->next, mode);
> + return MODE_OK;
>  }
>  EXPORT_SYMBOL(drm_bridge_chain_mode_valid);
>  
> @@ -251,13 +264,20 @@ EXPORT_SYMBOL(drm_bridge_chain_mode_valid);
>   */
>  void drm_bridge_chain_disable(struct drm_bridge *bridge)
>  {
> + struct drm_encoder *encoder;
> + struct drm_bridge *iter;
> +
>   if (!bridge)
>   return;
>  
> - drm_bridge_chain_disable(bridge->next);
> + encoder = bridge->encoder;
> + list_for_each_entry_reverse(iter, >bridge_chain, chain_node) {
> + if (iter->funcs->disable)
> + iter->funcs->disable(iter);
>  
> - if (bridge->funcs->disable)
> - 

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

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

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

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

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