Re: [PATCH v3 13/21] drm/bridge: megachips: add helper to create connector

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jul 11, 2020 at 01:34:30AM +0300, Laurent Pinchart wrote:
> Hi Sam,
> 
> Thank you for the patch.
> 
> On Fri, Jul 03, 2020 at 09:24:09PM +0200, Sam Ravnborg wrote:
> > Factor out connector creation to a small helper function.
> > 
> > Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> > Cc: Peter Senna Tschudin <peter.senna@xxxxxxxxx>
> > Cc: Martin Donnelly <martin.donnelly@xxxxxx>
> > Cc: Martyn Welch <martyn.welch@xxxxxxxxxxxxxxx>
> > Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> > Cc: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> > Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Cc: Jonas Karlman <jonas@xxxxxxxxx>
> > Cc: Jernej Skrabec <jernej.skrabec@xxxxxxxx>
> > ---
> >  .../bridge/megachips-stdpxxxx-ge-b850v3-fw.c  | 47 +++++++++++--------
> >  1 file changed, 27 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> > index 6200f12a37e6..258e0525cdcc 100644
> > --- a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> > +++ b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> > @@ -191,6 +191,32 @@ static const struct drm_connector_funcs ge_b850v3_lvds_connector_funcs = {
> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> >  };
> >  
> > +static int ge_b850v3_lvds_create_connector(struct drm_bridge *bridge)
> > +{
> > +	struct drm_connector *connector = &ge_b850v3_lvds_ptr->connector;
> 
> Wow, storing device state in a global variable... :-( How did this go
> past review ?
Took a short look at eliminating this.
But there are two module entries involved so I droppet it again.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> 
> > +	int ret;
> > +
> > +	if (!bridge->encoder) {
> > +		DRM_ERROR("Parent encoder object not found");
> > +		return -ENODEV;
> > +	}
> > +
> > +	connector->polled = DRM_CONNECTOR_POLL_HPD;
> > +
> > +	drm_connector_helper_add(connector,
> > +				 &ge_b850v3_lvds_connector_helper_funcs);
> > +
> > +	ret = drm_connector_init(bridge->dev, connector,
> > +				 &ge_b850v3_lvds_connector_funcs,
> > +				 DRM_MODE_CONNECTOR_DisplayPort);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to initialize connector with drm\n");
> > +		return ret;
> > +	}
> > +
> > +	return drm_connector_attach_encoder(connector, bridge->encoder);
> > +}
> > +
> >  static irqreturn_t ge_b850v3_lvds_irq_handler(int irq, void *dev_id)
> >  {
> >  	struct i2c_client *stdp4028_i2c
> > @@ -209,7 +235,6 @@ static irqreturn_t ge_b850v3_lvds_irq_handler(int irq, void *dev_id)
> >  static int ge_b850v3_lvds_attach(struct drm_bridge *bridge,
> >  				 enum drm_bridge_attach_flags flags)
> >  {
> > -	struct drm_connector *connector = &ge_b850v3_lvds_ptr->connector;
> >  	struct i2c_client *stdp4028_i2c
> >  			= ge_b850v3_lvds_ptr->stdp4028_i2c;
> >  	int ret;
> > @@ -219,25 +244,7 @@ static int ge_b850v3_lvds_attach(struct drm_bridge *bridge,
> >  		return -EINVAL;
> >  	}
> >  
> > -	if (!bridge->encoder) {
> > -		DRM_ERROR("Parent encoder object not found");
> > -		return -ENODEV;
> > -	}
> > -
> > -	connector->polled = DRM_CONNECTOR_POLL_HPD;
> > -
> > -	drm_connector_helper_add(connector,
> > -				 &ge_b850v3_lvds_connector_helper_funcs);
> > -
> > -	ret = drm_connector_init(bridge->dev, connector,
> > -				 &ge_b850v3_lvds_connector_funcs,
> > -				 DRM_MODE_CONNECTOR_DisplayPort);
> > -	if (ret) {
> > -		DRM_ERROR("Failed to initialize connector with drm\n");
> > -		return ret;
> > -	}
> > -
> > -	ret = drm_connector_attach_encoder(connector, bridge->encoder);
> > +	ret = ge_b850v3_lvds_create_connector(bridge);
> >  	if (ret)
> >  		return ret;
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux