Re: [PATCH v1 2/2] drm: bridge: add support for lontium LT9611UXC bridge

Laurent Pinchart
2020-08-28T15:05:10Z

On Fri, Aug 28, 2020 at 05:33:00PM +0300, Laurent Pinchart wrote:
> On Fri, Aug 28, 2020 at 07:48:48PM +0530, Vinod Koul wrote:
> > On 28-08-20, 15:04, Dmitry Baryshkov wrote:
> >
> > > +#define EDID_BLOCK_SIZE 128
> > > +#define EDID_NUM_BLOCKS 2
> >
> > tab or space either one, not both ;)
> >
> > > +static struct mipi_dsi_device *lt9611uxc_attach_dsi(struct lt9611uxc *lt9611uxc,
> > > + struct device_node *dsi_node)
> >
> > Please align this with open parenthesis of preceding line (checkpatch
> > with --strict option will check this)
> >
> > > +static int lt9611uxc_bridge_attach(struct drm_bridge *bridge,
> > > + enum drm_bridge_attach_flags flags)
> > > +{
> > > + struct lt9611uxc *lt9611uxc = bridge_to_lt9611uxc(bridge);
> > > + int ret;
> > > +
> > > + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> > > + dev_err(lt9611uxc->dev, "Fix bridge driver to make connector optional!");
> >
> > Can we support both modes as I have done in lt9611, that way once the
> > conversion is done we can drop the init part and support conversion.
>
> I was going to mention that :-) New drivers should support the
> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.

Please ignore this comment, I just realized that the driver supports
DRM_BRIDGE_ATTACH_NO_CONNECTOR, it's the !DRM_BRIDGE_ATTACH_NO_CONNECTOR
case that is not supported, and that's totally fine.

> > I have patch for msm driver to set DRM_BRIDGE_ATTACH_NO_CONNECTOR, you
> > can use that to test
> >
> > > +static int lt9611uxc_hdmi_hw_params(struct device *dev, void *data,
> > > + struct hdmi_codec_daifmt *fmt,
> > > + struct hdmi_codec_params *hparms)
> > > +{
> > > + /*
> > > + * LT9611UXC will automatically detect rate and sample size, so no need
> > > + * to setup anything here.
> > > + */
> > > + return 0;
> > > +}
> >
> > Do we need dummy function?

--
Regards,

Laurent Pinchart