On Thu, 22 Aug 2019 19:54:56 +0300
Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
> 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@xxxxxxxxxxxxxxxx>
> > > ---
> > > 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@xxxxxx>
> > > */
> > >
> > > -#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) ?
Well, this conversion is likely to take time, not to mention that other
drivers will go through the same process. And every time a bridge
driver is converted, it requires patching all display controller drivers
that are known to be connected to it before you can get rid of this
temporary solution. So yes, I still think it's worth adding those
helpers, maybe with a comment explaining that they should only be used
during the conversion phase (IOW, until the driver starts rejecting
the !DRM_BRIDGE_ATTACH_NO_CONNECTOR case).
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
