[GIT PULL FOR v4.12] Renesas R-Car Gen3 DU HDMI support

[GIT PULL FOR v4.12] Renesas R-Car Gen3 DU HDMI support

[Laurent Pinchart]

Hi Dave,

The following changes since commit e1b489d207c73e67810659a88c45b8db4bd62773:

  Merge tag 'omapdrm-4.12' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux into drm-next 
(2017-04-04 05:45:49 +1000)

are available in the git repository at:

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

for you to fetch changes up to 0dda563e571093f309d597cafaf7dd535496ecfb:

  drm: rcar-du: Add HDMI outputs to R8A7795 device description (2017-04-04 
17:04:21 +0300)

Note that the series contains 2 drm-panel patches since I need those to 
unblock the rest of the rcar-du patches.

----------------------------------------------------------------
Jacopo Mondi (1):
      drm: rcar-du: Make sure the VSP is initialized on platforms that need it

Koji Matsuoka (3):
      drm: rcar-du: Add Gen3 HDMI encoder support
      drm: rcar-du: Add DPLL support
      drm: rcar-du: Add HDMI outputs to R8A7795 device description

Laurent Pinchart (17):
      devicetree/bindings: display: Document common panel properties
      devicetree/bindings: display: Add bindings for LVDS panels
      devicetree/bindings: display: Add bindings for two Mitsubishi panels
      drm: Add data transmission order bus flag
      drm: panels: Add LVDS panel driver
      drm: rcar-du: Switch to encoder .atomic_mode_set() helper function
      drm: rcar-du: Handle event when disabling CRTCs
      drm: rcar-du: Clear handled event pointer in CRTC state
      drm: rcar-du: Use DRM core's atomic commit helper
      drm: rcar-du: Remove wait field from rcar_du_device structure
      drm: rcar-du: Document the vsps property in the DT bindings
      drm: rcar-du: Use the DRM panel API
      drm: rcar-du: Add support for LVDS mode selection
      drm: rcar-du: Replace manual bridge implementation with DRM bridge
      drm: rcar-du: Hardcode encoders types to DRM_MODE_ENCODER_NONE
      dt-bindings: display: renesas: Add R-Car Gen3 HDMI TX DT bindings
      drm: rcar-du: Skip disabled outputs

Wolfram Sang (1):
      drm: rcar-du: Don't open code of_device_get_match_data()

 .../bindings/display/bridge/renesas,dw-hdmi.txt         |  75 +++++++
 .../bindings/display/panel/mitsubishi,aa104xd12.txt     |  47 ++++
 .../bindings/display/panel/mitsubishi,aa121td01.txt     |  47 ++++
 .../devicetree/bindings/display/panel/panel-common.txt  |  91 ++++++++
 .../devicetree/bindings/display/panel/panel-lvds.txt    | 120 ++++++++++
 .../devicetree/bindings/display/renesas,du.txt          |   3 +
 MAINTAINERS                                             |   1 +
 drivers/gpu/drm/panel/Kconfig                           |  10 +
 drivers/gpu/drm/panel/Makefile                          |   1 +
 drivers/gpu/drm/panel/panel-lvds.c                      | 286 +++++++++++++++
 drivers/gpu/drm/rcar-du/Kconfig                         |  10 +-
 drivers/gpu/drm/rcar-du/Makefile                        |   6 +-
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c                  |  94 +++++++-
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h                  |   4 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c                   |  32 +--
 drivers/gpu/drm/rcar-du/rcar_du_drv.h                   |   8 +-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c               | 187 ++++++++------
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h               |  14 +-
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c               | 134 -----------
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.h               |  35 ---
 drivers/gpu/drm/rcar-du/rcar_du_kms.c                   | 143 ++----------
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c               |  68 ++----
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c               |  11 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h               |  13 ++
 drivers/gpu/drm/rcar-du/rcar_du_regs.h                  |  23 ++
 drivers/gpu/drm/rcar-du/rcar_du_vgacon.c                |  82 -------
 drivers/gpu/drm/rcar-du/rcar_du_vgacon.h                |  23 --
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h                   |   2 +-
 drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c                  | 100 +++++++++
 include/drm/drm_connector.h                             |   4 +
 30 files changed, 1109 insertions(+), 565 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt
 create mode 100644 
Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
 create mode 100644 
Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-
common.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-
lvds.txt
 create mode 100644 drivers/gpu/drm/panel/panel-lvds.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.h
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_vgacon.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_vgacon.h
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c

-- 
Regards,

Laurent Pinchart

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

Re: [GIT PULL FOR v4.12] Renesas R-Car Gen3 DU HDMI support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1769 bytes --]

On Tue, Apr 04, 2017 at 05:17:44PM +0300, Laurent Pinchart wrote:
> Hi Dave,
> 
> The following changes since commit e1b489d207c73e67810659a88c45b8db4bd62773:
> 
>   Merge tag 'omapdrm-4.12' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux into drm-next 
> (2017-04-04 05:45:49 +1000)
> 
> are available in the git repository at:
> 
>   git://linuxtv.org/pinchartl/media.git drm/next/du
> 
> for you to fetch changes up to 0dda563e571093f309d597cafaf7dd535496ecfb:
> 
>   drm: rcar-du: Add HDMI outputs to R8A7795 device description (2017-04-04 
> 17:04:21 +0300)
> 
> Note that the series contains 2 drm-panel patches since I need those to 
> unblock the rest of the rcar-du patches.
> 
> ----------------------------------------------------------------
> Jacopo Mondi (1):
>       drm: rcar-du: Make sure the VSP is initialized on platforms that need it
> 
> Koji Matsuoka (3):
>       drm: rcar-du: Add Gen3 HDMI encoder support
>       drm: rcar-du: Add DPLL support
>       drm: rcar-du: Add HDMI outputs to R8A7795 device description
> 
> Laurent Pinchart (17):
>       devicetree/bindings: display: Document common panel properties
>       devicetree/bindings: display: Add bindings for LVDS panels
>       devicetree/bindings: display: Add bindings for two Mitsubishi panels
>       drm: Add data transmission order bus flag
>       drm: panels: Add LVDS panel driver

Can you please add an entry to MAINTAINERS for this. We've had our
differences about this and the corresponding device tree bindings, but
it looks as if Rob doesn't have any objections and I've been overruled.
However, since I don't know where you want to take this I'm not going
to be able to do a good job maintaining it.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [GIT PULL FOR v4.12] Renesas R-Car Gen3 DU HDMI support

[Laurent Pinchart]

[-- Attachment #1.1: Type: text/plain, Size: 2381 bytes --]

Hi Thierry,

On Tuesday 04 Apr 2017 17:21:47 Thierry Reding wrote:
> On Tue, Apr 04, 2017 at 05:17:44PM +0300, Laurent Pinchart wrote:
> > Hi Dave,
> > 
> > The following changes since commit 
e1b489d207c73e67810659a88c45b8db4bd62773:
> >   Merge tag 'omapdrm-4.12' of
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux into drm-next
> > (2017-04-04 05:45:49 +1000)
> > 
> > are available in the git repository at:
> >   git://linuxtv.org/pinchartl/media.git drm/next/du
> > 
> > for you to fetch changes up to 0dda563e571093f309d597cafaf7dd535496ecfb:
> >   drm: rcar-du: Add HDMI outputs to R8A7795 device description (2017-04-04
> > 
> > 17:04:21 +0300)
> > 
> > Note that the series contains 2 drm-panel patches since I need those to
> > unblock the rest of the rcar-du patches.
> > 
> > ----------------------------------------------------------------
> > 
> > Jacopo Mondi (1):
> >       drm: rcar-du: Make sure the VSP is initialized on platforms that
> >       need it
> > 
> > Koji Matsuoka (3):
> >       drm: rcar-du: Add Gen3 HDMI encoder support
> >       drm: rcar-du: Add DPLL support
> >       drm: rcar-du: Add HDMI outputs to R8A7795 device description
> > 
> > Laurent Pinchart (17):
> >       devicetree/bindings: display: Document common panel properties
> >       devicetree/bindings: display: Add bindings for LVDS panels
> >       devicetree/bindings: display: Add bindings for two Mitsubishi panels
> >       drm: Add data transmission order bus flag
> >       drm: panels: Add LVDS panel driver
> 
> Can you please add an entry to MAINTAINERS for this. We've had our
> differences about this and the corresponding device tree bindings, but
> it looks as if Rob doesn't have any objections and I've been overruled.
> However, since I don't know where you want to take this I'm not going
> to be able to do a good job maintaining it.

I will do, sorry about missing it.

We certainly have different opinions on some matters related to panels, but 
please be assured that I had, and still have, no goal of overruling you for 
the sake of it. Some people might call me a utopian, but I believe 
collaboration is much better than confrontation :-) I would rather work with 
you on improving panel support in the kernel than working against each other.

-- 
Regards,

Laurent Pinchart

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [GIT PULL FOR v4.12] Renesas R-Car Gen3 DU HDMI support

[Daniel Vetter]

On Tue, Apr 04, 2017 at 06:38:15PM +0300, Laurent Pinchart wrote:
> Hi Thierry,
> 
> On Tuesday 04 Apr 2017 17:21:47 Thierry Reding wrote:
> > On Tue, Apr 04, 2017 at 05:17:44PM +0300, Laurent Pinchart wrote:
> > > Hi Dave,
> > > 
> > > The following changes since commit 
> e1b489d207c73e67810659a88c45b8db4bd62773:
> > >   Merge tag 'omapdrm-4.12' of
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux into drm-next
> > > (2017-04-04 05:45:49 +1000)
> > > 
> > > are available in the git repository at:
> > >   git://linuxtv.org/pinchartl/media.git drm/next/du
> > > 
> > > for you to fetch changes up to 0dda563e571093f309d597cafaf7dd535496ecfb:
> > >   drm: rcar-du: Add HDMI outputs to R8A7795 device description (2017-04-04
> > > 
> > > 17:04:21 +0300)
> > > 
> > > Note that the series contains 2 drm-panel patches since I need those to
> > > unblock the rest of the rcar-du patches.
> > > 
> > > ----------------------------------------------------------------
> > > 
> > > Jacopo Mondi (1):
> > >       drm: rcar-du: Make sure the VSP is initialized on platforms that
> > >       need it
> > > 
> > > Koji Matsuoka (3):
> > >       drm: rcar-du: Add Gen3 HDMI encoder support
> > >       drm: rcar-du: Add DPLL support
> > >       drm: rcar-du: Add HDMI outputs to R8A7795 device description
> > > 
> > > Laurent Pinchart (17):
> > >       devicetree/bindings: display: Document common panel properties
> > >       devicetree/bindings: display: Add bindings for LVDS panels
> > >       devicetree/bindings: display: Add bindings for two Mitsubishi panels
> > >       drm: Add data transmission order bus flag
> > >       drm: panels: Add LVDS panel driver
> > 
> > Can you please add an entry to MAINTAINERS for this. We've had our
> > differences about this and the corresponding device tree bindings, but
> > it looks as if Rob doesn't have any objections and I've been overruled.
> > However, since I don't know where you want to take this I'm not going
> > to be able to do a good job maintaining it.
> 
> I will do, sorry about missing it.
> 
> We certainly have different opinions on some matters related to panels, but 
> please be assured that I had, and still have, no goal of overruling you for 
> the sake of it. Some people might call me a utopian, but I believe 
> collaboration is much better than confrontation :-) I would rather work with 
> you on improving panel support in the kernel than working against each other.

Long term it might be good to group-maintain drm_panel within drm-misc.
But we're still figuring this out, and e.g. for drm_bridge (which is
similar situation, but with bigger drivers and more people) we're stil in
the process of figuring out how to best run things. And who all
might/should be listed as reviewers and who should all have commit rights.

Maybe once that has settled a bit more (in 1-2 releases perhaps) we could
try to get a drm_panel group up, maybe together with a bit a discussion
about what drm_panel is all about? At least looking back at some of the
past discussions, slightly misalignment on goals seems like part of the
reasons for disagreement here.

Thoughts?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Laurent Pinchart]

As the DRM LVDS panel driver uses a different approach to DT bindings
compared to what Thierry Reding advocates, add a specific MAINTAINERS
entry to avoid bothering Thierry with requests related to that driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fa8479e1799a..88eaaec064ca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4209,6 +4209,14 @@ F:	drivers/gpu/drm/panel/
 F:	include/drm/drm_panel.h
 F:	Documentation/devicetree/bindings/display/panel/
 
+DRM LVDS PANEL DRIVER
+M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+L:	dri-devel@lists.freedesktop.org
+T:	git git://people.freedesktop.org/~airlied/linux
+S:	Maintained
+F:	drivers/gpu/drm/panel/panel-lvds.c
+F:	Documentation/devicetree/bindings/display/panel/panel-lvds.txt
+
 INTEL DRM DRIVERS (excluding Poulsbo, Moorestown and derivative chipsets)
 M:	Daniel Vetter <daniel.vetter@intel.com>
 M:	Jani Nikula <jani.nikula@linux.intel.com>
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Laurent Pinchart]

Hi Dave,

On Wednesday 05 Apr 2017 09:51:27 Laurent Pinchart wrote:
> As the DRM LVDS panel driver uses a different approach to DT bindings
> compared to what Thierry Reding advocates, add a specific MAINTAINERS
> entry to avoid bothering Thierry with requests related to that driver.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Could you take this patch manually as part of "[GIT PULL FOR v4.12] Renesas R-
Car Gen3 DU HDMI support" or should I resend the pull request ?

> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fa8479e1799a..88eaaec064ca 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4209,6 +4209,14 @@ F:	drivers/gpu/drm/panel/
>  F:	include/drm/drm_panel.h
>  F:	Documentation/devicetree/bindings/display/panel/
> 
> +DRM LVDS PANEL DRIVER
> +M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +L:	dri-devel@lists.freedesktop.org
> +T:	git git://people.freedesktop.org/~airlied/linux
> +S:	Maintained
> +F:	drivers/gpu/drm/panel/panel-lvds.c
> +F:	Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> +
>  INTEL DRM DRIVERS (excluding Poulsbo, Moorestown and derivative chipsets)
>  M:	Daniel Vetter <daniel.vetter@intel.com>
>  M:	Jani Nikula <jani.nikula@linux.intel.com>

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Jani Nikula]

On Wed, 05 Apr 2017, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> Hi Dave,
>
> On Wednesday 05 Apr 2017 09:51:27 Laurent Pinchart wrote:
>> As the DRM LVDS panel driver uses a different approach to DT bindings
>> compared to what Thierry Reding advocates, add a specific MAINTAINERS
>> entry to avoid bothering Thierry with requests related to that driver.
>> 
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> Could you take this patch manually as part of "[GIT PULL FOR v4.12] Renesas R-
> Car Gen3 DU HDMI support" or should I resend the pull request ?

Or send a separate fixes pull for drm-next/-rc1 later?

BR,
Jani.

>
>> ---
>>  MAINTAINERS | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index fa8479e1799a..88eaaec064ca 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4209,6 +4209,14 @@ F:	drivers/gpu/drm/panel/
>>  F:	include/drm/drm_panel.h
>>  F:	Documentation/devicetree/bindings/display/panel/
>> 
>> +DRM LVDS PANEL DRIVER
>> +M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> +L:	dri-devel@lists.freedesktop.org
>> +T:	git git://people.freedesktop.org/~airlied/linux
>> +S:	Maintained
>> +F:	drivers/gpu/drm/panel/panel-lvds.c
>> +F:	Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> +
>>  INTEL DRM DRIVERS (excluding Poulsbo, Moorestown and derivative chipsets)
>>  M:	Daniel Vetter <daniel.vetter@intel.com>
>>  M:	Jani Nikula <jani.nikula@linux.intel.com>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Dave Airlie]

On 5 April 2017 at 16:51, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> As the DRM LVDS panel driver uses a different approach to DT bindings
> compared to what Thierry Reding advocates, add a specific MAINTAINERS
> entry to avoid bothering Thierry with requests related to that driver.

Could you document a bit more in the patch summary the finer points of
panel/dt doctrine,
as I haven't got as much knowledge as I'd like.

Just I believe, Thierry believes.

Otherwise just send a follow up git pull with this patch.

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 3197 bytes --]

On Fri, Apr 07, 2017 at 05:44:15AM +1000, Dave Airlie wrote:
> On 5 April 2017 at 16:51, Laurent Pinchart
> <laurent.pinchart+renesas@ideasonboard.com> wrote:
> > As the DRM LVDS panel driver uses a different approach to DT bindings
> > compared to what Thierry Reding advocates, add a specific MAINTAINERS
> > entry to avoid bothering Thierry with requests related to that driver.
> 
> Could you document a bit more in the patch summary the finer points of
> panel/dt doctrine, as I haven't got as much knowledge as I'd like.
> 
> Just I believe, Thierry believes.

I'm somewhat surprised how we arrived at the current situation. A very
long time ago when we first discussed device tree bindings for panels, a
number of attempts were made to generically describe everything in
device tree. All of those attempts failed because you simply couldn't
describe all of the required properties in DT in a sane way.

Eventually everyone involved agreed that we would have to stick with the
device-specific compatible, and in the best case we would be able to
support many panels with a fairly generic driver. I think we did pretty
well with the panel-simple driver. It started out very simple and then
got improved over time as necessary to deal with more panels. And for
cases where it wasn't suitable we simply added a custom driver. That's a
completely natural way to write drivers. We do the same thing in other
areas, nothing special here.

Ever since the simple-panel binding was introduced, which is now about
3 1/2 years ago, people have kept asking why we couldn't simply put all
data in DT and why kernel drivers had to be modified in order to add
support for a new panel. I kept repeating myself a number of times until
I finally wrote it all up[0], after which it was enough to point people
to it. Still not everyone was convinced, but the people that were there
when we made the decision all agreed that this was still the right thing
to do. So, despite the many complaints I stuck to what we had agreed on
because I am convinced that it is the right thing to do.

Now we have arrived at a point where apparently that decision has been
revoked, and I don't understand what's changed. This puts me in a very
difficult position. All of a sudden it's okay to do what everyone has
been asking for the last three years, and I'm the jerk who told everyone
that it couldn't be done.

Maybe the discussions that we had back at the time are now far enough in
the past that people have forgotten about the earlier failures. I still
don't see how this new panel-lvds would be any more successful in
solving the problems we failed to solve with simple-panel. The issues
are still fundamentally the same. Now if this was a generic driver that
dealt with a different subset of panels because they are different, that
would've been okay with me. What I don't understand is why this has to
deviate from the simple-panel binding in fundamental ways. Now we've got
two bindings and we make life miserable for people because they have to
choose between the two.

Thierry

[0]: https://sietch-tagr.blogspot.de/2016/04/display-panels-are-not-special.html

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [GIT PULL FOR v4.12] Renesas R-Car Gen3 DU HDMI support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 3679 bytes --]

On Tue, Apr 04, 2017 at 06:03:13PM +0200, Daniel Vetter wrote:
> On Tue, Apr 04, 2017 at 06:38:15PM +0300, Laurent Pinchart wrote:
> > Hi Thierry,
> > 
> > On Tuesday 04 Apr 2017 17:21:47 Thierry Reding wrote:
> > > On Tue, Apr 04, 2017 at 05:17:44PM +0300, Laurent Pinchart wrote:
> > > > Hi Dave,
> > > > 
> > > > The following changes since commit 
> > e1b489d207c73e67810659a88c45b8db4bd62773:
> > > >   Merge tag 'omapdrm-4.12' of
> > > > 
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux into drm-next
> > > > (2017-04-04 05:45:49 +1000)
> > > > 
> > > > are available in the git repository at:
> > > >   git://linuxtv.org/pinchartl/media.git drm/next/du
> > > > 
> > > > for you to fetch changes up to 0dda563e571093f309d597cafaf7dd535496ecfb:
> > > >   drm: rcar-du: Add HDMI outputs to R8A7795 device description (2017-04-04
> > > > 
> > > > 17:04:21 +0300)
> > > > 
> > > > Note that the series contains 2 drm-panel patches since I need those to
> > > > unblock the rest of the rcar-du patches.
> > > > 
> > > > ----------------------------------------------------------------
> > > > 
> > > > Jacopo Mondi (1):
> > > >       drm: rcar-du: Make sure the VSP is initialized on platforms that
> > > >       need it
> > > > 
> > > > Koji Matsuoka (3):
> > > >       drm: rcar-du: Add Gen3 HDMI encoder support
> > > >       drm: rcar-du: Add DPLL support
> > > >       drm: rcar-du: Add HDMI outputs to R8A7795 device description
> > > > 
> > > > Laurent Pinchart (17):
> > > >       devicetree/bindings: display: Document common panel properties
> > > >       devicetree/bindings: display: Add bindings for LVDS panels
> > > >       devicetree/bindings: display: Add bindings for two Mitsubishi panels
> > > >       drm: Add data transmission order bus flag
> > > >       drm: panels: Add LVDS panel driver
> > > 
> > > Can you please add an entry to MAINTAINERS for this. We've had our
> > > differences about this and the corresponding device tree bindings, but
> > > it looks as if Rob doesn't have any objections and I've been overruled.
> > > However, since I don't know where you want to take this I'm not going
> > > to be able to do a good job maintaining it.
> > 
> > I will do, sorry about missing it.
> > 
> > We certainly have different opinions on some matters related to panels, but 
> > please be assured that I had, and still have, no goal of overruling you for 
> > the sake of it. Some people might call me a utopian, but I believe 
> > collaboration is much better than confrontation :-) I would rather work with 
> > you on improving panel support in the kernel than working against each other.
> 
> Long term it might be good to group-maintain drm_panel within drm-misc.
> But we're still figuring this out, and e.g. for drm_bridge (which is
> similar situation, but with bigger drivers and more people) we're stil in
> the process of figuring out how to best run things. And who all
> might/should be listed as reviewers and who should all have commit rights.

Group maintainership seems like a good idea.

> Maybe once that has settled a bit more (in 1-2 releases perhaps) we could
> try to get a drm_panel group up, maybe together with a bit a discussion
> about what drm_panel is all about? At least looking back at some of the
> past discussions, slightly misalignment on goals seems like part of the
> reasons for disagreement here.

I'm not sure what you mean by misalignment on goals. I think the goal is
pretty well defined. If it isn't, I think we need to rectify that, and
the best way to do so seems to be to add better documentation.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Emil Velikov]

Hi Thierry,

I don't mean to stir up anything, just voicing "my 2c" as they say.

On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:

> Ever since the simple-panel binding was introduced, which is now about
> 3 1/2 years ago,

Do you have a link to these discussions? Your blog article does not
have any links and I only found the "Runtime Interpreted Power
Sequences" thread.
That in itself does not cover the pros/cons of storing HW information*
within DT.

Personally, the idea of having hardware information* in DT does not
sound all that bad. The simple panel driver(s) can use those
properties and any panels that require anything more complex will
still need their own driver.
For better or for worse, there's already a handful of drivers and
bindings that rely on/provide these. Using that information
consistently across the board, would be of a benefit, IMHO.

But at the end of the day, might as well go with whatever RobH and the
DT folk are happy with. It's on them to maintain the DT ABI ;-)

Regards,
Emil

* Display dimensions, display timings and even delays for the power
sequence. Not too sure on that last one though.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 3325 bytes --]

On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
> Hi Thierry,
> 
> I don't mean to stir up anything, just voicing "my 2c" as they say.
> 
> On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
> 
> > Ever since the simple-panel binding was introduced, which is now about
> > 3 1/2 years ago,
> 
> Do you have a link to these discussions? Your blog article does not
> have any links and I only found the "Runtime Interpreted Power
> Sequences" thread.
> That in itself does not cover the pros/cons of storing HW information*
> within DT.

There's some discussion here:

	https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html

which continues here:

	https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html

There are a couple of earlier threads, though, that discuss similar
issues. This one seems to be the earliest I can find that is publicly
archived:

	http://www.spinics.net/lists/devicetree/msg08497.html

Going over all these threads again wasn't a very pleasant experience. I
realize how much time I already spent discussing these, and I don't have
any desire to repeat that discussion.

We've had these differences ever since the very beginning. So we're now
again going in circles.

The main concern back at the time was that having to specify timings in
the driver would result in a complete mess because we have zillions of
panels that we need to support. That's turned out to be a complete non-
issue. We've got something on the order of 50 or 60 drivers supported in
the simple-panel driver, and for everything that's more complicated we
have a handful of separate drivers, all fairly simple as well.

So while I understand why people want to put all this information into
DT, we've repeatedly discussed the disadvantages that this would have.
And while we were never able to get everyone to agree, the current
solution has had enough agreement that we merged it. And it turned out
to be good enough. There's nothing in panel-lvds that I can see that
fundamentally changes this.

> Personally, the idea of having hardware information* in DT does not
> sound all that bad. The simple panel driver(s) can use those
> properties and any panels that require anything more complex will
> still need their own driver.

Again, the point is that you're going to have to modify the driver in
any case, because you need to support the new compatible string. Without
that compatible string you have zero information about the panel, and
matching on a generic one isn't going to give you a working panel. So if
you're already going to have to support a panel in a driver, why not go
all the way and fully describe its capabilities and properties? We do it
for all other devices. Panels are not at all special.

> For better or for worse, there's already a handful of drivers and
> bindings that rely on/provide these. Using that information
> consistently across the board, would be of a benefit, IMHO.

Would you mind pointing out which ones these are? I'm aware of only a
couple that seemed to have sneeked in because people were trying to
side-step adding drm/panel support for their boards, so I don't think
that qualifies as a reason to rethink how drm/panel works.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Laurent Pinchart]

Hi Thierry,

On Monday 10 Apr 2017 09:17:59 Thierry Reding wrote:
> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
> > Hi Thierry,
> > 
> > I don't mean to stir up anything, just voicing "my 2c" as they say.
> > 
> > On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
> > > Ever since the simple-panel binding was introduced, which is now about
> > > 3 1/2 years ago,
> > 
> > Do you have a link to these discussions? Your blog article does not
> > have any links and I only found the "Runtime Interpreted Power
> > Sequences" thread.
> > That in itself does not cover the pros/cons of storing HW information*
> > within DT.
> 
> There's some discussion here:
> 
> 	https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html
> 
> which continues here:
> 
> 	https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html
> 
> There are a couple of earlier threads, though, that discuss similar
> issues. This one seems to be the earliest I can find that is publicly
> archived:
> 
> 	http://www.spinics.net/lists/devicetree/msg08497.html
> 
> Going over all these threads again wasn't a very pleasant experience. I
> realize how much time I already spent discussing these, and I don't have
> any desire to repeat that discussion.
> 
> We've had these differences ever since the very beginning. So we're now
> again going in circles.
> 
> The main concern back at the time was that having to specify timings in
> the driver would result in a complete mess because we have zillions of
> panels that we need to support. That's turned out to be a complete non-
> issue. We've got something on the order of 50 or 60 drivers supported in
> the simple-panel driver, and for everything that's more complicated we
> have a handful of separate drivers, all fairly simple as well.
> 
> So while I understand why people want to put all this information into
> DT, we've repeatedly discussed the disadvantages that this would have.
> And while we were never able to get everyone to agree, the current
> solution has had enough agreement that we merged it. And it turned out
> to be good enough. There's nothing in panel-lvds that I can see that
> fundamentally changes this.
> 
> > Personally, the idea of having hardware information* in DT does not
> > sound all that bad. The simple panel driver(s) can use those
> > properties and any panels that require anything more complex will
> > still need their own driver.
> 
> Again, the point is that you're going to have to modify the driver in
> any case, because you need to support the new compatible string. Without
> that compatible string you have zero information about the panel, and
> matching on a generic one isn't going to give you a working panel.

It will *if* the panel doesn't need any device-specific handling. In all other 
cases I agree with you, panel-specific code will be needed in the kernel (to 
handle power sequences for instance).

> So if you're already going to have to support a panel in a driver, why not
> go all the way and fully describe its capabilities and properties? We do it
> for all other devices. Panels are not at all special.

That we agree on, panels are not special. They're not the only devices that 
store in DT information that could be hardcoded in the driver based on the 
compatible string. We have many devices whose compatible string contains the 
SoC version. Driver could then hardcode interrupts or clocks without any need 
to specific them in DT. We don't do that as it would be more complex to 
handle.

Regarding timings, I've long hesitated (albeit I confess I was more on the 
side of specifying them in DT) until Rob Herring convinced me with the generic 
rule that adding information in DT that are generally exposed by devices 
themselves makes sense. Displays traditionally expose video mode information 
through EDID, which is effectively device firmware. When a panel is integrated 
in a system, and the system designer decides to save money by removing the 
EDID I2C EEPROM, moving that piece of device firmware data to system firmware 
makes sense to me.

I certainly won't try to revive the power sequence discussions, I don't 
believe it belongs to DT.

> > For better or for worse, there's already a handful of drivers and
> > bindings that rely on/provide these. Using that information
> > consistently across the board, would be of a benefit, IMHO.
> 
> Would you mind pointing out which ones these are? I'm aware of only a
> couple that seemed to have sneeked in because people were trying to
> side-step adding drm/panel support for their boards, so I don't think
> that qualifies as a reason to rethink how drm/panel works.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Lucas Stach]

Am Montag, den 10.04.2017, 09:17 +0200 schrieb Thierry Reding:
> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
> > Hi Thierry,
> > 
> > I don't mean to stir up anything, just voicing "my 2c" as they say.

I normally try to keep away from this "political" discussions,
especially with regard to the panel stuff, but as this seems to pop up
again and again, I feel the need to speak up.

> > On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
> > 
> > > Ever since the simple-panel binding was introduced, which is now about
> > > 3 1/2 years ago,
> > 
> > Do you have a link to these discussions? Your blog article does not
> > have any links and I only found the "Runtime Interpreted Power
> > Sequences" thread.
> > That in itself does not cover the pros/cons of storing HW information*
> > within DT.
> 
> There's some discussion here:
> 
> 	https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html
> 
> which continues here:
> 
> 	https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html
> 
> There are a couple of earlier threads, though, that discuss similar
> issues. This one seems to be the earliest I can find that is publicly
> archived:
> 
> 	http://www.spinics.net/lists/devicetree/msg08497.html
> 
> Going over all these threads again wasn't a very pleasant experience. I
> realize how much time I already spent discussing these, and I don't have
> any desire to repeat that discussion.
> 
> We've had these differences ever since the very beginning. So we're now
> again going in circles.
> 
> The main concern back at the time was that having to specify timings in
> the driver would result in a complete mess because we have zillions of
> panels that we need to support. That's turned out to be a complete non-
> issue. We've got something on the order of 50 or 60 drivers supported in
> the simple-panel driver, and for everything that's more complicated we
> have a handful of separate drivers, all fairly simple as well.
> 
> So while I understand why people want to put all this information into
> DT, we've repeatedly discussed the disadvantages that this would have.
> And while we were never able to get everyone to agree, the current
> solution has had enough agreement that we merged it. And it turned out
> to be good enough. There's nothing in panel-lvds that I can see that
> fundamentally changes this.

I was personally opposed to the idea of specifying panels only by
compatible and carrying around a "modedb" in the form of the
simple-panel driver. Since then I've come to appreciate the flexibility
that having a real driver for a panel provides.

A panel is _much_ more than a simple mode. For LVDS panels it's in many
cases a complete range of timings that will work with the panel, it's
the enable sequencing with regulators and GPIOs, it's the size of the
panel, so we get proper DPI information, it's a bus format describing
all the oddities that vendors choose to implement when designing the
physical interface. It provides proper sequencing of backlight
enable/disable.

I very much support Thierrys view, that all this should be encoded by
the compatible. This allows us to add required properties to the panel
if needed. It certainly saved us a lot of backward compat issues in
imx-drm, that we could just add all the required information to the
panel, so everything is consistent within one kernel version.

> > Personally, the idea of having hardware information* in DT does not
> > sound all that bad. The simple panel driver(s) can use those
> > properties and any panels that require anything more complex will
> > still need their own driver.

Although some people seems to feel different on that matter, but DT is
supposed to be ABI. Newer kernels _must_ work on older DTs. Which means
it needs to encode _all_ information on the panel.

Personally I don't trust people, who are just enabling their one board
to go through and validate that all the panel information is correct. As
soon as a picture shows up they are going to ship the DT.

With having to go through a panel driver, there is at least some basic
review of the panel properties.

Also speaking from experience with a board with different display
configurations, where only the bootloader knows the specific attached
panel, it's very convenient to have the bootloader only patch in the
correct compatible, instead of all the panel properties. If any of those
properties turns out to be wrong, I can fix it with a simple kernel
update, instead of having to update the bootloader.

> Again, the point is that you're going to have to modify the driver in
> any case, because you need to support the new compatible string. Without
> that compatible string you have zero information about the panel, and
> matching on a generic one isn't going to give you a working panel. So if
> you're already going to have to support a panel in a driver, why not go
> all the way and fully describe its capabilities and properties? We do it
> for all other devices. Panels are not at all special.
> 
> > For better or for worse, there's already a handful of drivers and
> > bindings that rely on/provide these. Using that information
> > consistently across the board, would be of a benefit, IMHO.

Yes, some old DTs still provide panel properties. That's no reason to
not use the agreed upon way to describe panels in newer DTs. imx-drm
still provides some support for the old way as this predates the panel
stuff and we take DT backward compat serious, but newer kernels probably
will start to complain about this. Please don't advocate this for new
stuff.

So to sum up, I completely support Thierrys way of handling panels and
IMHO we can already see the brick wall the the "generic" (which is not
generic at all, but very specific in that it doesn't require any of the
handling a panel driver could provide) LVDS panel stuff is going to hit.
Requiring properties "data-mirror" to describe uncommon bit orderings
should be a sufficient warning sign.

Regards,
Lucas

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Emil Velikov]

On 10 April 2017 at 10:58, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Montag, den 10.04.2017, 09:17 +0200 schrieb Thierry Reding:
>> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
>> > Hi Thierry,
>> >
>> > I don't mean to stir up anything, just voicing "my 2c" as they say.
>
> I normally try to keep away from this "political" discussions,
> especially with regard to the panel stuff, but as this seems to pop up
> again and again, I feel the need to speak up.
>
Right, my intent did not plan as expected.

>> > On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
>> >
>> > > Ever since the simple-panel binding was introduced, which is now about
>> > > 3 1/2 years ago,
>> >
>> > Do you have a link to these discussions? Your blog article does not
>> > have any links and I only found the "Runtime Interpreted Power
>> > Sequences" thread.
>> > That in itself does not cover the pros/cons of storing HW information*
>> > within DT.
>>
>> There's some discussion here:
>>
>>       https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html
>>
>> which continues here:
>>
>>       https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html
>>
>> There are a couple of earlier threads, though, that discuss similar
>> issues. This one seems to be the earliest I can find that is publicly
>> archived:
>>
>>       http://www.spinics.net/lists/devicetree/msg08497.html
>>
>> Going over all these threads again wasn't a very pleasant experience. I
>> realize how much time I already spent discussing these, and I don't have
>> any desire to repeat that discussion.
>>
>> We've had these differences ever since the very beginning. So we're now
>> again going in circles.
>>
>> The main concern back at the time was that having to specify timings in
>> the driver would result in a complete mess because we have zillions of
>> panels that we need to support. That's turned out to be a complete non-
>> issue. We've got something on the order of 50 or 60 drivers supported in
>> the simple-panel driver, and for everything that's more complicated we
>> have a handful of separate drivers, all fairly simple as well.
>>
>> So while I understand why people want to put all this information into
>> DT, we've repeatedly discussed the disadvantages that this would have.
>> And while we were never able to get everyone to agree, the current
>> solution has had enough agreement that we merged it. And it turned out
>> to be good enough. There's nothing in panel-lvds that I can see that
>> fundamentally changes this.
>
> I was personally opposed to the idea of specifying panels only by
> compatible and carrying around a "modedb" in the form of the
> simple-panel driver. Since then I've come to appreciate the flexibility
> that having a real driver for a panel provides.
>
> A panel is _much_ more than a simple mode. For LVDS panels it's in many
> cases a complete range of timings that will work with the panel, it's
> the enable sequencing with regulators and GPIOs, it's the size of the
> panel, so we get proper DPI information, it's a bus format describing
> all the oddities that vendors choose to implement when designing the
> physical interface. It provides proper sequencing of backlight
> enable/disable.
>
> I very much support Thierrys view, that all this should be encoded by
> the compatible. This allows us to add required properties to the panel
> if needed. It certainly saved us a lot of backward compat issues in
> imx-drm, that we could just add all the required information to the
> panel, so everything is consistent within one kernel version.
>
>> > Personally, the idea of having hardware information* in DT does not
>> > sound all that bad. The simple panel driver(s) can use those
>> > properties and any panels that require anything more complex will
>> > still need their own driver.
>
> Although some people seems to feel different on that matter, but DT is
> supposed to be ABI. Newer kernels _must_ work on older DTs. Which means
> it needs to encode _all_ information on the panel.
>
> Personally I don't trust people, who are just enabling their one board
> to go through and validate that all the panel information is correct. As
> soon as a picture shows up they are going to ship the DT.
>
> With having to go through a panel driver, there is at least some basic
> review of the panel properties.
>
> Also speaking from experience with a board with different display
> configurations, where only the bootloader knows the specific attached
> panel, it's very convenient to have the bootloader only patch in the
> correct compatible, instead of all the panel properties. If any of those
> properties turns out to be wrong, I can fix it with a simple kernel
> update, instead of having to update the bootloader.
>
>> Again, the point is that you're going to have to modify the driver in
>> any case, because you need to support the new compatible string. Without
>> that compatible string you have zero information about the panel, and
>> matching on a generic one isn't going to give you a working panel. So if
>> you're already going to have to support a panel in a driver, why not go
>> all the way and fully describe its capabilities and properties? We do it
>> for all other devices. Panels are not at all special.
>>
>> > For better or for worse, there's already a handful of drivers and
>> > bindings that rely on/provide these. Using that information
>> > consistently across the board, would be of a benefit, IMHO.
>
> Yes, some old DTs still provide panel properties. That's no reason to
> not use the agreed upon way to describe panels in newer DTs. imx-drm
> still provides some support for the old way as this predates the panel
> stuff and we take DT backward compat serious, but newer kernels probably
> will start to complain about this. Please don't advocate this for new
> stuff.
>
> So to sum up, I completely support Thierrys way of handling panels and
> IMHO we can already see the brick wall the the "generic" (which is not
> generic at all, but very specific in that it doesn't require any of the
> handling a panel driver could provide) LVDS panel stuff is going to hit.
> Requiring properties "data-mirror" to describe uncommon bit orderings
> should be a sufficient warning sign.
>
If i understood things correctly, having the information within the
driver provides an extra amount of
 - verification - as one can simply add the information in DT without
properly checking that the panel works
 - flexibility - fixup the driver, as opposed to trying to fix the DT,
while preserving the ABI.

So, yes, having the timing information within the driver is good IMHO.

My closing thought from earlier was meant as:
If DT people do not see these as an issue and are OK with a)having DT
binding that are broken or b) breaking the DT ABI, so be it :-)

Either way, it's not my call so _please_ do not see any of my
questions as rocking the boat.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Dave Airlie]

On 10 April 2017 at 19:03, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Thierry,
>
> On Monday 10 Apr 2017 09:17:59 Thierry Reding wrote:
>> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
>> > Hi Thierry,
>> >
>> > I don't mean to stir up anything, just voicing "my 2c" as they say.
>> >
>> > On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
>> > > Ever since the simple-panel binding was introduced, which is now about
>> > > 3 1/2 years ago,
>> >
>> > Do you have a link to these discussions? Your blog article does not
>> > have any links and I only found the "Runtime Interpreted Power
>> > Sequences" thread.
>> > That in itself does not cover the pros/cons of storing HW information*
>> > within DT.
>>
>> There's some discussion here:
>>
>>       https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html
>>
>> which continues here:
>>
>>       https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html
>>
>> There are a couple of earlier threads, though, that discuss similar
>> issues. This one seems to be the earliest I can find that is publicly
>> archived:
>>
>>       http://www.spinics.net/lists/devicetree/msg08497.html
>>
>> Going over all these threads again wasn't a very pleasant experience. I
>> realize how much time I already spent discussing these, and I don't have
>> any desire to repeat that discussion.
>>
>> We've had these differences ever since the very beginning. So we're now
>> again going in circles.
>>
>> The main concern back at the time was that having to specify timings in
>> the driver would result in a complete mess because we have zillions of
>> panels that we need to support. That's turned out to be a complete non-
>> issue. We've got something on the order of 50 or 60 drivers supported in
>> the simple-panel driver, and for everything that's more complicated we
>> have a handful of separate drivers, all fairly simple as well.
>>
>> So while I understand why people want to put all this information into
>> DT, we've repeatedly discussed the disadvantages that this would have.
>> And while we were never able to get everyone to agree, the current
>> solution has had enough agreement that we merged it. And it turned out
>> to be good enough. There's nothing in panel-lvds that I can see that
>> fundamentally changes this.
>>
>> > Personally, the idea of having hardware information* in DT does not
>> > sound all that bad. The simple panel driver(s) can use those
>> > properties and any panels that require anything more complex will
>> > still need their own driver.
>>
>> Again, the point is that you're going to have to modify the driver in
>> any case, because you need to support the new compatible string. Without
>> that compatible string you have zero information about the panel, and
>> matching on a generic one isn't going to give you a working panel.
>
> It will *if* the panel doesn't need any device-specific handling. In all other
> cases I agree with you, panel-specific code will be needed in the kernel (to
> handle power sequences for instance).
>
>> So if you're already going to have to support a panel in a driver, why not
>> go all the way and fully describe its capabilities and properties? We do it
>> for all other devices. Panels are not at all special.
>
> That we agree on, panels are not special. They're not the only devices that
> store in DT information that could be hardcoded in the driver based on the
> compatible string. We have many devices whose compatible string contains the
> SoC version. Driver could then hardcode interrupts or clocks without any need
> to specific them in DT. We don't do that as it would be more complex to
> handle.
>
> Regarding timings, I've long hesitated (albeit I confess I was more on the
> side of specifying them in DT) until Rob Herring convinced me with the generic
> rule that adding information in DT that are generally exposed by devices
> themselves makes sense. Displays traditionally expose video mode information
> through EDID, which is effectively device firmware. When a panel is integrated
> in a system, and the system designer decides to save money by removing the
> EDID I2C EEPROM, moving that piece of device firmware data to system firmware
> makes sense to me.
>
> I certainly won't try to revive the power sequence discussions, I don't
> believe it belongs to DT.
>
>> > For better or for worse, there's already a handful of drivers and
>> > bindings that rely on/provide these. Using that information
>> > consistently across the board, would be of a benefit, IMHO.
>>
>> Would you mind pointing out which ones these are? I'm aware of only a
>> couple that seemed to have sneeked in because people were trying to
>> side-step adding drm/panel support for their boards, so I don't think
>> that qualifies as a reason to rethink how drm/panel works.

Okay I'm afraid I agree with Thierry here.

I don't want mode timings or EDID in DT files, I'm pretty sure I was one of the
people who helped decide that just having a compatible string and modes in
a driver makes sense. So if we have imported code to be the opposite of this
please work on removing it.

If you move EDID from over the wire into DT it's not really tied to the panel
anymore, and is now just a complicated way of specifying the modes from the
panel documentation.

EDID and DDC are a way for a panel to communicate to the host system,
timing constraints and mode info, if you remove the i2c link, why bother
encoding stuff in an EDID?

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Laurent Pinchart]

Hi Dave,

(CC'ing Rob)

On Tuesday 11 Apr 2017 05:27:04 Dave Airlie wrote:
> On 10 April 2017 at 19:03, Laurent Pinchart wrote:
> > On Monday 10 Apr 2017 09:17:59 Thierry Reding wrote:
> >> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
> >>> Hi Thierry,
> >>> 
> >>> I don't mean to stir up anything, just voicing "my 2c" as they say.
> >>> 
> >>> On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
> >>>> Ever since the simple-panel binding was introduced, which is now
> >>>> about 3 1/2 years ago,
> >>> 
> >>> Do you have a link to these discussions? Your blog article does not
> >>> have any links and I only found the "Runtime Interpreted Power
> >>> Sequences" thread.
> >>> That in itself does not cover the pros/cons of storing HW information*
> >>> within DT.
> >> 
> >> There's some discussion here:
> >>       https://lists.freedesktop.org/archives/dri-devel/2013-November/0495
> >>       09.html
> >> 
> >> which continues here:
> >>       https://lists.freedesktop.org/archives/dri-devel/2013-December/0500
> >>       82.html
> >> 
> >> There are a couple of earlier threads, though, that discuss similar
> >> issues. This one seems to be the earliest I can find that is publicly
> >> 
> >> archived:
> >>       http://www.spinics.net/lists/devicetree/msg08497.html
> >> 
> >> Going over all these threads again wasn't a very pleasant experience. I
> >> realize how much time I already spent discussing these, and I don't have
> >> any desire to repeat that discussion.
> >> 
> >> We've had these differences ever since the very beginning. So we're now
> >> again going in circles.
> >> 
> >> The main concern back at the time was that having to specify timings in
> >> the driver would result in a complete mess because we have zillions of
> >> panels that we need to support. That's turned out to be a complete non-
> >> issue. We've got something on the order of 50 or 60 drivers supported in
> >> the simple-panel driver, and for everything that's more complicated we
> >> have a handful of separate drivers, all fairly simple as well.
> >> 
> >> So while I understand why people want to put all this information into
> >> DT, we've repeatedly discussed the disadvantages that this would have.
> >> And while we were never able to get everyone to agree, the current
> >> solution has had enough agreement that we merged it. And it turned out
> >> to be good enough. There's nothing in panel-lvds that I can see that
> >> fundamentally changes this.
> >> 
> >>> Personally, the idea of having hardware information* in DT does not
> >>> sound all that bad. The simple panel driver(s) can use those
> >>> properties and any panels that require anything more complex will
> >>> still need their own driver.
> >> 
> >> Again, the point is that you're going to have to modify the driver in
> >> any case, because you need to support the new compatible string. Without
> >> that compatible string you have zero information about the panel, and
> >> matching on a generic one isn't going to give you a working panel.
> > 
> > It will *if* the panel doesn't need any device-specific handling. In all
> > other cases I agree with you, panel-specific code will be needed in the
> > kernel (to handle power sequences for instance).
> > 
> >> So if you're already going to have to support a panel in a driver, why
> >> not go all the way and fully describe its capabilities and properties? We
> >> do it for all other devices. Panels are not at all special.
> > 
> > That we agree on, panels are not special. They're not the only devices
> > that store in DT information that could be hardcoded in the driver based
> > on the compatible string. We have many devices whose compatible string
> > contains the SoC version. Driver could then hardcode interrupts or clocks
> > without any need to specific them in DT. We don't do that as it would be
> > more complex to handle.
> > 
> > Regarding timings, I've long hesitated (albeit I confess I was more on the
> > side of specifying them in DT) until Rob Herring convinced me with the
> > generic rule that adding information in DT that are generally exposed by
> > devices themselves makes sense. Displays traditionally expose video mode
> > information through EDID, which is effectively device firmware. When a
> > panel is integrated in a system, and the system designer decides to save
> > money by removing the EDID I2C EEPROM, moving that piece of device
> > firmware data to system firmware makes sense to me.
> > 
> > I certainly won't try to revive the power sequence discussions, I don't
> > believe it belongs to DT.
> > 
> >>> For better or for worse, there's already a handful of drivers and
> >>> bindings that rely on/provide these. Using that information
> >>> consistently across the board, would be of a benefit, IMHO.
> >> 
> >> Would you mind pointing out which ones these are? I'm aware of only a
> >> couple that seemed to have sneeked in because people were trying to
> >> side-step adding drm/panel support for their boards, so I don't think
> >> that qualifies as a reason to rethink how drm/panel works.
> 
> Okay I'm afraid I agree with Thierry here.
> 
> I don't want mode timings or EDID in DT files, I'm pretty sure I was one of
> the people who helped decide that just having a compatible string and modes
> in a driver makes sense. So if we have imported code to be the opposite of
> this please work on removing it.
> 
> If you move EDID from over the wire into DT it's not really tied to the
> panel anymore, and is now just a complicated way of specifying the modes
> from the panel documentation.
> 
> EDID and DDC are a way for a panel to communicate to the host system,
> timing constraints and mode info, if you remove the i2c link, why bother
> encoding stuff in an EDID?

Re-reading my previous e-mail I now see that I expressed myself poorly. I'm 
not advocating for specifying EDID information in DT, my point (and Rob's 
point) was that adding property to the device tree to store information 
typically provided through EDID is just moving firmware data from one place of 
the system to another, and thus makes sense. That information should certainly 
not be stored as a binary EDID blob, but as native DT properties instead.

The LVDS panel DT bindings don't contain EDID binary data, but contain the 
panel's physical size and video timings.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Laurent Pinchart]

Hi Lucas,

On Monday 10 Apr 2017 11:58:38 Lucas Stach wrote:
> Am Montag, den 10.04.2017, 09:17 +0200 schrieb Thierry Reding:
> > On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
> >> Hi Thierry,
> >> 
> >> I don't mean to stir up anything, just voicing "my 2c" as they say.
> 
> I normally try to keep away from this "political" discussions,
> especially with regard to the panel stuff, but as this seems to pop up
> again and again, I feel the need to speak up.
> 
> >> On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
> >>> Ever since the simple-panel binding was introduced, which is now about
> >>> 3 1/2 years ago,
> >> 
> >> Do you have a link to these discussions? Your blog article does not
> >> have any links and I only found the "Runtime Interpreted Power
> >> Sequences" thread.
> >> That in itself does not cover the pros/cons of storing HW information*
> >> within DT.
> > 
> > There's some discussion here:
> > 	https://lists.freedesktop.org/archives/dri-devel/2013-> > 	November/049509.html
> > 
> > which continues here:
> > 	https://lists.freedesktop.org/archives/dri-devel/2013-> > 	December/050082.html
> > 
> > There are a couple of earlier threads, though, that discuss similar
> > issues. This one seems to be the earliest I can find that is publicly
> > 
> > archived:
> > 	http://www.spinics.net/lists/devicetree/msg08497.html
> > 
> > Going over all these threads again wasn't a very pleasant experience. I
> > realize how much time I already spent discussing these, and I don't have
> > any desire to repeat that discussion.
> > 
> > We've had these differences ever since the very beginning. So we're now
> > again going in circles.
> > 
> > The main concern back at the time was that having to specify timings in
> > the driver would result in a complete mess because we have zillions of
> > panels that we need to support. That's turned out to be a complete non-
> > issue. We've got something on the order of 50 or 60 drivers supported in
> > the simple-panel driver, and for everything that's more complicated we
> > have a handful of separate drivers, all fairly simple as well.
> > 
> > So while I understand why people want to put all this information into
> > DT, we've repeatedly discussed the disadvantages that this would have.
> > And while we were never able to get everyone to agree, the current
> > solution has had enough agreement that we merged it. And it turned out
> > to be good enough. There's nothing in panel-lvds that I can see that
> > fundamentally changes this.
> 
> I was personally opposed to the idea of specifying panels only by
> compatible and carrying around a "modedb" in the form of the
> simple-panel driver. Since then I've come to appreciate the flexibility
> that having a real driver for a panel provides.
> 
> A panel is _much_ more than a simple mode. For LVDS panels it's in many
> cases a complete range of timings that will work with the panel, it's
> the enable sequencing with regulators and GPIOs, it's the size of the
> panel, so we get proper DPI information, it's a bus format describing
> all the oddities that vendors choose to implement when designing the
> physical interface. It provides proper sequencing of backlight
> enable/disable.
> 
> I very much support Thierrys view, that all this should be encoded by
> the compatible. This allows us to add required properties to the panel
> if needed. It certainly saved us a lot of backward compat issues in
> imx-drm, that we could just add all the required information to the
> panel, so everything is consistent within one kernel version.
> 
> >> Personally, the idea of having hardware information* in DT does not
> >> sound all that bad. The simple panel driver(s) can use those
> >> properties and any panels that require anything more complex will
> >> still need their own driver.
> 
> Although some people seems to feel different on that matter, but DT is
> supposed to be ABI. Newer kernels _must_ work on older DTs. Which means
> it needs to encode _all_ information on the panel.
> 
> Personally I don't trust people, who are just enabling their one board
> to go through and validate that all the panel information is correct. As
> soon as a picture shows up they are going to ship the DT.
> 
> With having to go through a panel driver, there is at least some basic
> review of the panel properties.

I don't think anyone will review the timings, and my hopes are not much higher 
for the rest of the panel properties, regardless of whether they're specified 
in DT or C code.

> Also speaking from experience with a board with different display
> configurations, where only the bootloader knows the specific attached
> panel, it's very convenient to have the bootloader only patch in the
> correct compatible, instead of all the panel properties. If any of those
> properties turns out to be wrong, I can fix it with a simple kernel
> update, instead of having to update the bootloader.

Aren't you essentially advocating for going back from DT to C board files 
because firmware is often buggy ? :-) I agree that boot loaders get things 
wrong more often than not, but that's not specific to panels.

> > Again, the point is that you're going to have to modify the driver in
> > any case, because you need to support the new compatible string. Without
> > that compatible string you have zero information about the panel, and
> > matching on a generic one isn't going to give you a working panel. So if
> > you're already going to have to support a panel in a driver, why not go
> > all the way and fully describe its capabilities and properties? We do it
> > for all other devices. Panels are not at all special.
> > 
> >> For better or for worse, there's already a handful of drivers and
> >> bindings that rely on/provide these. Using that information
> >> consistently across the board, would be of a benefit, IMHO.
> 
> Yes, some old DTs still provide panel properties. That's no reason to
> not use the agreed upon way to describe panels in newer DTs. imx-drm
> still provides some support for the old way as this predates the panel
> stuff and we take DT backward compat serious, but newer kernels probably
> will start to complain about this. Please don't advocate this for new
> stuff.
> 
> So to sum up, I completely support Thierrys way of handling panels and
> IMHO we can already see the brick wall the the "generic" (which is not
> generic at all, but very specific in that it doesn't require any of the
> handling a panel driver could provide) LVDS panel stuff is going to hit.
> Requiring properties "data-mirror" to describe uncommon bit orderings
> should be a sufficient warning sign.

I'm afraid I still don't agree. I don't see the panel-lvds driver as a one-
size-fits-them-all solution to support all LVDS panels without ever having to 
write a line of C code. Things as power sequences need to be implemented in 
drivers, there's no doubt about that. Any oddity that will make a panel not 
compatible with the panel-lvds DT bindings will require a specific driver (or 
specific code in an existing driver). I believe we agree on that.

The point I don't agree with is that timings have to be specified in C code. 
As pointed in in a comment to https://sietch-tagr.blogspot.fi/2016/04/display-panels-are-not-special.html, there's a huge number of display panels on the 
marked, and the Linux kernel supports only a very tiny fraction of them. I 
still haven't seen the scalability issue of a C mode DB being addressed, and 
that's a major blocker for me. The other comment about EMC and EMI (Electro-
Magnetic Compatibility and Electro-Magnetic Interference) is also valid in my 
opinion, and is why we had to specify clock frequencies in DT for cameras as 
they need to be fine-tuned for each particular system. The same applies to 
panels, where timings for the same panel integrated in different systems may 
need to differ.

I'm certainly willing to reconsider my position if someone can propose a way 
to solve those two issues without involving DT (and, of course, without too 
many dirty hacks either :-)).

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1663 bytes --]

On Tue, Apr 11, 2017 at 08:00:11AM +0300, Laurent Pinchart wrote:
[...]
> The point I don't agree with is that timings have to be specified in C code. 
> As pointed in in a comment to
> https://sietch-tagr.blogspot.fi/2016/04/display-panels-are-not-special.html,
> there's a huge number of display panels on the 
> marked, and the Linux kernel supports only a very tiny fraction of them. I 
> still haven't seen the scalability issue of a C mode DB being addressed, and 
> that's a major blocker for me.

How about we tackle that problem when it becomes real? drm/panel has
existed for about 3.5 years now and we support not even 100 different
panels.

> The other comment about EMC and EMI (Electro-
> Magnetic Compatibility and Electro-Magnetic Interference) is also valid in my 
> opinion, and is why we had to specify clock frequencies in DT for cameras as 
> they need to be fine-tuned for each particular system. The same applies to 
> panels, where timings for the same panel integrated in different systems may 
> need to differ.

We already have a mechanism to do this. Initially panel descriptors used
to contain a single mode, but it wasn't very long until we encountered a
situation where the same panel was used on different devices with slight
differences in capabilities, which cause the original mode not to work.

The solution that Philipp Zabel came up with is fairly elegant. Instead
of specifying a single mode, matching essentially the integration on one
board, panel descriptors can now contain display timing ranges. This
allows each display controller driver to tailor a mode that fits its
needs.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Rob Herring]

On Fri, Apr 7, 2017 at 12:33 PM, Thierry Reding <treding@nvidia.com> wrote:
> On Fri, Apr 07, 2017 at 05:44:15AM +1000, Dave Airlie wrote:
>> On 5 April 2017 at 16:51, Laurent Pinchart
>> <laurent.pinchart+renesas@ideasonboard.com> wrote:
>> > As the DRM LVDS panel driver uses a different approach to DT bindings
>> > compared to what Thierry Reding advocates, add a specific MAINTAINERS
>> > entry to avoid bothering Thierry with requests related to that driver.

This is not a good split. Panels should not be diverging based on
interface. That's not to say we can't have sub-classed bindings based
on interfaces.

>> Could you document a bit more in the patch summary the finer points of
>> panel/dt doctrine, as I haven't got as much knowledge as I'd like.
>>
>> Just I believe, Thierry believes.
>
> I'm somewhat surprised how we arrived at the current situation. A very
> long time ago when we first discussed device tree bindings for panels, a
> number of attempts were made to generically describe everything in
> device tree. All of those attempts failed because you simply couldn't
> describe all of the required properties in DT in a sane way.
>
> Eventually everyone involved agreed that we would have to stick with the
> device-specific compatible, and in the best case we would be able to
> support many panels with a fairly generic driver. I think we did pretty
> well with the panel-simple driver. It started out very simple and then
> got improved over time as necessary to deal with more panels. And for
> cases where it wasn't suitable we simply added a custom driver. That's a
> completely natural way to write drivers. We do the same thing in other
> areas, nothing special here.

That is still the case here.

> Ever since the simple-panel binding was introduced, which is now about
> 3 1/2 years ago, people have kept asking why we couldn't simply put all
> data in DT and why kernel drivers had to be modified in order to add
> support for a new panel. I kept repeating myself a number of times until
> I finally wrote it all up[0], after which it was enough to point people
> to it. Still not everyone was convinced, but the people that were there
> when we made the decision all agreed that this was still the right thing
> to do. So, despite the many complaints I stuck to what we had agreed on
> because I am convinced that it is the right thing to do.

The big difference was folks wanted "simple-panel" compatible strings
and nothing else. That remains wrong and is a constant discussion. I'd
say at least 30% of my reviews contain "needs a more specific
compatible string". Panels are not the only "simple" or "generic"
hardware. :)

Parameterizing everything is indeed a losing battle. That doesn't mean
we can't parameterize some things in DT if they are completely
standard. IMO, roughly anything that can be in EDID could be in DT. So
I don't have a big problem with timings or physical size of the
display in DT. After all, we can always just ignore it.

> Now we have arrived at a point where apparently that decision has been
> revoked, and I don't understand what's changed. This puts me in a very
> difficult position. All of a sudden it's okay to do what everyone has
> been asking for the last three years, and I'm the jerk who told everyone
> that it couldn't be done.
>
> Maybe the discussions that we had back at the time are now far enough in
> the past that people have forgotten about the earlier failures. I still
> don't see how this new panel-lvds would be any more successful in
> solving the problems we failed to solve with simple-panel. The issues
> are still fundamentally the same. Now if this was a generic driver that
> dealt with a different subset of panels because they are different, that
> would've been okay with me. What I don't understand is why this has to
> deviate from the simple-panel binding in fundamental ways. Now we've got
> two bindings and we make life miserable for people because they have to
> choose between the two.
>
> Thierry
>
> [0]: https://sietch-tagr.blogspot.de/2016/04/display-panels-are-not-special.html

I appreciate your excellent write-up very much. I've directed people
to it numerous times.

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Rob Herring]

On Mon, Apr 10, 2017 at 2:27 PM, Dave Airlie <airlied@gmail.com> wrote:
> On 10 April 2017 at 19:03, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>> Hi Thierry,
>>
>> On Monday 10 Apr 2017 09:17:59 Thierry Reding wrote:
>>> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
>>> > Hi Thierry,
>>> >
>>> > I don't mean to stir up anything, just voicing "my 2c" as they say.
>>> >
>>> > On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
>>> > > Ever since the simple-panel binding was introduced, which is now about
>>> > > 3 1/2 years ago,
>>> >
>>> > Do you have a link to these discussions? Your blog article does not
>>> > have any links and I only found the "Runtime Interpreted Power
>>> > Sequences" thread.
>>> > That in itself does not cover the pros/cons of storing HW information*
>>> > within DT.
>>>
>>> There's some discussion here:
>>>
>>>       https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html
>>>
>>> which continues here:
>>>
>>>       https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html
>>>
>>> There are a couple of earlier threads, though, that discuss similar
>>> issues. This one seems to be the earliest I can find that is publicly
>>> archived:
>>>
>>>       http://www.spinics.net/lists/devicetree/msg08497.html
>>>
>>> Going over all these threads again wasn't a very pleasant experience. I
>>> realize how much time I already spent discussing these, and I don't have
>>> any desire to repeat that discussion.
>>>
>>> We've had these differences ever since the very beginning. So we're now
>>> again going in circles.
>>>
>>> The main concern back at the time was that having to specify timings in
>>> the driver would result in a complete mess because we have zillions of
>>> panels that we need to support. That's turned out to be a complete non-
>>> issue. We've got something on the order of 50 or 60 drivers supported in
>>> the simple-panel driver, and for everything that's more complicated we
>>> have a handful of separate drivers, all fairly simple as well.
>>>
>>> So while I understand why people want to put all this information into
>>> DT, we've repeatedly discussed the disadvantages that this would have.
>>> And while we were never able to get everyone to agree, the current
>>> solution has had enough agreement that we merged it. And it turned out
>>> to be good enough. There's nothing in panel-lvds that I can see that
>>> fundamentally changes this.
>>>
>>> > Personally, the idea of having hardware information* in DT does not
>>> > sound all that bad. The simple panel driver(s) can use those
>>> > properties and any panels that require anything more complex will
>>> > still need their own driver.
>>>
>>> Again, the point is that you're going to have to modify the driver in
>>> any case, because you need to support the new compatible string. Without
>>> that compatible string you have zero information about the panel, and
>>> matching on a generic one isn't going to give you a working panel.
>>
>> It will *if* the panel doesn't need any device-specific handling. In all other
>> cases I agree with you, panel-specific code will be needed in the kernel (to
>> handle power sequences for instance).
>>
>>> So if you're already going to have to support a panel in a driver, why not
>>> go all the way and fully describe its capabilities and properties? We do it
>>> for all other devices. Panels are not at all special.
>>
>> That we agree on, panels are not special. They're not the only devices that
>> store in DT information that could be hardcoded in the driver based on the
>> compatible string. We have many devices whose compatible string contains the
>> SoC version. Driver could then hardcode interrupts or clocks without any need
>> to specific them in DT. We don't do that as it would be more complex to
>> handle.
>>
>> Regarding timings, I've long hesitated (albeit I confess I was more on the
>> side of specifying them in DT) until Rob Herring convinced me with the generic
>> rule that adding information in DT that are generally exposed by devices
>> themselves makes sense. Displays traditionally expose video mode information
>> through EDID, which is effectively device firmware. When a panel is integrated
>> in a system, and the system designer decides to save money by removing the
>> EDID I2C EEPROM, moving that piece of device firmware data to system firmware
>> makes sense to me.
>>
>> I certainly won't try to revive the power sequence discussions, I don't
>> believe it belongs to DT.
>>
>>> > For better or for worse, there's already a handful of drivers and
>>> > bindings that rely on/provide these. Using that information
>>> > consistently across the board, would be of a benefit, IMHO.
>>>
>>> Would you mind pointing out which ones these are? I'm aware of only a
>>> couple that seemed to have sneeked in because people were trying to
>>> side-step adding drm/panel support for their boards, so I don't think
>>> that qualifies as a reason to rethink how drm/panel works.
>
> Okay I'm afraid I agree with Thierry here.
>
> I don't want mode timings or EDID in DT files, I'm pretty sure I was one of the
> people who helped decide that just having a compatible string and modes in
> a driver makes sense. So if we have imported code to be the opposite of this
> please work on removing it.
>
> If you move EDID from over the wire into DT it's not really tied to the panel
> anymore, and is now just a complicated way of specifying the modes from the
> panel documentation.
>
> EDID and DDC are a way for a panel to communicate to the host system,
> timing constraints and mode info, if you remove the i2c link, why bother
> encoding stuff in an EDID?

EDID is a way to make the h/w discoverable as is DT. Both are just
data. Both are OS independent data. Only the mechanism of providing
the data differs. I can envision putting DT overlays into EEPROMs on
add-on boards which then would even have the same mechanism.

This is not an either or decision. DT can provide data and the OS can
decide whether it uses it or not as long as we continue with specific
compatible strings.

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 8152 bytes --]

On Tue, Apr 11, 2017 at 12:10:47PM -0500, Rob Herring wrote:
> On Fri, Apr 7, 2017 at 12:33 PM, Thierry Reding <treding@nvidia.com> wrote:
> > On Fri, Apr 07, 2017 at 05:44:15AM +1000, Dave Airlie wrote:
> >> On 5 April 2017 at 16:51, Laurent Pinchart
> >> <laurent.pinchart+renesas@ideasonboard.com> wrote:
> >> > As the DRM LVDS panel driver uses a different approach to DT bindings
> >> > compared to what Thierry Reding advocates, add a specific MAINTAINERS
> >> > entry to avoid bothering Thierry with requests related to that driver.
> 
> This is not a good split. Panels should not be diverging based on
> interface. That's not to say we can't have sub-classed bindings based
> on interfaces.
> 
> >> Could you document a bit more in the patch summary the finer points of
> >> panel/dt doctrine, as I haven't got as much knowledge as I'd like.
> >>
> >> Just I believe, Thierry believes.
> >
> > I'm somewhat surprised how we arrived at the current situation. A very
> > long time ago when we first discussed device tree bindings for panels, a
> > number of attempts were made to generically describe everything in
> > device tree. All of those attempts failed because you simply couldn't
> > describe all of the required properties in DT in a sane way.
> >
> > Eventually everyone involved agreed that we would have to stick with the
> > device-specific compatible, and in the best case we would be able to
> > support many panels with a fairly generic driver. I think we did pretty
> > well with the panel-simple driver. It started out very simple and then
> > got improved over time as necessary to deal with more panels. And for
> > cases where it wasn't suitable we simply added a custom driver. That's a
> > completely natural way to write drivers. We do the same thing in other
> > areas, nothing special here.
> 
> That is still the case here.
> 
> > Ever since the simple-panel binding was introduced, which is now about
> > 3 1/2 years ago, people have kept asking why we couldn't simply put all
> > data in DT and why kernel drivers had to be modified in order to add
> > support for a new panel. I kept repeating myself a number of times until
> > I finally wrote it all up[0], after which it was enough to point people
> > to it. Still not everyone was convinced, but the people that were there
> > when we made the decision all agreed that this was still the right thing
> > to do. So, despite the many complaints I stuck to what we had agreed on
> > because I am convinced that it is the right thing to do.
> 
> The big difference was folks wanted "simple-panel" compatible strings
> and nothing else. That remains wrong and is a constant discussion. I'd
> say at least 30% of my reviews contain "needs a more specific
> compatible string". Panels are not the only "simple" or "generic"
> hardware. :)

But that's really what panel-lvds is all about. While the binding
documentation says that it needs "panel-lvds" and a panel-specific
compatible string, the reality is that if we only use "panel-lvds"
there's no way to fully describe the panel, and I suspect that we
will be painting ourselves into a corner.

For example, the panel-lvds binding defines a couple of required and
optional properties. None of that includes any power supplies. These
power supplies are defined in panel-specific bindings, because they
really are panel-specific. Matching on panel-lvds doesn't give you
information about the regulators. The fact that the driver doesn't
handle power supplies at all and has a large TODO comment about it
doesn't give me much confidence that this is well thought out.

So if matching on "panel-lvds" doesn't give you enough information to do
anything useful with the panel, then I think it really shouldn't be
there at all in the first place. If you look back at the history of some
early simple-panel users, we used to include "simple-panel" in the list
of compatible strings as well. Then we realized that there's really no
use for it to be there because by itself it doesn't provide concrete
information about the device. The panel-specific property already
implies it. It's equivalent to having:

	compatible = "nvidia,tegra20-gpio", "gpio-controller";

"gpio-controller" is much too abstract to be useful in any way. And so
is "simple-panel" or "panel-lvds".

It's also totally confusing because, although I haven't checked in
detail, I'm fairly sure that a number of panels already supported by the
panel-simple driver are actually LVDS panels. But adding "panel-lvds" to
their compatible string list would be wrong and the panel-lvds driver
wouldn't know what to do about it.

Then there's the additional fact that since we can define so many
properties in DT, we can also make up a completely fake device. With the
current bindings its possible to add a panel with compatible
"mitsubishi,aa121td01" but use property values from a panel that's
actually compatible with "mitsubishi,aa104xd12".

As a side-note, there's a typo in the binding for the latter, it
requires the compatible string list to contain the compatible for the
former. The example has the correct one, though.

> Parameterizing everything is indeed a losing battle. That doesn't mean
> we can't parameterize some things in DT if they are completely
> standard. IMO, roughly anything that can be in EDID could be in DT. So
> I don't have a big problem with timings or physical size of the
> display in DT. After all, we can always just ignore it.

That's effectively saying that display timings, if we decide to add them
to device tree, should be optional. Because if we can just ignore them,
it means they have to be implied by the compatible string. So we would
require the display timing database within drivers anyway, and the only
real use-case for DT display timings would be to override the ones
provided in the driver. That's something I had already agreed to, only
we never actually implemented it because it has never been needed.

That's not, however, what the panel-lvds binding proposal is about. It
makes the display timings mandatory such that you can fully define the
panels in DT. It means that we have to trust the timings in DT because
they are the only data we have. Choosing to ignore them is no longer an
option because there's no way to initialize the panel without them. The
driver implementation matches on the generic "panel-lvds" compatible and
therefore can only ever rely on what's in the DT. There's no information
about the required power sequencing or anything.

Essentially it does everything that I've been arguing against for the
past 3.5 years.

> > Now we have arrived at a point where apparently that decision has been
> > revoked, and I don't understand what's changed. This puts me in a very
> > difficult position. All of a sudden it's okay to do what everyone has
> > been asking for the last three years, and I'm the jerk who told everyone
> > that it couldn't be done.
> >
> > Maybe the discussions that we had back at the time are now far enough in
> > the past that people have forgotten about the earlier failures. I still
> > don't see how this new panel-lvds would be any more successful in
> > solving the problems we failed to solve with simple-panel. The issues
> > are still fundamentally the same. Now if this was a generic driver that
> > dealt with a different subset of panels because they are different, that
> > would've been okay with me. What I don't understand is why this has to
> > deviate from the simple-panel binding in fundamental ways. Now we've got
> > two bindings and we make life miserable for people because they have to
> > choose between the two.
> >
> > Thierry
> >
> > [0]: https://sietch-tagr.blogspot.de/2016/04/display-panels-are-not-special.html
> 
> I appreciate your excellent write-up very much. I've directed people
> to it numerous times.

This confuses me. You say that you agree with that write-up, and at the
same time you've acked the panel-lvds binding. But they both contradict
each other.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 7484 bytes --]

On Tue, Apr 11, 2017 at 01:56:35PM -0500, Rob Herring wrote:
> On Mon, Apr 10, 2017 at 2:27 PM, Dave Airlie <airlied@gmail.com> wrote:
> > On 10 April 2017 at 19:03, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> >> Hi Thierry,
> >>
> >> On Monday 10 Apr 2017 09:17:59 Thierry Reding wrote:
> >>> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
> >>> > Hi Thierry,
> >>> >
> >>> > I don't mean to stir up anything, just voicing "my 2c" as they say.
> >>> >
> >>> > On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
> >>> > > Ever since the simple-panel binding was introduced, which is now about
> >>> > > 3 1/2 years ago,
> >>> >
> >>> > Do you have a link to these discussions? Your blog article does not
> >>> > have any links and I only found the "Runtime Interpreted Power
> >>> > Sequences" thread.
> >>> > That in itself does not cover the pros/cons of storing HW information*
> >>> > within DT.
> >>>
> >>> There's some discussion here:
> >>>
> >>>       https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html
> >>>
> >>> which continues here:
> >>>
> >>>       https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html
> >>>
> >>> There are a couple of earlier threads, though, that discuss similar
> >>> issues. This one seems to be the earliest I can find that is publicly
> >>> archived:
> >>>
> >>>       http://www.spinics.net/lists/devicetree/msg08497.html
> >>>
> >>> Going over all these threads again wasn't a very pleasant experience. I
> >>> realize how much time I already spent discussing these, and I don't have
> >>> any desire to repeat that discussion.
> >>>
> >>> We've had these differences ever since the very beginning. So we're now
> >>> again going in circles.
> >>>
> >>> The main concern back at the time was that having to specify timings in
> >>> the driver would result in a complete mess because we have zillions of
> >>> panels that we need to support. That's turned out to be a complete non-
> >>> issue. We've got something on the order of 50 or 60 drivers supported in
> >>> the simple-panel driver, and for everything that's more complicated we
> >>> have a handful of separate drivers, all fairly simple as well.
> >>>
> >>> So while I understand why people want to put all this information into
> >>> DT, we've repeatedly discussed the disadvantages that this would have.
> >>> And while we were never able to get everyone to agree, the current
> >>> solution has had enough agreement that we merged it. And it turned out
> >>> to be good enough. There's nothing in panel-lvds that I can see that
> >>> fundamentally changes this.
> >>>
> >>> > Personally, the idea of having hardware information* in DT does not
> >>> > sound all that bad. The simple panel driver(s) can use those
> >>> > properties and any panels that require anything more complex will
> >>> > still need their own driver.
> >>>
> >>> Again, the point is that you're going to have to modify the driver in
> >>> any case, because you need to support the new compatible string. Without
> >>> that compatible string you have zero information about the panel, and
> >>> matching on a generic one isn't going to give you a working panel.
> >>
> >> It will *if* the panel doesn't need any device-specific handling. In all other
> >> cases I agree with you, panel-specific code will be needed in the kernel (to
> >> handle power sequences for instance).
> >>
> >>> So if you're already going to have to support a panel in a driver, why not
> >>> go all the way and fully describe its capabilities and properties? We do it
> >>> for all other devices. Panels are not at all special.
> >>
> >> That we agree on, panels are not special. They're not the only devices that
> >> store in DT information that could be hardcoded in the driver based on the
> >> compatible string. We have many devices whose compatible string contains the
> >> SoC version. Driver could then hardcode interrupts or clocks without any need
> >> to specific them in DT. We don't do that as it would be more complex to
> >> handle.
> >>
> >> Regarding timings, I've long hesitated (albeit I confess I was more on the
> >> side of specifying them in DT) until Rob Herring convinced me with the generic
> >> rule that adding information in DT that are generally exposed by devices
> >> themselves makes sense. Displays traditionally expose video mode information
> >> through EDID, which is effectively device firmware. When a panel is integrated
> >> in a system, and the system designer decides to save money by removing the
> >> EDID I2C EEPROM, moving that piece of device firmware data to system firmware
> >> makes sense to me.
> >>
> >> I certainly won't try to revive the power sequence discussions, I don't
> >> believe it belongs to DT.
> >>
> >>> > For better or for worse, there's already a handful of drivers and
> >>> > bindings that rely on/provide these. Using that information
> >>> > consistently across the board, would be of a benefit, IMHO.
> >>>
> >>> Would you mind pointing out which ones these are? I'm aware of only a
> >>> couple that seemed to have sneeked in because people were trying to
> >>> side-step adding drm/panel support for their boards, so I don't think
> >>> that qualifies as a reason to rethink how drm/panel works.
> >
> > Okay I'm afraid I agree with Thierry here.
> >
> > I don't want mode timings or EDID in DT files, I'm pretty sure I was one of the
> > people who helped decide that just having a compatible string and modes in
> > a driver makes sense. So if we have imported code to be the opposite of this
> > please work on removing it.
> >
> > If you move EDID from over the wire into DT it's not really tied to the panel
> > anymore, and is now just a complicated way of specifying the modes from the
> > panel documentation.
> >
> > EDID and DDC are a way for a panel to communicate to the host system,
> > timing constraints and mode info, if you remove the i2c link, why bother
> > encoding stuff in an EDID?
> 
> EDID is a way to make the h/w discoverable as is DT. Both are just
> data. Both are OS independent data. Only the mechanism of providing
> the data differs. I can envision putting DT overlays into EEPROMs on
> add-on boards which then would even have the same mechanism.
> 
> This is not an either or decision. DT can provide data and the OS can
> decide whether it uses it or not as long as we continue with specific
> compatible strings.

How does the OS decide not to use the data provided by DT? What criteria
should be used to determine that the DT can't be trusted? Where do we
get fallback data from?

What we did with the panel-simple driver was to mandate that there is a
mode (later display timing ranges) hard-coded in the driver. This means
that for each supported panel we have trusted data that we know works
with it. But we also made provisions to get data from an EDID if it is
present. However there are also cases where the EDID can't be accessed,
or where it is corrupt. For such cases we can always fallback to the
hard-coded data in the driver.

There are no such mechanisms for panel-lvds. On the other hand I don't
see anything in panel-lvds that couldn't be implemented as part of the
panel-simple driver. We already have ways to describe everything it does
and more.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Daniel Vetter]

On Wed, Apr 12, 2017 at 5:44 PM, Thierry Reding <treding@nvidia.com> wrote:
> On Tue, Apr 11, 2017 at 01:56:35PM -0500, Rob Herring wrote:
>> On Mon, Apr 10, 2017 at 2:27 PM, Dave Airlie <airlied@gmail.com> wrote:
>> > On 10 April 2017 at 19:03, Laurent Pinchart
>> > <laurent.pinchart@ideasonboard.com> wrote:
>> >> Hi Thierry,
>> >>
>> >> On Monday 10 Apr 2017 09:17:59 Thierry Reding wrote:
>> >>> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
>> >>> > Hi Thierry,
>> >>> >
>> >>> > I don't mean to stir up anything, just voicing "my 2c" as they say.
>> >>> >
>> >>> > On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
>> >>> > > Ever since the simple-panel binding was introduced, which is now about
>> >>> > > 3 1/2 years ago,
>> >>> >
>> >>> > Do you have a link to these discussions? Your blog article does not
>> >>> > have any links and I only found the "Runtime Interpreted Power
>> >>> > Sequences" thread.
>> >>> > That in itself does not cover the pros/cons of storing HW information*
>> >>> > within DT.
>> >>>
>> >>> There's some discussion here:
>> >>>
>> >>>       https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html
>> >>>
>> >>> which continues here:
>> >>>
>> >>>       https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html
>> >>>
>> >>> There are a couple of earlier threads, though, that discuss similar
>> >>> issues. This one seems to be the earliest I can find that is publicly
>> >>> archived:
>> >>>
>> >>>       http://www.spinics.net/lists/devicetree/msg08497.html
>> >>>
>> >>> Going over all these threads again wasn't a very pleasant experience. I
>> >>> realize how much time I already spent discussing these, and I don't have
>> >>> any desire to repeat that discussion.
>> >>>
>> >>> We've had these differences ever since the very beginning. So we're now
>> >>> again going in circles.
>> >>>
>> >>> The main concern back at the time was that having to specify timings in
>> >>> the driver would result in a complete mess because we have zillions of
>> >>> panels that we need to support. That's turned out to be a complete non-
>> >>> issue. We've got something on the order of 50 or 60 drivers supported in
>> >>> the simple-panel driver, and for everything that's more complicated we
>> >>> have a handful of separate drivers, all fairly simple as well.
>> >>>
>> >>> So while I understand why people want to put all this information into
>> >>> DT, we've repeatedly discussed the disadvantages that this would have.
>> >>> And while we were never able to get everyone to agree, the current
>> >>> solution has had enough agreement that we merged it. And it turned out
>> >>> to be good enough. There's nothing in panel-lvds that I can see that
>> >>> fundamentally changes this.
>> >>>
>> >>> > Personally, the idea of having hardware information* in DT does not
>> >>> > sound all that bad. The simple panel driver(s) can use those
>> >>> > properties and any panels that require anything more complex will
>> >>> > still need their own driver.
>> >>>
>> >>> Again, the point is that you're going to have to modify the driver in
>> >>> any case, because you need to support the new compatible string. Without
>> >>> that compatible string you have zero information about the panel, and
>> >>> matching on a generic one isn't going to give you a working panel.
>> >>
>> >> It will *if* the panel doesn't need any device-specific handling. In all other
>> >> cases I agree with you, panel-specific code will be needed in the kernel (to
>> >> handle power sequences for instance).
>> >>
>> >>> So if you're already going to have to support a panel in a driver, why not
>> >>> go all the way and fully describe its capabilities and properties? We do it
>> >>> for all other devices. Panels are not at all special.
>> >>
>> >> That we agree on, panels are not special. They're not the only devices that
>> >> store in DT information that could be hardcoded in the driver based on the
>> >> compatible string. We have many devices whose compatible string contains the
>> >> SoC version. Driver could then hardcode interrupts or clocks without any need
>> >> to specific them in DT. We don't do that as it would be more complex to
>> >> handle.
>> >>
>> >> Regarding timings, I've long hesitated (albeit I confess I was more on the
>> >> side of specifying them in DT) until Rob Herring convinced me with the generic
>> >> rule that adding information in DT that are generally exposed by devices
>> >> themselves makes sense. Displays traditionally expose video mode information
>> >> through EDID, which is effectively device firmware. When a panel is integrated
>> >> in a system, and the system designer decides to save money by removing the
>> >> EDID I2C EEPROM, moving that piece of device firmware data to system firmware
>> >> makes sense to me.
>> >>
>> >> I certainly won't try to revive the power sequence discussions, I don't
>> >> believe it belongs to DT.
>> >>
>> >>> > For better or for worse, there's already a handful of drivers and
>> >>> > bindings that rely on/provide these. Using that information
>> >>> > consistently across the board, would be of a benefit, IMHO.
>> >>>
>> >>> Would you mind pointing out which ones these are? I'm aware of only a
>> >>> couple that seemed to have sneeked in because people were trying to
>> >>> side-step adding drm/panel support for their boards, so I don't think
>> >>> that qualifies as a reason to rethink how drm/panel works.
>> >
>> > Okay I'm afraid I agree with Thierry here.
>> >
>> > I don't want mode timings or EDID in DT files, I'm pretty sure I was one of the
>> > people who helped decide that just having a compatible string and modes in
>> > a driver makes sense. So if we have imported code to be the opposite of this
>> > please work on removing it.
>> >
>> > If you move EDID from over the wire into DT it's not really tied to the panel
>> > anymore, and is now just a complicated way of specifying the modes from the
>> > panel documentation.
>> >
>> > EDID and DDC are a way for a panel to communicate to the host system,
>> > timing constraints and mode info, if you remove the i2c link, why bother
>> > encoding stuff in an EDID?
>>
>> EDID is a way to make the h/w discoverable as is DT. Both are just
>> data. Both are OS independent data. Only the mechanism of providing
>> the data differs. I can envision putting DT overlays into EEPROMs on
>> add-on boards which then would even have the same mechanism.
>>
>> This is not an either or decision. DT can provide data and the OS can
>> decide whether it uses it or not as long as we continue with specific
>> compatible strings.
>
> How does the OS decide not to use the data provided by DT? What criteria
> should be used to determine that the DT can't be trusted? Where do we
> get fallback data from?
>
> What we did with the panel-simple driver was to mandate that there is a
> mode (later display timing ranges) hard-coded in the driver. This means
> that for each supported panel we have trusted data that we know works
> with it. But we also made provisions to get data from an EDID if it is
> present. However there are also cases where the EDID can't be accessed,
> or where it is corrupt. For such cases we can always fallback to the
> hard-coded data in the driver.
>
> There are no such mechanisms for panel-lvds. On the other hand I don't
> see anything in panel-lvds that couldn't be implemented as part of the
> panel-simple driver. We already have ways to describe everything it does
> and more.

I think I have some relevant experience here, since for i915.ko and
dsi panels we have a fully generic description in VBT. And if you make
DT into a real machine desription, then I expect you'll roughly end up
in a similar position.

For the "when should you trust the firmware data" issue: We have tons
of dmi-based quirks (which more or less equals the board id). No, it's
not pretty.

For the mode description: We do have the full mode in VBT. We've had
that for a long time in lvds, and used it as the override EDID (except
when blacklisted with a dmi-based entry) even when there was one on
the i2c bus. Probably lots of littel integration issues that were
"fixed" by frobbing the mode in VBT. The only panel type where we
don't depend much upon VBT is eDP. But even there there's stuff like
link training values in the VBT, to avoid link training on boot-up an
optimize them more.

What's even more fun in the context of power sequences is that we also
have that in the VBT, as some kind of bytecode. For PWMs, gpios and
i2c buses we have the equivalent of phandles that point at the right
endpoint (which is provided by some different pmic driver entirely
independent of i915.ko in some cases), and load ordering is handled
with deferred probing.

I'm not advertisting that we do this in DT, just saying that there's
at least 1 system which does this (well, the equivalent in a
proprietary format), it's in upstream and it's shipping. Food for
thought :-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Alex Deucher]

On Wed, Apr 12, 2017 at 1:42 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Apr 12, 2017 at 5:44 PM, Thierry Reding <treding@nvidia.com> wrote:
>> On Tue, Apr 11, 2017 at 01:56:35PM -0500, Rob Herring wrote:
>>> On Mon, Apr 10, 2017 at 2:27 PM, Dave Airlie <airlied@gmail.com> wrote:
>>> > On 10 April 2017 at 19:03, Laurent Pinchart
>>> > <laurent.pinchart@ideasonboard.com> wrote:
>>> >> Hi Thierry,
>>> >>
>>> >> On Monday 10 Apr 2017 09:17:59 Thierry Reding wrote:
>>> >>> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
>>> >>> > Hi Thierry,
>>> >>> >
>>> >>> > I don't mean to stir up anything, just voicing "my 2c" as they say.
>>> >>> >
>>> >>> > On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
>>> >>> > > Ever since the simple-panel binding was introduced, which is now about
>>> >>> > > 3 1/2 years ago,
>>> >>> >
>>> >>> > Do you have a link to these discussions? Your blog article does not
>>> >>> > have any links and I only found the "Runtime Interpreted Power
>>> >>> > Sequences" thread.
>>> >>> > That in itself does not cover the pros/cons of storing HW information*
>>> >>> > within DT.
>>> >>>
>>> >>> There's some discussion here:
>>> >>>
>>> >>>       https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html
>>> >>>
>>> >>> which continues here:
>>> >>>
>>> >>>       https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html
>>> >>>
>>> >>> There are a couple of earlier threads, though, that discuss similar
>>> >>> issues. This one seems to be the earliest I can find that is publicly
>>> >>> archived:
>>> >>>
>>> >>>       http://www.spinics.net/lists/devicetree/msg08497.html
>>> >>>
>>> >>> Going over all these threads again wasn't a very pleasant experience. I
>>> >>> realize how much time I already spent discussing these, and I don't have
>>> >>> any desire to repeat that discussion.
>>> >>>
>>> >>> We've had these differences ever since the very beginning. So we're now
>>> >>> again going in circles.
>>> >>>
>>> >>> The main concern back at the time was that having to specify timings in
>>> >>> the driver would result in a complete mess because we have zillions of
>>> >>> panels that we need to support. That's turned out to be a complete non-
>>> >>> issue. We've got something on the order of 50 or 60 drivers supported in
>>> >>> the simple-panel driver, and for everything that's more complicated we
>>> >>> have a handful of separate drivers, all fairly simple as well.
>>> >>>
>>> >>> So while I understand why people want to put all this information into
>>> >>> DT, we've repeatedly discussed the disadvantages that this would have.
>>> >>> And while we were never able to get everyone to agree, the current
>>> >>> solution has had enough agreement that we merged it. And it turned out
>>> >>> to be good enough. There's nothing in panel-lvds that I can see that
>>> >>> fundamentally changes this.
>>> >>>
>>> >>> > Personally, the idea of having hardware information* in DT does not
>>> >>> > sound all that bad. The simple panel driver(s) can use those
>>> >>> > properties and any panels that require anything more complex will
>>> >>> > still need their own driver.
>>> >>>
>>> >>> Again, the point is that you're going to have to modify the driver in
>>> >>> any case, because you need to support the new compatible string. Without
>>> >>> that compatible string you have zero information about the panel, and
>>> >>> matching on a generic one isn't going to give you a working panel.
>>> >>
>>> >> It will *if* the panel doesn't need any device-specific handling. In all other
>>> >> cases I agree with you, panel-specific code will be needed in the kernel (to
>>> >> handle power sequences for instance).
>>> >>
>>> >>> So if you're already going to have to support a panel in a driver, why not
>>> >>> go all the way and fully describe its capabilities and properties? We do it
>>> >>> for all other devices. Panels are not at all special.
>>> >>
>>> >> That we agree on, panels are not special. They're not the only devices that
>>> >> store in DT information that could be hardcoded in the driver based on the
>>> >> compatible string. We have many devices whose compatible string contains the
>>> >> SoC version. Driver could then hardcode interrupts or clocks without any need
>>> >> to specific them in DT. We don't do that as it would be more complex to
>>> >> handle.
>>> >>
>>> >> Regarding timings, I've long hesitated (albeit I confess I was more on the
>>> >> side of specifying them in DT) until Rob Herring convinced me with the generic
>>> >> rule that adding information in DT that are generally exposed by devices
>>> >> themselves makes sense. Displays traditionally expose video mode information
>>> >> through EDID, which is effectively device firmware. When a panel is integrated
>>> >> in a system, and the system designer decides to save money by removing the
>>> >> EDID I2C EEPROM, moving that piece of device firmware data to system firmware
>>> >> makes sense to me.
>>> >>
>>> >> I certainly won't try to revive the power sequence discussions, I don't
>>> >> believe it belongs to DT.
>>> >>
>>> >>> > For better or for worse, there's already a handful of drivers and
>>> >>> > bindings that rely on/provide these. Using that information
>>> >>> > consistently across the board, would be of a benefit, IMHO.
>>> >>>
>>> >>> Would you mind pointing out which ones these are? I'm aware of only a
>>> >>> couple that seemed to have sneeked in because people were trying to
>>> >>> side-step adding drm/panel support for their boards, so I don't think
>>> >>> that qualifies as a reason to rethink how drm/panel works.
>>> >
>>> > Okay I'm afraid I agree with Thierry here.
>>> >
>>> > I don't want mode timings or EDID in DT files, I'm pretty sure I was one of the
>>> > people who helped decide that just having a compatible string and modes in
>>> > a driver makes sense. So if we have imported code to be the opposite of this
>>> > please work on removing it.
>>> >
>>> > If you move EDID from over the wire into DT it's not really tied to the panel
>>> > anymore, and is now just a complicated way of specifying the modes from the
>>> > panel documentation.
>>> >
>>> > EDID and DDC are a way for a panel to communicate to the host system,
>>> > timing constraints and mode info, if you remove the i2c link, why bother
>>> > encoding stuff in an EDID?
>>>
>>> EDID is a way to make the h/w discoverable as is DT. Both are just
>>> data. Both are OS independent data. Only the mechanism of providing
>>> the data differs. I can envision putting DT overlays into EEPROMs on
>>> add-on boards which then would even have the same mechanism.
>>>
>>> This is not an either or decision. DT can provide data and the OS can
>>> decide whether it uses it or not as long as we continue with specific
>>> compatible strings.
>>
>> How does the OS decide not to use the data provided by DT? What criteria
>> should be used to determine that the DT can't be trusted? Where do we
>> get fallback data from?
>>
>> What we did with the panel-simple driver was to mandate that there is a
>> mode (later display timing ranges) hard-coded in the driver. This means
>> that for each supported panel we have trusted data that we know works
>> with it. But we also made provisions to get data from an EDID if it is
>> present. However there are also cases where the EDID can't be accessed,
>> or where it is corrupt. For such cases we can always fallback to the
>> hard-coded data in the driver.
>>
>> There are no such mechanisms for panel-lvds. On the other hand I don't
>> see anything in panel-lvds that couldn't be implemented as part of the
>> panel-simple driver. We already have ways to describe everything it does
>> and more.
>
> I think I have some relevant experience here, since for i915.ko and
> dsi panels we have a fully generic description in VBT. And if you make
> DT into a real machine desription, then I expect you'll roughly end up
> in a similar position.
>
> For the "when should you trust the firmware data" issue: We have tons
> of dmi-based quirks (which more or less equals the board id). No, it's
> not pretty.
>
> For the mode description: We do have the full mode in VBT. We've had
> that for a long time in lvds, and used it as the override EDID (except
> when blacklisted with a dmi-based entry) even when there was one on
> the i2c bus. Probably lots of littel integration issues that were
> "fixed" by frobbing the mode in VBT. The only panel type where we
> don't depend much upon VBT is eDP. But even there there's stuff like
> link training values in the VBT, to avoid link training on boot-up an
> optimize them more.
>
> What's even more fun in the context of power sequences is that we also
> have that in the VBT, as some kind of bytecode. For PWMs, gpios and
> i2c buses we have the equivalent of phandles that point at the right
> endpoint (which is provided by some different pmic driver entirely
> independent of i915.ko in some cases), and load ordering is handled
> with deferred probing.
>
> I'm not advertisting that we do this in DT, just saying that there's
> at least 1 system which does this (well, the equivalent in a
> proprietary format), it's in upstream and it's shipping. Food for
> thought :-)

We have something similar on AMD GPUs.  The entire display topology
(encoders, connectors, i2c and aux routing, etc.) as well as the mode
timing for integrated panels is stored in vbios tables and it's
reliable.

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

Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Rob Herring]

On Wed, Apr 12, 2017 at 10:26 AM, Thierry Reding <treding@nvidia.com> wrote:
> On Tue, Apr 11, 2017 at 12:10:47PM -0500, Rob Herring wrote:
>> On Fri, Apr 7, 2017 at 12:33 PM, Thierry Reding <treding@nvidia.com> wrote:
>> > On Fri, Apr 07, 2017 at 05:44:15AM +1000, Dave Airlie wrote:
>> >> On 5 April 2017 at 16:51, Laurent Pinchart
>> >> <laurent.pinchart+renesas@ideasonboard.com> wrote:
>> >> > As the DRM LVDS panel driver uses a different approach to DT bindings
>> >> > compared to what Thierry Reding advocates, add a specific MAINTAINERS
>> >> > entry to avoid bothering Thierry with requests related to that driver.
>>
>> This is not a good split. Panels should not be diverging based on
>> interface. That's not to say we can't have sub-classed bindings based
>> on interfaces.
>>
>> >> Could you document a bit more in the patch summary the finer points of
>> >> panel/dt doctrine, as I haven't got as much knowledge as I'd like.
>> >>
>> >> Just I believe, Thierry believes.
>> >
>> > I'm somewhat surprised how we arrived at the current situation. A very
>> > long time ago when we first discussed device tree bindings for panels, a
>> > number of attempts were made to generically describe everything in
>> > device tree. All of those attempts failed because you simply couldn't
>> > describe all of the required properties in DT in a sane way.
>> >
>> > Eventually everyone involved agreed that we would have to stick with the
>> > device-specific compatible, and in the best case we would be able to
>> > support many panels with a fairly generic driver. I think we did pretty
>> > well with the panel-simple driver. It started out very simple and then
>> > got improved over time as necessary to deal with more panels. And for
>> > cases where it wasn't suitable we simply added a custom driver. That's a
>> > completely natural way to write drivers. We do the same thing in other
>> > areas, nothing special here.
>>
>> That is still the case here.
>>
>> > Ever since the simple-panel binding was introduced, which is now about
>> > 3 1/2 years ago, people have kept asking why we couldn't simply put all
>> > data in DT and why kernel drivers had to be modified in order to add
>> > support for a new panel. I kept repeating myself a number of times until
>> > I finally wrote it all up[0], after which it was enough to point people
>> > to it. Still not everyone was convinced, but the people that were there
>> > when we made the decision all agreed that this was still the right thing
>> > to do. So, despite the many complaints I stuck to what we had agreed on
>> > because I am convinced that it is the right thing to do.
>>
>> The big difference was folks wanted "simple-panel" compatible strings
>> and nothing else. That remains wrong and is a constant discussion. I'd
>> say at least 30% of my reviews contain "needs a more specific
>> compatible string". Panels are not the only "simple" or "generic"
>> hardware. :)
>
> But that's really what panel-lvds is all about. While the binding
> documentation says that it needs "panel-lvds" and a panel-specific
> compatible string, the reality is that if we only use "panel-lvds"
> there's no way to fully describe the panel, and I suspect that we
> will be painting ourselves into a corner.

Using only "panel-lvds" is not valid. Now I generally don't think the
kernel should validate DTs, but if abusing it is a concern let's add a
big fat WARN if that's the only compatible.

> For example, the panel-lvds binding defines a couple of required and
> optional properties. None of that includes any power supplies. These
> power supplies are defined in panel-specific bindings, because they
> really are panel-specific. Matching on panel-lvds doesn't give you
> information about the regulators. The fact that the driver doesn't
> handle power supplies at all and has a large TODO comment about it
> doesn't give me much confidence that this is well thought out.

Right. The only way you should ever match on panel-lvds is if the
panel needs no power control. That can evolve with the OS over time as
needed, but the bindings shouldn't change.

simple-panel suffers from the same problem here. power supplies are
often not being described correctly when there is more than one. It's
only later or on another board that both the binding and driver get
fixed. I personally think having "power-supply" in simple-panel may
have been a mistake because folks just omit describing the supplies
correctly. I've been getting stricter on this as you've seen.

> So if matching on "panel-lvds" doesn't give you enough information to do
> anything useful with the panel, then I think it really shouldn't be
> there at all in the first place. If you look back at the history of some
> early simple-panel users, we used to include "simple-panel" in the list
> of compatible strings as well. Then we realized that there's really no
> use for it to be there because by itself it doesn't provide concrete
> information about the device. The panel-specific property already
> implies it. It's equivalent to having:
>
>         compatible = "nvidia,tegra20-gpio", "gpio-controller";
>
> "gpio-controller" is much too abstract to be useful in any way. And so
> is "simple-panel" or "panel-lvds".

Plus nodes are already tagged with "gpio-controller" as a boolean, but
I get your point. This is useful from a validation standpoint though.
If there is a property or compatible or standard node name to key off
of, we can validate the rest of the properties. Of course if we had
per compatible schema then we wouldn't need any of that.

For panel-lvds, minimally it tells me a) what the interface is which
could be useful for the upstream (parent) device and b) that various
LVDS related properties may be present. "simple-panel" never provided
any of that. Another compatible string is just additional data.

OTOH, I would not really object to removing the compatible either.

> It's also totally confusing because, although I haven't checked in
> detail, I'm fairly sure that a number of panels already supported by the
> panel-simple driver are actually LVDS panels. But adding "panel-lvds" to
> their compatible string list would be wrong and the panel-lvds driver
> wouldn't know what to do about it.

Yes, unfortunately the kernel can't work out multiple matches and pick
the driver with the most specific match. Generally, we're relying on
linking order here. That's a bigger issue in general.

IMO for this case, it is a problem because the drivers are split.
That's really a separate issue from the bindings. Dropping driver
matching on "panel-lvds" would solve it.

> Then there's the additional fact that since we can define so many
> properties in DT, we can also make up a completely fake device. With the
> current bindings its possible to add a panel with compatible
> "mitsubishi,aa121td01" but use property values from a panel that's
> actually compatible with "mitsubishi,aa104xd12".

If people lie and do stupid shit, then they get to keep it... That
only works until if the power control is the same or absent.

> As a side-note, there's a typo in the binding for the latter, it
> requires the compatible string list to contain the compatible for the
> former. The example has the correct one, though.
>
>> Parameterizing everything is indeed a losing battle. That doesn't mean
>> we can't parameterize some things in DT if they are completely
>> standard. IMO, roughly anything that can be in EDID could be in DT. So
>> I don't have a big problem with timings or physical size of the
>> display in DT. After all, we can always just ignore it.
>
> That's effectively saying that display timings, if we decide to add them
> to device tree, should be optional. Because if we can just ignore them,
> it means they have to be implied by the compatible string. So we would
> require the display timing database within drivers anyway, and the only
> real use-case for DT display timings would be to override the ones
> provided in the driver. That's something I had already agreed to, only
> we never actually implemented it because it has never been needed.

DT overriding the driver is completely backwards. The DT is part of
firmware. When firmware is wrong we override it. Just like if EDID is
crap, we'd ignore it (or perhaps adjust it).

> That's not, however, what the panel-lvds binding proposal is about. It
> makes the display timings mandatory such that you can fully define the
> panels in DT. It means that we have to trust the timings in DT because
> they are the only data we have. Choosing to ignore them is no longer an
> option because there's no way to initialize the panel without them. The
> driver implementation matches on the generic "panel-lvds" compatible and
> therefore can only ever rely on what's in the DT. There's no information
> about the required power sequencing or anything.

Mandatory for DT does not equate to the driver must use them. The
driver currently doesn't do much, but that doesn't mean it can't.
IIRC, when Laurent adds power sequencing support then that panel will
switch to matching on its specific compatible. The driver can match on
the specific compatible and it there's driver data with timings use
that. If not, fall back to using what is in DT.

Power supplies are similar. The DT may require them, but the OS
support for them is not yet in place and relies on everything being
powered on already.

> Essentially it does everything that I've been arguing against for the
> past 3.5 years.
>
>> > Now we have arrived at a point where apparently that decision has been
>> > revoked, and I don't understand what's changed. This puts me in a very
>> > difficult position. All of a sudden it's okay to do what everyone has
>> > been asking for the last three years, and I'm the jerk who told everyone
>> > that it couldn't be done.
>> >
>> > Maybe the discussions that we had back at the time are now far enough in
>> > the past that people have forgotten about the earlier failures. I still
>> > don't see how this new panel-lvds would be any more successful in
>> > solving the problems we failed to solve with simple-panel. The issues
>> > are still fundamentally the same. Now if this was a generic driver that
>> > dealt with a different subset of panels because they are different, that
>> > would've been okay with me. What I don't understand is why this has to
>> > deviate from the simple-panel binding in fundamental ways. Now we've got
>> > two bindings and we make life miserable for people because they have to
>> > choose between the two.
>> >
>> > Thierry
>> >
>> > [0]: https://sietch-tagr.blogspot.de/2016/04/display-panels-are-not-special.html
>>
>> I appreciate your excellent write-up very much. I've directed people
>> to it numerous times.
>
> This confuses me. You say that you agree with that write-up, and at the
> same time you've acked the panel-lvds binding. But they both contradict
> each other.

In general, I think we agree on most points. We always need a specific
compatible (I still have to argue that one). Having other properties
that are not related to describing connections is quite common. Having
specific properties vs. implying them from the compatible is not a
black and white situation. It's often a judgement call. For me, I
think timings falls on one side and power control falls on the other.

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