Re: [GIT PULL FOR v3.19] R-Car DU changes

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

Hi Daniel,

Thank you for the review.

On Friday 07 November 2014 10:19:14 Daniel Vetter wrote:
> On Fri, Nov 07, 2014 at 08:25:32AM +0200, Laurent Pinchart wrote:
> > Hi Dave,
> > 
> > Here's a pull request that adds HDMI support to the R-Car DU driver,
> > including a new slave encoder driver for the adv7511.
> > 
> > The branch is based on a merge of drm-next and Simon's
> > tags/renesas-dt-du-for- v3.19 available at
> > git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git, for which
> > a pull request has been submitted for v3.19 to the ARM SoC maintainers.
> > 
> > I'm leaving now for two weeks of holidays and would like to avoid missing
> > the merge window. Simon, could you please confirm that
> > tags/renesas-dt-du-for- v3.19 is stable and ack this pull request (and
> > possibly ping Dave on my behalf ;-)) ?
> > 
> > The following changes since commit 
19de43d4dea8712b3574d72d3d8aa16cd17e7fed:
> >   Merge tag 'tags/renesas-dt-du-for-v3.19' into drm/next/adv7511-base
> > 
> > (2014-11-07 08:06:16 +0200)
> > 
> > are available in the git repository at:
> >   git://linuxtv.org/pinchartl/fbdev.git
> > 
> > for you to fetch changes up to cc47927ca851110d2c387a7d9fd7086112abb92e:
> >   drm: Add adv7511 encoder driver (2014-11-07 08:19:06 +0200)
> > 
> > ----------------------------------------------------------------
> > 
> > Lars-Peter Clausen (2):
> >       drm: Decouple EDID parsing from I2C adapter
> 
> So somehow the pull request seems to lack the branch so I can't actually
> take a lookc.

git request-pull is playing trick with me :-/ I'll try to fix that.

The branch is available at

        git://linuxtv.org/pinchartl/fbdev.git drm/next/du

(http://git.linuxtv.org/cgit.cgi/pinchartl/fbdev.git/commit/?h=drm/next/du)

> But this patch here seems to lack review:
> - kerneldoc for the newly exported function is missing

I'll fix that.

> - the interface looks rather backwards: Either this still does i2c reads,
>   and then you'd just need a i2c-over-whatever adapter to make it work. Or
>   you have other magic means to optain an edid block, in which case just
>   do that and then feed the edid drm_add_edid_modes.

I have a magic way to get EDID over I2C :-) Basically the ADV7511 controls the 
DDC bus, and exposes EDID data over I2C using vendor commands. To read an EDID 
block I have to write an ADV7511 register over I2C with the block number, wait 
for an interrupt, read a status register to check whether EDID data is 
available or whether an error occurred, and then read EDID data from the 
ADV7511 over I2C in 64-bytes chunks. This needs to be repeated for every 
block. I thus can't use drm_get_edid() directly.

> Nack from me in this form. And can we try not to be quite this sneaky with
> drm core patches going in through driver trees, please?
> 
> Thanks, Daniel
> 
> >       drm: Add adv7511 encoder driver
> > 
> > Laurent Pinchart (6):
> >       drm: rcar-du: Remove platform data support
> >       drm: rcar-du: Pass the encoder DT node to rcar_du_encoder_init()
> >       drm: rcar-du: Replace direct DRM encoder access with cast macro
> >       drm: rcar-du: Replace drm_encoder with drm_slave_encoder
> >       drm: rcar-du: Add HDMI encoder and connector support
> >       video: Add ADV751[13] DT bindings documentation
> >  
> >  Documentation/devicetree/bindings/video/adi,adv7511.txt |   88 ++
> >  drivers/gpu/drm/drm_edid.c                              |   27 +-
> >  drivers/gpu/drm/i2c/Kconfig                             |    6 +
> >  drivers/gpu/drm/i2c/Makefile                            |    2 +
> >  drivers/gpu/drm/i2c/adv7511.c                           | 1010 ++++++++++
> >  drivers/gpu/drm/i2c/adv7511.h                           |  289 +++++++
> >  drivers/gpu/drm/rcar-du/Kconfig                         |   11 +-
> >  drivers/gpu/drm/rcar-du/Makefile                        |    2 +
> >  drivers/gpu/drm/rcar-du/rcar_du_crtc.h                  |   10 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c                   |    4 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_drv.h                   |    2 -
> >  drivers/gpu/drm/rcar-du/rcar_du_encoder.c               |   45 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_encoder.h               |   23 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c               |  118 +++
> >  drivers/gpu/drm/rcar-du/rcar_du_hdmicon.h               |   31 +
> >  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c               |  151 ++++
> >  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.h               |   35 +
> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c                   |   53 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c               |   31 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h               |    2 -
> >  drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h               |    1 -
> >  drivers/gpu/drm/rcar-du/rcar_du_vgacon.c                |    5 +-
> >  include/drm/drm_edid.h                                  |    5 +
> >  include/linux/platform_data/rcar-du.h                   |   74 --
> >  24 files changed, 1845 insertions(+), 180 deletions(-)
> >  create mode 100644
> >  Documentation/devicetree/bindings/video/adi,adv7511.txt
> >  create mode 100644 drivers/gpu/drm/i2c/adv7511.c
> >  create mode 100644 drivers/gpu/drm/i2c/adv7511.h
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmicon.h
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.h
> >  delete mode 100644 include/linux/platform_data/rcar-du.h

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel