[PATCH v2 15/50] drm/bridge: tfp410: Replace manual connector handling with bridge

[Laurent Pinchart]

Hi Boris,

On Thu, Aug 22, 2019 at 06:36:45PM +0200, Boris Brezillon wrote:
> On Tue, 20 Aug 2019 04:16:46 +0300 Laurent Pinchart wrote:
> 
> > Now that a driver is available for display connectors, replace the
> > manual connector handling code with usage of the DRM bridge API. The
> > tfp410 driver doesn't deal with the display connector directly anymore,
> > but still delegates drm_connector operations to the next bridge. This
> > brings us one step closer to having the tfp410 driver handling the
> > TFP410 only.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  drivers/gpu/drm/bridge/ti-tfp410.c | 195 ++++++++++-------------------
> >  1 file changed, 68 insertions(+), 127 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> > index 4a468f44ef69..65651ae6c553 100644
> > --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> > @@ -4,14 +4,12 @@
> >   * Author: Jyri Sarha <jsarha at ti.com>
> >   */
> >  
> > -#include <linux/delay.h>
> > -#include <linux/fwnode.h>
> >  #include <linux/gpio/consumer.h>
> >  #include <linux/i2c.h>
> > -#include <linux/irq.h>
> >  #include <linux/module.h>
> >  #include <linux/of_graph.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/workqueue.h>
> >  
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_bridge.h>
> > @@ -24,16 +22,13 @@
> >  struct tfp410 {
> >  	struct drm_bridge	bridge;
> >  	struct drm_connector	connector;
> > -	unsigned int		connector_type;
> >  
> >  	u32			bus_format;
> > -	struct i2c_adapter	*ddc;
> > -	struct gpio_desc	*hpd;
> > -	int			hpd_irq;
> >  	struct delayed_work	hpd_work;
> >  	struct gpio_desc	*powerdown;
> >  
> >  	struct drm_bridge_timings timings;
> > +	struct drm_bridge	*next_bridge;
> >  
> >  	struct device *dev;
> >  };
> > @@ -56,10 +51,10 @@ static int tfp410_get_modes(struct drm_connector *connector)
> >  	struct edid *edid;
> >  	int ret;
> >  
> > -	if (!dvi->ddc)
> > +	if (!(dvi->next_bridge->ops & DRM_BRIDGE_OP_EDID))
> >  		goto fallback;
> >  
> > -	edid = drm_get_edid(connector, dvi->ddc);
> > +	edid = dvi->next_bridge->funcs->get_edid(dvi->next_bridge, connector);
> 
> Can we create a drm_bridge_get_edid() wrapper for that?
> Something like:
> 
> int drm_bridge_get_edid(struct drm_bridge *bridge,
> 			struct drm_connector *conn)
> {
> 	if (!(dvi->next_bridge->ops & DRM_BRIDGE_OP_EDID))
> 		return -ENOTSUPP;

I assume you mean ERR_PTR(-ENOTSUPP) with the return type changed to
struct drm_edid *.

> 
> 	return bridge->funcs->get_edid(bridge, connector);
> }

I've thought about it, but I'm not sure it's worth it. These operations
are not meant to be called manually by bridges like in here. This is an
interim solution until support for drm_connector can be dropped from the
tfp410 driver, once its users will be converted. Do you think I should
still create a wrapper (which I would make static inline then) ?

> >  	if (!edid) {
> >  		dev_info(dvi->dev,
> >  			 "EDID read failed. Fallback to standard modes\n");
> > @@ -93,21 +88,10 @@ tfp410_connector_detect(struct drm_connector *connector, bool force)
> >  {
> >  	struct tfp410 *dvi = drm_connector_to_tfp410(connector);
> >  
> > -	if (dvi->hpd) {
> > -		if (gpiod_get_value_cansleep(dvi->hpd))
> > -			return connector_status_connected;
> > -		else
> > -			return connector_status_disconnected;
> > -	}
> > +	if (!(dvi->next_bridge->ops & DRM_BRIDGE_OP_DETECT))
> > +		return connector_status_unknown;
> >  
> > -	if (dvi->ddc) {
> > -		if (drm_probe_ddc(dvi->ddc))
> > -			return connector_status_connected;
> > -		else
> > -			return connector_status_disconnected;
> > -	}
> > -
> > -	return connector_status_unknown;
> > +	return dvi->next_bridge->funcs->detect(dvi->next_bridge);
> 
> Same here for the detect hook.
> 

-- 
Regards,

Laurent Pinchart