diff mbox series

[PATCH/RFC,06/15] drm: bridge: thc63: Report input bus mode through bridge timings

Message ID 20190306232345.23052-7-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series R-Car DU: LVDS dual-link mode support | expand

Commit Message

Laurent Pinchart March 6, 2019, 11:23 p.m. UTC
Set a drm_bridge_timings in the drm_bridge, and use it to report the
input bus mode (single-link or dual-link). The other fields of the
timings structure are kept to 0 as they do not apply to LVDS buses.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 11 deletions(-)

Comments

Jacopo Mondi March 8, 2019, 5:32 p.m. UTC | #1
Hi Laurent,

On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> Set a drm_bridge_timings in the drm_bridge, and use it to report the
> input bus mode (single-link or dual-link). The other fields of the
> timings structure are kept to 0 as they do not apply to LVDS buses.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> index b083a740565c..206b0af5e154 100644
> --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> @@ -31,6 +31,8 @@ struct thc63_dev {
>
>  	struct drm_bridge bridge;
>  	struct drm_bridge *next;
> +
> +	struct drm_bridge_timings timings;
>  };
>
>  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
>  static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
>  					const struct drm_display_mode *mode)
>  {
> +	struct thc63_dev *thc63 = to_thc63(bridge);
> +	unsigned int min_freq;
> +	unsigned int max_freq;
> +
>  	/*
> -	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> -	 * mode. Note that the limits are different in dual-in, single-out mode,
> -	 * and will need to be adjusted accordingly.
> +	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> +	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> +	 * isn't supported by the driver yet, simply derive the limits from the
> +	 * input mode.
>  	 */
> -	if (mode->clock < 8000)
> +	if (thc63->timings.dual_link) {
> +		min_freq = 40000;
> +		max_freq = 150000;
> +	} else {
> +		min_freq = 8000;
> +		max_freq = 135000;
> +	}
> +
> +	if (mode->clock < min_freq)
>  		return MODE_CLOCK_LOW;
>
> -	if (mode->clock > 135000)
> +	if (mode->clock > max_freq)
>  		return MODE_CLOCK_HIGH;
>
>  	return MODE_OK;
> @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
>
>  static int thc63_parse_dt(struct thc63_dev *thc63)
>  {
> -	struct device_node *thc63_out;
> +	struct device_node *endpoint;
>  	struct device_node *remote;
>
> -	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> -						  THC63_RGB_OUT0, -1);
> -	if (!thc63_out) {
> +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> +						 THC63_RGB_OUT0, -1);
> +	if (!endpoint) {
>  		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
>  			THC63_RGB_OUT0);
>  		return -ENODEV;
>  	}
>
> -	remote = of_graph_get_remote_port_parent(thc63_out);
> -	of_node_put(thc63_out);
> +	remote = of_graph_get_remote_port_parent(endpoint);
> +	of_node_put(endpoint);
>  	if (!remote) {
>  		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
>  			THC63_RGB_OUT0);
> @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
>  	if (!thc63->next)
>  		return -EPROBE_DEFER;
>
> +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> +						 THC63_LVDS_IN1, -1);
> +	of_node_put(endpoint);
> +

Should you check if endpoint is enabled?

By the way, this seems to me works properly, as in [12/15] if skip
creation of the LVDS1 encoder if it is operating in dual link mode.
Without that, we would hit again the issue of matching on device
nodes, as the same bridge would be attached twice, am I wrong?

I feel we've been there already :)
https://lkml.org/lkml/2018/3/20/341

and it is not just an issue on matching on endpoints, it's that the
drm bridge itself that would need to handle multiple attach/detaches..

> +	thc63->timings.dual_link = endpoint != NULL;
> +	dev_dbg(thc63->dev, "operating in %s-link mode\n",
> +		thc63->timings.dual_link ? "dual" : "single");
> +
>  	return 0;
>  }
>
> @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
>  	thc63->bridge.driver_private = thc63;
>  	thc63->bridge.of_node = pdev->dev.of_node;
>  	thc63->bridge.funcs = &thc63_bridge_func;
> +	thc63->bridge.timings = &thc63->timings;
>
>  	drm_bridge_add(&thc63->bridge);
>
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart March 8, 2019, 6 p.m. UTC | #2
Hi Jacopo,

On Fri, Mar 08, 2019 at 06:32:59PM +0100, Jacopo Mondi wrote:
> On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> > Set a drm_bridge_timings in the drm_bridge, and use it to report the
> > input bus mode (single-link or dual-link). The other fields of the
> > timings structure are kept to 0 as they do not apply to LVDS buses.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
> >  1 file changed, 35 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > index b083a740565c..206b0af5e154 100644
> > --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> > +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > @@ -31,6 +31,8 @@ struct thc63_dev {
> >
> >  	struct drm_bridge bridge;
> >  	struct drm_bridge *next;
> > +
> > +	struct drm_bridge_timings timings;
> >  };
> >
> >  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> > @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
> >  static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
> >  					const struct drm_display_mode *mode)
> >  {
> > +	struct thc63_dev *thc63 = to_thc63(bridge);
> > +	unsigned int min_freq;
> > +	unsigned int max_freq;
> > +
> >  	/*
> > -	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> > -	 * mode. Note that the limits are different in dual-in, single-out mode,
> > -	 * and will need to be adjusted accordingly.
> > +	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> > +	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> > +	 * isn't supported by the driver yet, simply derive the limits from the
> > +	 * input mode.
> >  	 */
> > -	if (mode->clock < 8000)
> > +	if (thc63->timings.dual_link) {
> > +		min_freq = 40000;
> > +		max_freq = 150000;
> > +	} else {
> > +		min_freq = 8000;
> > +		max_freq = 135000;
> > +	}
> > +
> > +	if (mode->clock < min_freq)
> >  		return MODE_CLOCK_LOW;
> >
> > -	if (mode->clock > 135000)
> > +	if (mode->clock > max_freq)
> >  		return MODE_CLOCK_HIGH;
> >
> >  	return MODE_OK;
> > @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
> >
> >  static int thc63_parse_dt(struct thc63_dev *thc63)
> >  {
> > -	struct device_node *thc63_out;
> > +	struct device_node *endpoint;
> >  	struct device_node *remote;
> >
> > -	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > -						  THC63_RGB_OUT0, -1);
> > -	if (!thc63_out) {
> > +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > +						 THC63_RGB_OUT0, -1);
> > +	if (!endpoint) {
> >  		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
> >  			THC63_RGB_OUT0);
> >  		return -ENODEV;
> >  	}
> >
> > -	remote = of_graph_get_remote_port_parent(thc63_out);
> > -	of_node_put(thc63_out);
> > +	remote = of_graph_get_remote_port_parent(endpoint);
> > +	of_node_put(endpoint);
> >  	if (!remote) {
> >  		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
> >  			THC63_RGB_OUT0);
> > @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
> >  	if (!thc63->next)
> >  		return -EPROBE_DEFER;
> >
> > +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > +						 THC63_LVDS_IN1, -1);
> > +	of_node_put(endpoint);
> > +
> 
> Should you check if endpoint is enabled?

Can endpoints be disabled ?

> By the way, this seems to me works properly, as in [12/15] if skip
> creation of the LVDS1 encoder if it is operating in dual link mode.
> Without that, we would hit again the issue of matching on device
> nodes, as the same bridge would be attached twice, am I wrong?
> 
> I feel we've been there already :)
> https://lkml.org/lkml/2018/3/20/341
> 
> and it is not just an issue on matching on endpoints, it's that the
> drm bridge itself that would need to handle multiple attach/detaches..

Correct. Thanks to 12/15 we don't need this yet :-)

> > +	thc63->timings.dual_link = endpoint != NULL;
> > +	dev_dbg(thc63->dev, "operating in %s-link mode\n",
> > +		thc63->timings.dual_link ? "dual" : "single");
> > +
> >  	return 0;
> >  }
> >
> > @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
> >  	thc63->bridge.driver_private = thc63;
> >  	thc63->bridge.of_node = pdev->dev.of_node;
> >  	thc63->bridge.funcs = &thc63_bridge_func;
> > +	thc63->bridge.timings = &thc63->timings;
> >
> >  	drm_bridge_add(&thc63->bridge);
> >
Jacopo Mondi March 9, 2019, 11:24 a.m. UTC | #3
Hi Laurent

On Fri, Mar 08, 2019 at 08:00:28PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Fri, Mar 08, 2019 at 06:32:59PM +0100, Jacopo Mondi wrote:
> > On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> > > Set a drm_bridge_timings in the drm_bridge, and use it to report the
> > > input bus mode (single-link or dual-link). The other fields of the
> > > timings structure are kept to 0 as they do not apply to LVDS buses.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >  drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
> > >  1 file changed, 35 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > > index b083a740565c..206b0af5e154 100644
> > > --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> > > +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > > @@ -31,6 +31,8 @@ struct thc63_dev {
> > >
> > >  	struct drm_bridge bridge;
> > >  	struct drm_bridge *next;
> > > +
> > > +	struct drm_bridge_timings timings;
> > >  };
> > >
> > >  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> > > @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
> > >  static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
> > >  					const struct drm_display_mode *mode)
> > >  {
> > > +	struct thc63_dev *thc63 = to_thc63(bridge);
> > > +	unsigned int min_freq;
> > > +	unsigned int max_freq;
> > > +
> > >  	/*
> > > -	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> > > -	 * mode. Note that the limits are different in dual-in, single-out mode,
> > > -	 * and will need to be adjusted accordingly.
> > > +	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> > > +	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> > > +	 * isn't supported by the driver yet, simply derive the limits from the
> > > +	 * input mode.
> > >  	 */
> > > -	if (mode->clock < 8000)
> > > +	if (thc63->timings.dual_link) {
> > > +		min_freq = 40000;
> > > +		max_freq = 150000;
> > > +	} else {
> > > +		min_freq = 8000;
> > > +		max_freq = 135000;
> > > +	}
> > > +
> > > +	if (mode->clock < min_freq)
> > >  		return MODE_CLOCK_LOW;
> > >
> > > -	if (mode->clock > 135000)
> > > +	if (mode->clock > max_freq)
> > >  		return MODE_CLOCK_HIGH;
> > >
> > >  	return MODE_OK;
> > > @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
> > >
> > >  static int thc63_parse_dt(struct thc63_dev *thc63)
> > >  {
> > > -	struct device_node *thc63_out;
> > > +	struct device_node *endpoint;
> > >  	struct device_node *remote;
> > >
> > > -	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > > -						  THC63_RGB_OUT0, -1);
> > > -	if (!thc63_out) {
> > > +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > > +						 THC63_RGB_OUT0, -1);
> > > +	if (!endpoint) {
> > >  		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
> > >  			THC63_RGB_OUT0);
> > >  		return -ENODEV;
> > >  	}
> > >
> > > -	remote = of_graph_get_remote_port_parent(thc63_out);
> > > -	of_node_put(thc63_out);
> > > +	remote = of_graph_get_remote_port_parent(endpoint);
> > > +	of_node_put(endpoint);
> > >  	if (!remote) {
> > >  		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
> > >  			THC63_RGB_OUT0);
> > > @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
> > >  	if (!thc63->next)
> > >  		return -EPROBE_DEFER;
> > >
> > > +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > > +						 THC63_LVDS_IN1, -1);
> > > +	of_node_put(endpoint);
> > > +
> >
> > Should you check if endpoint is enabled?
>
> Can endpoints be disabled ?

I meant the remote port parent, sorry about that.

Thanks
  j

>
> > By the way, this seems to me works properly, as in [12/15] if skip
> > creation of the LVDS1 encoder if it is operating in dual link mode.
> > Without that, we would hit again the issue of matching on device
> > nodes, as the same bridge would be attached twice, am I wrong?
> >
> > I feel we've been there already :)
> > https://lkml.org/lkml/2018/3/20/341
> >
> > and it is not just an issue on matching on endpoints, it's that the
> > drm bridge itself that would need to handle multiple attach/detaches..
>
> Correct. Thanks to 12/15 we don't need this yet :-)
>
> > > +	thc63->timings.dual_link = endpoint != NULL;
> > > +	dev_dbg(thc63->dev, "operating in %s-link mode\n",
> > > +		thc63->timings.dual_link ? "dual" : "single");
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
> > >  	thc63->bridge.driver_private = thc63;
> > >  	thc63->bridge.of_node = pdev->dev.of_node;
> > >  	thc63->bridge.funcs = &thc63_bridge_func;
> > > +	thc63->bridge.timings = &thc63->timings;
> > >
> > >  	drm_bridge_add(&thc63->bridge);
> > >
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart March 9, 2019, 11:45 a.m. UTC | #4
Hi Jacopo,

On Sat, Mar 09, 2019 at 12:24:08PM +0100, Jacopo Mondi wrote:
> On Fri, Mar 08, 2019 at 08:00:28PM +0200, Laurent Pinchart wrote:
> > On Fri, Mar 08, 2019 at 06:32:59PM +0100, Jacopo Mondi wrote:
> >> On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> >>> Set a drm_bridge_timings in the drm_bridge, and use it to report the
> >>> input bus mode (single-link or dual-link). The other fields of the
> >>> timings structure are kept to 0 as they do not apply to LVDS buses.
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>>  drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ++++++++++++++++++++-------
> >>>  1 file changed, 35 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
> >>> index b083a740565c..206b0af5e154 100644
> >>> --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> >>> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> >>> @@ -31,6 +31,8 @@ struct thc63_dev {
> >>>
> >>>  	struct drm_bridge bridge;
> >>>  	struct drm_bridge *next;
> >>> +
> >>> +	struct drm_bridge_timings timings;
> >>>  };
> >>>
> >>>  static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> >>> @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
> >>>  static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
> >>>  					const struct drm_display_mode *mode)
> >>>  {
> >>> +	struct thc63_dev *thc63 = to_thc63(bridge);
> >>> +	unsigned int min_freq;
> >>> +	unsigned int max_freq;
> >>> +
> >>>  	/*
> >>> -	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> >>> -	 * mode. Note that the limits are different in dual-in, single-out mode,
> >>> -	 * and will need to be adjusted accordingly.
> >>> +	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> >>> +	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> >>> +	 * isn't supported by the driver yet, simply derive the limits from the
> >>> +	 * input mode.
> >>>  	 */
> >>> -	if (mode->clock < 8000)
> >>> +	if (thc63->timings.dual_link) {
> >>> +		min_freq = 40000;
> >>> +		max_freq = 150000;
> >>> +	} else {
> >>> +		min_freq = 8000;
> >>> +		max_freq = 135000;
> >>> +	}
> >>> +
> >>> +	if (mode->clock < min_freq)
> >>>  		return MODE_CLOCK_LOW;
> >>>
> >>> -	if (mode->clock > 135000)
> >>> +	if (mode->clock > max_freq)
> >>>  		return MODE_CLOCK_HIGH;
> >>>
> >>>  	return MODE_OK;
> >>> @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
> >>>
> >>>  static int thc63_parse_dt(struct thc63_dev *thc63)
> >>>  {
> >>> -	struct device_node *thc63_out;
> >>> +	struct device_node *endpoint;
> >>>  	struct device_node *remote;
> >>>
> >>> -	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> >>> -						  THC63_RGB_OUT0, -1);
> >>> -	if (!thc63_out) {
> >>> +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> >>> +						 THC63_RGB_OUT0, -1);
> >>> +	if (!endpoint) {
> >>>  		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
> >>>  			THC63_RGB_OUT0);
> >>>  		return -ENODEV;
> >>>  	}
> >>>
> >>> -	remote = of_graph_get_remote_port_parent(thc63_out);
> >>> -	of_node_put(thc63_out);
> >>> +	remote = of_graph_get_remote_port_parent(endpoint);
> >>> +	of_node_put(endpoint);
> >>>  	if (!remote) {
> >>>  		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
> >>>  			THC63_RGB_OUT0);
> >>> @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
> >>>  	if (!thc63->next)
> >>>  		return -EPROBE_DEFER;
> >>>
> >>> +	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> >>> +						 THC63_LVDS_IN1, -1);
> >>> +	of_node_put(endpoint);
> >>> +
> >>
> >> Should you check if endpoint is enabled?
> >
> > Can endpoints be disabled ?
> 
> I meant the remote port parent, sorry about that.

Strictly speaking, you're right. I don't know if there's a practical use
for this, but I'll fix that in v2.

> >> By the way, this seems to me works properly, as in [12/15] if skip
> >> creation of the LVDS1 encoder if it is operating in dual link mode.
> >> Without that, we would hit again the issue of matching on device
> >> nodes, as the same bridge would be attached twice, am I wrong?
> >>
> >> I feel we've been there already :)
> >> https://lkml.org/lkml/2018/3/20/341
> >>
> >> and it is not just an issue on matching on endpoints, it's that the
> >> drm bridge itself that would need to handle multiple attach/detaches..
> >
> > Correct. Thanks to 12/15 we don't need this yet :-)
> >
> >>> +	thc63->timings.dual_link = endpoint != NULL;
> >>> +	dev_dbg(thc63->dev, "operating in %s-link mode\n",
> >>> +		thc63->timings.dual_link ? "dual" : "single");
> >>> +
> >>>  	return 0;
> >>>  }
> >>>
> >>> @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
> >>>  	thc63->bridge.driver_private = thc63;
> >>>  	thc63->bridge.of_node = pdev->dev.of_node;
> >>>  	thc63->bridge.funcs = &thc63_bridge_func;
> >>> +	thc63->bridge.timings = &thc63->timings;
> >>>
> >>>  	drm_bridge_add(&thc63->bridge);
> >>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
index b083a740565c..206b0af5e154 100644
--- a/drivers/gpu/drm/bridge/thc63lvd1024.c
+++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
@@ -31,6 +31,8 @@  struct thc63_dev {
 
 	struct drm_bridge bridge;
 	struct drm_bridge *next;
+
+	struct drm_bridge_timings timings;
 };
 
 static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
@@ -48,15 +50,28 @@  static int thc63_attach(struct drm_bridge *bridge)
 static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
 					const struct drm_display_mode *mode)
 {
+	struct thc63_dev *thc63 = to_thc63(bridge);
+	unsigned int min_freq;
+	unsigned int max_freq;
+
 	/*
-	 * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
-	 * mode. Note that the limits are different in dual-in, single-out mode,
-	 * and will need to be adjusted accordingly.
+	 * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
+	 * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
+	 * isn't supported by the driver yet, simply derive the limits from the
+	 * input mode.
 	 */
-	if (mode->clock < 8000)
+	if (thc63->timings.dual_link) {
+		min_freq = 40000;
+		max_freq = 150000;
+	} else {
+		min_freq = 8000;
+		max_freq = 135000;
+	}
+
+	if (mode->clock < min_freq)
 		return MODE_CLOCK_LOW;
 
-	if (mode->clock > 135000)
+	if (mode->clock > max_freq)
 		return MODE_CLOCK_HIGH;
 
 	return MODE_OK;
@@ -101,19 +116,19 @@  static const struct drm_bridge_funcs thc63_bridge_func = {
 
 static int thc63_parse_dt(struct thc63_dev *thc63)
 {
-	struct device_node *thc63_out;
+	struct device_node *endpoint;
 	struct device_node *remote;
 
-	thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
-						  THC63_RGB_OUT0, -1);
-	if (!thc63_out) {
+	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
+						 THC63_RGB_OUT0, -1);
+	if (!endpoint) {
 		dev_err(thc63->dev, "Missing endpoint in port@%u\n",
 			THC63_RGB_OUT0);
 		return -ENODEV;
 	}
 
-	remote = of_graph_get_remote_port_parent(thc63_out);
-	of_node_put(thc63_out);
+	remote = of_graph_get_remote_port_parent(endpoint);
+	of_node_put(endpoint);
 	if (!remote) {
 		dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
 			THC63_RGB_OUT0);
@@ -132,6 +147,14 @@  static int thc63_parse_dt(struct thc63_dev *thc63)
 	if (!thc63->next)
 		return -EPROBE_DEFER;
 
+	endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
+						 THC63_LVDS_IN1, -1);
+	of_node_put(endpoint);
+
+	thc63->timings.dual_link = endpoint != NULL;
+	dev_dbg(thc63->dev, "operating in %s-link mode\n",
+		thc63->timings.dual_link ? "dual" : "single");
+
 	return 0;
 }
 
@@ -188,6 +211,7 @@  static int thc63_probe(struct platform_device *pdev)
 	thc63->bridge.driver_private = thc63;
 	thc63->bridge.of_node = pdev->dev.of_node;
 	thc63->bridge.funcs = &thc63_bridge_func;
+	thc63->bridge.timings = &thc63->timings;
 
 	drm_bridge_add(&thc63->bridge);