Re: [PATCH/RFC v3 08/19] video: display: Add MIPI DBI bus support

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Vikas,

On Wednesday 04 September 2013 18:22:45 Vikas Sajjan wrote:
> On 9 August 2013 22:44, Laurent Pinchart wrote:
> > MIPI DBI is a configurable-width parallel display bus that transmits
> > commands and data.
> > 
> > Add a new DBI Linux bus type that implements the usual bus
> > infrastructure (including devices and drivers (un)registration and
> > matching, and bus configuration and access functions).
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/video/display/Kconfig        |   8 ++
> >  drivers/video/display/Makefile       |   1 +
> >  drivers/video/display/mipi-dbi-bus.c | 234 ++++++++++++++++++++++++++++++
> >  include/video/display.h              |   4 +
> >  include/video/mipi-dbi-bus.h         | 125 +++++++++++++++++++
> >  5 files changed, 372 insertions(+)
> >  create mode 100644 drivers/video/display/mipi-dbi-bus.c
> >  create mode 100644 include/video/mipi-dbi-bus.h

[snip]

> > diff --git a/drivers/video/display/mipi-dbi-bus.c
> > b/drivers/video/display/mipi-dbi-bus.c new file mode 100644
> > index 0000000..791fb4d
> > --- /dev/null
> > +++ b/drivers/video/display/mipi-dbi-bus.c

[snip]

> > +/* ----------------------------------------------------------------------
> > + * Bus operations
> > + */
> > +
> > +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int
> > width)
> > +{
> > +       if (width != 8 && width != 16)
> > +               return -EINVAL;
> > +
> > +       dev->data_width = width;
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(mipi_dbi_set_data_width);
> > +
> > +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd)
> > +{
> > +       return dev->bus->ops->write_command(dev->bus, dev, cmd);
> 
> Can you help me in pointing out where these function pointer
> (ops->write_command) are assigned in case MIPI DBI.
> 
> In case of exynos (drivers/video/exynos/exynos_mipi_dsi.c), we
> assign them as below and register DSI as a platform device
> 
>  static struct mipi_dsim_master_ops master_ops = {
>         .cmd_read                       = exynos_mipi_dsi_rd_data,
>         .cmd_write                      = exynos_mipi_dsi_wr_data,
>         .get_dsim_frame_done            =
> exynos_mipi_dsi_get_frame_done_status,
>         .clear_dsim_frame_done          = exynos_mipi_dsi_clear_frame_done,
>         .set_early_blank_mode           = exynos_mipi_dsi_early_blank_mode,
>         .set_blank_mode                 = exynos_mipi_dsi_blank_mode, };
> 
> Since now you are saying to have it as linux BUS, how should we
> register these ops and how we can configure the MIPI DSI hw itself if
> we register it as bus. I could not find any help in mipi-dbi-bus.c,
> how we actually configure the MIPI DBI h/w itself.
> 
> ideally mipi-dbi-bus.c should have done 2 things
> ======================================
> 
> 1. provide a framework to register the DBI kind of panel  = which is
> supported
> 
> 2. provide a framework to register the actuall MIPI DBI H/W itself =
> which i think is missing (correct me, if i am missing anything)

Indeed, you're right... I'll go hide in a dark corner now.

Thinking about it, I should instead implement the missing code, that would 
more helpful.

The patch is missing MIPI DBI bus registration. If you have already written 
bus registration code feel free to send a patch, otherwise I'll fix it (I'll 
wait for your confirmation first).

> > +}

-- 
Regards,

Laurent Pinchart