subject:"\[PATCH v2 6\/8\] drm\: Introduce drm_bridge_mode

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-15 Thread Daniel Vetter

On Fri, May 12, 2017 at 02:01:49PM +0300, Laurent Pinchart wrote:
> Hi Archit,
> 
> On Friday 12 May 2017 16:20:07 Archit Taneja wrote:
> > On 05/12/2017 03:08 PM, Laurent Pinchart wrote:
> > > On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:
> > >> On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:
> > >>> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
> >  Introduce a new helper function which calls mode_valid() callback
> >  for all bridges in an encoder chain.
> >  
> >  Signed-off-by: Jose Abreu 
> >  Cc: Carlos Palminha 
> >  Cc: Alexey Brodkin 
> >  Cc: Ville Syrjälä 
> >  Cc: Daniel Vetter 
> >  Cc: Dave Airlie 
> >  Cc: Andrzej Hajda 
> >  Cc: Archit Taneja 
> >  ---
> >  
> >   drivers/gpu/drm/drm_bridge.c | 33 +
> >   include/drm/drm_bridge.h |  2 ++
> >   2 files changed, 35 insertions(+)
> >  
> >  diff --git a/drivers/gpu/drm/drm_bridge.c
> >  b/drivers/gpu/drm/drm_bridge.c
> >  index 86a7637..dc8cdfe 100644
> >  --- a/drivers/gpu/drm/drm_bridge.c
> >  +++ b/drivers/gpu/drm/drm_bridge.c
> >  @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
> >  *bridge,
> >  
> >   EXPORT_SYMBOL(drm_bridge_mode_fixup);
> >   
> >   /**
> >  
> >  + * drm_bridge_mode_valid - validate the mode against all bridges in
> >  the
> >  + *   encoder chain.
> >  + * @bridge: bridge control structure
> >  + * @mode: desired mode to be validated
> >  + *
> >  + * Calls _bridge_funcs.mode_valid for all the bridges in the
> >  encoder
> >  + * chain, starting from the first bridge to the last. If at least one
> >  bridge + * does not accept the mode the function returns the error
> >  code.
> >  + *
> >  + * Note: the bridge passed should be the one closest to the encoder.
> >  + *
> >  + * RETURNS:
> >  + * MODE_OK on success, drm_mode_status Enum error code on failure
> >  + */
> >  +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> >  + const struct 
> drm_display_mode
> > > 
> > > *mode)
> > > 
> >  +{
> >  +  enum drm_mode_status ret = MODE_OK;
> >  +
> >  +  if (!bridge)
> >  +  return ret;
> >  +
> >  +  if (bridge->funcs->mode_valid)
> >  +  ret = bridge->funcs->mode_valid(bridge, mode);
> >  +
> >  +  if (ret != MODE_OK)
> >  +  return ret;
> >  +
> >  +  return drm_bridge_mode_valid(bridge->next, mode);
> > >>> 
> > >>> Looks like it should be pretty trivial to avoid the recursion.
> > >>> 
> > >>> Am I correct in interpreting this that bridges have some kind of
> > >>> a hand rolled linked list implementation? Reusing the standard
> > >>> linked lists would allow you to use list_for_each() etc.
> > >> 
> > >> Yeah it's a hand-rolled list, but current hw also has a bridge nesting
> > >> depth of 2, so it really doesn't matter. I guess once we have real long
> > >> chains of bridges we can fix this (and just using list_head sounds like a
> > >> great idea).
> > > 
> > > Even if not really needed right now, it's a pretty easy cleanup, if Jose
> > > has time to handle it in v3 of this series let's not postpone it ;-)
> > 
> > jfyi, some of the bridge functions call the ops from the last bridge in the
> > chain to first, so we'd need to use list_for_each_entry_prev() (or something
> > like that) for them.
> 
> And now that I think about it, for some of the operations (especially 
> enable/disable) I believe that the bridge should be able to decide whether to 
> call the next/previous bridge first or to configure its hardware first. I can 
> image bridges that need the previous bridge in the chain to provide a valid 
> clock before they get started, as well as bridges that need to be started 
> with 
> the incoming video signal stopped.

That's why we have pre_/post_ hooks ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-15 Thread Daniel Vetter

On Fri, May 12, 2017 at 02:01:49PM +0300, Laurent Pinchart wrote:
> Hi Archit,
> 
> On Friday 12 May 2017 16:20:07 Archit Taneja wrote:
> > On 05/12/2017 03:08 PM, Laurent Pinchart wrote:
> > > On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:
> > >> On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:
> > >>> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
> >  Introduce a new helper function which calls mode_valid() callback
> >  for all bridges in an encoder chain.
> >  
> >  Signed-off-by: Jose Abreu 
> >  Cc: Carlos Palminha 
> >  Cc: Alexey Brodkin 
> >  Cc: Ville Syrjälä 
> >  Cc: Daniel Vetter 
> >  Cc: Dave Airlie 
> >  Cc: Andrzej Hajda 
> >  Cc: Archit Taneja 
> >  ---
> >  
> >   drivers/gpu/drm/drm_bridge.c | 33 +
> >   include/drm/drm_bridge.h |  2 ++
> >   2 files changed, 35 insertions(+)
> >  
> >  diff --git a/drivers/gpu/drm/drm_bridge.c
> >  b/drivers/gpu/drm/drm_bridge.c
> >  index 86a7637..dc8cdfe 100644
> >  --- a/drivers/gpu/drm/drm_bridge.c
> >  +++ b/drivers/gpu/drm/drm_bridge.c
> >  @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
> >  *bridge,
> >  
> >   EXPORT_SYMBOL(drm_bridge_mode_fixup);
> >   
> >   /**
> >  
> >  + * drm_bridge_mode_valid - validate the mode against all bridges in
> >  the
> >  + *   encoder chain.
> >  + * @bridge: bridge control structure
> >  + * @mode: desired mode to be validated
> >  + *
> >  + * Calls _bridge_funcs.mode_valid for all the bridges in the
> >  encoder
> >  + * chain, starting from the first bridge to the last. If at least one
> >  bridge + * does not accept the mode the function returns the error
> >  code.
> >  + *
> >  + * Note: the bridge passed should be the one closest to the encoder.
> >  + *
> >  + * RETURNS:
> >  + * MODE_OK on success, drm_mode_status Enum error code on failure
> >  + */
> >  +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> >  + const struct 
> drm_display_mode
> > > 
> > > *mode)
> > > 
> >  +{
> >  +  enum drm_mode_status ret = MODE_OK;
> >  +
> >  +  if (!bridge)
> >  +  return ret;
> >  +
> >  +  if (bridge->funcs->mode_valid)
> >  +  ret = bridge->funcs->mode_valid(bridge, mode);
> >  +
> >  +  if (ret != MODE_OK)
> >  +  return ret;
> >  +
> >  +  return drm_bridge_mode_valid(bridge->next, mode);
> > >>> 
> > >>> Looks like it should be pretty trivial to avoid the recursion.
> > >>> 
> > >>> Am I correct in interpreting this that bridges have some kind of
> > >>> a hand rolled linked list implementation? Reusing the standard
> > >>> linked lists would allow you to use list_for_each() etc.
> > >> 
> > >> Yeah it's a hand-rolled list, but current hw also has a bridge nesting
> > >> depth of 2, so it really doesn't matter. I guess once we have real long
> > >> chains of bridges we can fix this (and just using list_head sounds like a
> > >> great idea).
> > > 
> > > Even if not really needed right now, it's a pretty easy cleanup, if Jose
> > > has time to handle it in v3 of this series let's not postpone it ;-)
> > 
> > jfyi, some of the bridge functions call the ops from the last bridge in the
> > chain to first, so we'd need to use list_for_each_entry_prev() (or something
> > like that) for them.
> 
> And now that I think about it, for some of the operations (especially 
> enable/disable) I believe that the bridge should be able to decide whether to 
> call the next/previous bridge first or to configure its hardware first. I can 
> image bridges that need the previous bridge in the chain to provide a valid 
> clock before they get started, as well as bridges that need to be started 
> with 
> the incoming video signal stopped.

That's why we have pre_/post_ hooks ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-14 Thread Archit Taneja




On 05/12/2017 04:31 PM, Laurent Pinchart wrote:

Hi Archit,

On Friday 12 May 2017 16:20:07 Archit Taneja wrote:

On 05/12/2017 03:08 PM, Laurent Pinchart wrote:

On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:

On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:

On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:

Introduce a new helper function which calls mode_valid() callback
for all bridges in an encoder chain.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Alexey Brodkin 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 
Cc: Andrzej Hajda 
Cc: Archit Taneja 
---

 drivers/gpu/drm/drm_bridge.c | 33 +
 include/drm/drm_bridge.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c
b/drivers/gpu/drm/drm_bridge.c
index 86a7637..dc8cdfe 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
*bridge,

 EXPORT_SYMBOL(drm_bridge_mode_fixup);

 /**

+ * drm_bridge_mode_valid - validate the mode against all bridges in
the
+ *encoder chain.
+ * @bridge: bridge control structure
+ * @mode: desired mode to be validated
+ *
+ * Calls _bridge_funcs.mode_valid for all the bridges in the
encoder
+ * chain, starting from the first bridge to the last. If at least one
bridge + * does not accept the mode the function returns the error
code.
+ *
+ * Note: the bridge passed should be the one closest to the encoder.
+ *
+ * RETURNS:
+ * MODE_OK on success, drm_mode_status Enum error code on failure
+ */
+enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
+  const struct

drm_display_mode


*mode)


+{
+   enum drm_mode_status ret = MODE_OK;
+
+   if (!bridge)
+   return ret;
+
+   if (bridge->funcs->mode_valid)
+   ret = bridge->funcs->mode_valid(bridge, mode);
+
+   if (ret != MODE_OK)
+   return ret;
+
+   return drm_bridge_mode_valid(bridge->next, mode);


Looks like it should be pretty trivial to avoid the recursion.

Am I correct in interpreting this that bridges have some kind of
a hand rolled linked list implementation? Reusing the standard
linked lists would allow you to use list_for_each() etc.


Yeah it's a hand-rolled list, but current hw also has a bridge nesting
depth of 2, so it really doesn't matter. I guess once we have real long
chains of bridges we can fix this (and just using list_head sounds like a
great idea).


Even if not really needed right now, it's a pretty easy cleanup, if Jose
has time to handle it in v3 of this series let's not postpone it ;-)


jfyi, some of the bridge functions call the ops from the last bridge in the
chain to first, so we'd need to use list_for_each_entry_prev() (or something
like that) for them.


And now that I think about it, for some of the operations (especially
enable/disable) I believe that the bridge should be able to decide whether to
call the next/previous bridge first or to configure its hardware first. I can
image bridges that need the previous bridge in the chain to provide a valid
clock before they get started, as well as bridges that need to be started with
the incoming video signal stopped.


I guess converting into list would be a good start to achieve this. We'd 
probably
need to extend/redo the drm_bridge_attach() API to tweak the order in the which
the ops are called.

Thanks,
Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-14 Thread Archit Taneja




On 05/12/2017 04:31 PM, Laurent Pinchart wrote:

Hi Archit,

On Friday 12 May 2017 16:20:07 Archit Taneja wrote:

On 05/12/2017 03:08 PM, Laurent Pinchart wrote:

On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:

On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:

On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:

Introduce a new helper function which calls mode_valid() callback
for all bridges in an encoder chain.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Alexey Brodkin 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 
Cc: Andrzej Hajda 
Cc: Archit Taneja 
---

 drivers/gpu/drm/drm_bridge.c | 33 +
 include/drm/drm_bridge.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c
b/drivers/gpu/drm/drm_bridge.c
index 86a7637..dc8cdfe 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
*bridge,

 EXPORT_SYMBOL(drm_bridge_mode_fixup);

 /**

+ * drm_bridge_mode_valid - validate the mode against all bridges in
the
+ *encoder chain.
+ * @bridge: bridge control structure
+ * @mode: desired mode to be validated
+ *
+ * Calls _bridge_funcs.mode_valid for all the bridges in the
encoder
+ * chain, starting from the first bridge to the last. If at least one
bridge + * does not accept the mode the function returns the error
code.
+ *
+ * Note: the bridge passed should be the one closest to the encoder.
+ *
+ * RETURNS:
+ * MODE_OK on success, drm_mode_status Enum error code on failure
+ */
+enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
+  const struct

drm_display_mode


*mode)


+{
+   enum drm_mode_status ret = MODE_OK;
+
+   if (!bridge)
+   return ret;
+
+   if (bridge->funcs->mode_valid)
+   ret = bridge->funcs->mode_valid(bridge, mode);
+
+   if (ret != MODE_OK)
+   return ret;
+
+   return drm_bridge_mode_valid(bridge->next, mode);


Looks like it should be pretty trivial to avoid the recursion.

Am I correct in interpreting this that bridges have some kind of
a hand rolled linked list implementation? Reusing the standard
linked lists would allow you to use list_for_each() etc.


Yeah it's a hand-rolled list, but current hw also has a bridge nesting
depth of 2, so it really doesn't matter. I guess once we have real long
chains of bridges we can fix this (and just using list_head sounds like a
great idea).


Even if not really needed right now, it's a pretty easy cleanup, if Jose
has time to handle it in v3 of this series let's not postpone it ;-)


jfyi, some of the bridge functions call the ops from the last bridge in the
chain to first, so we'd need to use list_for_each_entry_prev() (or something
like that) for them.


And now that I think about it, for some of the operations (especially
enable/disable) I believe that the bridge should be able to decide whether to
call the next/previous bridge first or to configure its hardware first. I can
image bridges that need the previous bridge in the chain to provide a valid
clock before they get started, as well as bridges that need to be started with
the incoming video signal stopped.


I guess converting into list would be a good start to achieve this. We'd 
probably
need to extend/redo the drm_bridge_attach() API to tweak the order in the which
the ops are called.

Thanks,
Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-12 Thread Laurent Pinchart

Hi Archit,

On Friday 12 May 2017 16:20:07 Archit Taneja wrote:
> On 05/12/2017 03:08 PM, Laurent Pinchart wrote:
> > On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:
> >> On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:
> >>> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
>  Introduce a new helper function which calls mode_valid() callback
>  for all bridges in an encoder chain.
>  
>  Signed-off-by: Jose Abreu 
>  Cc: Carlos Palminha 
>  Cc: Alexey Brodkin 
>  Cc: Ville Syrjälä 
>  Cc: Daniel Vetter 
>  Cc: Dave Airlie 
>  Cc: Andrzej Hajda 
>  Cc: Archit Taneja 
>  ---
>  
>   drivers/gpu/drm/drm_bridge.c | 33 +
>   include/drm/drm_bridge.h |  2 ++
>   2 files changed, 35 insertions(+)
>  
>  diff --git a/drivers/gpu/drm/drm_bridge.c
>  b/drivers/gpu/drm/drm_bridge.c
>  index 86a7637..dc8cdfe 100644
>  --- a/drivers/gpu/drm/drm_bridge.c
>  +++ b/drivers/gpu/drm/drm_bridge.c
>  @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
>  *bridge,
>  
>   EXPORT_SYMBOL(drm_bridge_mode_fixup);
>   
>   /**
>  
>  + * drm_bridge_mode_valid - validate the mode against all bridges in
>  the
>  + * encoder chain.
>  + * @bridge: bridge control structure
>  + * @mode: desired mode to be validated
>  + *
>  + * Calls _bridge_funcs.mode_valid for all the bridges in the
>  encoder
>  + * chain, starting from the first bridge to the last. If at least one
>  bridge + * does not accept the mode the function returns the error
>  code.
>  + *
>  + * Note: the bridge passed should be the one closest to the encoder.
>  + *
>  + * RETURNS:
>  + * MODE_OK on success, drm_mode_status Enum error code on failure
>  + */
>  +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>  +   const struct 
drm_display_mode
> > 
> > *mode)
> > 
>  +{
>  +enum drm_mode_status ret = MODE_OK;
>  +
>  +if (!bridge)
>  +return ret;
>  +
>  +if (bridge->funcs->mode_valid)
>  +ret = bridge->funcs->mode_valid(bridge, mode);
>  +
>  +if (ret != MODE_OK)
>  +return ret;
>  +
>  +return drm_bridge_mode_valid(bridge->next, mode);
> >>> 
> >>> Looks like it should be pretty trivial to avoid the recursion.
> >>> 
> >>> Am I correct in interpreting this that bridges have some kind of
> >>> a hand rolled linked list implementation? Reusing the standard
> >>> linked lists would allow you to use list_for_each() etc.
> >> 
> >> Yeah it's a hand-rolled list, but current hw also has a bridge nesting
> >> depth of 2, so it really doesn't matter. I guess once we have real long
> >> chains of bridges we can fix this (and just using list_head sounds like a
> >> great idea).
> > 
> > Even if not really needed right now, it's a pretty easy cleanup, if Jose
> > has time to handle it in v3 of this series let's not postpone it ;-)
> 
> jfyi, some of the bridge functions call the ops from the last bridge in the
> chain to first, so we'd need to use list_for_each_entry_prev() (or something
> like that) for them.

And now that I think about it, for some of the operations (especially 
enable/disable) I believe that the bridge should be able to decide whether to 
call the next/previous bridge first or to configure its hardware first. I can 
image bridges that need the previous bridge in the chain to provide a valid 
clock before they get started, as well as bridges that need to be started with 
the incoming video signal stopped.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-12 Thread Laurent Pinchart

Hi Archit,

On Friday 12 May 2017 16:20:07 Archit Taneja wrote:
> On 05/12/2017 03:08 PM, Laurent Pinchart wrote:
> > On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:
> >> On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:
> >>> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
>  Introduce a new helper function which calls mode_valid() callback
>  for all bridges in an encoder chain.
>  
>  Signed-off-by: Jose Abreu 
>  Cc: Carlos Palminha 
>  Cc: Alexey Brodkin 
>  Cc: Ville Syrjälä 
>  Cc: Daniel Vetter 
>  Cc: Dave Airlie 
>  Cc: Andrzej Hajda 
>  Cc: Archit Taneja 
>  ---
>  
>   drivers/gpu/drm/drm_bridge.c | 33 +
>   include/drm/drm_bridge.h |  2 ++
>   2 files changed, 35 insertions(+)
>  
>  diff --git a/drivers/gpu/drm/drm_bridge.c
>  b/drivers/gpu/drm/drm_bridge.c
>  index 86a7637..dc8cdfe 100644
>  --- a/drivers/gpu/drm/drm_bridge.c
>  +++ b/drivers/gpu/drm/drm_bridge.c
>  @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
>  *bridge,
>  
>   EXPORT_SYMBOL(drm_bridge_mode_fixup);
>   
>   /**
>  
>  + * drm_bridge_mode_valid - validate the mode against all bridges in
>  the
>  + * encoder chain.
>  + * @bridge: bridge control structure
>  + * @mode: desired mode to be validated
>  + *
>  + * Calls _bridge_funcs.mode_valid for all the bridges in the
>  encoder
>  + * chain, starting from the first bridge to the last. If at least one
>  bridge + * does not accept the mode the function returns the error
>  code.
>  + *
>  + * Note: the bridge passed should be the one closest to the encoder.
>  + *
>  + * RETURNS:
>  + * MODE_OK on success, drm_mode_status Enum error code on failure
>  + */
>  +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>  +   const struct 
drm_display_mode
> > 
> > *mode)
> > 
>  +{
>  +enum drm_mode_status ret = MODE_OK;
>  +
>  +if (!bridge)
>  +return ret;
>  +
>  +if (bridge->funcs->mode_valid)
>  +ret = bridge->funcs->mode_valid(bridge, mode);
>  +
>  +if (ret != MODE_OK)
>  +return ret;
>  +
>  +return drm_bridge_mode_valid(bridge->next, mode);
> >>> 
> >>> Looks like it should be pretty trivial to avoid the recursion.
> >>> 
> >>> Am I correct in interpreting this that bridges have some kind of
> >>> a hand rolled linked list implementation? Reusing the standard
> >>> linked lists would allow you to use list_for_each() etc.
> >> 
> >> Yeah it's a hand-rolled list, but current hw also has a bridge nesting
> >> depth of 2, so it really doesn't matter. I guess once we have real long
> >> chains of bridges we can fix this (and just using list_head sounds like a
> >> great idea).
> > 
> > Even if not really needed right now, it's a pretty easy cleanup, if Jose
> > has time to handle it in v3 of this series let's not postpone it ;-)
> 
> jfyi, some of the bridge functions call the ops from the last bridge in the
> chain to first, so we'd need to use list_for_each_entry_prev() (or something
> like that) for them.

And now that I think about it, for some of the operations (especially 
enable/disable) I believe that the bridge should be able to decide whether to 
call the next/previous bridge first or to configure its hardware first. I can 
image bridges that need the previous bridge in the chain to provide a valid 
clock before they get started, as well as bridges that need to be started with 
the incoming video signal stopped.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-12 Thread Archit Taneja




On 05/12/2017 03:08 PM, Laurent Pinchart wrote:

Hi Daniel,

On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:

On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:

On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:

Introduce a new helper function which calls mode_valid() callback
for all bridges in an encoder chain.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Alexey Brodkin 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 
Cc: Andrzej Hajda 
Cc: Archit Taneja 
---

 drivers/gpu/drm/drm_bridge.c | 33 +
 include/drm/drm_bridge.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 86a7637..dc8cdfe 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
*bridge,

 EXPORT_SYMBOL(drm_bridge_mode_fixup);

 /**

+ * drm_bridge_mode_valid - validate the mode against all bridges in the
+ *encoder chain.
+ * @bridge: bridge control structure
+ * @mode: desired mode to be validated
+ *
+ * Calls _bridge_funcs.mode_valid for all the bridges in the
encoder
+ * chain, starting from the first bridge to the last. If at least one
bridge + * does not accept the mode the function returns the error
code.
+ *
+ * Note: the bridge passed should be the one closest to the encoder.
+ *
+ * RETURNS:
+ * MODE_OK on success, drm_mode_status Enum error code on failure
+ */
+enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
+  const struct drm_display_mode

*mode)

+{
+   enum drm_mode_status ret = MODE_OK;
+
+   if (!bridge)
+   return ret;
+
+   if (bridge->funcs->mode_valid)
+   ret = bridge->funcs->mode_valid(bridge, mode);
+
+   if (ret != MODE_OK)
+   return ret;
+
+   return drm_bridge_mode_valid(bridge->next, mode);


Looks like it should be pretty trivial to avoid the recursion.

Am I correct in interpreting this that bridges have some kind of
a hand rolled linked list implementation? Reusing the standard
linked lists would allow you to use list_for_each() etc.


Yeah it's a hand-rolled list, but current hw also has a bridge nesting
depth of 2, so it really doesn't matter. I guess once we have real long
chains of bridges we can fix this (and just using list_head sounds like a
great idea).


Even if not really needed right now, it's a pretty easy cleanup, if Jose has
time to handle it in v3 of this series let's not postpone it ;-)


jfyi, some of the bridge functions call the ops from the last bridge in the
chain to first, so we'd need to use list_for_each_entry_prev() (or something
like that) for them.

Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-12 Thread Archit Taneja




On 05/12/2017 03:08 PM, Laurent Pinchart wrote:

Hi Daniel,

On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:

On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:

On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:

Introduce a new helper function which calls mode_valid() callback
for all bridges in an encoder chain.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Alexey Brodkin 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 
Cc: Andrzej Hajda 
Cc: Archit Taneja 
---

 drivers/gpu/drm/drm_bridge.c | 33 +
 include/drm/drm_bridge.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 86a7637..dc8cdfe 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
*bridge,

 EXPORT_SYMBOL(drm_bridge_mode_fixup);

 /**

+ * drm_bridge_mode_valid - validate the mode against all bridges in the
+ *encoder chain.
+ * @bridge: bridge control structure
+ * @mode: desired mode to be validated
+ *
+ * Calls _bridge_funcs.mode_valid for all the bridges in the
encoder
+ * chain, starting from the first bridge to the last. If at least one
bridge + * does not accept the mode the function returns the error
code.
+ *
+ * Note: the bridge passed should be the one closest to the encoder.
+ *
+ * RETURNS:
+ * MODE_OK on success, drm_mode_status Enum error code on failure
+ */
+enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
+  const struct drm_display_mode

*mode)

+{
+   enum drm_mode_status ret = MODE_OK;
+
+   if (!bridge)
+   return ret;
+
+   if (bridge->funcs->mode_valid)
+   ret = bridge->funcs->mode_valid(bridge, mode);
+
+   if (ret != MODE_OK)
+   return ret;
+
+   return drm_bridge_mode_valid(bridge->next, mode);


Looks like it should be pretty trivial to avoid the recursion.

Am I correct in interpreting this that bridges have some kind of
a hand rolled linked list implementation? Reusing the standard
linked lists would allow you to use list_for_each() etc.


Yeah it's a hand-rolled list, but current hw also has a bridge nesting
depth of 2, so it really doesn't matter. I guess once we have real long
chains of bridges we can fix this (and just using list_head sounds like a
great idea).


Even if not really needed right now, it's a pretty easy cleanup, if Jose has
time to handle it in v3 of this series let's not postpone it ;-)


jfyi, some of the bridge functions call the ops from the last bridge in the
chain to first, so we'd need to use list_for_each_entry_prev() (or something
like that) for them.

Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-12 Thread Laurent Pinchart

Hi Daniel,

On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:
> On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:
> > On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
> > > Introduce a new helper function which calls mode_valid() callback
> > > for all bridges in an encoder chain.
> > > 
> > > Signed-off-by: Jose Abreu 
> > > Cc: Carlos Palminha 
> > > Cc: Alexey Brodkin 
> > > Cc: Ville Syrjälä 
> > > Cc: Daniel Vetter 
> > > Cc: Dave Airlie 
> > > Cc: Andrzej Hajda 
> > > Cc: Archit Taneja 
> > > ---
> > > 
> > >  drivers/gpu/drm/drm_bridge.c | 33 +
> > >  include/drm/drm_bridge.h |  2 ++
> > >  2 files changed, 35 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index 86a7637..dc8cdfe 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
> > > *bridge,
> > > 
> > >  EXPORT_SYMBOL(drm_bridge_mode_fixup);
> > >  
> > >  /**
> > > 
> > > + * drm_bridge_mode_valid - validate the mode against all bridges in the
> > > + *  encoder chain.
> > > + * @bridge: bridge control structure
> > > + * @mode: desired mode to be validated
> > > + *
> > > + * Calls _bridge_funcs.mode_valid for all the bridges in the
> > > encoder
> > > + * chain, starting from the first bridge to the last. If at least one
> > > bridge + * does not accept the mode the function returns the error
> > > code.
> > > + *
> > > + * Note: the bridge passed should be the one closest to the encoder.
> > > + *
> > > + * RETURNS:
> > > + * MODE_OK on success, drm_mode_status Enum error code on failure
> > > + */
> > > +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> > > +const struct drm_display_mode 
*mode)
> > > +{
> > > + enum drm_mode_status ret = MODE_OK;
> > > +
> > > + if (!bridge)
> > > + return ret;
> > > +
> > > + if (bridge->funcs->mode_valid)
> > > + ret = bridge->funcs->mode_valid(bridge, mode);
> > > +
> > > + if (ret != MODE_OK)
> > > + return ret;
> > > +
> > > + return drm_bridge_mode_valid(bridge->next, mode);
> > 
> > Looks like it should be pretty trivial to avoid the recursion.
> > 
> > Am I correct in interpreting this that bridges have some kind of
> > a hand rolled linked list implementation? Reusing the standard
> > linked lists would allow you to use list_for_each() etc.
> 
> Yeah it's a hand-rolled list, but current hw also has a bridge nesting
> depth of 2, so it really doesn't matter. I guess once we have real long
> chains of bridges we can fix this (and just using list_head sounds like a
> great idea).

Even if not really needed right now, it's a pretty easy cleanup, if Jose has 
time to handle it in v3 of this series let's not postpone it ;-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-12 Thread Laurent Pinchart

Hi Daniel,

On Wednesday 10 May 2017 17:14:33 Daniel Vetter wrote:
> On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:
> > On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
> > > Introduce a new helper function which calls mode_valid() callback
> > > for all bridges in an encoder chain.
> > > 
> > > Signed-off-by: Jose Abreu 
> > > Cc: Carlos Palminha 
> > > Cc: Alexey Brodkin 
> > > Cc: Ville Syrjälä 
> > > Cc: Daniel Vetter 
> > > Cc: Dave Airlie 
> > > Cc: Andrzej Hajda 
> > > Cc: Archit Taneja 
> > > ---
> > > 
> > >  drivers/gpu/drm/drm_bridge.c | 33 +
> > >  include/drm/drm_bridge.h |  2 ++
> > >  2 files changed, 35 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index 86a7637..dc8cdfe 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge
> > > *bridge,
> > > 
> > >  EXPORT_SYMBOL(drm_bridge_mode_fixup);
> > >  
> > >  /**
> > > 
> > > + * drm_bridge_mode_valid - validate the mode against all bridges in the
> > > + *  encoder chain.
> > > + * @bridge: bridge control structure
> > > + * @mode: desired mode to be validated
> > > + *
> > > + * Calls _bridge_funcs.mode_valid for all the bridges in the
> > > encoder
> > > + * chain, starting from the first bridge to the last. If at least one
> > > bridge + * does not accept the mode the function returns the error
> > > code.
> > > + *
> > > + * Note: the bridge passed should be the one closest to the encoder.
> > > + *
> > > + * RETURNS:
> > > + * MODE_OK on success, drm_mode_status Enum error code on failure
> > > + */
> > > +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> > > +const struct drm_display_mode 
*mode)
> > > +{
> > > + enum drm_mode_status ret = MODE_OK;
> > > +
> > > + if (!bridge)
> > > + return ret;
> > > +
> > > + if (bridge->funcs->mode_valid)
> > > + ret = bridge->funcs->mode_valid(bridge, mode);
> > > +
> > > + if (ret != MODE_OK)
> > > + return ret;
> > > +
> > > + return drm_bridge_mode_valid(bridge->next, mode);
> > 
> > Looks like it should be pretty trivial to avoid the recursion.
> > 
> > Am I correct in interpreting this that bridges have some kind of
> > a hand rolled linked list implementation? Reusing the standard
> > linked lists would allow you to use list_for_each() etc.
> 
> Yeah it's a hand-rolled list, but current hw also has a bridge nesting
> depth of 2, so it really doesn't matter. I guess once we have real long
> chains of bridges we can fix this (and just using list_head sounds like a
> great idea).

Even if not really needed right now, it's a pretty easy cleanup, if Jose has 
time to handle it in v3 of this series let's not postpone it ;-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-10 Thread Daniel Vetter

On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:
> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
> > Introduce a new helper function which calls mode_valid() callback
> > for all bridges in an encoder chain.
> > 
> > Signed-off-by: Jose Abreu 
> > Cc: Carlos Palminha 
> > Cc: Alexey Brodkin 
> > Cc: Ville Syrjälä 
> > Cc: Daniel Vetter 
> > Cc: Dave Airlie 
> > Cc: Andrzej Hajda 
> > Cc: Archit Taneja 
> > ---
> >  drivers/gpu/drm/drm_bridge.c | 33 +
> >  include/drm/drm_bridge.h |  2 ++
> >  2 files changed, 35 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 86a7637..dc8cdfe 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
> >  EXPORT_SYMBOL(drm_bridge_mode_fixup);
> >  
> >  /**
> > + * drm_bridge_mode_valid - validate the mode against all bridges in the
> > + *encoder chain.
> > + * @bridge: bridge control structure
> > + * @mode: desired mode to be validated
> > + *
> > + * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
> > + * chain, starting from the first bridge to the last. If at least one 
> > bridge
> > + * does not accept the mode the function returns the error code.
> > + *
> > + * Note: the bridge passed should be the one closest to the encoder.
> > + *
> > + * RETURNS:
> > + * MODE_OK on success, drm_mode_status Enum error code on failure
> > + */
> > +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> > +  const struct drm_display_mode *mode)
> > +{
> > +   enum drm_mode_status ret = MODE_OK;
> > +
> > +   if (!bridge)
> > +   return ret;
> > +
> > +   if (bridge->funcs->mode_valid)
> > +   ret = bridge->funcs->mode_valid(bridge, mode);
> > +
> > +   if (ret != MODE_OK)
> > +   return ret;
> > +
> > +   return drm_bridge_mode_valid(bridge->next, mode);
> 
> Looks like it should be pretty trivial to avoid the recursion.
> 
> Am I correct in interpreting this that bridges have some kind of
> a hand rolled linked list implementation? Reusing the standard
> linked lists would allow you to use list_for_each() etc.

Yeah it's a hand-rolled list, but current hw also has a bridge nesting
depth of 2, so it really doesn't matter. I guess once we have real long
chains of bridges we can fix this (and just using list_head sounds like a
great idea).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-10 Thread Daniel Vetter

On Wed, May 10, 2017 at 04:41:09PM +0300, Ville Syrjälä wrote:
> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
> > Introduce a new helper function which calls mode_valid() callback
> > for all bridges in an encoder chain.
> > 
> > Signed-off-by: Jose Abreu 
> > Cc: Carlos Palminha 
> > Cc: Alexey Brodkin 
> > Cc: Ville Syrjälä 
> > Cc: Daniel Vetter 
> > Cc: Dave Airlie 
> > Cc: Andrzej Hajda 
> > Cc: Archit Taneja 
> > ---
> >  drivers/gpu/drm/drm_bridge.c | 33 +
> >  include/drm/drm_bridge.h |  2 ++
> >  2 files changed, 35 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 86a7637..dc8cdfe 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
> >  EXPORT_SYMBOL(drm_bridge_mode_fixup);
> >  
> >  /**
> > + * drm_bridge_mode_valid - validate the mode against all bridges in the
> > + *encoder chain.
> > + * @bridge: bridge control structure
> > + * @mode: desired mode to be validated
> > + *
> > + * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
> > + * chain, starting from the first bridge to the last. If at least one 
> > bridge
> > + * does not accept the mode the function returns the error code.
> > + *
> > + * Note: the bridge passed should be the one closest to the encoder.
> > + *
> > + * RETURNS:
> > + * MODE_OK on success, drm_mode_status Enum error code on failure
> > + */
> > +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> > +  const struct drm_display_mode *mode)
> > +{
> > +   enum drm_mode_status ret = MODE_OK;
> > +
> > +   if (!bridge)
> > +   return ret;
> > +
> > +   if (bridge->funcs->mode_valid)
> > +   ret = bridge->funcs->mode_valid(bridge, mode);
> > +
> > +   if (ret != MODE_OK)
> > +   return ret;
> > +
> > +   return drm_bridge_mode_valid(bridge->next, mode);
> 
> Looks like it should be pretty trivial to avoid the recursion.
> 
> Am I correct in interpreting this that bridges have some kind of
> a hand rolled linked list implementation? Reusing the standard
> linked lists would allow you to use list_for_each() etc.

Yeah it's a hand-rolled list, but current hw also has a bridge nesting
depth of 2, so it really doesn't matter. I guess once we have real long
chains of bridges we can fix this (and just using list_head sounds like a
great idea).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-10 Thread Jose Abreu

Hi Ville,


On 10-05-2017 15:01, Jose Abreu wrote:
> Hi Ville,
>
>
> On 10-05-2017 14:41, Ville Syrjälä wrote:
>> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
>>> Introduce a new helper function which calls mode_valid() callback
>>> for all bridges in an encoder chain.
>>>
>>> Signed-off-by: Jose Abreu 
>>> Cc: Carlos Palminha 
>>> Cc: Alexey Brodkin 
>>> Cc: Ville Syrjälä 
>>> Cc: Daniel Vetter 
>>> Cc: Dave Airlie 
>>> Cc: Andrzej Hajda 
>>> Cc: Archit Taneja 
>>> ---
>>>  drivers/gpu/drm/drm_bridge.c | 33 +
>>>  include/drm/drm_bridge.h |  2 ++
>>>  2 files changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>>> index 86a7637..dc8cdfe 100644
>>> --- a/drivers/gpu/drm/drm_bridge.c
>>> +++ b/drivers/gpu/drm/drm_bridge.c
>>> @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>>>  EXPORT_SYMBOL(drm_bridge_mode_fixup);
>>>  
>>>  /**
>>> + * drm_bridge_mode_valid - validate the mode against all bridges in the
>>> + *encoder chain.
>>> + * @bridge: bridge control structure
>>> + * @mode: desired mode to be validated
>>> + *
>>> + * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
>>> + * chain, starting from the first bridge to the last. If at least one 
>>> bridge
>>> + * does not accept the mode the function returns the error code.
>>> + *
>>> + * Note: the bridge passed should be the one closest to the encoder.
>>> + *
>>> + * RETURNS:
>>> + * MODE_OK on success, drm_mode_status Enum error code on failure
>>> + */
>>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>>> +  const struct drm_display_mode *mode)
>>> +{
>>> +   enum drm_mode_status ret = MODE_OK;
>>> +
>>> +   if (!bridge)
>>> +   return ret;
>>> +
>>> +   if (bridge->funcs->mode_valid)
>>> +   ret = bridge->funcs->mode_valid(bridge, mode);
>>> +
>>> +   if (ret != MODE_OK)
>>> +   return ret;
>>> +
>>> +   return drm_bridge_mode_valid(bridge->next, mode);
>> Looks like it should be pretty trivial to avoid the recursion.
>>
>> Am I correct in interpreting this that bridges have some kind of
>> a hand rolled linked list implementation? Reusing the standard
>> linked lists would allow you to use list_for_each() etc.
> I reused the drm_bridge_mode_fixup but now I see how its done
> like that: so that the fixup is propagated in the correct order.
> As for mode_valid we just need to check if ret != MODE_OK then I
> think we can use the list_for_each_entry(bridge->list).

Oops, I got this wrong sorry. I meant there is a list but its for
all the system bridges. This is a "custom" linked list yeah.

Best regards,
Jose Miguel Abreu

>
> Best regards,
> Jose Miguel Abreu
>
>>> +}
>>> +EXPORT_SYMBOL(drm_bridge_mode_valid);
>>> +
>>> +/**
>>>   * drm_bridge_disable - disables all bridges in the encoder chain
>>>   * @bridge: bridge control structure
>>>   *
>>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>>> index 00c6c36..8358eb3 100644
>>> --- a/include/drm/drm_bridge.h
>>> +++ b/include/drm/drm_bridge.h
>>> @@ -233,6 +233,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, 
>>> struct drm_bridge *bridge,
>>>  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>>> const struct drm_display_mode *mode,
>>> struct drm_display_mode *adjusted_mode);
>>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>>> +  const struct drm_display_mode *mode);
>>>  void drm_bridge_disable(struct drm_bridge *bridge);
>>>  void drm_bridge_post_disable(struct drm_bridge *bridge);
>>>  void drm_bridge_mode_set(struct drm_bridge *bridge,
>>> -- 
>>> 1.9.1
>>>

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-10 Thread Jose Abreu

Hi Ville,


On 10-05-2017 15:01, Jose Abreu wrote:
> Hi Ville,
>
>
> On 10-05-2017 14:41, Ville Syrjälä wrote:
>> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
>>> Introduce a new helper function which calls mode_valid() callback
>>> for all bridges in an encoder chain.
>>>
>>> Signed-off-by: Jose Abreu 
>>> Cc: Carlos Palminha 
>>> Cc: Alexey Brodkin 
>>> Cc: Ville Syrjälä 
>>> Cc: Daniel Vetter 
>>> Cc: Dave Airlie 
>>> Cc: Andrzej Hajda 
>>> Cc: Archit Taneja 
>>> ---
>>>  drivers/gpu/drm/drm_bridge.c | 33 +
>>>  include/drm/drm_bridge.h |  2 ++
>>>  2 files changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>>> index 86a7637..dc8cdfe 100644
>>> --- a/drivers/gpu/drm/drm_bridge.c
>>> +++ b/drivers/gpu/drm/drm_bridge.c
>>> @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>>>  EXPORT_SYMBOL(drm_bridge_mode_fixup);
>>>  
>>>  /**
>>> + * drm_bridge_mode_valid - validate the mode against all bridges in the
>>> + *encoder chain.
>>> + * @bridge: bridge control structure
>>> + * @mode: desired mode to be validated
>>> + *
>>> + * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
>>> + * chain, starting from the first bridge to the last. If at least one 
>>> bridge
>>> + * does not accept the mode the function returns the error code.
>>> + *
>>> + * Note: the bridge passed should be the one closest to the encoder.
>>> + *
>>> + * RETURNS:
>>> + * MODE_OK on success, drm_mode_status Enum error code on failure
>>> + */
>>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>>> +  const struct drm_display_mode *mode)
>>> +{
>>> +   enum drm_mode_status ret = MODE_OK;
>>> +
>>> +   if (!bridge)
>>> +   return ret;
>>> +
>>> +   if (bridge->funcs->mode_valid)
>>> +   ret = bridge->funcs->mode_valid(bridge, mode);
>>> +
>>> +   if (ret != MODE_OK)
>>> +   return ret;
>>> +
>>> +   return drm_bridge_mode_valid(bridge->next, mode);
>> Looks like it should be pretty trivial to avoid the recursion.
>>
>> Am I correct in interpreting this that bridges have some kind of
>> a hand rolled linked list implementation? Reusing the standard
>> linked lists would allow you to use list_for_each() etc.
> I reused the drm_bridge_mode_fixup but now I see how its done
> like that: so that the fixup is propagated in the correct order.
> As for mode_valid we just need to check if ret != MODE_OK then I
> think we can use the list_for_each_entry(bridge->list).

Oops, I got this wrong sorry. I meant there is a list but its for
all the system bridges. This is a "custom" linked list yeah.

Best regards,
Jose Miguel Abreu

>
> Best regards,
> Jose Miguel Abreu
>
>>> +}
>>> +EXPORT_SYMBOL(drm_bridge_mode_valid);
>>> +
>>> +/**
>>>   * drm_bridge_disable - disables all bridges in the encoder chain
>>>   * @bridge: bridge control structure
>>>   *
>>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>>> index 00c6c36..8358eb3 100644
>>> --- a/include/drm/drm_bridge.h
>>> +++ b/include/drm/drm_bridge.h
>>> @@ -233,6 +233,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, 
>>> struct drm_bridge *bridge,
>>>  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>>> const struct drm_display_mode *mode,
>>> struct drm_display_mode *adjusted_mode);
>>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>>> +  const struct drm_display_mode *mode);
>>>  void drm_bridge_disable(struct drm_bridge *bridge);
>>>  void drm_bridge_post_disable(struct drm_bridge *bridge);
>>>  void drm_bridge_mode_set(struct drm_bridge *bridge,
>>> -- 
>>> 1.9.1
>>>

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-10 Thread Jose Abreu

Hi Ville,


On 10-05-2017 14:41, Ville Syrjälä wrote:
> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
>> Introduce a new helper function which calls mode_valid() callback
>> for all bridges in an encoder chain.
>>
>> Signed-off-by: Jose Abreu 
>> Cc: Carlos Palminha 
>> Cc: Alexey Brodkin 
>> Cc: Ville Syrjälä 
>> Cc: Daniel Vetter 
>> Cc: Dave Airlie 
>> Cc: Andrzej Hajda 
>> Cc: Archit Taneja 
>> ---
>>  drivers/gpu/drm/drm_bridge.c | 33 +
>>  include/drm/drm_bridge.h |  2 ++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>> index 86a7637..dc8cdfe 100644
>> --- a/drivers/gpu/drm/drm_bridge.c
>> +++ b/drivers/gpu/drm/drm_bridge.c
>> @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>>  EXPORT_SYMBOL(drm_bridge_mode_fixup);
>>  
>>  /**
>> + * drm_bridge_mode_valid - validate the mode against all bridges in the
>> + * encoder chain.
>> + * @bridge: bridge control structure
>> + * @mode: desired mode to be validated
>> + *
>> + * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
>> + * chain, starting from the first bridge to the last. If at least one bridge
>> + * does not accept the mode the function returns the error code.
>> + *
>> + * Note: the bridge passed should be the one closest to the encoder.
>> + *
>> + * RETURNS:
>> + * MODE_OK on success, drm_mode_status Enum error code on failure
>> + */
>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>> +   const struct drm_display_mode *mode)
>> +{
>> +enum drm_mode_status ret = MODE_OK;
>> +
>> +if (!bridge)
>> +return ret;
>> +
>> +if (bridge->funcs->mode_valid)
>> +ret = bridge->funcs->mode_valid(bridge, mode);
>> +
>> +if (ret != MODE_OK)
>> +return ret;
>> +
>> +return drm_bridge_mode_valid(bridge->next, mode);
> Looks like it should be pretty trivial to avoid the recursion.
>
> Am I correct in interpreting this that bridges have some kind of
> a hand rolled linked list implementation? Reusing the standard
> linked lists would allow you to use list_for_each() etc.

I reused the drm_bridge_mode_fixup but now I see how its done
like that: so that the fixup is propagated in the correct order.
As for mode_valid we just need to check if ret != MODE_OK then I
think we can use the list_for_each_entry(bridge->list).

Best regards,
Jose Miguel Abreu

>
>> +}
>> +EXPORT_SYMBOL(drm_bridge_mode_valid);
>> +
>> +/**
>>   * drm_bridge_disable - disables all bridges in the encoder chain
>>   * @bridge: bridge control structure
>>   *
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index 00c6c36..8358eb3 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -233,6 +233,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, 
>> struct drm_bridge *bridge,
>>  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>>  const struct drm_display_mode *mode,
>>  struct drm_display_mode *adjusted_mode);
>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>> +   const struct drm_display_mode *mode);
>>  void drm_bridge_disable(struct drm_bridge *bridge);
>>  void drm_bridge_post_disable(struct drm_bridge *bridge);
>>  void drm_bridge_mode_set(struct drm_bridge *bridge,
>> -- 
>> 1.9.1
>>

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-10 Thread Jose Abreu

Hi Ville,


On 10-05-2017 14:41, Ville Syrjälä wrote:
> On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
>> Introduce a new helper function which calls mode_valid() callback
>> for all bridges in an encoder chain.
>>
>> Signed-off-by: Jose Abreu 
>> Cc: Carlos Palminha 
>> Cc: Alexey Brodkin 
>> Cc: Ville Syrjälä 
>> Cc: Daniel Vetter 
>> Cc: Dave Airlie 
>> Cc: Andrzej Hajda 
>> Cc: Archit Taneja 
>> ---
>>  drivers/gpu/drm/drm_bridge.c | 33 +
>>  include/drm/drm_bridge.h |  2 ++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>> index 86a7637..dc8cdfe 100644
>> --- a/drivers/gpu/drm/drm_bridge.c
>> +++ b/drivers/gpu/drm/drm_bridge.c
>> @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>>  EXPORT_SYMBOL(drm_bridge_mode_fixup);
>>  
>>  /**
>> + * drm_bridge_mode_valid - validate the mode against all bridges in the
>> + * encoder chain.
>> + * @bridge: bridge control structure
>> + * @mode: desired mode to be validated
>> + *
>> + * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
>> + * chain, starting from the first bridge to the last. If at least one bridge
>> + * does not accept the mode the function returns the error code.
>> + *
>> + * Note: the bridge passed should be the one closest to the encoder.
>> + *
>> + * RETURNS:
>> + * MODE_OK on success, drm_mode_status Enum error code on failure
>> + */
>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>> +   const struct drm_display_mode *mode)
>> +{
>> +enum drm_mode_status ret = MODE_OK;
>> +
>> +if (!bridge)
>> +return ret;
>> +
>> +if (bridge->funcs->mode_valid)
>> +ret = bridge->funcs->mode_valid(bridge, mode);
>> +
>> +if (ret != MODE_OK)
>> +return ret;
>> +
>> +return drm_bridge_mode_valid(bridge->next, mode);
> Looks like it should be pretty trivial to avoid the recursion.
>
> Am I correct in interpreting this that bridges have some kind of
> a hand rolled linked list implementation? Reusing the standard
> linked lists would allow you to use list_for_each() etc.

I reused the drm_bridge_mode_fixup but now I see how its done
like that: so that the fixup is propagated in the correct order.
As for mode_valid we just need to check if ret != MODE_OK then I
think we can use the list_for_each_entry(bridge->list).

Best regards,
Jose Miguel Abreu

>
>> +}
>> +EXPORT_SYMBOL(drm_bridge_mode_valid);
>> +
>> +/**
>>   * drm_bridge_disable - disables all bridges in the encoder chain
>>   * @bridge: bridge control structure
>>   *
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index 00c6c36..8358eb3 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -233,6 +233,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, 
>> struct drm_bridge *bridge,
>>  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>>  const struct drm_display_mode *mode,
>>  struct drm_display_mode *adjusted_mode);
>> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
>> +   const struct drm_display_mode *mode);
>>  void drm_bridge_disable(struct drm_bridge *bridge);
>>  void drm_bridge_post_disable(struct drm_bridge *bridge);
>>  void drm_bridge_mode_set(struct drm_bridge *bridge,
>> -- 
>> 1.9.1
>>

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-10 Thread Ville Syrjälä

On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
> Introduce a new helper function which calls mode_valid() callback
> for all bridges in an encoder chain.
> 
> Signed-off-by: Jose Abreu 
> Cc: Carlos Palminha 
> Cc: Alexey Brodkin 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: Andrzej Hajda 
> Cc: Archit Taneja 
> ---
>  drivers/gpu/drm/drm_bridge.c | 33 +
>  include/drm/drm_bridge.h |  2 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 86a7637..dc8cdfe 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>  EXPORT_SYMBOL(drm_bridge_mode_fixup);
>  
>  /**
> + * drm_bridge_mode_valid - validate the mode against all bridges in the
> + *  encoder chain.
> + * @bridge: bridge control structure
> + * @mode: desired mode to be validated
> + *
> + * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
> + * chain, starting from the first bridge to the last. If at least one bridge
> + * does not accept the mode the function returns the error code.
> + *
> + * Note: the bridge passed should be the one closest to the encoder.
> + *
> + * RETURNS:
> + * MODE_OK on success, drm_mode_status Enum error code on failure
> + */
> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> +const struct drm_display_mode *mode)
> +{
> + enum drm_mode_status ret = MODE_OK;
> +
> + if (!bridge)
> + return ret;
> +
> + if (bridge->funcs->mode_valid)
> + ret = bridge->funcs->mode_valid(bridge, mode);
> +
> + if (ret != MODE_OK)
> + return ret;
> +
> + return drm_bridge_mode_valid(bridge->next, mode);

Looks like it should be pretty trivial to avoid the recursion.

Am I correct in interpreting this that bridges have some kind of
a hand rolled linked list implementation? Reusing the standard
linked lists would allow you to use list_for_each() etc.

> +}
> +EXPORT_SYMBOL(drm_bridge_mode_valid);
> +
> +/**
>   * drm_bridge_disable - disables all bridges in the encoder chain
>   * @bridge: bridge control structure
>   *
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 00c6c36..8358eb3 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -233,6 +233,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
> drm_bridge *bridge,
>  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>   const struct drm_display_mode *mode,
>   struct drm_display_mode *adjusted_mode);
> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> +const struct drm_display_mode *mode);
>  void drm_bridge_disable(struct drm_bridge *bridge);
>  void drm_bridge_post_disable(struct drm_bridge *bridge);
>  void drm_bridge_mode_set(struct drm_bridge *bridge,
> -- 
> 1.9.1
> 

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-10 Thread Ville Syrjälä

On Tue, May 09, 2017 at 06:00:13PM +0100, Jose Abreu wrote:
> Introduce a new helper function which calls mode_valid() callback
> for all bridges in an encoder chain.
> 
> Signed-off-by: Jose Abreu 
> Cc: Carlos Palminha 
> Cc: Alexey Brodkin 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: Andrzej Hajda 
> Cc: Archit Taneja 
> ---
>  drivers/gpu/drm/drm_bridge.c | 33 +
>  include/drm/drm_bridge.h |  2 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 86a7637..dc8cdfe 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>  EXPORT_SYMBOL(drm_bridge_mode_fixup);
>  
>  /**
> + * drm_bridge_mode_valid - validate the mode against all bridges in the
> + *  encoder chain.
> + * @bridge: bridge control structure
> + * @mode: desired mode to be validated
> + *
> + * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
> + * chain, starting from the first bridge to the last. If at least one bridge
> + * does not accept the mode the function returns the error code.
> + *
> + * Note: the bridge passed should be the one closest to the encoder.
> + *
> + * RETURNS:
> + * MODE_OK on success, drm_mode_status Enum error code on failure
> + */
> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> +const struct drm_display_mode *mode)
> +{
> + enum drm_mode_status ret = MODE_OK;
> +
> + if (!bridge)
> + return ret;
> +
> + if (bridge->funcs->mode_valid)
> + ret = bridge->funcs->mode_valid(bridge, mode);
> +
> + if (ret != MODE_OK)
> + return ret;
> +
> + return drm_bridge_mode_valid(bridge->next, mode);

Looks like it should be pretty trivial to avoid the recursion.

Am I correct in interpreting this that bridges have some kind of
a hand rolled linked list implementation? Reusing the standard
linked lists would allow you to use list_for_each() etc.

> +}
> +EXPORT_SYMBOL(drm_bridge_mode_valid);
> +
> +/**
>   * drm_bridge_disable - disables all bridges in the encoder chain
>   * @bridge: bridge control structure
>   *
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 00c6c36..8358eb3 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -233,6 +233,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
> drm_bridge *bridge,
>  bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>   const struct drm_display_mode *mode,
>   struct drm_display_mode *adjusted_mode);
> +enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
> +const struct drm_display_mode *mode);
>  void drm_bridge_disable(struct drm_bridge *bridge);
>  void drm_bridge_post_disable(struct drm_bridge *bridge);
>  void drm_bridge_mode_set(struct drm_bridge *bridge,
> -- 
> 1.9.1
> 

-- 
Ville Syrjälä
Intel OTC

[PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-09 Thread Jose Abreu

Introduce a new helper function which calls mode_valid() callback
for all bridges in an encoder chain.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Alexey Brodkin 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 
Cc: Andrzej Hajda 
Cc: Archit Taneja 
---
 drivers/gpu/drm/drm_bridge.c | 33 +
 include/drm/drm_bridge.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 86a7637..dc8cdfe 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
 EXPORT_SYMBOL(drm_bridge_mode_fixup);
 
 /**
+ * drm_bridge_mode_valid - validate the mode against all bridges in the
+ *encoder chain.
+ * @bridge: bridge control structure
+ * @mode: desired mode to be validated
+ *
+ * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
+ * chain, starting from the first bridge to the last. If at least one bridge
+ * does not accept the mode the function returns the error code.
+ *
+ * Note: the bridge passed should be the one closest to the encoder.
+ *
+ * RETURNS:
+ * MODE_OK on success, drm_mode_status Enum error code on failure
+ */
+enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
+  const struct drm_display_mode *mode)
+{
+   enum drm_mode_status ret = MODE_OK;
+
+   if (!bridge)
+   return ret;
+
+   if (bridge->funcs->mode_valid)
+   ret = bridge->funcs->mode_valid(bridge, mode);
+
+   if (ret != MODE_OK)
+   return ret;
+
+   return drm_bridge_mode_valid(bridge->next, mode);
+}
+EXPORT_SYMBOL(drm_bridge_mode_valid);
+
+/**
  * drm_bridge_disable - disables all bridges in the encoder chain
  * @bridge: bridge control structure
  *
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 00c6c36..8358eb3 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -233,6 +233,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
drm_bridge *bridge,
 bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode);
+enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
+  const struct drm_display_mode *mode);
 void drm_bridge_disable(struct drm_bridge *bridge);
 void drm_bridge_post_disable(struct drm_bridge *bridge);
 void drm_bridge_mode_set(struct drm_bridge *bridge,
-- 
1.9.1

[PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

2017-05-09 Thread Jose Abreu

Introduce a new helper function which calls mode_valid() callback
for all bridges in an encoder chain.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Alexey Brodkin 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 
Cc: Andrzej Hajda 
Cc: Archit Taneja 
---
 drivers/gpu/drm/drm_bridge.c | 33 +
 include/drm/drm_bridge.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 86a7637..dc8cdfe 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -206,6 +206,39 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
 EXPORT_SYMBOL(drm_bridge_mode_fixup);
 
 /**
+ * drm_bridge_mode_valid - validate the mode against all bridges in the
+ *encoder chain.
+ * @bridge: bridge control structure
+ * @mode: desired mode to be validated
+ *
+ * Calls _bridge_funcs.mode_valid for all the bridges in the encoder
+ * chain, starting from the first bridge to the last. If at least one bridge
+ * does not accept the mode the function returns the error code.
+ *
+ * Note: the bridge passed should be the one closest to the encoder.
+ *
+ * RETURNS:
+ * MODE_OK on success, drm_mode_status Enum error code on failure
+ */
+enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
+  const struct drm_display_mode *mode)
+{
+   enum drm_mode_status ret = MODE_OK;
+
+   if (!bridge)
+   return ret;
+
+   if (bridge->funcs->mode_valid)
+   ret = bridge->funcs->mode_valid(bridge, mode);
+
+   if (ret != MODE_OK)
+   return ret;
+
+   return drm_bridge_mode_valid(bridge->next, mode);
+}
+EXPORT_SYMBOL(drm_bridge_mode_valid);
+
+/**
  * drm_bridge_disable - disables all bridges in the encoder chain
  * @bridge: bridge control structure
  *
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 00c6c36..8358eb3 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -233,6 +233,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct 
drm_bridge *bridge,
 bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode);
+enum drm_mode_status drm_bridge_mode_valid(struct drm_bridge *bridge,
+  const struct drm_display_mode *mode);
 void drm_bridge_disable(struct drm_bridge *bridge);
 void drm_bridge_post_disable(struct drm_bridge *bridge);
 void drm_bridge_mode_set(struct drm_bridge *bridge,
-- 
1.9.1

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

Re: [PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

[PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

[PATCH v2 6/8] drm: Introduce drm_bridge_mode_valid()

20 matches

Site Navigation

Mail list logo

Footer information