Re: [PATCH v2 00/15] DRM fbconv helpers for converting fbdev drivers

2019-10-14 Thread Sam Ravnborg
Hi Thomas.

On Mon, Oct 14, 2019 at 04:04:01PM +0200, Thomas Zimmermann wrote:
> (was: DRM driver for fbdev devices)
> 
> This is version 2 of the fbdev conversion helpers. It's more or less a
> rewrite of the original patchset.
> 
> The fbdev subsystem is considered legacy and will probably be removed at
> some point. This would mean the loss of a signifanct number of drivers.
> Some of the affected hardware is not in use any longer, but some hardware
> is still around and provides good(-enough) framebuffers.
> 
> The fbconv helpers allow for running the current DRM stack on top of fbdev
> drivers. It's a set of functions that convert between fbdev interfaces and
> DRM interfaces. Based on SHMEM and simple KMS helpers, it only offers the
> basic functionality of a framebuffer, but should be compatible with most
> existing fbdev drivers.
> 
> A DRM driver using fbconv helpers consists of
> 
>   * DRM stub code that calls into fbconv helpers, and
>   * the original fbdev driver code.
> 
> The fbdev driver code has to be modified to register itself with the
> stub driver instead of the fbdev core framework. A tutorial on how to use
> the helpers is part of this patchset. The resulting driver hybrid can be
> refactored into a first-class DRM driver. The fbconv helpers contain a
> number of comments, labeled 'DRM porting note', which explain the required
> steps.
> 
> I tested the current patchset with the following drivers: atyfb, aty128fb,
> matroxfb, pm2fb, pm3fb, rivafb, s3fb, savagefb, sisfb, tdfxfb and tridentfb.
> With each, I was able to successfully start with fbcon enabled, run weston and
> X11. The drivers are available at [1]. For reference, the patchset includes
> the Matrox stub driver.

In general I like the idea of modernizing the existing fbdev drivers.
What I fail to read in your intro above is if this allows us to phase
out the migrated fbdev drivers sooner?
Or do we end up with two drivers to maintain?

Obviously a full migration to a DRM driver was preferred - but this may
serve as a step in that direction.
But we should not end up with two drivers doing almost the same.

Another general question. Do we want the modernized DRM drivers to end
up in staging? Why should they not go direct into drm/*
I know they are not fully atomic but this is not new drivers so maybe
they can be excused.
Problem is that drm drivers in staging live a secret nonvisible life
where they are easy to forget when we change interfaces and such.

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

Re: [PATCH v2 05/15] drm/fbconv: Add DRM <-> fbdev pixel-format conversion

2019-10-14 Thread Sam Ravnborg
Hi Thomas.

On Mon, Oct 14, 2019 at 04:04:06PM +0200, Thomas Zimmermann wrote:
> DRM uses FOURCC constants to describe pixel formats, fbdev uses a
> per-component bitfield structure. The functions in this patch convert
> between the two.
> 

A few nits below.


> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/drm_fbconv_helper.c | 435 
>  include/drm/drm_fbconv_helper.h |  23 ++
>  2 files changed, 458 insertions(+)
>  create mode 100644 include/drm/drm_fbconv_helper.h
> 
> diff --git a/drivers/gpu/drm/drm_fbconv_helper.c 
> b/drivers/gpu/drm/drm_fbconv_helper.c
> index 0cb46d2c98c3..af45358a156a 100644
> --- a/drivers/gpu/drm/drm_fbconv_helper.c
> +++ b/drivers/gpu/drm/drm_fbconv_helper.c
> @@ -1 +1,436 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include 
> +
> +#include 

 after 
So we in this way pick the more general include file first.

> +
> +struct format_map {
> + bool (*is_format)(const struct fb_var_screeninfo *fb_var);
> + uint32_t format;
> +};
We are in the kernel - where I think u32 is preferred over the longer
uint32_t.
If I grep in drm/* then they seems be be equally popular, so feel free
to ignore this comment.


> +static void set_fb_bitfield(struct fb_bitfield *bits, __u32 offset,
> + __u32 length)

This is not uapi - so u32 is preferred.

Both comments apply to the whole file.

I did not see that this was wired into the kernel-doc in Documentation/
but maybe I just missed it.

With my comments considered you can add:
Acked-by: Sam Ravnborg 

All code looks sane, but as I have not grasped the bigger picture
this can hardly be a review.

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

Re: [PATCH 0/3] drm/vboxvideo: Use generic fbdev and framebuffer

2019-10-12 Thread Sam Ravnborg
Hi Thomas.

On Fri, Oct 11, 2019 at 03:48:05PM +0200, Thomas Zimmermann wrote:
> The vboxvideo driver provides its own implementation for fbdev emulation
> and framebuffers. Both can be replaced by DRM's generic code.
> 
> All patches have been tested on VirtualBox 6.0.12.
> 
> Thomas Zimmermann (3):
>   drm/vboxvideo: Switch to generic fbdev emulation
>   drm/vboxvideo: Switch to drm_atomic_helper_dirty_fb()
>   drm/vboxvideo: Replace struct vram_framebuffer with generic
> implemenation
> 
>  drivers/gpu/drm/vboxvideo/Makefile|   2 +-
>  drivers/gpu/drm/vboxvideo/vbox_drv.c  |  14 +--
>  drivers/gpu/drm/vboxvideo/vbox_drv.h  |  25 -
>  drivers/gpu/drm/vboxvideo/vbox_fb.c   | 149 --
>  drivers/gpu/drm/vboxvideo/vbox_main.c | 119 +---
>  drivers/gpu/drm/vboxvideo/vbox_mode.c |  86 +++
>  6 files changed, 49 insertions(+), 346 deletions(-)
>  delete mode 100644 drivers/gpu/drm/vboxvideo/vbox_fb.c
Nice work. Browsed through the changes and enjoyed all the deleted code.

You can add my:

Acked-by: Sam Ravnborg 

to the whole series.

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

Re: [PATCH v2 04/21] drm/exynos: Fix potential unbalanced calls to pm_runtime_put

2019-10-12 Thread Sam Ravnborg
Hi Boris/Andrzej.

> 
> > 
> > 
> > > Note that -ENOSYS is actually a valid case, it just
> > > means the panel driver does not implement the hook.  
> > 
> > 
> > It would be good then to fix it in panel framework, ie without hook
> > drm_panel_* function should return 0, ENOSYS makes no sense here.
> 
> I'm fine with that. Thierry, Sam, any opinion?

Agreed, I have following patch in my panel patch queue:

drm/drm_panel: no error when no callback

The callbacks in drm_panel_funcs are optional, so do not
return an error just because no callback is assigned.

Signed-off-by: Sam Ravnborg 

If I get time this weekend I will rebase/test and send the
set of patches out.

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

Re: [PATCH v2 13/50] drm/bridge: panel: Implement bridge connector operations

2019-10-09 Thread Sam Ravnborg
Hi Tomi

On Thu, Oct 03, 2019 at 08:56:15AM +0300, Tomi Valkeinen wrote:
> Hi Sam,
> 
> On 20/08/2019 13:37, Sam Ravnborg wrote:
> 
> > > @@ -123,6 +123,18 @@ static void panel_bridge_post_disable(struct 
> > > drm_bridge *bridge)
> > >   drm_panel_unprepare(panel_bridge->panel);
> > >   }
> > > +static int panel_bridge_get_modes(struct drm_bridge *bridge,
> > > +   struct drm_connector *connector)
> > > +{
> > > + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
> > > +
> > > + /*
> > > +  * FIXME: drm_panel_get_modes() should take the connector as an
> > > +  * argument.
> > > +  */
> > Noted, I have patches to fix this. Needs a little testing/polishing
> > before I post them.
> 
> Do you have any testable patches for this?
> 
> I was testing this series with a Toshiba DPI-2-DSI bridge and a DSI panel,
> and was hitting a crash as simple-panel couldn't get the connector.


If time permits I will refresh the patches this weekend and send them
out.
But right now cannot promise much as I am awful behind.

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

Re: [PATCH v1 0/2]: Finally delete drmP.h

2019-10-08 Thread Sam Ravnborg
On Mon, Oct 07, 2019 at 07:12:22PM +0200, Sam Ravnborg wrote:
> One user of drmP.h sneaked in after the merge window.
> Drop the use of drmP.h and delete the header file for good.
> 
> Small band-aid on top of not going to xdc :-)
> 
> Build tested with various architectures and configs.

Applied and pushed to drm-misc-next.

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

Re: [PATCH 0/5] Fix SPI module alias for panels used by omapdrm

2019-10-07 Thread Sam Ravnborg
Hi Laurent.
On Mon, Oct 07, 2019 at 08:07:56PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> This patch series fixes a module alias issue with the five recently
> added panel drivers used by omapdrm.
> 
> Before those panel drivers, omapdrm had custom drivers for the panels,
> and prefixed the OF compatible strings with an "omapdss," prefix. The
> SPI device IDs are constructed by stripping the OF compatible string
> from the prefix, resulting in the "omapdss," prefix being removed, but
> the subsequence OF vendor prefix being kept. The SPI drivers thus had
> modules aliases that contained the vendor prefix.
> 
> Now that the panels are supported by standard drivers and the "omapdss,"
> prefix is removed, the SPI device IDs are stripped from the OF vendor
> prefix. As the new panel drivers copied the module aliases from the
> omapdrm-specific drivers, they contain the vendor prefix in their SPI
> module aliases, and are thus not loaded automatically.
> 
> Fix this by removing the vendor prefix from the SPI modules aliases in
> the drivers. For consistency reason, the manual module aliases are also
> moved to use an SPI module table.

Good explanation - thanks.

> 
> These patches are based on the drm-misc-fixes branch as they fix v5.4
> regressions.
> 
> Laurent Pinchart (5):
>   drm/panel: lg-lb035q02: Fix SPI alias
>   drm/panel: nec-nl8048hl11: Fix SPI alias
>   drm/panel: sony-acx565akm: Fix SPI alias
>   drm/panel: tpo-td028ttec1: Fix SPI alias
>   drm/panel: tpo-td043mtea1: Fix SPI alias

Full series is:
Acked-by: Sam Ravnborg 

I expect someone else to pick them up or that you apply them.

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

[PATCH v1 1/2] drm_dp_cec: drop use of drmP.h

2019-10-07 Thread Sam Ravnborg
drmP.h is deprecated and will be deleted.
Replace use with proper header.

Divide header includes in blocks while touching these.

Build tested with various archtectures and configs.

Signed-off-by: Sam Ravnborg 
Fixes: ae85b0df124f6928 ("drm_dp_cec: add connector info support.")
Cc: Dariusz Marcinkiewicz 
Cc: Hans Verkuil 
Cc: Lyude Paul 
Cc: Ben Skeggs 
---
 drivers/gpu/drm/drm_dp_cec.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
index b457c16c3a8b..3ab2609f9ec7 100644
--- a/drivers/gpu/drm/drm_dp_cec.c
+++ b/drivers/gpu/drm/drm_dp_cec.c
@@ -8,10 +8,12 @@
 #include 
 #include 
 #include 
+
+#include 
+
 #include 
+#include 
 #include 
-#include 
-#include 
 
 /*
  * Unfortunately it turns out that we have a chicken-and-egg situation
-- 
2.20.1

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

[PATCH v1 2/2] drm: delete drmP.h + drm_os_linux.h

2019-10-07 Thread Sam Ravnborg
There is finally no more users left in the kernel of drmP.h
and drm_os_linux.h (drmP.h was the only user left).
Delete the header files and delete the corresponding todo entry.

When we started this quest there was more than 700 users of drmP.h.
And drmP.h was a huge cover-it-all header file.

Daniel Vetter is the one that followed the work from start
to the end and in between many people have contributed to the
removal process - thanks to everyone!

Signed-off-by: Sam Ravnborg 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
---
 Documentation/gpu/todo.rst |  12 -
 include/drm/drmP.h | 103 -
 include/drm/drm_os_linux.h |  55 
 3 files changed, 170 deletions(-)
 delete mode 100644 include/drm/drmP.h
 delete mode 100644 include/drm/drm_os_linux.h

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 8dc147c93c9c..79785559d711 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -299,18 +299,6 @@ connector register/unregister fixes
 Core refactorings
 =
 
-Clean up the DRM header mess
-
-
-The DRM subsystem originally had only one huge global header, ``drmP.h``. This
-is now split up, but many source files still include it. The remaining part of
-the cleanup work here is to replace any ``#include `` by only the
-headers needed (and fixing up any missing pre-declarations in the headers).
-
-In the end no .c file should need to include ``drmP.h`` anymore.
-
-Contact: Daniel Vetter
-
 Make panic handling work
 
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
deleted file mode 100644
index 037b1f7a87a5..
--- a/include/drm/drmP.h
+++ /dev/null
@@ -1,103 +0,0 @@
-/*
- * Internal Header for the Direct Rendering Manager
- *
- * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
- * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
- * Copyright (c) 2009-2010, Code Aurora Forum.
- * All rights reserved.
- *
- * Author: Rickard E. (Rik) Faith 
- * Author: Gareth Hughes 
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- */
-
-#ifndef _DRM_P_H_
-#define _DRM_P_H_
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-#include 
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-struct module;
-
-struct device_node;
-struct videomode;
-struct dma_resv;
-struct dma_buf_attachment;
-
-struct pci_dev;
-struct pci_controller;
-
-/*
- * NOTE: drmP.h is obsolete - do NOT add anything to this file
- *
- * Do not include drmP.h in new files.
- * Work is ongoing to remove drmP.h includes from existing files
- */
-
-#endif
diff --git a/include/drm/drm_os_linux.h b/include/drm/drm_os_linux.h
deleted file mode 100644
index ee8d61b64f29..
--- a/include/drm/drm_os_linux.h
+++ /dev/null
@@ -1,55 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/**
- * \file drm_os_linux.h
- * OS abstraction macros.
- */
-
-#include/* For task queue support */
-#include 
-#include 
-#include 
-
-/** Current process ID */
-#define DRM_CURRENTPID task_pid_nr(current)
-#define DRM_UDELAY(d)  udelay(d)
-/** Read a byte from a MMIO region */
-#define DRM_READ8(map, offset) readb(((void __iomem *)(map)->handle) + 
(offset))
-/** Read a word from a MMIO region */
-#define DRM_READ16(map, offset) readw(((void __iomem *)(map)->hand

[PATCH v1 0/2]: Finally delete drmP.h

2019-10-07 Thread Sam Ravnborg
One user of drmP.h sneaked in after the merge window.
Drop the use of drmP.h and delete the header file for good.

Small band-aid on top of not going to xdc :-)

Build tested with various architectures and configs.

Sam

Sam Ravnborg (2):
  drm_dp_cec: drop use of drmP.h
  drm: delete drmP.h + drm_os_linux.h

 Documentation/gpu/todo.rst   |  12 -
 drivers/gpu/drm/drm_dp_cec.c |   6 ++-
 include/drm/drmP.h   | 103 ---
 include/drm/drm_os_linux.h   |  55 ---
 4 files changed, 4 insertions(+), 172 deletions(-)

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

Re: [PATCH] drm: Fix comment doc for format_modifiers

2019-10-02 Thread Sam Ravnborg
Hi Andrzej

Always good to have clear documentation.

On Wed, Oct 02, 2019 at 05:43:49PM +0200, Andrzej Pietrasiewicz wrote:
> To human readers
> 
> "array of struct drm_format modifiers" is almost indistinguishable from
> "array of struct drm_format_modifiers", especially given that
> struct drm_format_modifier does exist.
> 
> And indeed the parameter passes an array of uint64_t rather than an array
> of structs, but the first words of the comment suggest that it passes
> an array of structs.
> 
> Signed-off-by: Andrzej Pietrasiewicz 
> ---
>  drivers/gpu/drm/drm_plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index d6ad60ab0d38..df05d8a0dd63 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -160,7 +160,7 @@ static int create_in_format_blob(struct drm_device *dev, 
> struct drm_plane *plane
>   * @funcs: callbacks for the new plane
>   * @formats: array of supported formats (DRM_FORMAT\_\*)
>   * @format_count: number of elements in @formats
> - * @format_modifiers: array of struct drm_format modifiers terminated by
> + * @format_modifiers: array of modifiers of struct drm_format terminated by
>   *DRM_FORMAT_MOD_INVALID
@format_modifiers is an array of DRM_FORMAT_* which are defined as:

#define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
 ((__u32)(c) << 16) | ((__u32)(d) << 24))


But the array is a u64[] like this:
static const u64 tegra20_modifiers[] = {
DRM_FORMAT_MOD_LINEAR,
DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED,
DRM_FORMAT_MOD_INVALID
};

So this is not struct drm_format.

Can you try to give it a shot more?

Sam


Re: [PATCH] drm/tilcdc: include linux/pinctrl/consumer.h again

2019-09-30 Thread Sam Ravnborg
On Mon, Sep 30, 2019 at 05:15:53PM +0300, Jyri Sarha wrote:
> From: Arnd Bergmann 
> 
> This was apparently dropped by accident in a recent
> cleanup, causing a build failure in some configurations now:
> 
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c:296:12: error: implicit declaration of 
> function 'devm_pinctrl_get_select_default' 
> [-Werror,-Wimplicit-function-declaration]
> 
> Fixes: fcb57664172e ("drm/tilcdc: drop use of drmP.h")
> Signed-off-by: Arnd Bergmann 
> Reviewed-by: Laurent Pinchart 
> Signed-off-by: Jyri Sarha 

Thanks
Acked-by: Sam Ravnborg 

> ---
> Just reordered the includes in aplhapetical order and applied
> Laurent's Reviewed-by. I'll apply this to drm-misc-next.
> 
>  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> index 525dc1c0f1c1..530edb3b51cc 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> @@ -7,6 +7,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 22/36] drm/atmel-hlcdc: use bpp instead of cpp for drm_format_info

2019-09-23 Thread Sam Ravnborg
Hi Sandy.

Thanks for taking care of this, but...

On Mon, Sep 23, 2019 at 08:51:45PM +0800, Sandy Huang wrote:
> cpp[BytePerPlane] can't describe the 10bit data format correctly,
> So we use bpp[BitPerPlane] to instead cpp.
> 
> Signed-off-by: Sandy Huang 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 89f5a75..ab7d423 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -644,7 +644,7 @@ static int atmel_hlcdc_plane_atomic_check(struct 
> drm_plane *p,
>   int xdiv = i ? fb->format->hsub : 1;
>   int ydiv = i ? fb->format->vsub : 1;
>  
> - state->bpp[i] = fb->format->cpp[i];
> + state->bpp[i] = fb->format->bpp[i] / 8;
>   if (!state->bpp[i])
>   return -EINVAL;

Awaiting conclusion on Daniels comment on PATCH 1 and has dropped this
patch for now.
And please address the concerns Rob has about bisectability in your
cover letter for v2.

Sam


Re: [PATCH v3] drm: panel-lvds: Potential Oops in probe error handling

2019-09-22 Thread Sam Ravnborg
Hi Dan.

On Wed, Sep 11, 2019 at 01:49:28PM +0300, Dan Carpenter wrote:
> The "lvds->backlight" pointer could be NULL in situations where
> of_parse_phandle() returns NULL.  This code is cleaner if we use the
> managed devm_of_find_backlight() so the clean up is automatic.
> 
> Fixes: 7c9dff5bd643 ("drm: panels: Add LVDS panel driver")
> Signed-off-by: Dan Carpenter 
> ---
> v3: Use devm_of_find_backlight().  This version is quite a bit more
> ambitious, and I haven't tested it so please review carefully.
Looks good.
Applied and pushed to drm-misc-next.

It will hit upstream not at this but next merge window.

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

Re: [PATCH 3/3] ARM: logicpd-torpedo-37xx-devkit-28: Reference new DRM panel

2019-09-21 Thread Sam Ravnborg
Hi Adam.

On Tue, Sep 17, 2019 at 11:12:13AM -0500, Adam Ford wrote:
> With the removal of the panel-dpi from the omap drivers, the
> LCD no longer works.  This patch points the device tree to
> a newly created panel named "logicpd,type28"
> 
> Fixes: 8bf4b1621178 ("drm/omap: Remove panel-dpi driver")
> 
> Signed-off-by: Adam Ford 
Looks good.
One nit below.

With this addressed:

Acked-by: Sam Ravnborg 

Sam
> 
> diff --git a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit-28.dts 
> b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit-28.dts
> index 07ac99b9cda6..00c426bd51a0 100644
> --- a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit-28.dts
> +++ b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit-28.dts
> @@ -11,22 +11,8 @@
>  #include "logicpd-torpedo-37xx-devkit.dts"
>  
>   {
> -
> + /* This isn't the exact LCD, but the timings meet spec */
> + /* To make it work, set CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK=4 */
> + compatible = "logicpd,type28";
>   label = "28";
You left this property - but us it documented and what use has it?

> -
> - panel-timing {
> - clock-frequency = <900>;
> - hactive = <480>;
> - vactive = <272>;
> - hfront-porch = <3>;
> - hback-porch = <2>;
> - hsync-len = <42>;
> - vback-porch = <3>;
> - vfront-porch = <2>;
> - vsync-len = <11>;
> - hsync-active = <1>;
> - vsync-active = <1>;
> - de-active = <1>;
> - pixelclk-active = <0>;
> - };
>  };
> -- 
> 2.17.1


Re: [PATCH 1/3] drm/panel: simple: Add Logic PD Type 28 display support

2019-09-21 Thread Sam Ravnborg
Hi Adam.

On Tue, Sep 17, 2019 at 11:12:11AM -0500, Adam Ford wrote:
> Previously, there was an omap panel-dpi driver that would
> read generic timings from the device tree and set the display
> timing accordingly.  This driver was removed so the screen
> no longer functions.  This patch modifies the panel-simple
> file to setup the timings to the same values previously used.
> 
> Fixes: 8bf4b1621178 ("drm/omap: Remove panel-dpi driver")
> 
> Signed-off-by: Adam Ford 

Patch looks good.
Reviewed-by: Sam Ravnborg 
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index 5a93c4edf1e4..c86c30f3a8a1 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1900,6 +1900,40 @@ static const struct drm_display_mode 
> mitsubishi_aa070mc01_mode = {
>   .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
>  };
>  
> +static const struct drm_display_mode logicpd_type_28_mode = {
> + .clock = 9000,
> + .hdisplay = 480,
> + .hsync_start = 480 + 3,
> + .hsync_end = 480 + 3 + 42,
> + .htotal = 480 + 3 + 42 + 2,
> +
> + .vdisplay = 272,
> + .vsync_start = 272 + 2,
> + .vsync_end = 272 + 2 + 11,
> + .vtotal = 272 + 2 + 11 + 3,
> + .vrefresh = 60,
> + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
> +};
> +
> +static const struct panel_desc logicpd_type_28 = {
> + .modes = _type_28_mode,
> + .num_modes = 1,
> + .bpc = 8,
> + .size = {
> + .width = 105,
> + .height = 67,
> + },
> + .delay = {
> + .prepare = 200,
> + .enable = 200,
> + .unprepare = 200,
> + .disable = 200,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
> +  DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE,
> +};
> +
>  static const struct panel_desc mitsubishi_aa070mc01 = {
>   .modes = _aa070mc01_mode,
>   .num_modes = 1,
> @@ -2948,6 +2982,9 @@ static const struct of_device_id platform_of_match[] = {
>   }, {
>   .compatible = "lg,lp129qe",
>   .data = _lp129qe,
> + }, {
> + .compatible = "logicpd,type28",
> + .data = _type_28,
>   }, {
>   .compatible = "mitsubishi,aa070mc01-ca1",
>   .data = _aa070mc01,
> -- 
> 2.17.1


Re: [PATCH 2/3] dt-bindings: Add Logic PD Type 28 display panel

2019-09-21 Thread Sam Ravnborg
Hi Adam.

Good with even more panels.
But for new bindings please use meta-schema (.yaml) format.
This is what we use for new bindings as it allows better
validation.

Sam

On Tue, Sep 17, 2019 at 11:12:12AM -0500, Adam Ford wrote:
> This patch adds documentation of device tree bindings for the WVGA panel
> Logic PD Type 28 display.
> 
> Signed-off-by: Adam Ford 
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/logicpd,type28.txt 
> b/Documentation/devicetree/bindings/display/panel/logicpd,type28.txt
> new file mode 100644
> index ..829fc5210e06
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/logicpd,type28.txt
> @@ -0,0 +1,26 @@
> +Logic PD Type 28 4.3" WQVGA TFT LCD panel
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
> +
> +Required properties:
> +- compatible: should be "logicpd,type28"
> +
> +Optional properties:
> +- power-supply: regulator to provide the supply voltage
> +- enable-gpios: GPIO pin to enable or disable the panel
> +- backlight: phandle of the backlight device attached to the panel
Is it correct that these are optional for the descrivbed panel?

> +
> +Optional nodes:
> +- Video port for RGB input.
> +
> +Example:
> + lcd0: display {
> + compatible = "logicpd,type28";
> + enable-gpios = < 27 GPIO_ACTIVE_HIGH>;
> + port {
> + lcd_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> -- 
> 2.17.1


Re: [PATCH v5 0/8] add driver for boe, tv101wum-nl6, boe, tv101wum-n53, auo, kd101n80-45na and auo, b101uan08.3 panels

2019-09-16 Thread Sam Ravnborg
Hi Jitao.

> Changes since v4:

You are hit by some of the progress made in the kernel.
New dispaly bindings are preferred to be in meta-schma formal (.yaml
files).
This allows more formals checks and this is the format that we
hope all display bindigns will migrate over to use once
someone steps up and do a mass conversion.

This is a bit extra work now, but much better to have it done
by someone who knows the HW.

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

Re: [PATCH v3 0/2] drm/panel: Extend panels to report their types

2019-09-08 Thread Sam Ravnborg
Hi Laurent.

> > Laurent Pinchart (2):
> >   drm/panel: Add and fill drm_panel type field
> >   drm/bridge: panel: Infer connector type from panel by default
> 
> Applied all three patches (despite this shortlog only shows two
> patches).

I guess you noticed that I have been away a little.
This will continue for at least another week.
day-time job + some travelling ahead.

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

Re: [PATCH v3 0/2] drm/panel: Extend panels to report their types

2019-09-08 Thread Sam Ravnborg
Hi Laurent.

On Wed, Sep 04, 2019 at 04:28:02PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> This series, whose previous version was named "[PATCH v2 0/4] drm/panel:
> Extend panels to report their types" and is available at
> https://www.spinics.net/lists/dri-devel/msg224579.html, allows panels to
> report their type, in order to create drm_connector instances with
> appropriate types in the upper layers.
> 
> In patch 1/2 the drm_panel structure receives a new connector_type field
> to report its type, set through drm_panel_init(), and all connector
> drivers are updated accordingly. The panel-simple driver however only
> reports the LVDS connector type for known-to-be-LVDS panels, while all
> other leave the field initialised to 0, corresponding to
> DRM_MODE_CONNECTOR_Unknown. Panels supported by that driver will need to
> be reviewed one by one and their type updated. This was done to avoid
> reporting an incorrect type, allowing upper layers to catch
> DRM_MODE_CONNECTOR_Unknown and WARN() to trigger an update of the
> corresponding panel.
> 
> Patch 2/2 then modifies drm_panel_bridge_add() and its devm_ counterpart
> to replace the connector type argument with the type reported by the
> panel. This can't unfortunately be forced upon all drivers as several of
> them hardcode a DRM_MODE_CONNECTOR_Unknown type, and would then change
> the connector type reported to userspace, leading to possible breakages.
> A new function, drm_panel_bridge_add_typed(), is added with the existing
> behaviour of drm_panel_bridge_add() to create a panel bridge with a
> forced connector type, and drivers are switched to using that function.
> They should then be switched back one by one to drm_panel_bridge_add()
> after careful review (and clever handling of the connector type change
> issue). The drm_panel_bridge_add_typed() function is marked as
> deprecated and should not be used in new code.
> 
> During review of v2, the question of whether to introduce a new
> DRM_MODE_CONNECTOR_PANEL was raised. This is still being discussed, but
> such a change would still need to expose the existing panel types for
> backward compatibility, and this series wouldn't hinder this in any way.
> I thus believe that we should merge it sooner than later without waiting
> for the DRM_MODE_CONNECTOR_PANEL discussion to settle.
Agreed.

> 
> The patches are available at
> 
>   git://linuxtv.org/pinchartl/media.git omapdrm/panels
> 
> Laurent Pinchart (2):
>   drm/panel: Add and fill drm_panel type field
>   drm/bridge: panel: Infer connector type from panel by default

Applied all three patches (despite this shortlog only shows two
patches).

Pushed to drm-misc-next.

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

Re: [PATCH v2] drm: panel-lvds: Potential Oops in probe error handling

2019-09-08 Thread Sam Ravnborg
Hi Dan.

On Wed, Sep 04, 2019 at 09:55:07PM +0300, Dan Carpenter wrote:
> The "lvds->backlight" pointer could be NULL in situations were
> of_parse_phandle() returns NULL.  Also it's slightly cleaner to use
> backlight_put() which already has a check for NULL built in.
> 
> Fixes: 7c9dff5bd643 ("drm: panels: Add LVDS panel driver")
> Signed-off-by: Dan Carpenter 

A much better fix would be to introduce use of devm_of_find_backlight().
Then you do not have to worry about put().

Care to respin a v3 that does this?

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

Re: [PATCH] drm/ingenic: Hardcode panel type to DPI

2019-08-26 Thread Sam Ravnborg
On Fri, Aug 23, 2019 at 11:30:09PM +0200, Paul Cercueil wrote:
> Hi Laurent,
> 
> 
> Le ven. 23 août 2019 à 23:23, Laurent Pinchart
>  a écrit :
> > The ingenic driver supports DPI panels only at the moment, so hardcode
> > their type to DPI instead of Unknown.
> > 
> > Signed-off-by: Laurent Pinchart 
> > ---
> > Paul, as the driver has been merged in v5.3-rc1, this is a candidate for
> > a v5.3 fix. Keeping the connector type as unknown could cause a
> > userspace dependency on it, preventing it from being changed later.
> 
> Makes sense.
> 
> Reviewed-by: Paul Cercueil 

In another mail we discuss CONNECTOR_PANEL. But we should not hold up
due to this.
Reviewed-by: Sam Ravnborg 

Paul - will you apply to drm-misc-fixes?

Sam


> 
> 
> > 
> > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c
> > b/drivers/gpu/drm/ingenic/ingenic-drm.c
> > index ce1fae3a78a9..2e2ed653e9c6 100644
> > --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> > +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> > @@ -675,10 +675,9 @@ static int ingenic_drm_probe(struct platform_device
> > *pdev)
> > return ret;
> > }
> > 
> > -   if (panel) {
> > +   if (panel)
> > bridge = devm_drm_panel_bridge_add(dev, panel,
> > -  DRM_MODE_CONNECTOR_Unknown);
> > -   }
> > +  DRM_MODE_CONNECTOR_DPI);
> > 
> > priv->dma_hwdesc = dma_alloc_coherent(dev, sizeof(*priv->dma_hwdesc),
> >   >dma_hwdesc_phys,
> > --
> > Regards,
> > 
> > Laurent Pinchart
> > 
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

DRM_MODE_CONNECTOR_PANEL? [Was: drm/panel: Add and fill drm_panel type field]

2019-08-24 Thread Sam Ravnborg
Hi Laurent et all.

On Fri, Aug 23, 2019 at 10:32:44PM +0300, Laurent Pinchart wrote:
> Add a type field to the drm_panel structure to report the panel type,
> using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS,
> eDP, DSI and DPI). This will be used to initialise the corresponding
> connector type.

As discussed on irc before applying this we should evaluate the
idea to introduce DRM_MODE_CONNECTOR_PANEL

Today we have:
DRM_MODE_CONNECTOR_DPI
DRM_MODE_CONNECTOR_DBI
DRM_MODE_CONNECTOR_SPI
DRM_MODE_CONNECTOR_LVDS

The question if what benefit ig gives to distingush between the
different ways panels are connected.

For example for VGA and HDMI there is a physical connector involved so
here it make good sense.
But why should userspace care if the panel is connected via DPI or LVDS?

The more generic DRM_MODE_CONNECTOR_PANEL would be equally descriptive.

We will also start to see new drivers where there will be support
for more than one type of connector interface.

Like for example ili9322:
 - 8-bit serial RGB interface
 - 24-bit parallel RGB interface
 - 8-bit ITU-R BT.601 interface
 - 8-bit ITU-R BT.656 interface

Adding DRM_MODE_CONNECTOR_PANEL may create an xkcd 927 situation.
But we should then deprecate _DPI, _DBI, _SPI, _LVDS.

Sam


> 
> Update all panel drivers accordingly. The panel-simple driver only
> specifies the type for the known to be LVDS panels, while all other
> panels are left as unknown and will be converted on a case-by-case
> basis as they all need to be carefully reviewed.
> 
> Signed-off-by: Laurent Pinchart 
> ---
> Changes since v1:
> 
> - Rename the type field to connector_type
> - Pass the type to the drm_panel_init() function
> - Keep connector type as unknown for non-LVDS panels in panel-simple
> - Flag the toshiba_lt089ac29000 panel as LVDS as reported by Boris
> ---
>  drivers/gpu/drm/drm_panel.c   |  5 +++-
>  drivers/gpu/drm/panel/panel-arm-versatile.c   |  3 ++-
>  .../drm/panel/panel-feiyang-fy07024di26a30d.c |  3 ++-
>  drivers/gpu/drm/panel/panel-ilitek-ili9322.c  |  3 ++-
>  drivers/gpu/drm/panel/panel-ilitek-ili9881c.c |  3 ++-
>  drivers/gpu/drm/panel/panel-innolux-p079zca.c |  3 ++-
>  .../gpu/drm/panel/panel-jdi-lt070me05000.c|  3 ++-
>  .../drm/panel/panel-kingdisplay-kd097d04.c|  2 +-
>  drivers/gpu/drm/panel/panel-lg-lb035q02.c |  3 ++-
>  drivers/gpu/drm/panel/panel-lg-lg4573.c   |  3 ++-
>  drivers/gpu/drm/panel/panel-lvds.c|  3 ++-
>  drivers/gpu/drm/panel/panel-nec-nl8048hl11.c  |  3 ++-
>  drivers/gpu/drm/panel/panel-novatek-nt39016.c |  3 ++-
>  .../drm/panel/panel-olimex-lcd-olinuxino.c|  3 ++-
>  .../gpu/drm/panel/panel-orisetech-otm8009a.c  |  3 ++-
>  .../drm/panel/panel-osd-osd101t2587-53ts.c|  2 +-
>  .../drm/panel/panel-panasonic-vvx10f034n00.c  |  2 +-
>  .../drm/panel/panel-raspberrypi-touchscreen.c |  3 ++-
>  drivers/gpu/drm/panel/panel-raydium-rm67191.c |  3 ++-
>  drivers/gpu/drm/panel/panel-raydium-rm68200.c |  3 ++-
>  .../drm/panel/panel-rocktech-jh057n00900.c|  3 ++-
>  drivers/gpu/drm/panel/panel-ronbo-rb070d30.c  |  3 ++-
>  drivers/gpu/drm/panel/panel-samsung-ld9040.c  |  3 ++-
>  drivers/gpu/drm/panel/panel-samsung-s6d16d0.c |  3 ++-
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c |  3 ++-
>  .../gpu/drm/panel/panel-samsung-s6e63j0x03.c  |  3 ++-
>  drivers/gpu/drm/panel/panel-samsung-s6e63m0.c |  3 ++-
>  drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c |  3 ++-
>  drivers/gpu/drm/panel/panel-seiko-43wvf1g.c   |  3 ++-
>  .../gpu/drm/panel/panel-sharp-lq101r1sx01.c   |  3 ++-
>  .../gpu/drm/panel/panel-sharp-ls037v7dw01.c   |  3 ++-
>  .../gpu/drm/panel/panel-sharp-ls043t1le01.c   |  2 +-
>  drivers/gpu/drm/panel/panel-simple.c  | 26 ++-
>  drivers/gpu/drm/panel/panel-sitronix-st7701.c |  3 ++-
>  .../gpu/drm/panel/panel-sitronix-st7789v.c|  3 ++-
>  drivers/gpu/drm/panel/panel-sony-acx565akm.c  |  3 ++-
>  drivers/gpu/drm/panel/panel-tpo-td028ttec1.c  |  3 ++-
>  drivers/gpu/drm/panel/panel-tpo-td043mtea1.c  |  3 ++-
>  drivers/gpu/drm/panel/panel-tpo-tpg110.c  |  3 ++-
>  drivers/gpu/drm/panel/panel-truly-nt35597.c   |  3 ++-
>  include/drm/drm_panel.h   | 12 -
>  41 files changed, 112 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index ba2fad4c9648..ed7985c0535a 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -46,16 +46,19 @@ static LIST_HEAD(panel_list);
>   * @panel: DRM panel
>   * @dev: parent device of the panel
>   * @funcs: panel operations
> + * @connector_type: the connector type (DRM_MODE_CONNECTOR_*) corresponding 
> to
> + *   the panel interface
>   *
>   * Initialize the panel structure for subsequent registration with
>   * drm_panel_add().
>   */
>  void drm_panel_init(struct drm_panel *panel, struct device *dev,
> - const struct drm_panel_funcs 

Re: [PATCH v2 2/4] drm/panel: Initialise panel dev and funcs through drm_panel_init()

2019-08-24 Thread Sam Ravnborg
On Fri, Aug 23, 2019 at 10:32:43PM +0300, Laurent Pinchart wrote:
> Instead of requiring all drivers to set the dev and funcs fields of
> drm_panel manually after calling drm_panel_init(), pass the data as
> arguments to the function. This simplifies the panel drivers, and will
> help future refactoring when adding new arguments to drm_panel_init().
> 
> The panel drivers have been updated with the following Coccinelle
> semantic patch, with manual inspection to verify that no call to
> drm_panel_init() with a single argument still exists.
> 
> @@
> expression panel;
> expression device;
> identifier ops;
> @@
>  drm_panel_init(
> + , device, 
>  );
>  ...
> (
> -panel.dev = device;
> -panel.funcs = 
> |
> -panel.funcs = 
> -panel.dev = device;
> )
> 
> Suggested-by: Sam Ravnborg 
> Signed-off-by: Laurent Pinchart 

Thanks, applied to drm-misc-next.
Nice use of Coccinelle.

Sam


> ---
>  drivers/gpu/drm/drm_panel.c   | 11 ---
>  drivers/gpu/drm/panel/panel-arm-versatile.c   |  4 +---
>  drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c |  4 +---
>  drivers/gpu/drm/panel/panel-ilitek-ili9322.c  |  4 +---
>  drivers/gpu/drm/panel/panel-ilitek-ili9881c.c |  4 +---
>  drivers/gpu/drm/panel/panel-innolux-p079zca.c |  4 +---
>  drivers/gpu/drm/panel/panel-jdi-lt070me05000.c|  4 +---
>  drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c|  5 ++---
>  drivers/gpu/drm/panel/panel-lg-lb035q02.c |  4 +---
>  drivers/gpu/drm/panel/panel-lg-lg4573.c   |  4 +---
>  drivers/gpu/drm/panel/panel-lvds.c|  4 +---
>  drivers/gpu/drm/panel/panel-nec-nl8048hl11.c  |  4 +---
>  drivers/gpu/drm/panel/panel-novatek-nt39016.c |  4 +---
>  drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c|  4 +---
>  drivers/gpu/drm/panel/panel-orisetech-otm8009a.c  |  4 +---
>  drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c|  5 ++---
>  drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c  |  5 ++---
>  drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c |  4 +---
>  drivers/gpu/drm/panel/panel-raydium-rm67191.c |  4 +---
>  drivers/gpu/drm/panel/panel-raydium-rm68200.c |  4 +---
>  drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c|  4 +---
>  drivers/gpu/drm/panel/panel-ronbo-rb070d30.c  |  4 +---
>  drivers/gpu/drm/panel/panel-samsung-ld9040.c  |  4 +---
>  drivers/gpu/drm/panel/panel-samsung-s6d16d0.c |  4 +---
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c |  4 +---
>  drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c  |  4 +---
>  drivers/gpu/drm/panel/panel-samsung-s6e63m0.c |  4 +---
>  drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c |  4 +---
>  drivers/gpu/drm/panel/panel-seiko-43wvf1g.c   |  4 +---
>  drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c   |  4 +---
>  drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c   |  4 +---
>  drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c   |  5 ++---
>  drivers/gpu/drm/panel/panel-simple.c  |  4 +---
>  drivers/gpu/drm/panel/panel-sitronix-st7701.c |  4 +---
>  drivers/gpu/drm/panel/panel-sitronix-st7789v.c|  4 +---
>  drivers/gpu/drm/panel/panel-sony-acx565akm.c  |  4 +---
>  drivers/gpu/drm/panel/panel-tpo-td028ttec1.c  |  4 +---
>  drivers/gpu/drm/panel/panel-tpo-td043mtea1.c  |  4 +---
>  drivers/gpu/drm/panel/panel-tpo-tpg110.c  |  4 +---
>  drivers/gpu/drm/panel/panel-truly-nt35597.c   |  4 +---
>  include/drm/drm_panel.h   |  3 ++-
>  41 files changed, 53 insertions(+), 121 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 6b0bf42039cf..ba2fad4c9648 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -44,13 +44,18 @@ static LIST_HEAD(panel_list);
>  /**
>   * drm_panel_init - initialize a panel
>   * @panel: DRM panel
> + * @dev: parent device of the panel
> + * @funcs: panel operations
>   *
> - * Sets up internal fields of the panel so that it can subsequently be added
> - * to the registry.
> + * Initialize the panel structure for subsequent registration with
> + * drm_panel_add().
>   */
> -void drm_panel_init(struct drm_panel *panel)
> +void drm_panel_init(struct drm_panel *panel, struct device *dev,
> + const struct drm_panel_funcs *funcs)
>  {
>   INIT_LIST_HEAD(>list);
> + panel->dev = dev;
> + panel->funcs = funcs;
>  }
>  EXPORT_SYMBOL(drm_panel_init);
>  
> diff --git a/drivers/gpu/drm/pan

Re: [PATCH v2 1/4] drm/panel: Add missing drm_panel_init() in panel drivers

2019-08-24 Thread Sam Ravnborg
On Fri, Aug 23, 2019 at 10:32:42PM +0300, Laurent Pinchart wrote:
> Panels must be initialised with drm_panel_init(). Add the missing
> function call in the panel-raspberrypi-touchscreen.c and
> panel-sitronix-st7789v.c drivers.
> 
> Signed-off-by: Laurent Pinchart 

Thanks, good to have this done in the right way.

This does not solve any know bugs visible to users. At least there are
no reports I know off. So for now only applied to drm-misc-next.

Sam

> ---
>  drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c | 1 +
>  drivers/gpu/drm/panel/panel-sitronix-st7789v.c| 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c 
> b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> index b5b14aa059ea..2aa89eaecf6f 100644
> --- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> +++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> @@ -426,6 +426,7 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c,
>   return PTR_ERR(ts->dsi);
>   }
>  
> + drm_panel_init(>base);
>   ts->base.dev = dev;
>   ts->base.funcs = _touchscreen_funcs;
>  
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c 
> b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> index 5e3e92ea9ea6..3b2612ae931e 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -381,6 +381,7 @@ static int st7789v_probe(struct spi_device *spi)
>   spi_set_drvdata(spi, ctx);
>   ctx->spi = spi;
>  
> + drm_panel_init(>panel);
>   ctx->panel.dev = >dev;
>   ctx->panel.funcs = _drm_funcs;
>  
> -- 
> Regards,
> 
> Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panel: Add and fill drm_panel type field

2019-08-22 Thread Sam Ravnborg
Hi Laurent.

Thanks for looking into this, but you will not be happy in a minute...

On Fri, Aug 23, 2019 at 04:40:32AM +0300, Laurent Pinchart wrote:
> Add a type field to the drm_panel structure to report the panel type,
> using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS,
> eDP, DSI and DPI).

> This will be used to initialise the corresponding connector type.
Ohh, that explains what this should be used for.
I had missed that we have the panel when we create the drm_connector.

(I had seen we had to use the connector type to match a panel with a
connector or something like that).

> diff --git a/drivers/gpu/drm/panel/panel-arm-versatile.c 
> b/drivers/gpu/drm/panel/panel-arm-versatile.c
> index 5f72c922a04b..5c335fc1632b 100644
> --- a/drivers/gpu/drm/panel/panel-arm-versatile.c
> +++ b/drivers/gpu/drm/panel/panel-arm-versatile.c
> @@ -353,6 +353,7 @@ static int versatile_panel_probe(struct platform_device 
> *pdev)
>   drm_panel_init(>panel);
>   vpanel->panel.dev = dev;
>   vpanel->panel.funcs = _panel_drm_funcs;
> + vpanel->panel.type = DRM_MODE_CONNECTOR_DPI;

The pattern where we call a simple init like here,
and then have a set of mandatory assignments right after
is not a good way to help the driver writer.

We should instead do:

drm_panel_init(>panel, dev, _panel_drm_funcs, 
DRM_MODE_CONNECTOR_DPI);

Then all drm_panel users are forced to supply the init parameters,
and we do not secretly rely on some defaults for panels that do not have it.
Also new panels will fail to build if they do not specify the new field.



This may also allow us to chatch that:

> diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c 
> b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> index b5b14aa059ea..cac074939e2c 100644
> --- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> +++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> @@ -428,6 +428,7 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c,
>  
>   ts->base.dev = dev;
>   ts->base.funcs = _touchscreen_funcs;
> + ts->base.type = DRM_MODE_CONNECTOR_DSI;

forgets to call drm_panel_init()


> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index 28fa6ba7b767..6d5d0c51e97e 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -94,6 +94,7 @@ struct panel_desc {
>  
>   u32 bus_format;
>   u32 bus_flags;
> + unsigned int type;
>  };
>  
>  struct panel_simple {
> @@ -467,6 +468,7 @@ static int panel_simple_probe(struct device *dev, const 
> struct panel_desc *desc)
>   drm_panel_init(>base);
>   panel->base.dev = dev;
>   panel->base.funcs = _simple_funcs;
> + panel->base.type = desc->type ? desc->type : DRM_MODE_CONNECTOR_DPI;
>  
>   err = drm_panel_add(>base);
>   if (err < 0)
> @@ -833,6 +835,7 @@ static const struct panel_desc auo_g133han01 = {
>   .unprepare = 1000,
>   },
>   .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA,
> + .type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct display_timing auo_g185han01_timings = {
> @@ -862,6 +865,7 @@ static const struct panel_desc auo_g185han01 = {
>   .unprepare = 1000,
>   },
>   .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> + .type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct display_timing auo_p320hvn03_timings = {
> @@ -890,6 +894,7 @@ static const struct panel_desc auo_p320hvn03 = {
>   .unprepare = 500,
>   },
>   .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> + .type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode auo_t215hvn01_mode = {
> @@ -1205,6 +1210,7 @@ static const struct panel_desc dlc_dlc0700yzg_1 = {
>   .disable = 200,
>   },
>   .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> + .type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct display_timing dlc_dlc1010gig_timing = {
> @@ -1235,6 +1241,7 @@ static const struct panel_desc dlc_dlc1010gig = {
>   .unprepare = 60,
>   },
>   .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> + .type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode edt_et035012dm6_mode = {
> @@ -1501,6 +1508,7 @@ static const struct panel_desc hannstar_hsd070pww1 = {
>   .height = 94,
>   },
>   .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> + .type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct display_timing hannstar_hsd100pxn1_timing = {
> @@ -1525,6 +1533,7 @@ static const struct panel_desc hannstar_hsd100pxn1 = {
>   .height = 152,
>   },
>   .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> + .type = DRM_MODE_CONNECTOR_LVDS,
>  };
>  
>  static const struct drm_display_mode hitachi_tx23d38vm0caa_mode = {
> @@ -1631,6 +1640,7 @@ static const struct panel_desc 

Re: [PATCH v5 00/25] drm: Kirin driver cleanups to prep for Kirin960 support

2019-08-21 Thread Sam Ravnborg
Hi John.

On Tue, Aug 20, 2019 at 11:06:01PM +, John Stultz wrote:
> Sending this out again (apologies!), to address a few issues Sam
> found.
> 
> This patchset contains one fix (in the front, so its easier to
> eventually backport), and a series of changes from YiPing to
> refactor the kirin drm driver so that it can be used on both
> kirin620 based devices (like the original HiKey board) as well
> as kirin960 based devices (like the HiKey960 board).
> 
> The full kirin960 drm support is still being refactored, but as
> this base kirin rework was getting to be substantial, I wanted
> to send out the first chunk, so that the review burden wasn't
> overwhelming.
> 
> The full HiKey960 patch stack can be found here:
>   
> https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/hikey960-mainline-WIP

Applied from the mails - as this is what my tooling expect.
Pushed to drm-misc-next.

Sam


Re: [PATCH v4 00/25] drm: Kirin driver cleanups to prep for Kirin960 support

2019-08-20 Thread Sam Ravnborg
Hi John.

On Mon, Aug 19, 2019 at 11:02:56PM +, John Stultz wrote:
> Sending this out again, to get it based on drm-misc-next.
> 
> This patchset contains one fix (in the front, so its easier to
> eventually backport), and a series of changes from YiPing to
> refactor the kirin drm driver so that it can be used on both
> kirin620 based devices (like the original HiKey board) as well
> as kirin960 based devices (like the HiKey960 board).
> 
> The full kirin960 drm support is still being refactored, but as
> this base kirin rework was getting to be substantial, I wanted
> to send out the first chunk, so that the review burden wasn't
> overwhelming.
> 
> The full HiKey960 patch stack can be found here:
>   
> https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/hikey960-mainline-WIP
> 
> thanks
> -john
> 
> 
> New in v4:
> * Rebased to drm-misc-next, minor tweaks to merge changes
> * Dropped "drm: kirin: Get rid of drmP.h includes" as similar change
>   was already in drm-misc next
> * Added acked-by tag from Xinliang

There was some checkpatch noises in some of the patches - please verify
with "--strict".
Mostly alignment of parameters with open parantesis
Sample - but there was similar issues in other patches:

8788b59decc8 (HEAD -> drm-misc-next) drm: kirin: Move ade drm init to kirin drm 
drv
-:208: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#208: FILE: drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c:41:
+static int kirin_drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
+   struct drm_plane *plane,

-:244: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#244: FILE: drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c:77:
+   ret = drm_universal_plane_init(dev, plane, 1, data->plane_funcs,
+   data->channel_formats,

-:271: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#271: FILE: drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c:104:
+static int kirin_drm_private_init(struct drm_device *dev,
+   const struct kirin_drm_data *driver_data)




And then the build failed like this:
 LD [M]  drivers/gpu/drm/hisilicon/kirin/kirin-drm.o
aarch64-linux-gnu-ld: drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.o: in function 
`init_module':
dw_drm_dsi.c:(.init.text+0x0): multiple definition of `init_module'; 
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.o:kirin_drm_drv.c:(.init.text+0x0):
 first defined here
aarch64-linux-gnu-ld: drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.o: in function 
`cleanup_module':
dw_drm_dsi.c:(.exit.text+0x0): multiple definition of `cleanup_module'; 
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.o:kirin_drm_drv.c:(.exit.text+0x0):
 first defined here
make[3]: *** [/home/sam/drm/linux.git/scripts/Makefile.build:464: 
drivers/gpu/drm/hisilicon/kirin/kirin-drm.o] Error 1
make[2]: *** [/home/sam/drm/linux.git/scripts/Makefile.build:490: 
drivers/gpu/drm/hisilicon/kirin] Error 2
make[1]: *** [/home/sam/drm/linux.git/Makefile:1776: 
drivers/gpu/drm/hisilicon/] Error 2
make[1]: Leaving directory '/home/sam/drm/linux.git/.build/arm64-allmodconfig'
make: *** [Makefile:179: sub-make] Error 2

It was a simple allmodconfig build where I did:

make drivers/gpu/drm/hisilicon/

Please fix and resend. I did not look further.

Sam


Re: [PATCH v2 13/50] drm/bridge: panel: Implement bridge connector operations

2019-08-20 Thread Sam Ravnborg
Hi Laurent.

On Tue, Aug 20, 2019 at 04:16:44AM +0300, Laurent Pinchart wrote:
> Implement the newly added bridge connector operations, allowing the
> usage of drm_bridge_panel with drm_bridge_connector.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/bridge/panel.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index f5b8e55301ac..1c7f5b648f05 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -60,7 +60,7 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
>   int ret;
>  
>   if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> - return -EINVAL;
> + return 0;
>  
>   if (!bridge->encoder) {
>   DRM_ERROR("Missing encoder\n");
> @@ -123,6 +123,18 @@ static void panel_bridge_post_disable(struct drm_bridge 
> *bridge)
>   drm_panel_unprepare(panel_bridge->panel);
>  }
>  
> +static int panel_bridge_get_modes(struct drm_bridge *bridge,
> +   struct drm_connector *connector)
> +{
> + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
> +
> + /*
> +  * FIXME: drm_panel_get_modes() should take the connector as an
> +  * argument.
> +  */
Noted, I have patches to fix this. Needs a little testing/polishing
before I post them.

> + return drm_panel_get_modes(panel_bridge->panel);
> +}
> +
>  static const struct drm_bridge_funcs panel_bridge_bridge_funcs = {
>   .attach = panel_bridge_attach,
>   .detach = panel_bridge_detach,
> @@ -130,6 +142,7 @@ static const struct drm_bridge_funcs 
> panel_bridge_bridge_funcs = {
>   .enable = panel_bridge_enable,
>   .disable = panel_bridge_disable,
>   .post_disable = panel_bridge_post_disable,
> + .get_modes = panel_bridge_get_modes,
>  };
>  
>  /**
> @@ -175,6 +188,9 @@ struct drm_bridge *drm_panel_bridge_add(struct drm_panel 
> *panel,
>  #ifdef CONFIG_OF
>   panel_bridge->bridge.of_node = panel->dev->of_node;
>  #endif
> + panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES;
> + /* FIXME: The panel should report its type. */
> + panel_bridge->bridge.type = DRM_MODE_CONNECTOR_DPI;
Confused.
We move the connector to the display controller.
So the panel does not know the type.

In others words - please put a few more words on this FIXME.

Sam

>  
>   drm_bridge_add(_bridge->bridge);
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] video: omapfb2: Make standard and custom panel drivers mutually exclusive

2019-08-20 Thread Sam Ravnborg
Hi Daniel

> > 
> > Ohh, and dim came to my rescue. My Fixes: syntax was wrong but it was
> > caught in my "dim push" - nice.
> 
> Aside: dim fixes and dim cite should generate conformant commit citations.

Maybe I should invest more time in the wonders of dim.
So far my use is limited to:

dim update-branches
dim push
dim apply
dim add-missing-cc

But I can see dim has a bit more possibilities.
Well, time to read some of the documentation.

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

Re: [PATCH RFC 07/19] drm/msm: Use drm_attach_bridge() to attach a bridge to an encoder

2019-08-19 Thread Sam Ravnborg
On Thu, Aug 08, 2019 at 05:11:38PM +0200, Boris Brezillon wrote:
> This is part of our attempt to make the bridge chain a double-linked
> list based on the generic list helpers. In order to do that, we must
> patch all drivers manipulating the encoder->bridge field directly.
> 
> Signed-off-by: Boris Brezillon 
Reviewed-by: Sam Ravnborg 

Sean, this patch looks like a nice cleanup we can apply
outside the series.
It would be good that drivers do not poke direct in
the encoder data that this patch fixes.

Sam

> ---
>  drivers/gpu/drm/msm/edp/edp.c   | 4 +++-
>  drivers/gpu/drm/msm/hdmi/hdmi.c | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/edp/edp.c b/drivers/gpu/drm/msm/edp/edp.c
> index 0f312ac5b624..ad4e963ccd9b 100644
> --- a/drivers/gpu/drm/msm/edp/edp.c
> +++ b/drivers/gpu/drm/msm/edp/edp.c
> @@ -178,7 +178,9 @@ int msm_edp_modeset_init(struct msm_edp *edp, struct 
> drm_device *dev,
>   goto fail;
>   }
>  
> - encoder->bridge = edp->bridge;
> + ret = drm_bridge_attach(encoder, edp->bridge, NULL);
> + if (ret)
> + goto fail;
>  
>   priv->bridges[priv->num_bridges++]   = edp->bridge;
>   priv->connectors[priv->num_connectors++] = edp->connector;
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 0e4217be3f00..55b9a8c8312b 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -327,7 +327,9 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>   goto fail;
>   }
>  
> - encoder->bridge = hdmi->bridge;
> + ret = drm_bridge_attach(encoder, hdmi->bridge, NULL);
> + if (ret)
> + goto fail;
>  
>   priv->bridges[priv->num_bridges++]   = hdmi->bridge;
>   priv->connectors[priv->num_connectors++] = hdmi->connector;
> -- 
> 2.21.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/drv: Use // for comments in example code

2019-08-19 Thread Sam Ravnborg
Hi Jonathan.

Thanks for making this more readable in the html output.

On Thu, Aug 08, 2019 at 06:36:28PM +0200, Jonathan Neuschäfer wrote:
> This improves Sphinx output in two ways:
> 
> - It avoids an unmatched single-quote ('), about which Sphinx complained:
> 
>   /.../Documentation/gpu/drm-internals.rst:298:
> WARNING: Could not lex literal_block as "c". Highlighting skipped.
> 
>   An alternative approach would be to replace "can't" with a word that
>   doesn't have a single-quote.
> 
> - It lets Sphinx format the comments in italics and grey, making the
>   code slightly easier to read.
> 
> Signed-off-by: Jonathan Neuschäfer 

I got Acked-by from Daniel Vetter and has now applied this patch
to drm-misc-next.
It will show up in linux-next after the merge window.

Sam


Re: [PATCH 1/2] video: omapfb2: Make standard and custom panel drivers mutually exclusive

2019-08-19 Thread Sam Ravnborg
Hi Bartlomiej

On Mon, Aug 19, 2019 at 04:16:26PM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> On 8/16/19 3:32 PM, Laurent Pinchart wrote:
> > On Fri, Aug 16, 2019 at 04:20:46PM +0300, Tomi Valkeinen wrote:
> >> On 16/08/2019 15:22, Laurent Pinchart wrote:
> >>> Standard DRM panel drivers for several panels used by omapfb2 are now
> >>> available. Their module name clashes with the modules from
> >>> drivers/gpu/drm/omapdrm/displays/, part of the deprecated omapfb2 fbdev
> >>
> >> Shouldn't that path be drivers/video/fbdev/omap2/omapfb/displays?
> > 
> > Absolutely :-) Could this be fixed when applying ? Otherwise I'll
> > submit a v2.
> > 
> >>> driver. As omapfb2 can only be compiled when the omapdrm driver is
> >>> disabled, and the DRM panel drivers are useless in that case, make the
> >>> omapfb2 panels depend on the standard DRM panels being disabled to fix
> >>> the name clash.
> >>>
> >>> Signed-off-by: Laurent Pinchart 
> >>> ---
> >>>   drivers/video/fbdev/omap2/omapfb/displays/Kconfig | 5 +
> >>>   1 file changed, 5 insertions(+)
> >>
> >> Cc'd Bartlomiej.
> 
> Thanks Tomi.
> 
> > Oops, sorry, forgot to do that :-S
> > 
> >> Reviewed-by: Tomi Valkeinen 
> 
> Acked-by: Bartlomiej Zolnierkiewicz 

I took the liberty to apply it already only with Tomi's r-b.
Let me know if you want me to wait for your ack in the future.
This was touching two sub-systems and it can
in such cases sometimes be a challenge to get all acks.

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

Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535

2019-08-19 Thread Sam Ravnborg
Hi Bogdan.

> > >   adv7533_detach_dsi(adv7511);
> > >   i2c_unregister_device(adv7511->i2c_cec);
> > >   if (adv7511->cec_clk)
> > > @@ -1266,8 +1278,9 @@ static const struct i2c_device_id
> > > adv7511_i2c_ids[] = {
> > >   { "adv7511", ADV7511 },
> > >   { "adv7511w", ADV7511 },
> > >   { "adv7513", ADV7511 },
> > > -#ifdef CONFIG_DRM_I2C_ADV7533
> > > +#ifdef CONFIG_DRM_I2C_ADV753x
> > >   { "adv7533", ADV7533 },
> > > + { "adv7535", ADV7535 },
> > >  #endif
> > 
> > This ifdef may not be needed??
> > If we did not get this type we will not look it up.
> But if we have defined in DT adv7533/5 device but
> CONFIG_DRM_I2C_ADV753x not selected probe will fail with ENODEV. That
> would be ok?

What do we gain from this complexity in the end.
Why not let the driver always support all variants.

If this result in a simpler driver, and less choices in Kconfig
then it is a win-win.

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

Re: [RESEND][PATCH v3 00/26] drm: Kirin driver cleanups to prep for Kirin960 support

2019-08-18 Thread Sam Ravnborg
Hi Xinliang

> > As Maintainers can we please get some feedback from one of you.
> > Just an "OK to commit" would do it.
> > But preferably an ack or a review on the individual patches.
> 
> As I have done a pre-review and talked with the  author before sending out
> the patches.
> So, for this serial patches,
> Acked-by: Xinliang Liu 

Thanks!
We all know how it is to be busy, especially when trying to keep up
after role changes.
Unless someone beats me, then I will apply tonight or tomorrow.

> > If the reality is that John is the Maintainer today,
> > then we should update MAINTAINERS to reflect this.
> 
> I am assuming you are talking about the kirin[1] drm driver not the hibmc[2]
> one, right?
> I really appreciate John's awesome work at kirin drm driver all the way.
> Honestly, after my work change from mobile to server years ago, I am always
> waiting for some guy who is stably working at kirin drm driver to take the
> maintenance work.
> John, surely is a such guy.  Please add up a patch to update the maintainer
> as John, if John agree so.  Then John can push the patch set to drm
> maintainer himself.
> *Note* that the maintainer patch should break hisilicon drivers into kirin
> and hibmc two parts, like bellow:
> 
> DRM DRIVERS FOR HISILICON HIBMC
> M:  Xinliang Liu 
> ...
> F:  drivers/gpu/drm/hisilicon/hibmc
> ...
> 
> DRM DRIVERS FOR HISILICON KIRIN
> M:  John Stultz 
> ...
> F:  drivers/gpu/drm/hisilicon/kirin
> ...
> 
> [1] drivers/gpu/drm/hisilicon/kirin # for kirin mobile display driver
> [2] drivers/gpu/drm/hisilicon/hibmc # for server VGA driver

Hi John

Up to the challenge?
If yes then please consider to apply for commit rights to drm-misc-next.

And read:
https://drm.pages.freedesktop.org/maintainer-tools/index.html

See this to get an account:
https://www.freedesktop.org/wiki/AccountRequests/

You will need an ssh account for drm-misc-next as it is not (yet?)
gitlab enabled.

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

Re: [PATCH] drm/arm: drop use of drmP.h

2019-08-17 Thread Sam Ravnborg
Hi Sidong

On Sat, Aug 17, 2019 at 08:41:15AM +0100, Sidong Yang wrote:
> Drop use of deprecated drmP.h header file.
> Remove drmP.h includes and add some include headers for function or
> struct that used in code.

Thanks for your patch.
We already have a similiar patch in drm-misc-next, that
drop the use of drmP.h from arm so this patch is obsoleted.

But keep up the spirit and send us other good stuff.

Sam


Re: [PATCH][drm-next] drm/panel: remove redundant assignment to val

2019-08-17 Thread Sam Ravnborg
On Sat, Aug 17, 2019 at 01:21:24PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Variable val is initialized to a value in a for-loop that is
> never read and hence it is redundant. Remove it.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King 

Thanks. Applied and pushed to drm-misc-next.

Sam

> ---
>  drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c 
> b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c
> index 3b4f30c0fdae..84370562910f 100644
> --- a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c
> +++ b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c
> @@ -116,7 +116,7 @@ static void td043mtea1_write_gamma(struct 
> td043mtea1_panel *lcd)
>   td043mtea1_write(lcd, 0x13, val);
>  
>   /* gamma bits [7:0] */
> - for (val = i = 0; i < 12; i++)
> + for (i = 0; i < 12; i++)
>   td043mtea1_write(lcd, 0x14 + i, gamma[i] & 0xff);
>  }
>  
> -- 
> 2.20.1


Re: [PATCH 1/2] video: omapfb2: Make standard and custom panel drivers mutually exclusive

2019-08-16 Thread Sam Ravnborg
Hi Laurent.

On Fri, Aug 16, 2019 at 09:39:05PM +0300, Laurent Pinchart wrote:
> Hi Sam,
> 
> On Fri, Aug 16, 2019 at 07:31:05PM +0200, Sam Ravnborg wrote:
> > Hi Laurent
> > 
> > Thanks for beating me on this!
> > 
> > On Fri, Aug 16, 2019 at 03:22:27PM +0300, Laurent Pinchart wrote:
> > > Standard DRM panel drivers for several panels used by omapfb2 are now
> > > available. Their module name clashes with the modules from
> > > drivers/gpu/drm/omapdrm/displays/, part of the deprecated omapfb2 fbdev
> > > driver. As omapfb2 can only be compiled when the omapdrm driver is
> > > disabled, and the DRM panel drivers are useless in that case, make the
> > > omapfb2 panels depend on the standard DRM panels being disabled to fix
> > > the name clash.
> > > 
> > > Signed-off-by: Laurent Pinchart 
> > > ---
> > >  drivers/video/fbdev/omap2/omapfb/displays/Kconfig | 5 +
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/video/fbdev/omap2/omapfb/displays/Kconfig 
> > > b/drivers/video/fbdev/omap2/omapfb/displays/Kconfig
> > > index 8c1c5a4cfe18..744416dc530e 100644
> > > --- a/drivers/video/fbdev/omap2/omapfb/displays/Kconfig
> > > +++ b/drivers/video/fbdev/omap2/omapfb/displays/Kconfig
> > > @@ -49,6 +49,7 @@ config FB_OMAP2_PANEL_DSI_CM
> > >  config FB_OMAP2_PANEL_SONY_ACX565AKM
> > >   tristate "ACX565AKM Panel"
> > >   depends on SPI && BACKLIGHT_CLASS_DEVICE
> > > + depends on DRM_PANEL_SONY_ACX565AKM = n
> > 
> > Would:
> > depends on !DRM_PANEL_SONY_ACX565AKM
> > 
> > do the same?
> > 
> > Example:
> > config FB_INTEL
> > tristate "Intel 
> > 830M/845G/852GM/855GM/865G/915G/945G/945GM/965G/965GM support"
> > depends on FB && PCI && X86 && AGP_INTEL && EXPERT
> > ...
> > depends on !DRM_I915
> > 
> > 
> > DRM_915 is a tristate symbol.
> > 
> > It is maybe bikeshedding, but the ! seems more logical/readable to 
> > me.
> 
> The two are equivalent as far as I can tell, so I'm fine with !.
> Would you change that when applying, or should I send a v2 ?

I failed to convince myself they are equivalent.

From kconfig-languae.rst:

"
'!'(6)
(6) Returns the result of (2-/expr/).

An expression can have a value of 'n', 'm' or 'y' (or 0, 1, 2
respectively for calculations). A menu entry becomes visible when its
expression evaluates to 'm' or 'y'.
"

So ! equals 1 for m and 0 for y. If I get it right.

My testing was not fully conclusive.

So I left it as is.
Added a few Fixes: etc. tags.
Applied and pushed to drm-misc-next.

Ohh, and dim came to my rescue. My Fixes: syntax was wrong but it was
caught in my "dim push" - nice.

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

Re: [PATCH 0/2] Fix module name clash with omapdrm and DRM panel modules

2019-08-16 Thread Sam Ravnborg
Hi all

On Fri, Aug 16, 2019 at 03:22:26PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> The patch series "[PATCH v4 0/9] DRM panel drivers for omapdrm" added
> new DRM panel drivers for a set of panels used on OMAP platforms, with
> the omapdrm and omapfb2 drivers. Both those drivers contain
> omap-specific panel drivers, which the new DRM panel drivers aims to
> replace for omapdrm. The new drivers use the same module name as the old
> ones (with one exception), resulting in module name clashes.
> 
> This needs to be fixed separately for omapfb2 and omapdrm.
> 
> For omapfb2, we can disable the omapfb2-specific panel drivers in
> Kconfig when the standard drivers are enabled. As omapfb2 and omapdrm
> are mutually exclusive, the DRM panel drivers are not useful with
> omapfb2 in any case. This is done in patch 1/2.
> 
> For omapdrm, we can simply drop the omapdrm-specific panel drivers, as
> the new drivers replace them. This is done in patch 2/2, which was
> already part of the larger omapdrm patch series "[PATCH 00/60] drm/omap:
> Replace custom display drivers with drm_bridge and drm_panel".
> 
> Laurent Pinchart (2):
>   video: omapfb2: Make standard and custom panel drivers mutually
> exclusive
>   drm/omap: displays: Remove unused panel drivers

Applied and pushed to drm-misc-next.

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

Re: [PATCH 1/2] video: omapfb2: Make standard and custom panel drivers mutually exclusive

2019-08-16 Thread Sam Ravnborg
Hi Laurent

Thanks for beating me on this!

On Fri, Aug 16, 2019 at 03:22:27PM +0300, Laurent Pinchart wrote:
> Standard DRM panel drivers for several panels used by omapfb2 are now
> available. Their module name clashes with the modules from
> drivers/gpu/drm/omapdrm/displays/, part of the deprecated omapfb2 fbdev
> driver. As omapfb2 can only be compiled when the omapdrm driver is
> disabled, and the DRM panel drivers are useless in that case, make the
> omapfb2 panels depend on the standard DRM panels being disabled to fix
> the name clash.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/video/fbdev/omap2/omapfb/displays/Kconfig | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/video/fbdev/omap2/omapfb/displays/Kconfig 
> b/drivers/video/fbdev/omap2/omapfb/displays/Kconfig
> index 8c1c5a4cfe18..744416dc530e 100644
> --- a/drivers/video/fbdev/omap2/omapfb/displays/Kconfig
> +++ b/drivers/video/fbdev/omap2/omapfb/displays/Kconfig
> @@ -49,6 +49,7 @@ config FB_OMAP2_PANEL_DSI_CM
>  config FB_OMAP2_PANEL_SONY_ACX565AKM
>   tristate "ACX565AKM Panel"
>   depends on SPI && BACKLIGHT_CLASS_DEVICE
> + depends on DRM_PANEL_SONY_ACX565AKM = n

Would:
depends on !DRM_PANEL_SONY_ACX565AKM

do the same?

Example:
config FB_INTEL
tristate "Intel 830M/845G/852GM/855GM/865G/915G/945G/945GM/965G/965GM 
support"
depends on FB && PCI && X86 && AGP_INTEL && EXPERT
...
depends on !DRM_I915


DRM_915 is a tristate symbol.

It is maybe bikeshedding, but the ! seems more logical/readable to me.

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

Re: linux-next: build warning after merge of the drm-misc tree

2019-08-15 Thread Sam Ravnborg
Hi Stephen.

On Fri, Aug 16, 2019 at 01:31:32PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the drm-misc tree, today's linux-next build (x86_64
> allmodconfig) produced this warning:
> 
> warning: same module names found:
>   drivers/video/fbdev/omap2/omapfb/displays/panel-nec-nl8048hl11.ko
>   drivers/gpu/drm/panel/panel-nec-nl8048hl11.ko
> warning: same module names found:
>   drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.ko
>   drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.ko
> warning: same module names found:
>   drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.ko
>   drivers/gpu/drm/panel/panel-sony-acx565akm.ko
> warning: same module names found:
>   drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.ko
>   drivers/gpu/drm/panel/panel-tpo-td028ttec1.ko
> warning: same module names found:
>   drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td043mtea1.ko
>   drivers/gpu/drm/panel/panel-tpo-td043mtea1.ko
> 
> Introduced by commits
> 
>   df439abe6501 ("drm/panel: Add driver for the NEC NL8048HL11 panel")
>   c9cf4c2a3bd3 ("drm/panel: Add driver for the Sharp LS037V7DW01 panel")
>   1c8fc3f0c5d2 ("drm/panel: Add driver for the Sony ACX565AKM panel")
>   415b8dd08711 ("drm/panel: Add driver for the Toppoly TD028TTEC1 panel")
>   dc2e1e5b2799 ("drm/panel: Add driver for the Toppoly TD043MTEA1 panel")

Ups, had not seen this one coming.
We are in the process of removing the drivers in 
drivers/video/fbdev/omap2/omapfb/
and decided to introduce the new drivers early to get them out of a
longer patch series.

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

Re: [PATCH v2 0/9] Add dual-LVDS panel support to EK874

2019-08-15 Thread Sam Ravnborg
Hi Fabrizio

> it appears that Rob has been busy converting the dt-bindings relevant to this
> series, and his changes are now found in linux-next. Most notably
> Documentation/devicetree/bindings/display/panel/panel-lvds.txt has now become
> Documentation/devicetree/bindings/display/panel/lvds.yaml
> 
> You have already taken patch:
> https://patchwork.kernel.org/patch/11072749/
> 
> as such the patch I am sending you is still related to:
> Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> 
> Patch "dt-bindings: display: Add bindings for Advantech IDK-2121WR" is still
> assuming the format is .txt, as I am not too sure about what the protocol is 
> in
> this case? Shall we take this patch and convert it later to .yaml or shall I
> convert it to .yaml straight away?
> 
> Please, let me know what's the best course of action.

It is preferred that all new display and panel bindings uses the new
meta-schema format.
And that the writers uses the available tools to verify the bindings
submission.
This is one way to avoid simple mistakes in examples.

Sam


Re: [PATCH v2 5/9] drm/panel: Add timings field to drm_panel

2019-08-15 Thread Sam Ravnborg
Hi Fabrizio

On Thu, Aug 15, 2019 at 12:04:29PM +0100, Fabrizio Castro wrote:
> We need to know if the panel supports dual-link, similarly
> to bridges, therefore add a reference to drm_timings in
> drm_panel.

Why do we need to know this?
Why is it needed in drm_panel and not in some driver specific struct?

I cannot see the full series, as I was copied only on some mails.
Awaiting dri-devel moderator before I can see the rest.

Sam

> 
> Signed-off-by: Fabrizio Castro 
> 
> ---
> v1->v2:
> * new patch
> 
>  include/drm/drm_panel.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 8c738c0..cd6ff07 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -26,6 +26,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  struct device_node;
>  struct drm_connector;
> @@ -81,6 +82,7 @@ struct drm_panel_funcs {
>   * struct drm_panel - DRM panel object
>   * @drm: DRM device owning the panel
>   * @connector: DRM connector that the panel is attached to
> + * @timings: timing information
>   * @dev: parent device of the panel
>   * @link: link from panel device (supplier) to DRM device (consumer)
>   * @funcs: operations that can be performed on the panel
> @@ -89,6 +91,7 @@ struct drm_panel_funcs {
>  struct drm_panel {
>   struct drm_device *drm;
>   struct drm_connector *connector;
> + const struct drm_timings *timings;
>   struct device *dev;
>  
>   const struct drm_panel_funcs *funcs;
> -- 
> 2.7.4


Re: [PATCH v4 0/9] DRM panel drivers for omapdrm

2019-08-14 Thread Sam Ravnborg
Hi Laurent.

On Wed, Aug 14, 2019 at 06:44:26PM +0200, Sam Ravnborg wrote:
> Hi Laurent.
> 
> On Tue, Aug 13, 2019 at 11:10:52PM +0300, Laurent Pinchart wrote:
> > Hello everybody,
> > 
> > This patch series adds DT bindings and drivers for 6 panels used by
> > omapdrm. They are meant to replace the corresponding omapdrm-specific
> > drivers from drivers/gpu/drm/omapdrm/displays/ that will be removed in a
> > subsequent patch series, once the omapdrm driver switches fully to the
> > drm_panel API.
> > 
> > There's nothing very special here. The first three patches add DT vendor
> > prefixes and DT bindings. The last six patches add new panel drivers.
> > 
> > Please see individual patches for changelogs. Sam, all the patches have
> > now been acked (resulting in a TODO list in 7/9 and a rework of 8/9).
> > Would you merge this in drm-misc ?
> > 
> > The patches are based on top of drm-misc-next and can be found at
> > 
> > git://linuxtv.org/pinchartl/media.git omapdrm/panels
> > 
> > Laurent Pinchart (9):
> >   dt-bindings: Add vendor prefix for LG Display
> >   dt-bindings: Add legacy 'toppoly' vendor prefix
> >   dt-bindings: display: panel: Add bindings for NEC NL8048HL11 panel
> >   drm/panel: Add driver for the LG Philips LB035Q02 panel
> >   drm/panel: Add driver for the NEC NL8048HL11 panel
> >   drm/panel: Add driver for the Sharp LS037V7DW01 panel
> >   drm/panel: Add driver for the Sony ACX565AKM panel
> >   drm/panel: Add driver for the Toppoly TD028TTEC1 panel
> >   drm/panel: Add driver for the Toppoly TD043MTEA1 panel
> 
> dim was not too happy with the patches.
> checkpatch --strict triggers too much:
> 
> drm/panel: Add driver for the LG Philips LB035Q02 panel
> -:24: WARNING:CONFIG_DESCRIPTION: please write a paragraph that describes the 
> config symbol fully
> #24: FILE: drivers/gpu/drm/panel/Kconfig:106:
> +config DRM_PANEL_LG_LB035Q02
> 
> -:235: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!lcd"
> #235: FILE: drivers/gpu/drm/panel/panel-lg-lb035q02.c:183:
> + if (lcd == NULL)
> 
> drm/panel: Add driver for the NEC NL8048HL11 panel
> -:23: WARNING:CONFIG_DESCRIPTION: please write a paragraph that describes the 
> config symbol fully
> #23: FILE: drivers/gpu/drm/panel/Kconfig:122:
> +config DRM_PANEL_NEC_NL8048HL11
> 
> -:47: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does 
> MAINTAINERS need updating?
> #47:
> new file mode 100644
> 
> -:136: CHECK:USLEEP_RANGE: usleep_range is preferred over udelay; see 
> Documentation/timers/timers-howto.rst
> #136: FILE: drivers/gpu/drm/panel/panel-nec-nl8048hl11.c:85:
> + udelay(20);
> 
> -:234: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!lcd"
> #234: FILE: drivers/gpu/drm/panel/panel-nec-nl8048hl11.c:183:
> + if (lcd == NULL)
> 
> etc..
> 
> Nothing serious but please give them an extra round so we do
> not have extra patches flowing in to fix these trivial things.
> There is a few "line too long" that I personally would ignore.
> But warnings lige (lcd == NULL) => (!lcd) I would fix.
> 
> And there was too much to fix while applying.

Forget this. I could use this existing little task to avoid some boring
work stuff.
Updated all checkpatch issues I considered relevant:
o (lcd == NULL) => (!lcd)
o <1 << X) => BIT(X)

And removed the __exit_p() annotation for a remove function to fix a
build warning.

Build tested and applied to drm-misc-next.

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

Re: [RESEND][PATCH v3 00/26] drm: Kirin driver cleanups to prep for Kirin960 support

2019-08-14 Thread Sam Ravnborg
Hi Xinliang, Rongrong, Xinwei, Chen

On Wed, Aug 14, 2019 at 06:46:36PM +, John Stultz wrote:
> Just wanted to resend this patch set so I didn't have to
> continue carrying it forever to keep the HiKey960 board running.
> 
> This patchset contains one fix (in the front, so its easier to
> eventually backport), and a series of changes from YiPing to
> refactor the kirin drm driver so that it can be used on both
> kirin620 based devices (like the original HiKey board) as well
> as kirin960 based devices (like the HiKey960 board).
> 
> The full kirin960 drm support is still being refactored, but as
> this base kirin rework was getting to be substantial, I wanted
> to send out the first chunk, so that the review burden wasn't
> overwhelming.

As Maintainers can we please get some feedback from one of you.
Just an "OK to commit" would do it.
But preferably an ack or a review on the individual patches.

If the reality is that John is the Maintainer today,
then we should update MAINTAINERS to reflect this.

Thanks!

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

Re: [PATCH] drm/vboxvideo: Make structure vbox_fb_helper_funcs constant

2019-08-14 Thread Sam Ravnborg
On Wed, Aug 14, 2019 at 07:51:37PM +0200, Daniel Vetter wrote:
> On Wed, Aug 14, 2019 at 07:36:55PM +0200, Hans de Goede wrote:
> > Hi,
> > 
> > On 14-08-19 19:26, Daniel Vetter wrote:
> > > On Tue, Aug 13, 2019 at 09:57:19AM +0200, Hans de Goede wrote:
> > > > Hi,
> > > > 
> > > > On 13-08-19 08:25, Nishka Dasgupta wrote:
> > > > > The static structure vbox_fb_helper_funcs, of type 
> > > > > drm_fb_helper_funcs,
> > > > > is used only when it is passed as the third argument to
> > > > > drm_fb_helper_fbdev_setup(), which does not modify it. Hence make it
> > > > > constant to protect it from unintended modifications.
> > > > > Issue found with Coccinelle.
> > > > > 
> > > > > Signed-off-by: Nishka Dasgupta 
> > > > 
> > > > Thank you for the patch, this looks good to me:
> > > > 
> > > > Reviewed-by: Hans de Goede 
> > > 
> > > I'm assuming you'll apply this to drm-misc-next too? Good to state that,
> > > to avoid confusion and coordination issues.
> > 
> > Actually I'm so used to the workflow in other subsystems I was
> > expecting the subsys maintainer to pick it up. But I know that
> > is not how it works for the drm subsys and since I'm the vboxvideo
> > maintainer I guess it makes sense for me to pick this up and push it.
> 
> Yeah, drm subsystem maintainers are exceedingly lazy bastards. Same
> applies to subtree maintainers (at least in most cases).

Be careful, this could end up in some popular news site.

Sam
>  
> > So yes I will pick this up and push it to drm-misc-next, sorry
> > for the confusion.
> 
> Thanks, Daniel
> 
> > 
> > Regards,
> > 
> > Hans
> > 
> > 
> > > > > ---
> > > > >drivers/gpu/drm/vboxvideo/vbox_drv.c | 2 +-
> > > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c 
> > > > > b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > > > > index 02537ab9cc08..2b57ea3195f2 100644
> > > > > --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > > > > +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > > > > @@ -32,7 +32,7 @@ static const struct pci_device_id pciidlist[] = {
> > > > >};
> > > > >MODULE_DEVICE_TABLE(pci, pciidlist);
> > > > > -static struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
> > > > > +static const struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
> > > > >   .fb_probe = vboxfb_create,
> > > > >};
> > > > > 
> > > 
> 
> -- 
> 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 0/9] DRM panel drivers for omapdrm

2019-08-14 Thread Sam Ravnborg
Hi Laurent.

On Tue, Aug 13, 2019 at 11:10:52PM +0300, Laurent Pinchart wrote:
> Hello everybody,
> 
> This patch series adds DT bindings and drivers for 6 panels used by
> omapdrm. They are meant to replace the corresponding omapdrm-specific
> drivers from drivers/gpu/drm/omapdrm/displays/ that will be removed in a
> subsequent patch series, once the omapdrm driver switches fully to the
> drm_panel API.
> 
> There's nothing very special here. The first three patches add DT vendor
> prefixes and DT bindings. The last six patches add new panel drivers.
> 
> Please see individual patches for changelogs. Sam, all the patches have
> now been acked (resulting in a TODO list in 7/9 and a rework of 8/9).
> Would you merge this in drm-misc ?
> 
> The patches are based on top of drm-misc-next and can be found at
> 
>   git://linuxtv.org/pinchartl/media.git omapdrm/panels
> 
> Laurent Pinchart (9):
>   dt-bindings: Add vendor prefix for LG Display
>   dt-bindings: Add legacy 'toppoly' vendor prefix
>   dt-bindings: display: panel: Add bindings for NEC NL8048HL11 panel
>   drm/panel: Add driver for the LG Philips LB035Q02 panel
>   drm/panel: Add driver for the NEC NL8048HL11 panel
>   drm/panel: Add driver for the Sharp LS037V7DW01 panel
>   drm/panel: Add driver for the Sony ACX565AKM panel
>   drm/panel: Add driver for the Toppoly TD028TTEC1 panel
>   drm/panel: Add driver for the Toppoly TD043MTEA1 panel

dim was not too happy with the patches.
checkpatch --strict triggers too much:

drm/panel: Add driver for the LG Philips LB035Q02 panel
-:24: WARNING:CONFIG_DESCRIPTION: please write a paragraph that describes the 
config symbol fully
#24: FILE: drivers/gpu/drm/panel/Kconfig:106:
+config DRM_PANEL_LG_LB035Q02

-:235: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!lcd"
#235: FILE: drivers/gpu/drm/panel/panel-lg-lb035q02.c:183:
+   if (lcd == NULL)

drm/panel: Add driver for the NEC NL8048HL11 panel
-:23: WARNING:CONFIG_DESCRIPTION: please write a paragraph that describes the 
config symbol fully
#23: FILE: drivers/gpu/drm/panel/Kconfig:122:
+config DRM_PANEL_NEC_NL8048HL11

-:47: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does 
MAINTAINERS need updating?
#47:
new file mode 100644

-:136: CHECK:USLEEP_RANGE: usleep_range is preferred over udelay; see 
Documentation/timers/timers-howto.rst
#136: FILE: drivers/gpu/drm/panel/panel-nec-nl8048hl11.c:85:
+   udelay(20);

-:234: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "!lcd"
#234: FILE: drivers/gpu/drm/panel/panel-nec-nl8048hl11.c:183:
+   if (lcd == NULL)

etc..

Nothing serious but please give them an extra round so we do
not have extra patches flowing in to fix these trivial things.
There is a few "line too long" that I personally would ignore.
But warnings lige (lcd == NULL) => (!lcd) I would fix.

And there was too much to fix while applying.

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

Re: [PATCH v2 0/4] drm: drop drmP in tda998x, tegra, arm, armada

2019-08-14 Thread Sam Ravnborg
> > This set of patches is one of the final steps before
> > we have succeeded to stop using drmP.h in all of drm/.
> > 
> > There is a few patches in flight through other trees
> > and the plan is that all users shall be gone in the
> > upstream kernel after next merge window.
> > 
> > The patches has seen build test with various configs
> > with various architectures.
> > 
> > The patches has been sent before, but to my best knowledge
> > they have not been applied anywhere.
> > All four patches are based on drm-misc-next,
> > but I checked that the tda998x patch can be applied to
> > the tda998x tree.
> > 
> > There are no dependencies between the patches.
> > 
> > v2:
> > - rebase on top of drm-misc-next
> > 
> > To maintainers: (Assuming the patch are OK)
> > Please let me know if you take the patch, or request
> > me to apply it to drm-misc-next.
> > Or let me me know if the patch should be based on another tree.
> 
> ping...
> 
> This patchset is one of the last steps to get rid of drmP.h.
> Other patches are applied to various sub-system trees.
> 
> The idea is that after next merge window can drop drmP.h.
> As long as we keep drmP.h around new users will sneak in.

Thierry reviewed all patches - thanks!

Applied to drm-misc-next and pushed out.

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

Re: [PATCH v3 7/9] drm/panel: Add driver for the Sony ACX565AKM panel

2019-08-13 Thread Sam Ravnborg
Hi Laurent.


On Tue, Aug 13, 2019 at 04:33:09PM +0300, Laurent Pinchart wrote:
> This panel is used on the Nokia N900.
> 
> The code is based on the omapdrm-specific panel-sony-acx565akm driver.
> 
> Signed-off-by: Laurent Pinchart 

Looking at the backlight support in this driver there is some potential
for using more of the infrastructure.
iFor example there should be no need for acx565akm_panel.mutex,
as the core handles locking.

Backlight should maybe use devm_backlight_device_register()
as part of using more infrastructure, so clean-up is automatic.

Also the implementation of the enable/disable callbacks looks like
most of the implementation belongs in prepare/unprepare callbacks.

And enable() could be as trivial as:

backlight_update_status(acx565akm_panel->backlight);


My feedback to this patch would be:
"This is how it was in the original driver so we can fix it in a
follow-up patch. Follow-up patch needs HW to test the changes."

If you add a TODO like this:
TODO:
- Update backlight support to use backlight_update_status() etc.
- Use prepare/unprepare for the basic power on/off of the backligt

(Or some other way to remember we need to do this) then:
Reviewed-by: Sam Ravnborg 

Even better would be a follow-up patch to actually do these things.

Sam


> ---
> Changes since v2:
> 
> - Call drm_panel_unprepare() in .remove() handler
> 
> Changes since v1:
> 
> - Mention boards using the panel in Kconfig
> - Renamed acx565akm_device to acx565akm_panel
> - Comments updates
> - Store width_mm and height_mm in drm_display_mode
> - Use drm_panel_disable() in .remove() handler
> ---
>  drivers/gpu/drm/panel/Kconfig|   8 +
>  drivers/gpu/drm/panel/Makefile   |   1 +
>  drivers/gpu/drm/panel/panel-sony-acx565akm.c | 694 +++
>  3 files changed, 703 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-sony-acx565akm.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 8d9a8cdb704e..b05649b3118a 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -316,6 +316,14 @@ config DRM_PANEL_SITRONIX_ST7789V
> Say Y here if you want to enable support for the Sitronix
> ST7789V controller for 240x320 LCD panels
>  
> +config DRM_PANEL_SONY_ACX565AKM
> + tristate "Sony ACX565AKM panel"
> + depends on GPIOLIB && OF && SPI
> + depends on BACKLIGHT_CLASS_DEVICE

> + help
> +   Say Y here if you want to enable support for the Sony ACX565AKM
> +   800x600 3.5" panel (found on the Nokia N900).
> +
>  config DRM_PANEL_TPO_TPG110
>   tristate "TPO TPG 800x400 panel"
>   depends on OF && SPI && GPIOLIB
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 14d1c49ef3ab..28cf2332fd06 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -33,5 +33,6 @@ obj-$(CONFIG_DRM_PANEL_SHARP_LS037V7DW01) += 
> panel-sharp-ls037v7dw01.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
>  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o
>  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> +obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
>  obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
>  obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
> diff --git a/drivers/gpu/drm/panel/panel-sony-acx565akm.c 
> b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
> new file mode 100644
> index ..c8c82163e24d
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-sony-acx565akm.c
> @@ -0,0 +1,694 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sony ACX565AKM LCD Panel driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated
> + *
> + * Based on the omapdrm-specific panel-sony-acx565akm driver
> + *
> + * Copyright (C) 2010 Nokia Corporation
> + * Author: Imre Deak 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#define CTRL_DISP_BRIGHTNESS_CTRL_ON (1 << 5)
> +#define CTRL_DISP_AMBIENT_LIGHT_CTRL_ON  (1 << 4)
> +#define CTRL_DISP_BACKLIGHT_ON   (1 << 2)
> +#define CTRL_DISP_AUTO_BRIGHTNESS_ON (1 << 1)
> +
> +#define MIPID_CMD_WRITE_CABC 0x55
> +#define MIPID_CMD_READ_CABC  0x56
> +
> +#define MIPID_VER_LPH89233
> +#define MIPID_VER_LS041Y34
> +#define MIPID_VER_L4

Re: [PATCH v3 9/9] drm/panel: Add driver for the Toppoly TD043MTEA1 panel

2019-08-13 Thread Sam Ravnborg
On Tue, Aug 13, 2019 at 04:33:11PM +0300, Laurent Pinchart wrote:
> This panel is used on the OMAP3 Pandora.
> 
> The code is based on the omapdrm-specific panel-tpo-td043mtea1 driver.
> 
> Signed-off-by: Laurent Pinchart 
Reviewed-by: Sam Ravnborg 

> ---
> Changes since v2:
> 
> - Call drm_panel_disable() in .remove() handler
> 
> Changes since v1:
> 
> - Mention boards using the panel in Kconfig
> - Renamed td043mtea1_device to td043mtea1_panel
> - Fixed typo in Toppoly name
> - Comments updates
> - Store width_mm and height_mm in drm_display_mode
> - Use drm_panel_unprepare() in .remove() handler
> - Replace enable/disable with prepare/unprepare
> ---
>  drivers/gpu/drm/panel/Kconfig|   7 +
>  drivers/gpu/drm/panel/Makefile   |   1 +
>  drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 509 +++
>  3 files changed, 517 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-tpo-td043mtea1.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index c2689420d2dd..f152bc4eeb53 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -332,6 +332,13 @@ config DRM_PANEL_TPO_TD028TTEC1
> Say Y here if you want to enable support for TPO TD028TTEC1 480x640
> 2.8" panel (found on the OpenMoko Neo FreeRunner and Neo 1973).
>  
> +config DRM_PANEL_TPO_TD043MTEA1
> + tristate "Toppoly (TPO) TD043MTEA1 panel driver"
> + depends on GPIOLIB && OF && REGULATOR && SPI
> + help
> +   Say Y here if you want to enable support for TPO TD043MTEA1 800x480
> +   4.3" panel (found on the OMAP3 Pandora board).
> +
>  config DRM_PANEL_TPO_TPG110
>   tristate "TPO TPG 800x400 panel"
>   depends on OF && SPI && GPIOLIB
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 27b776737ad3..b6cd39fe0f20 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -35,5 +35,6 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += 
> panel-sitronix-st7701.o
>  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
>  obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
>  obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
> +obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
>  obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
>  obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
> diff --git a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c 
> b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c
> new file mode 100644
> index ..71824a334af7
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c
> @@ -0,0 +1,509 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Toppoly TD043MTEA1 Panel Driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated
> + *
> + * Based on the omapdrm-specific panel-tpo-td043mtea1 driver
> + *
> + * Author: Gražvydas Ignotas 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#define TPO_R02_MODE(x)  ((x) & 7)
> +#define TPO_R02_MODE_800x480 7
> +#define TPO_R02_NCLK_RISING  BIT(3)
> +#define TPO_R02_HSYNC_HIGH   BIT(4)
> +#define TPO_R02_VSYNC_HIGH   BIT(5)
> +
> +#define TPO_R03_NSTANDBY BIT(0)
> +#define TPO_R03_EN_CP_CLKBIT(1)
> +#define TPO_R03_EN_VGL_PUMP  BIT(2)
> +#define TPO_R03_EN_PWM   BIT(3)
> +#define TPO_R03_DRIVING_CAP_100  BIT(4)
> +#define TPO_R03_EN_PRE_CHARGEBIT(6)
> +#define TPO_R03_SOFTWARE_CTL BIT(7)
> +
> +#define TPO_R04_NFLIP_H  BIT(0)
> +#define TPO_R04_NFLIP_V  BIT(1)
> +#define TPO_R04_CP_CLK_FREQ_1H   BIT(2)
> +#define TPO_R04_VGL_FREQ_1H  BIT(4)
> +
> +#define TPO_R03_VAL_NORMAL \
> + (TPO_R03_NSTANDBY | TPO_R03_EN_CP_CLK | TPO_R03_EN_VGL_PUMP | \
> +  TPO_R03_EN_PWM | TPO_R03_DRIVING_CAP_100 | TPO_R03_EN_PRE_CHARGE | \
> +  TPO_R03_SOFTWARE_CTL)
> +
> +#define TPO_R03_VAL_STANDBY \
> + (TPO_R03_DRIVING_CAP_100 | TPO_R03_EN_PRE_CHARGE | \
> +  TPO_R03_SOFTWARE_CTL)
> +
> +static const u16 td043mtea1_def_gamma[12] = {
> + 105, 315, 381, 431, 490, 537, 579, 686, 780, 837, 880, 1023
> +};
> +
> +struct td043mtea1_panel {
> + struct drm_panel panel;
> +
> + struct spi_device *spi;
> + struct regulator *vcc_reg;
> + struct gpio_desc *reset_gpio;
> 

Re: [PATCH v3 8/9] drm/panel: Add driver for the Toppoly TD028TTEC1 panel

2019-08-13 Thread Sam Ravnborg
Hi Laurent.

On Tue, Aug 13, 2019 at 04:33:10PM +0300, Laurent Pinchart wrote:
> This panel is used on the OpenMoko Neo FreeRunner and Neo 1973.
> 
> The code is based on the omapdrm-specific panel-tpo-td028ttec1 driver.
> 
> Signed-off-by: Laurent Pinchart 

This driver have everything in the enable/disable pair.
The driver should use prepare/unprepare for the things that belongs
there.

With that fixed:
Reviewed-by: Sam Ravnborg 

> ---
> Changes since v2:
> 
> - Call drm_panel_unprepare() in .remove() handler
> 
> Changes since v1:
> 
> - Mention boards using the panel in Kconfig
> - Renamed td028ttec1_device to td028ttec1_panel
> - Comments updates
> - Removed unneeded ret check
> - Store width_mm and height_mm in drm_display_mode
> - Use drm_panel_disable() in .remove() handler
> ---
>  drivers/gpu/drm/panel/Kconfig|   8 +
>  drivers/gpu/drm/panel/Makefile   |   1 +
>  drivers/gpu/drm/panel/panel-tpo-td028ttec1.c | 382 +++
>  3 files changed, 391 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-tpo-td028ttec1.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index b05649b3118a..c2689420d2dd 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -324,6 +324,14 @@ config DRM_PANEL_SONY_ACX565AKM
> Say Y here if you want to enable support for the Sony ACX565AKM
> 800x600 3.5" panel (found on the Nokia N900).
>  
> +config DRM_PANEL_TPO_TD028TTEC1
> + tristate "Toppoly (TPO) TD028TTEC1 panel driver"
> + depends on OF && SPI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> +   Say Y here if you want to enable support for TPO TD028TTEC1 480x640
> +   2.8" panel (found on the OpenMoko Neo FreeRunner and Neo 1973).
> +
>  config DRM_PANEL_TPO_TPG110
>   tristate "TPO TPG 800x400 panel"
>   depends on OF && SPI && GPIOLIB
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 28cf2332fd06..27b776737ad3 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -34,5 +34,6 @@ obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += 
> panel-sharp-ls043t1le01.o
>  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o
>  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
>  obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
> +obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
>  obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
>  obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
> diff --git a/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c 
> b/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c
> new file mode 100644
> index ..dd69fac9808f
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Toppoly TD028TTEC1 Panel Driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated
> + *
> + * Based on the omapdrm-specific panel-tpo-td028ttec1 driver
> + *
> + * Copyright (C) 2008 Nokia Corporation
> + * Author: Tomi Valkeinen 
> + *
> + * Neo 1973 code (jbt6k74.c):
> + * Copyright (C) 2006-2007 OpenMoko, Inc.
> + * Author: Harald Welte 
> + *
> + * Ported and adapted from Neo 1973 U-Boot by:
> + * H. Nikolaus Schaller 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#define JBT_COMMAND  0x000
> +#define JBT_DATA 0x100
> +
> +#define JBT_REG_SLEEP_IN 0x10
> +#define JBT_REG_SLEEP_OUT0x11
> +
> +#define JBT_REG_DISPLAY_OFF  0x28
> +#define JBT_REG_DISPLAY_ON   0x29
> +
> +#define JBT_REG_RGB_FORMAT   0x3a
> +#define JBT_REG_QUAD_RATE0x3b
> +
> +#define JBT_REG_POWER_ON_OFF 0xb0
> +#define JBT_REG_BOOSTER_OP   0xb1
> +#define JBT_REG_BOOSTER_MODE 0xb2
> +#define JBT_REG_BOOSTER_FREQ 0xb3
> +#define JBT_REG_OPAMP_SYSCLK 0xb4
> +#define JBT_REG_VSC_VOLTAGE  0xb5
> +#define JBT_REG_VCOM_VOLTAGE 0xb6
> +#define JBT_REG_EXT_DISPL0xb7
> +#define JBT_REG_OUTPUT_CONTROL   0xb8
> +#define JBT_REG_DCCLK_DCEV   0xb9
> +#define JBT_REG_DISPLAY_MODE10xba
> +#define JBT_REG_DISPLAY_MODE20xbb
> +#define JBT_REG_DISPLAY_MODE 0xbc
> +#define JBT_REG_ASW_SLEW 0xbd
> +#define JBT_REG_DUMMY_DISPLAY0xbe
> +#define JB

Re: [PATCH v3 6/9] drm/panel: Add driver for the Sharp LS037V7DW01 panel

2019-08-13 Thread Sam Ravnborg
On Tue, Aug 13, 2019 at 04:33:08PM +0300, Laurent Pinchart wrote:
> This panel is used on the TI SDP3430 board.
> 
> The code is based on the omapdrm-specific panel-sharp-ls037v7dw01
> driver.
> 
> Signed-off-by: Laurent Pinchart 
Reviewed-by: Sam Ravnborg 

> ---
> Changes since v1:
> 
> - Mention boards using the panel in Kconfig
> - Renamed ls037v7dw01_device to ls037v7dw01_panel
> - Renamed vcc to vdd
> - Comments updates
> - Store width_mm and height_mm in drm_display_mode
> - Use drm_panel_disable() in .remove() handler
> - Use devm_gpiod_get() where applicable
> - Remove NULL-check on vdd
> - Order Kconfig entries alphabetically
> ---
>  drivers/gpu/drm/panel/Kconfig |   7 +
>  drivers/gpu/drm/panel/Makefile|   1 +
>  .../gpu/drm/panel/panel-sharp-ls037v7dw01.c   | 226 ++
>  3 files changed, 234 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index d28133c6aa0a..8d9a8cdb704e 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -282,6 +282,13 @@ config DRM_PANEL_SHARP_LQ101R1SX01
> To compile this driver as a module, choose M here: the module
> will be called panel-sharp-lq101r1sx01.
>  
> +config DRM_PANEL_SHARP_LS037V7DW01
> + tristate "Sharp LS037V7DW01 VGA LCD panel"
> + depends on GPIOLIB && OF && REGULATOR
> + help
> +   Say Y here if you want to enable support for Sharp LS037V7DW01 VGA
> +   (480x640) LCD panel (found on the TI SDP3430 board).
> +
>  config DRM_PANEL_SHARP_LS043T1LE01
>   tristate "Sharp LS043T1LE01 qHD video mode panel"
>   depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 8052f9a7ad60..14d1c49ef3ab 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += 
> panel-samsung-s6e63m0.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>  obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> +obj-$(CONFIG_DRM_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
>  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o
>  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> diff --git a/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c 
> b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
> new file mode 100644
> index ..2aaea507cc1f
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
> @@ -0,0 +1,226 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sharp LS037V7DW01 LCD Panel Driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated
> + *
> + * Based on the omapdrm-specific panel-sharp-ls037v7dw01 driver
> + *
> + * Copyright (C) 2013 Texas Instruments Incorporated
> + * Author: Tomi Valkeinen 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +struct ls037v7dw01_panel {
> + struct drm_panel panel;
> + struct platform_device *pdev;
> +
> + struct regulator *vdd;
> + struct gpio_desc *resb_gpio;/* low = reset active min 20 us */
> + struct gpio_desc *ini_gpio; /* high = power on */
> + struct gpio_desc *mo_gpio;  /* low = 480x640, high = 240x320 */
> + struct gpio_desc *lr_gpio;  /* high = conventional horizontal 
> scanning */
> + struct gpio_desc *ud_gpio;  /* high = conventional vertical 
> scanning */
> +};
> +
> +#define to_ls037v7dw01_device(p) \
> + container_of(p, struct ls037v7dw01_panel, panel)
> +
> +static int ls037v7dw01_disable(struct drm_panel *panel)
> +{
> + struct ls037v7dw01_panel *lcd = to_ls037v7dw01_device(panel);
> +
> + gpiod_set_value_cansleep(lcd->ini_gpio, 0);
> + gpiod_set_value_cansleep(lcd->resb_gpio, 0);
> +
> + /* Wait at least 5 vsyncs after disabling the LCD. */
> + msleep(100);
> +
> + return 0;
> +}
> +
> +static int ls037v7dw01_unprepare(struct drm_panel *panel)
> +{
> + struct ls037v7dw01_panel *lcd = to_ls037v7dw01_device(panel);
> +
> + regulator_disable(lcd->vdd);
> + return 0;
> +}
> +
> +static int ls037v7dw01_prepare(struct drm_panel *panel)
> +{
> + struct ls037v7dw01_panel *lcd = to_ls037v7dw01_device

Re: [PATCH v3 5/9] drm/panel: Add driver for the NEC NL8048HL11 panel

2019-08-13 Thread Sam Ravnborg
On Tue, Aug 13, 2019 at 04:33:07PM +0300, Laurent Pinchart wrote:
> This panel is used on the Zoom2/3/3630 SDP boards.
> 
> The code is based on the omapdrm-specific panel-nec-nl8048hl11 driver
> 
> Signed-off-by: Laurent Pinchart 
Reviewed-by: Sam Ravnborg 

> ---
> Changes since v2:
> 
> - Call drm_panel_unprepare() in .remove() handler
> 
> Changes since v1:
> 
> - Mention boards using the panel in Kconfig
> - Renamed nl8048_device to nl8048_panel
> - Comments updates
> - Store width_mm and height_mm in drm_display_mode
> - Use drm_panel_disable() in .remove() handler
> ---
>  drivers/gpu/drm/panel/Kconfig|   8 +
>  drivers/gpu/drm/panel/Makefile   |   1 +
>  drivers/gpu/drm/panel/panel-nec-nl8048hl11.c | 248 +++
>  3 files changed, 257 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-nec-nl8048hl11.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 87cdc330672b..d28133c6aa0a 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -119,6 +119,14 @@ config DRM_PANEL_LG_LG4573
> Say Y here if you want to enable support for LG4573 RGB panel.
> To compile this driver as a module, choose M here.
>  
> +config DRM_PANEL_NEC_NL8048HL11
> + tristate "NEC NL8048HL11 RGB panel"
> + depends on GPIOLIB && OF && SPI
> + help
> +   Say Y here if you want to enable support for the NEC NL8048HL11 RGB
> +   panel (found on the Zoom2/3/3630 SDP boards). To compile this driver
> +   as a module, choose M here.
> +
>  config DRM_PANEL_NOVATEK_NT39016
>   tristate "Novatek NT39016 RGB/SPI panel"
>   depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 9bc6f3c5bf96..8052f9a7ad60 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += 
> panel-jdi-lt070me05000.o
>  obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o
>  obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> +obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
>  obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
>  obj-$(CONFIG_DRM_PANEL_OLIMEX_LCD_OLINUXINO) += panel-olimex-lcd-olinuxino.o
>  obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
> diff --git a/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c 
> b/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c
> new file mode 100644
> index ..21bae2d0a23a
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c
> @@ -0,0 +1,248 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * NEC NL8048HL11 Panel Driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated
> + *
> + * Based on the omapdrm-specific panel-nec-nl8048hl11 driver
> + *
> + * Copyright (C) 2010 Texas Instruments Incorporated
> + * Author: Erik Gilling 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +struct nl8048_panel {
> + struct drm_panel panel;
> +
> + struct spi_device *spi;
> + struct gpio_desc *reset_gpio;
> +};
> +
> +#define to_nl8048_device(p) container_of(p, struct nl8048_panel, panel)
> +
> +static int nl8048_write(struct nl8048_panel *lcd, unsigned char addr,
> + unsigned char value)
> +{
> + u8 data[4] = { value, 0x01, addr, 0x00 };
> + int ret;
> +
> + ret = spi_write(lcd->spi, data, sizeof(data));
> + if (ret)
> + dev_err(>spi->dev, "SPI write to %u failed: %d\n",
> + addr, ret);
> +
> + return ret;
> +}
> +
> +static int nl8048_init(struct nl8048_panel *lcd)
> +{
> + static const struct {
> + unsigned char addr;
> + unsigned char data;
> + } nl8048_init_seq[] = {
> + {   3, 0x01 }, {   0, 0x00 }, {   1, 0x01 }, {   4, 0x00 },
> + {   5, 0x14 }, {   6, 0x24 }, {  16, 0xd7 }, {  17, 0x00 },
> + {  18, 0x00 }, {  19, 0x55 }, {  20, 0x01 }, {  21, 0x70 },
> + {  22, 0x1e }, {  23, 0x25 }, {  24, 0x25 }, {  25, 0x02 },
> + {  26, 0x02 }, {  27, 0xa0 }, {  32, 0x2f }, {  33, 0x0f },
> + {  34, 0x0f }, {  35, 0x0f }, {  36, 0x0f }, {  37, 0x0f },
> + {  38, 0x0f }, {  39, 0x00 }, {  40, 0x02 }, {  41, 0x02 },
> + {  42, 0x02 }, {  43, 0x0f }, { 

Re: [PATCH v1 1/1] drm/exynos: drop use of drmP.h (2nd round)

2019-08-12 Thread Sam Ravnborg
Hi Inki.

On Tue, Aug 13, 2019 at 10:03:51PM +0900, Inki Dae wrote:
> Hi,
> 
> On 19. 8. 13. 오전 3:51, Sam Ravnborg wrote:
> > Hi Inki.
> > 
> > Any plans to take this to the exynos tree?
> > 
> 
> It will be merged to next.
Thanks, one more off the list.

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

Re: [PATCH v2 0/4] drm: drop drmP in tda998x, tegra, arm, armada

2019-08-12 Thread Sam Ravnborg
Hi all.

On Sun, Aug 04, 2019 at 11:41:28AM +0200, Sam Ravnborg wrote:
> This set of patches is one of the final steps before
> we have succeeded to stop using drmP.h in all of drm/.
> 
> There is a few patches in flight through other trees
> and the plan is that all users shall be gone in the
> upstream kernel after next merge window.
> 
> The patches has seen build test with various configs
> with various architectures.
> 
> The patches has been sent before, but to my best knowledge
> they have not been applied anywhere.
> All four patches are based on drm-misc-next,
> but I checked that the tda998x patch can be applied to
> the tda998x tree.
> 
> There are no dependencies between the patches.
> 
> v2:
> - rebase on top of drm-misc-next
> 
> To maintainers: (Assuming the patch are OK)
> Please let me know if you take the patch, or request
> me to apply it to drm-misc-next.
> Or let me me know if the patch should be based on another tree.

ping...

This patchset is one of the last steps to get rid of drmP.h.
Other patches are applied to various sub-system trees.

The idea is that after next merge window can drop drmP.h.
As long as we keep drmP.h around new users will sneak in.

Sam

> 
> Sam Ravnborg (4):
>   drm/i2c/tda998x: drop use of drmP.h
>   drm/tegra: drop use of drmP.h
>   drm/armada: drop use of drmP.h
>   drm/arm: drop use of drmP.h
> 
>  drivers/gpu/drm/arm/hdlcd_crtc.c| 12 +++-
>  drivers/gpu/drm/arm/hdlcd_drv.c |  7 ++-
>  drivers/gpu/drm/arm/malidp_crtc.c   | 11 +++
>  drivers/gpu/drm/arm/malidp_drv.c|  8 +---
>  drivers/gpu/drm/arm/malidp_drv.h|  7 ---
>  drivers/gpu/drm/arm/malidp_hw.c |  7 ++-
>  drivers/gpu/drm/arm/malidp_mw.c |  5 +++--
>  drivers/gpu/drm/arm/malidp_planes.c |  4 +++-
>  drivers/gpu/drm/armada/armada_crtc.c| 10 +++---
>  drivers/gpu/drm/armada/armada_debugfs.c |  8 ++--
>  drivers/gpu/drm/armada/armada_drm.h |  5 -
>  drivers/gpu/drm/armada/armada_drv.c |  8 
>  drivers/gpu/drm/armada/armada_fb.c  |  3 +++
>  drivers/gpu/drm/armada/armada_fbdev.c   |  3 +++
>  drivers/gpu/drm/armada/armada_gem.c |  7 ++-
>  drivers/gpu/drm/armada/armada_overlay.c |  8 +---
>  drivers/gpu/drm/armada/armada_plane.c   |  4 +++-
>  drivers/gpu/drm/armada/armada_trace.h   |  5 -
>  drivers/gpu/drm/i2c/tda998x_drv.c   |  2 +-
>  drivers/gpu/drm/tegra/dc.c  | 13 +
>  drivers/gpu/drm/tegra/dpaux.c   |  5 +++--
>  drivers/gpu/drm/tegra/drm.c |  8 
>  drivers/gpu/drm/tegra/drm.h |  3 +--
>  drivers/gpu/drm/tegra/dsi.c |  8 +---
>  drivers/gpu/drm/tegra/fb.c  |  6 --
>  drivers/gpu/drm/tegra/gem.c |  3 +++
>  drivers/gpu/drm/tegra/gem.h |  1 -
>  drivers/gpu/drm/tegra/gr2d.c|  1 +
>  drivers/gpu/drm/tegra/hdmi.c|  5 +
>  drivers/gpu/drm/tegra/hub.c |  3 ++-
>  drivers/gpu/drm/tegra/hub.h |  1 -
>  drivers/gpu/drm/tegra/plane.c   |  1 +
>  drivers/gpu/drm/tegra/sor.c |  3 +++
>  drivers/gpu/drm/tegra/vic.c |  1 +
>  34 files changed, 137 insertions(+), 49 deletions(-)
> 
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/1] drm/exynos: drop use of drmP.h (2nd round)

2019-08-12 Thread Sam Ravnborg
Hi Inki.

Any plans to take this to the exynos tree?

Sam

On Sat, Aug 03, 2019 at 04:57:35PM +0200, Sam Ravnborg wrote:
> There was a few uses of drmP that was missed in the last
> patch removing this header from exynos.
> 
> Remove the final uses of this header.
> 
> Signed-off-by: Sam Ravnborg 
> Cc: Inki Dae 
> Cc: Joonyoung Shim 
> Cc: Seung-Woo Kim 
> Cc: Kyungmin Park 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Kukjin Kim 
> Cc: Krzysztof Kozlowski 
> Cc: Jingoo Han 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 
>  drivers/gpu/drm/exynos/exynos_drm_fimc.c| 2 ++
>  drivers/gpu/drm/exynos/exynos_drm_gsc.c | 2 ++
>  drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 -
>  drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 --
>  drivers/gpu/drm/exynos/exynos_drm_rotator.c | 2 ++
>  drivers/gpu/drm/exynos/exynos_drm_scaler.c  | 1 +
>  7 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index cc53dcad25e4..8a03a33c32cb 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -8,12 +8,20 @@
>   */
>  
>  #include 
> +#include 
> +#include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
> +#include 
> +#include 
>  #include 
> +#include 
>  #include 
>  
>  #include "exynos_drm_drv.h"
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c 
> b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index a594ab7be2c0..5cf0ed0d714c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -17,6 +17,8 @@
>  #include 
>  #include 
>  
> +#include 
> +#include 
>  #include 
>  
>  #include "exynos_drm_drv.h"
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c 
> b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> index 1e4b21c49a06..a0fc9a2a58f6 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> @@ -16,6 +16,8 @@
>  #include 
>  #include 
>  
> +#include 
> +#include 
>  #include 
>  
>  #include "exynos_drm_drv.h"
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c 
> b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index d45bfab6fe40..4f2b7551b251 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -16,7 +16,10 @@
>   * all copies or substantial portions of the Software.
>   */
>  
> -#include 
> +#include 
> +
> +#include 
> +#include 
>  #include 
>  #include 
>  
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h 
> b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> index 9cbbc301bec9..67a0805ee009 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> @@ -6,8 +6,6 @@
>  #ifndef _EXYNOS_DRM_IPP_H_
>  #define _EXYNOS_DRM_IPP_H_
>  
> -#include 
> -
>  struct exynos_drm_ipp;
>  struct exynos_drm_ipp_task;
>  
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c 
> b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
> index 8ebad2740ad5..b98482990d1a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
> @@ -15,7 +15,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
> +#include 
>  #include 
>  
>  #include "exynos_drm_drv.h"
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_scaler.c 
> b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
> index 9af096479e1c..f082f259ca6d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_scaler.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  
>  #include "exynos_drm_drv.h"
> -- 
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/mipi-dbi: Support command mode panel drivers

2019-08-12 Thread Sam Ravnborg
Hi Noralf.

> > - drm_panel has proper support for modes.
> >   This is today duplicated in mipi_dbi.
> >   Could we make it so that when a panel is used then the panel
> >   has the mode info - as we then use the panel more in the way we do
> >   in other cases?
> >   As it is now the mode is specified in mipi_dbi_dev_init()
> >   The drm_connector would then, if a panel is used, ask the panel for
> >   the mode.
> >   I did not really think to the end of this, but it seems wrong that
> >   we introduce drm_panel and then keep modes in mipi_dbi.
> > 
> 
> I considered that, but it would would just generate duplicate code for
> the connector. It would make sense to refactor this when/if all mipi dbi
> drivers are turned into panel drivers.

The objective should be that all mipi dbi drivers could be refactored.
And so it makes sense to do it right from the beginning.
It will be some duplicated code until all are migrated.
But as the number of mipi dbi drivers are low it is doable if a few
people throw some time after it.
I volunteer to assist.

In drm_mipi_dbi.c we could just add:

static int mipi_dbi_connector_get_modes(struct drm_connector *connector)
{
...
if (dbidev->panel)
return drm_panel_get_modes(dbidev->panel);


Then if there is a panel we would use the mode specified by the panel.
To make this work we would need a drm_panel_attach() in
drm_mipi_dbi_panel_register() to give the panel access to the connector.
I have patches that makes connector an argument to drm_panel_get_modes()
but they need a bit more baking, so you cannot benefit from them yet.

Maybe this is the same argument as backlight?
We can introduce this when the drm_panel core is better prepared.

> >> +int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct 
> >> mipi_dbi_dev *dbidev,
> >> +  struct drm_driver *driver, const struct 
> >> drm_display_mode *mode,
> >> +  u32 rotation)
> > Can we make this use enum drm_panel_orientation - so we can use
> > of_drm_get_panel_orientation() in the callers?
> > of_drm_get_panel_orientation() is not merged yet, but we could do so if
> > this patch-set needs it.
> > 
> > I know that this would require mipi_dbi_dev_init() and all users to be
> > updated. But it is a simpler interface so worth it.
> > 
> 
> That would break rotation on userspace that doesn't know about the panel
> orientation property which is a recent addition. These panels are mostly
> used in the embedded world not desktop. It also would break fbdev 90/270
> rotation (see drm_client_rotation()).

I think it is possible to move most of drm over to one way to spicify
rotation.
But let's wait with that battle until another day.
It should not hinder this series.

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

Re: [PATCH 4/4] drm/panel/ili9341: Support DPI panels

2019-08-12 Thread Sam Ravnborg
Hi Laurent/Noralf.

On Mon, Aug 12, 2019 at 06:35:42PM +0300, Laurent Pinchart wrote:
> On Mon, Aug 12, 2019 at 02:13:54PM +0200, Noralf Trønnes wrote:
> > Den 11.08.2019 18.41, skrev Sam Ravnborg:
> > > On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote:
> > >> Add support for panels that use the DPI interface.
> > >> ILI9341 has onboard RAM so the assumption made here is that all such
> > >> panels support pixel upload over DBI.
> > >>
> > >> The presence/absense of the Device Tree 'port' node decides which
> > >> interface is used for pixel transfer.
> > >
> > > Have you consiered if the compatible could be used to determine the use
> > > of a panel? Then it is more explicit how the HW is described in DT.
> > 
> > Is that common to use the compatible to tell which interface to use?
> > I don't know what best practice is here.
> 
> Usually the compatible identifies the device, not the interface.
> Additional properties are preferred to describe the interface.
Thanks Laurent.
Then the ports idea as implmented by the patch seems to fly.

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

Best practice for embedded code samles? [Was: drm/drv: Use // for comments in example code]

2019-08-11 Thread Sam Ravnborg
On Thu, Aug 08, 2019 at 06:36:28PM +0200, Jonathan Neuschäfer wrote:
> This improves Sphinx output in two ways:
> 
> - It avoids an unmatched single-quote ('), about which Sphinx complained:
> 
>   /.../Documentation/gpu/drm-internals.rst:298:
> WARNING: Could not lex literal_block as "c". Highlighting skipped.
> 
>   An alternative approach would be to replace "can't" with a word that
>   doesn't have a single-quote.
> 
> - It lets Sphinx format the comments in italics and grey, making the
>   code slightly easier to read.
> 
> Signed-off-by: Jonathan Neuschäfer 

The result looks much better now - thanks.

I wonder if there is a better way to embed a code sample
than reverting to // style comments.

As the kernel do not like // comments we should try to avoid them in
examples.

Mauro/Jon?

Sam

> ---
>  drivers/gpu/drm/drm_drv.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 9d00947ca447..769feeff 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -328,11 +328,9 @@ void drm_minor_release(struct drm_minor *minor)
>   *   struct drm_device *drm;
>   *   int ret;
>   *
> - *   [
> - * devm_kzalloc() can't be used here because the drm_device
> - * lifetime can exceed the device lifetime if driver unbind
> - * happens when userspace still has open file descriptors.
> - *   ]
> + *   // devm_kzalloc() can't be used here because the drm_device '
> + *   // lifetime can exceed the device lifetime if driver unbind
> + *   // happens when userspace still has open file descriptors.
>   *   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>   *   if (!priv)
>   *   return -ENOMEM;
> @@ -355,7 +353,7 @@ void drm_minor_release(struct drm_minor *minor)
>   *   if (IS_ERR(priv->pclk))
>   *   return PTR_ERR(priv->pclk);
>   *
> - *   [ Further setup, display pipeline etc ]
> + *   // Further setup, display pipeline etc
>   *
>   *   platform_set_drvdata(pdev, drm);
>   *
> @@ -370,7 +368,7 @@ void drm_minor_release(struct drm_minor *minor)
>   *   return 0;
>   *   }
>   *
> - *   [ This function is called before the devm_ resources are released ]
> + *   // This function is called before the devm_ resources are released
>   *   static int driver_remove(struct platform_device *pdev)
>   *   {
>   *   struct drm_device *drm = platform_get_drvdata(pdev);
> @@ -381,7 +379,7 @@ void drm_minor_release(struct drm_minor *minor)
>   *   return 0;
>   *   }
>   *
> - *   [ This function is called on kernel restart and shutdown ]
> + *   // This function is called on kernel restart and shutdown
>   *   static void driver_shutdown(struct platform_device *pdev)
>   *   {
>   *   drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
> --
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 0/4] Add drivers for auo, kd101n80-45na and boe, tv101wum-nl6 panels

2019-08-11 Thread Sam Ravnborg
Hi Jitao.

>  .../display/panel/auo,kd101n80-45na.txt   |  34 +
>  .../display/panel/boe,tv101wum-nl6.txt|  34 +

panel bindings are in the process of being migrated to the new
meta-schema format.
Therefore new bindings should preferably also follow the new format.

Can you please look into this.
In upstream and drm-misc-next there is already some examples.

Note: It is not a hard rule that new bindings shall be in
the new meta-schema format (.yaml extension), but as this is
best practice now it is preferred.
Same goes for display bindings btw.

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

Re: [PATCH v2 4/9] drm/panel: Add driver for the LG Philips LB035Q02 panel

2019-08-11 Thread Sam Ravnborg
Hi Noralf.

> >>> + /* register value */
> >>> + buffer[4] = 0x72;
> >>> + buffer[5] = val >> 8;
> >>> + buffer[6] = val;
> >>> + value_xfer.tx_buf = buffer + 4;
> >>> + spi_message_add_tail(_xfer, );
> >>> +
> >>> + return spi_sync(lcd->spi, );
> >>> +}
> >>
> >> Just a note to Sam:
> >> This is the same spi protocol that the ili9325 controller on the hy28b
> >> panel uses.
> >>
> >> I remembered that I have experimented with a regmap implementation:
> >> https://github.com/notro/tinydrm/blob/master/tinydrm-ili9325.c#L191
> > 
> > regmap seems a too limited interface to use when trying to generalize
> > this.
> > We should rather go for a ili-lib or something that can be used in all
> > the present and future ili based drivers.
> > Obviously it depends on how similar the ili based chips are.
> > 
> > I did a quick look and this driver did not match the ili9325 datasheet
> > as different bits was are in register 0x1.
> > So it smeels like another, similar. ili varaint.
> > 
> > So for this driver we would just use the hardcoded varaint already
> > present. Then we may re-visit ili-lib idea later if we see too much
> > similar code.
> > This is as I see it for now...
> > 
> 
> Yeah, no point in changing this driver until there are more similar
> controllers. Just wanted to point out that the hy28b panel you ordered
> uses the same protocol.
Ohh, that was your point. Thanks!

The display is still in the mail somewhere from China..
Right now I do not have time to hack on it so not a big deal.

For the fun of it I ordred a few other displays. One was ssd1306 based,
and do not recall the rest.

Sam

> 
> A web search helped my memory, these 3 supported by staging/fbtft use a
> startbyte: ili9320, ili9325 and hx8347d. The ili chips are almost
> identical. The search revealed many more including:
> st7793, st7781r, S6E63D6, +many ilitek.
> 
> Noralf.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/9] DRM panel drivers for omapdrm

2019-08-11 Thread Sam Ravnborg
Hi Laurent.

On Sun, Aug 11, 2019 at 02:10:39AM +0300, Laurent Pinchart wrote:
> Hello everybody,
> 
> These 9 patches have initially been posted as part of the larger "[PATCH
> 00/60] drm/omap: Replace custom display drivers with drm_bridge and
> drm_panel" series, hence the v2 in the subject prefix.
> 
> I'm posting this second version separately per Sam's request as the rest
> of the original series is expected to take more time to process through
> review.
Thanks, make good sense (to me) that we process these now while waiting
for the dust to settel on the other more invasive patches.

> 
> There's nothing very special here. The first three patches add DT vendor
> prefixes and DT bindings. Since v1 patch 3/9 has been converted from
> text to YAML, and as I'm not very familiar with YAML DT bindings, I'm
> eagerly waiting for reviews.
> 
> The last six patches add new panel drivers. The code originates from the
> corresponding omapdrm-specific panel drivers, which explains why only
> one new DT binding is needed as most of them are already present.
> 
> Please see individual patches for changelogs. Sam, I believe I've taken
> all your comments into account, I hope none fell through the cracks.
I have been through the patches - all looked good.
One generel comment about drm_panel_remove().

The DT for the NEC NL8048HL11 needs to be reviewed, and then we can
land all patches in one go.
Unless someone else put some review effort in and find something.

Sam


> 
> The patches are based on top of drm-misc-next and can be found at
> 
>   git://linuxtv.org/pinchartl/media.git omapdrm/panels
> 
> Laurent Pinchart (9):
>   dt-bindings: Add vendor prefix for LG Display
>   dt-bindings: Add legacy 'toppoly' vendor prefix
>   dt-bindings: display: panel: Add bindings for NEC NL8048HL11 panel
>   drm/panel: Add driver for the LG Philips LB035Q02 panel
>   drm/panel: Add driver for the NEC NL8048HL11 panel
>   drm/panel: Add driver for the Sharp LS037V7DW01 panel
>   drm/panel: Add driver for the Sony ACX565AKM panel
>   drm/panel: Add driver for the Toppoly TD028TTEC1 panel
>   drm/panel: Add driver for the Toppoly TD043MTEA1 panel
> 
>  .../display/panel/nec,nl8048hl11.yaml |  49 ++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   5 +
>  drivers/gpu/drm/panel/Kconfig |  46 ++
>  drivers/gpu/drm/panel/Makefile|   6 +
>  drivers/gpu/drm/panel/panel-lg-lb035q02.c | 237 ++
>  drivers/gpu/drm/panel/panel-nec-nl8048hl11.c  | 247 +++
>  .../gpu/drm/panel/panel-sharp-ls037v7dw01.c   | 226 ++
>  drivers/gpu/drm/panel/panel-sony-acx565akm.c  | 693 ++
>  drivers/gpu/drm/panel/panel-tpo-td028ttec1.c  | 381 ++
>  drivers/gpu/drm/panel/panel-tpo-td043mtea1.c  | 508 +
>  10 files changed, 2398 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/nec,nl8048hl11.yaml
>  create mode 100644 drivers/gpu/drm/panel/panel-lg-lb035q02.c
>  create mode 100644 drivers/gpu/drm/panel/panel-nec-nl8048hl11.c
>  create mode 100644 drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
>  create mode 100644 drivers/gpu/drm/panel/panel-sony-acx565akm.c
>  create mode 100644 drivers/gpu/drm/panel/panel-tpo-td028ttec1.c
>  create mode 100644 drivers/gpu/drm/panel/panel-tpo-td043mtea1.c
> 
> -- 
> Regards,
> 
> Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 5/9] drm/panel: Add driver for the NEC NL8048HL11 panel

2019-08-11 Thread Sam Ravnborg
Hi Laurent.

On Sun, Aug 11, 2019 at 02:10:44AM +0300, Laurent Pinchart wrote:
> This panel is used on the Zoom2/3/3630 SDP boards.
> 
> The code is based on the omapdrm-specific panel-nec-nl8048hl11 driver
> 
> Signed-off-by: Laurent Pinchart 
> ---
> Changes since v1:
> 
> - Mention boards using the panel in Kconfig
> - Renamed nl8048_device to nl8048_panel
> - Comments updates
> - Store width_mm and height_mm in drm_display_mode
> - Use drm_panel_disable() in .remove() handler
> ---
>  drivers/gpu/drm/panel/Kconfig|   8 +
>  drivers/gpu/drm/panel/Makefile   |   1 +
>  drivers/gpu/drm/panel/panel-nec-nl8048hl11.c | 247 +++
>  3 files changed, 256 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-nec-nl8048hl11.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 87cdc330672b..d28133c6aa0a 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -119,6 +119,14 @@ config DRM_PANEL_LG_LG4573
> Say Y here if you want to enable support for LG4573 RGB panel.
> To compile this driver as a module, choose M here.
>  
> +config DRM_PANEL_NEC_NL8048HL11
> + tristate "NEC NL8048HL11 RGB panel"
> + depends on GPIOLIB && OF && SPI
> + help
> +   Say Y here if you want to enable support for the NEC NL8048HL11 RGB
> +   panel (found on the Zoom2/3/3630 SDP boards). To compile this driver
> +   as a module, choose M here.
> +
>  config DRM_PANEL_NOVATEK_NT39016
>   tristate "Novatek NT39016 RGB/SPI panel"
>   depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 9bc6f3c5bf96..8052f9a7ad60 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += 
> panel-jdi-lt070me05000.o
>  obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o
>  obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> +obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
>  obj-$(CONFIG_DRM_PANEL_NOVATEK_NT39016) += panel-novatek-nt39016.o
>  obj-$(CONFIG_DRM_PANEL_OLIMEX_LCD_OLINUXINO) += panel-olimex-lcd-olinuxino.o
>  obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
> diff --git a/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c 
> b/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c
> new file mode 100644
> index ..7c1b77a0b024
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c
> @@ -0,0 +1,247 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * NEC NL8048HL11 Panel Driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated
> + *
> + * Based on the omapdrm-specific panel-nec-nl8048hl11 driver
> + *
> + * Copyright (C) 2010 Texas Instruments Incorporated
> + * Author: Erik Gilling 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +struct nl8048_panel {
> + struct drm_panel panel;
> +
> + struct spi_device *spi;
> + struct gpio_desc *reset_gpio;
> +};
> +
> +#define to_nl8048_device(p) container_of(p, struct nl8048_panel, panel)
> +
> +static int nl8048_write(struct nl8048_panel *lcd, unsigned char addr,
> + unsigned char value)
> +{
> + u8 data[4] = { value, 0x01, addr, 0x00 };
> + int ret;
> +
> + ret = spi_write(lcd->spi, data, sizeof(data));
> + if (ret)
> + dev_err(>spi->dev, "SPI write to %u failed: %d\n",
> + addr, ret);
> +
> + return ret;
> +}
> +
> +static int nl8048_init(struct nl8048_panel *lcd)
> +{
> + static const struct {
> + unsigned char addr;
> + unsigned char data;
> + } nl8048_init_seq[] = {
> + {   3, 0x01 }, {   0, 0x00 }, {   1, 0x01 }, {   4, 0x00 },
> + {   5, 0x14 }, {   6, 0x24 }, {  16, 0xd7 }, {  17, 0x00 },
> + {  18, 0x00 }, {  19, 0x55 }, {  20, 0x01 }, {  21, 0x70 },
> + {  22, 0x1e }, {  23, 0x25 }, {  24, 0x25 }, {  25, 0x02 },
> + {  26, 0x02 }, {  27, 0xa0 }, {  32, 0x2f }, {  33, 0x0f },
> + {  34, 0x0f }, {  35, 0x0f }, {  36, 0x0f }, {  37, 0x0f },
> + {  38, 0x0f }, {  39, 0x00 }, {  40, 0x02 }, {  41, 0x02 },
> + {  42, 0x02 }, {  43, 0x0f }, {  44, 0x0f }, {  45, 0x0f },
> + {  46, 0x0f }, {  47, 0x0f }, {  48, 0x0f }, {  49, 0x0f },
> + {  50, 0x00 }, {  51, 0x02 }, {  52, 0x02 }, {  53, 0x02 },
> + {  80, 0x0c }, {  83, 0x42 }, {  84, 0x42 }, {  85, 0x41 },
> + {  86, 0x14 }, {  89, 0x88 }, {  90, 0x01 }, {  91, 0x00 },
> + {  92, 0x02 }, {  93, 0x0c }, {  94, 0x1c }, {  95, 0x27 },
> + {  98, 0x49 }, {  99, 0x27 }, { 102, 0x76 }, { 103, 0x27 },
> + { 112, 0x01 }, { 113, 

Re: [PATCH 4/4] drm/panel/ili9341: Support DPI panels

2019-08-11 Thread Sam Ravnborg
Hi Noralf.

On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote:
> Add support for panels that use the DPI interface.
> ILI9341 has onboard RAM so the assumption made here is that all such
> panels support pixel upload over DBI.
> 
> The presence/absense of the Device Tree 'port' node decides which
> interface is used for pixel transfer.
> 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 
>  1 file changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c 
> b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> index f6082fa2a389..7cbfd739c7fd 100644
> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -53,11 +54,13 @@
>  struct ili9341_config {
>   const struct drm_panel_funcs *funcs;
>   const struct drm_display_mode *mode;
> + bool no_dpi;
>  };
>  
>  struct ili9341 {
>   struct mipi_dbi_dev dbidev; /* This must be the first entry */
>   struct drm_panel panel;
> + bool use_dpi;
>   struct regulator *regulator;
>   struct backlight_device *backlight;
>   const struct ili9341_config *conf;
> @@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = {
>  static const struct ili9341_config yx240qv29_data = {
>   .funcs = _funcs,
>   .mode = _mode,
> + .no_dpi = true,
>  };
>  
>  static int mi0283qt_prepare(struct drm_panel *panel)
> @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = {
>  static const struct ili9341_config mi0283qt_data = {
>   .funcs = _drm_funcs,
>   .mode = _mode,
> + .no_dpi = true,
>  };
>  
>  /* Legacy, DRM driver name is ABI */
> @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi)
>   const struct spi_device_id *spi_id;
>   struct device *dev = >dev;
>   struct drm_driver *driver;
> + struct device_node *port;
>   struct mipi_dbi *dbi;
>   struct gpio_desc *dc;
>   struct ili9341 *ili;
> @@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi)
>   ili->panel.dev = dev;
>   ili->panel.funcs = ili->conf->funcs;
>  
> - if (ili->conf == _data)
> - driver = _drm_driver;
> - else
> - driver = _drm_driver;
>  
> - return drm_mipi_dbi_panel_register(>panel, >dbidev, driver,
> -ili->conf->mode, rotation);
> + port = of_get_child_by_name(dev->of_node, "port");
> + if (port) {
> + of_node_put(port);
> + ili->use_dpi = true;
> + }
> +
> + if (ili->conf->no_dpi)
> + ili->use_dpi = false;
> +
> + if (ili->use_dpi) {
> + ret = drm_panel_add(>panel);
> + } else {
> + if (ili->conf == _data)
> + driver = _drm_driver;
> + else
> + driver = _drm_driver;
> +
> + ret = drm_mipi_dbi_panel_register(>panel, >dbidev, 
> driver,
> +   ili->conf->mode, rotation);
> + }
> +
> + return ret;
>  }
>  
>  static int ili9341_remove(struct spi_device *spi)
>  {
>   struct ili9341 *ili = spi_get_drvdata(spi);
>  
> - drm_dev_unplug(>dbidev.drm);
> - drm_atomic_helper_shutdown(>dbidev.drm);
> + if (ili->use_dpi) {
> + drm_panel_remove(>panel);
> + drm_panel_disable(>panel);
> + drm_panel_unprepare(>panel);
> + kfree(ili);
At first I thought - order is wrong.
But drm_panel_remove() prevents display drivers from using the driver.
And this will not invalidate the other calls.
Maybe add a short comment?

Sam


> + } else {
> + drm_dev_unplug(>dbidev.drm);
> + drm_atomic_helper_shutdown(>dbidev.drm);
> + }
>  
>   return 0;
>  }
> @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi)
>  {
>   struct ili9341 *ili = spi_get_drvdata(spi);
>  
> - drm_atomic_helper_shutdown(>dbidev.drm);
> + if (!ili->use_dpi)
> + drm_atomic_helper_shutdown(>dbidev.drm);
>  }
>  
>  static int __maybe_unused ili9341_pm_suspend(struct device *dev)
>  {
>   struct ili9341 *ili = dev_get_drvdata(dev);
>  
> - return drm_mode_config_helper_suspend(>dbidev.drm);
> + if (!ili->use_dpi)
> + return drm_mode_config_helper_suspend(>dbidev.drm);
> +
> + return 0;
>  }
>  
>  static int __maybe_unused ili9341_pm_resume(struct device *dev)
>  {
>   struct ili9341 *ili = dev_get_drvdata(dev);
>  
> - drm_mode_config_helper_resume(>dbidev.drm);
> + if (!ili->use_dpi)
> + drm_mode_config_helper_resume(>dbidev.drm);
>  
>   return 0;
>  }
> -- 
> 2.20.1
___
dri-devel mailing list

Re: [PATCH v2 3/9] dt-bindings: display: panel: Add bindings for NEC NL8048HL11 panel

2019-08-11 Thread Sam Ravnborg
Hi Laurent.

My meta-schemas foo is very limited, but I noticed a few things.
Hopefully Rob finds time soon to review.

Sam

On Sun, Aug 11, 2019 at 02:10:42AM +0300, Laurent Pinchart wrote:
> The NEC NL8048HL11 is a 10.4cm WVGA (800x480) panel with a 24-bit RGB
> parallel data interface and an SPI control interface.
> 
> Signed-off-by: Laurent Pinchart 
> ---
> Changes since v1:
> 
> - Convert to YAML
> ---
>  .../display/panel/nec,nl8048hl11.yaml | 49 +++
>  1 file changed, 49 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/nec,nl8048hl11.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/nec,nl8048hl11.yaml 
> b/Documentation/devicetree/bindings/display/panel/nec,nl8048hl11.yaml
> new file mode 100644
> index ..cc3d40975828
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/nec,nl8048hl11.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/nec,nl8048hl11.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NEC NL8048HL11 4.1" WVGA TFT LCD panel
> +
> +description:
> +  The NEC NL8048HL11 is a 4.1" WVGA TFT LCD panel with a 24-bit RGB parallel
> +  data interface and an SPI control interface.
> +
> +maintainers:
> +  - Laurent Pinchart 
> +
> +allOf:
> +  - $ref: panel-common.yaml#
I *think* we are missing a reference to spi-controller.yaml here.


> +
> +properties:
> +  compatible:
> +const: nec,nl8048hl11
> +
> +  label: true
> +  reset-gpios: true
> +  port: true
> +
> +required:
> +  - compatible
> +  - reset-gpios
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +lcd_panel: panel {
> +  compatible = "nec,nl8048hl11";
> +  spi-max-frequency = <1000>;
Not sure, but should there be a reg property here for spi too?

> +
> +  reset-gpios = < 7 GPIO_ACTIVE_LOW>;
> +
> +  port {
> +lcd_in: endpoint {
> +  remote-endpoint = <_out>;
> +};
> +  };
> +};
> +
> +...
> -- 
> Regards,
> 
> Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/9] drm/panel: Add driver for the LG Philips LB035Q02 panel

2019-08-11 Thread Sam Ravnborg
Hi Noralf.

> > +static int lb035q02_write(struct lb035q02_device *lcd, u16 reg, u16 val)
> > +{
> > +   struct spi_message msg;
> > +   struct spi_transfer index_xfer = {
> > +   .len= 3,
> > +   .cs_change  = 1,
> > +   };
> > +   struct spi_transfer value_xfer = {
> > +   .len= 3,
> > +   };
> > +   u8  buffer[16];
> > +
> > +   spi_message_init();
> > +
> > +   /* register index */
> > +   buffer[0] = 0x70;
> > +   buffer[1] = 0x00;
> > +   buffer[2] = reg & 0x7f;
> > +   index_xfer.tx_buf = buffer;
> > +   spi_message_add_tail(_xfer, );
> > +
> > +   /* register value */
> > +   buffer[4] = 0x72;
> > +   buffer[5] = val >> 8;
> > +   buffer[6] = val;
> > +   value_xfer.tx_buf = buffer + 4;
> > +   spi_message_add_tail(_xfer, );
> > +
> > +   return spi_sync(lcd->spi, );
> > +}
> 
> Just a note to Sam:
> This is the same spi protocol that the ili9325 controller on the hy28b
> panel uses.
> 
> I remembered that I have experimented with a regmap implementation:
> https://github.com/notro/tinydrm/blob/master/tinydrm-ili9325.c#L191

regmap seems a too limited interface to use when trying to generalize
this.
We should rather go for a ili-lib or something that can be used in all
the present and future ili based drivers.
Obviously it depends on how similar the ili based chips are.

I did a quick look and this driver did not match the ili9325 datasheet
as different bits was are in register 0x1.
So it smeels like another, similar. ili varaint.

So for this driver we would just use the hardcoded varaint already
present. Then we may re-visit ili-lib idea later if we see too much
similar code.
This is as I see it for now...

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

Re: [PATCH 1/2] drm: gm12u320: Do not take a mutex from a wait_event condition

2019-08-11 Thread Sam Ravnborg
Hi Hans.

On Sun, Aug 11, 2019 at 04:37:24PM +0200, Hans de Goede wrote:
> I made the condition of the wait_event_timeout call in
> gm12u320_fb_update_work a helper which takes a mutex to make sure
> that any writes to fb_update.run or fb_update.fb from other CPU cores
> are seen before the check is done.
> 
> This is not necessary as the wait_event helpers contain the necessary
> barriers for this themselves.
> 
> More over it is harmfull since by the time the check is done the task
> is no longer in the TASK_RUNNING state and calling mutex_lock while not
> in task-running is not allowed, leading to this warning when the kernel
> is build with some extra locking checks enabled:
> 
> [11947.450011] do not call blocking ops when !TASK_RUNNING; state=2 set at
>[<e4306de6>] prepare_to_wait_event+0x61/0x190
> 
> This commit fixes this by dropping the helper and simply directly
> checking the condition (without unnecessary locking) in the
> wait_event_timeout call.
> 
> Signed-off-by: Hans de Goede 

Both patches are:
Acked-by: Sam Ravnborg 

Code looks sane and matches your explanations.
But with limited clue on locking or USB this is not a proper review.

Sam

> ---
>  drivers/gpu/drm/tiny/gm12u320.c | 14 ++
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index 4c47aa4de09b..4d66765b1125 100644
> --- a/drivers/gpu/drm/tiny/gm12u320.c
> +++ b/drivers/gpu/drm/tiny/gm12u320.c
> @@ -342,17 +342,6 @@ static void gm12u320_copy_fb_to_blocks(struct 
> gm12u320_device *gm12u320)
>   mutex_unlock(>fb_update.lock);
>  }
>  
> -static int gm12u320_fb_update_ready(struct gm12u320_device *gm12u320)
> -{
> - int ret;
> -
> - mutex_lock(>fb_update.lock);
> - ret = !gm12u320->fb_update.run || gm12u320->fb_update.fb != NULL;
> - mutex_unlock(>fb_update.lock);
> -
> - return ret;
> -}
> -
>  static void gm12u320_fb_update_work(struct work_struct *work)
>  {
>   struct gm12u320_device *gm12u320 =
> @@ -426,7 +415,8 @@ static void gm12u320_fb_update_work(struct work_struct 
> *work)
>* switches back to showing its logo.
>*/
>   wait_event_timeout(gm12u320->fb_update.waitq,
> -gm12u320_fb_update_ready(gm12u320),
> +!gm12u320->fb_update.run ||
> + gm12u320->fb_update.fb != NULL,
>  IDLE_TIMEOUT);
>   }
>   return;
> -- 
> 2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/panel/ili9341: Support DPI panels

2019-08-11 Thread Sam Ravnborg
Hi Noralf.

On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote:
> Add support for panels that use the DPI interface.
> ILI9341 has onboard RAM so the assumption made here is that all such
> panels support pixel upload over DBI.
> 
> The presence/absense of the Device Tree 'port' node decides which
> interface is used for pixel transfer.
Have you consiered if the compatible could be used to determine the use
of a panel?
Then it is more explicit how the HW is described in DT.

Sam

> 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 
>  1 file changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c 
> b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> index f6082fa2a389..7cbfd739c7fd 100644
> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -53,11 +54,13 @@
>  struct ili9341_config {
>   const struct drm_panel_funcs *funcs;
>   const struct drm_display_mode *mode;
> + bool no_dpi;
>  };
>  
>  struct ili9341 {
>   struct mipi_dbi_dev dbidev; /* This must be the first entry */
>   struct drm_panel panel;
> + bool use_dpi;
>   struct regulator *regulator;
>   struct backlight_device *backlight;
>   const struct ili9341_config *conf;
> @@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = {
>  static const struct ili9341_config yx240qv29_data = {
>   .funcs = _funcs,
>   .mode = _mode,
> + .no_dpi = true,
>  };
>  
>  static int mi0283qt_prepare(struct drm_panel *panel)
> @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = {
>  static const struct ili9341_config mi0283qt_data = {
>   .funcs = _drm_funcs,
>   .mode = _mode,
> + .no_dpi = true,
>  };
>  
>  /* Legacy, DRM driver name is ABI */
> @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi)
>   const struct spi_device_id *spi_id;
>   struct device *dev = >dev;
>   struct drm_driver *driver;
> + struct device_node *port;
>   struct mipi_dbi *dbi;
>   struct gpio_desc *dc;
>   struct ili9341 *ili;
> @@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi)
>   ili->panel.dev = dev;
>   ili->panel.funcs = ili->conf->funcs;
>  
> - if (ili->conf == _data)
> - driver = _drm_driver;
> - else
> - driver = _drm_driver;
>  
> - return drm_mipi_dbi_panel_register(>panel, >dbidev, driver,
> -ili->conf->mode, rotation);
> + port = of_get_child_by_name(dev->of_node, "port");
> + if (port) {
> + of_node_put(port);
> + ili->use_dpi = true;
> + }
> +
> + if (ili->conf->no_dpi)
> + ili->use_dpi = false;
> +
> + if (ili->use_dpi) {
> + ret = drm_panel_add(>panel);
> + } else {
> + if (ili->conf == _data)
> + driver = _drm_driver;
> + else
> + driver = _drm_driver;
> +
> + ret = drm_mipi_dbi_panel_register(>panel, >dbidev, 
> driver,
> +   ili->conf->mode, rotation);
> + }
> +
> + return ret;
>  }
>  
>  static int ili9341_remove(struct spi_device *spi)
>  {
>   struct ili9341 *ili = spi_get_drvdata(spi);
>  
> - drm_dev_unplug(>dbidev.drm);
> - drm_atomic_helper_shutdown(>dbidev.drm);
> + if (ili->use_dpi) {
> + drm_panel_remove(>panel);
> + drm_panel_disable(>panel);
> + drm_panel_unprepare(>panel);
> + kfree(ili);
> + } else {
> + drm_dev_unplug(>dbidev.drm);
> + drm_atomic_helper_shutdown(>dbidev.drm);
> + }
>  
>   return 0;
>  }
> @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi)
>  {
>   struct ili9341 *ili = spi_get_drvdata(spi);
>  
> - drm_atomic_helper_shutdown(>dbidev.drm);
> + if (!ili->use_dpi)
> + drm_atomic_helper_shutdown(>dbidev.drm);
>  }
>  
>  static int __maybe_unused ili9341_pm_suspend(struct device *dev)
>  {
>   struct ili9341 *ili = dev_get_drvdata(dev);
>  
> - return drm_mode_config_helper_suspend(>dbidev.drm);
> + if (!ili->use_dpi)
> + return drm_mode_config_helper_suspend(>dbidev.drm);
> +
> + return 0;
>  }
>  
>  static int __maybe_unused ili9341_pm_resume(struct device *dev)
>  {
>   struct ili9341 *ili = dev_get_drvdata(dev);
>  
> - drm_mode_config_helper_resume(>dbidev.drm);
> + if (!ili->use_dpi)
> + drm_mode_config_helper_resume(>dbidev.drm);
>  
>   return 0;
>  }
> -- 
> 2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org

Re: [PATCH 2/4] drm/tiny/ili9341: Move driver to drm/panel

2019-08-11 Thread Sam Ravnborg
Hi Noralf.

Most feedback on this driver was covered in comment to 1/4.
Only a few things caught my eye.

On Thu, Aug 01, 2019 at 03:52:47PM +0200, Noralf Trønnes wrote:
> Move the driver to drm/panel and take advantage of the new panel support
> in drm_mipi_dbi. Change the file name to match the naming standard in
> drm/panel. The DRM driver name is kept since it is ABI.
> 
> Add missing MAINTAINERS entry.
> 
> Cc: David Lechner 
> Signed-off-by: Noralf Trønnes 
> ---
>  MAINTAINERS   |   7 +
>  drivers/gpu/drm/panel/Kconfig |  12 ++
>  drivers/gpu/drm/panel/Makefile|   1 +
>  .../panel-ilitek-ili9341.c}   | 174 ++
>  drivers/gpu/drm/tiny/Kconfig  |  13 --
>  drivers/gpu/drm/tiny/Makefile |   1 -
>  6 files changed, 113 insertions(+), 95 deletions(-)
>  rename drivers/gpu/drm/{tiny/ili9341.c => panel/panel-ilitek-ili9341.c} (66%)
> 
> +
> +struct ili9341 {
> + struct mipi_dbi_dev dbidev; /* This must be the first entry */

Can we avoid the need for this to be the first entry?


> -static struct drm_driver ili9341_driver = {
> - .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> - .fops   = _fops,
> - .release= mipi_dbi_release,
> - DRM_GEM_CMA_VMAP_DRIVER_OPS,
> - .debugfs_init   = mipi_dbi_debugfs_init,
> - .name   = "ili9341",
> - .desc   = "Ilitek ILI9341",
> - .date   = "20180514",
> - .major  = 1,
> - .minor  = 0,

> +DEFINE_DRM_MIPI_DBI_PANEL_DRIVER(ili9341, "Ilitek ILI9341", "20180514");
Update the date. This is a major change so let it be refelcted in the
date.

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

Re: [PATCH 1/4] drm/mipi-dbi: Support command mode panel drivers

2019-08-11 Thread Sam Ravnborg
Hi Noralf.

I really like how this allows us to have a single file for all
the uses of a driver IC.
And this patch-set is much easier to grasp than the first RFC.

General things:

- The current design have a tight connection between the display
  controller and the panel. Will this hurt in the long run?
  In other words, should we try to add a panel_bridge in-between?
  For now I think this would just make something simple more
  complicated.
  So this note was to say - no I think we should not use panel_bridge.

- drm_panel has proper support for modes.
  This is today duplicated in mipi_dbi.
  Could we make it so that when a panel is used then the panel
  has the mode info - as we then use the panel more in the way we do
  in other cases?
  As it is now the mode is specified in mipi_dbi_dev_init()
  The drm_connector would then, if a panel is used, ask the panel for
  the mode.
  I did not really think to the end of this, but it seems wrong that
  we introduce drm_panel and then keep modes in mipi_dbi.

- For backlight support please move this to drm_panel.
  I have patches that makes it simple to use backlight with drm_panel,
  and this will then benefit from it.
  The drm_panel backlight supports requires that a backlight
  phandle is specified in the DT node of the panel.

Some more specific comments in the following.

Sam

On Thu, Aug 01, 2019 at 03:52:46PM +0200, Noralf Trønnes wrote:
> This adds a function that registers a DRM driver for use with MIPI DBI
> panels in command mode. That is, framebuffer upload over DBI.
> 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/drm_mipi_dbi.c | 92 ++
>  include/drm/drm_mipi_dbi.h | 34 +
>  2 files changed, 126 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index 1961f713aaab..797a20e3a017 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -17,11 +17,13 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -597,6 +599,96 @@ void mipi_dbi_release(struct drm_device *drm)
>  }
>  EXPORT_SYMBOL(mipi_dbi_release);
>  
> +static void drm_mipi_dbi_panel_pipe_enable(struct drm_simple_display_pipe 
> *pipe,
> +struct drm_crtc_state *crtc_state,
> +struct drm_plane_state *plane_state)
> +{
> + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
> + struct drm_panel *panel = dbidev->panel;
> + int ret, idx;
> +
> + if (!drm_dev_enter(pipe->crtc.dev, ))
> + return;
> +
> + DRM_DEBUG_KMS("\n");
Still usefull?

> +
> + ret = drm_panel_prepare(panel);
> + if (ret)
> + goto out_exit;
> +
> + mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
> +
> + drm_panel_enable(panel);
> +out_exit:
nit - blank line above label?

> + drm_dev_exit(idx);
> +}
> +
> +static void drm_mipi_dbi_panel_pipe_disable(struct drm_simple_display_pipe 
> *pipe)
> +{
> + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
> + struct drm_panel *panel = dbidev->panel;
> +
> + if (!dbidev->enabled)
> + return;
> +
> + DRM_DEBUG_KMS("\n");
Still usefull?
> +
> + dbidev->enabled = false;
> + drm_panel_disable(panel);
> + drm_panel_unprepare(panel);
> +}
> +
> +static const struct drm_simple_display_pipe_funcs drm_mipi_dbi_pipe_funcs = {
> + .enable = drm_mipi_dbi_panel_pipe_enable,
> + .disable = drm_mipi_dbi_panel_pipe_disable,
> + .update = mipi_dbi_pipe_update,
> + .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +};
> +
> +/**
> + * drm_mipi_dbi_panel_register - Register a MIPI DBI DRM driver
> + * @panel: DRM Panel
> + * @dbidev: MIPI DBI device structure to initialize
> + * @mode: Display mode
> + *
> + * This function registeres a DRM driver with @panel attached.
> + *
> + * Returns:
> + * Zero on success, negative error code on failure.
> + */
> +int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev 
> *dbidev,
> + struct drm_driver *driver, const struct 
> drm_display_mode *mode,
> + u32 rotation)
Can we make this use enum drm_panel_orientation - so we can use
of_drm_get_panel_orientation() in the callers?
of_drm_get_panel_orientation() is not merged yet, but we could do so if
this patch-set needs it.

I know that this would require mipi_dbi_dev_init() and all users to be
updated. But it is a simpler interface so worth it.

> +{
> + struct device *dev = panel->dev;
> + struct drm_device *drm;
> + int ret;
> +
> + dbidev->panel = panel;
> +
> + drm = >drm;
> + ret = devm_drm_dev_init(dev, drm, driver);
> + if (ret) {
> + kfree(dbidev);
> + return ret;
> + 

Re: [PATCH 0/5] kbuild: allow big modules to sub-divide Makefiles

2019-08-10 Thread Sam Ravnborg
Hi Masahiro

On Tue, Aug 06, 2019 at 03:39:18PM +0900, Masahiro Yamada wrote:
> 
> Recently, Jani Nikula requests a better build system support
> for drivers spanning multiple directories.
> (better kbuild support for drivers spanning multiple directories?)
> 
> I implemented it, so please take a look at it.
> 
> Note:
> The single targets do not work correctly.
> 
> The single targets have never worked correctly:

It works in most cases, but now always.
I dunno how much it is used.
Myself I almost always do make /drivers/foo/bar/
> 
> [1] For instance, "make drivers/foo/bar/baz.o" will descend into
> drivers/foo/bar/Makefile, which may not necessarily specify
> the build rule of baz.o
> 
> It is possible for drivers/foo/Makefile having
> obj-$(CONFIG_BAZ) += bar/baz.o
> 
> [2] subdir-ccflags-y does not work.
> 
> The single targets directly descend into the directory of
> that file resides.
> 
> It missed subdir-ccflags-y if it is specifies in parent
> Makefiles.
> 
> Perhaps, I will have to manage correct implementation of single targets.
The day that kbuild has a separate step to read all Makefiles
and then without using recursive make can build the kernel we can have
this fixed.
Until then we can accpet it as is - as fixing this may not be simple.

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

Re: [PATCH] drm: Fix kerneldoc warns in connector-related docs

2019-08-09 Thread Sam Ravnborg
Hi Sean.

> > > > > --- a/include/drm/drm_connector.h
> > > > > +++ b/include/drm/drm_connector.h
> > > > > @@ -543,8 +543,8 @@ struct drm_connector_state {
> > > > >*
> > > > >* This is also used in the atomic helpers to map encoders to 
> > > > > their
> > > > >* current and previous connectors, see
> > > > > -  * _atomic_get_old_connector_for_encoder() and
> > > > > -  * _atomic_get_new_connector_for_encoder().
> > > > > +  * _atomic_get_old_connector_for_encoder and
> > > > > +  * _atomic_get_new_connector_for_encoder.
> > > > Please fix this to use () for the functions and drop the "&".
> > > > This is what we use in drm kernel-doc for functions.
> > > > See for example function references in doc of writeback_job in the same 
> > > > file.
> > > 
> > > Doing this won't get a hyperlink in the docs. It seems like these hooks 
> > > aren't
> > > recognized as functions by sphinx (not sure didn't look into it too 
> > > deeply). The
> > > other "_funcs.*" hooks are also handled with '&' (there are lots of 
> > > examples in
> > > drm_connector.h).
> > > 
> > > I think preserving the hyperlinks probably outweighs the missing (), 
> > > thoughts?
> > 
> > Hmm, I actually tested it here - with sphinx_1.4.9.
> > The links was preserved, the only difference was that they had the ()
> > prefixed to their name.
> > 
> > Do you happen to use an older sphinx version?
> 
> I'm on 1.7.9. I just rechecked and was a bit confused in my last mail. The
> drm_atomic_get_*_connector_for_encoder links do work with (), it's the ones
> drm_connector_helper_funcs in the paragraph above that need the '&'. So I'll
> switch up these 2 and leave the others as-is. Cool?
Perfect!

You can add my:

Reviewed-by: Sam Ravnborg 

and then apply.

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

Re: [PATCH] drm: Fix kerneldoc warns in connector-related docs

2019-08-09 Thread Sam Ravnborg
Hi Sean.

> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -543,8 +543,8 @@ struct drm_connector_state {
> > >*
> > >* This is also used in the atomic helpers to map encoders to their
> > >* current and previous connectors, see
> > > -  * _atomic_get_old_connector_for_encoder() and
> > > -  * _atomic_get_new_connector_for_encoder().
> > > +  * _atomic_get_old_connector_for_encoder and
> > > +  * _atomic_get_new_connector_for_encoder.
> > Please fix this to use () for the functions and drop the "&".
> > This is what we use in drm kernel-doc for functions.
> > See for example function references in doc of writeback_job in the same 
> > file.
> 
> Doing this won't get a hyperlink in the docs. It seems like these hooks aren't
> recognized as functions by sphinx (not sure didn't look into it too deeply). 
> The
> other "_funcs.*" hooks are also handled with '&' (there are lots of examples 
> in
> drm_connector.h).
> 
> I think preserving the hyperlinks probably outweighs the missing (), thoughts?

Hmm, I actually tested it here - with sphinx_1.4.9.
The links was preserved, the only difference was that they had the ()
prefixed to their name.

Do you happen to use an older sphinx version?

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

Re: [PATCH v2 2/2] drm: bridge: adv7511: Add support for ADV7535

2019-08-09 Thread Sam Ravnborg
Hi Bogdan.

This patch triggered a few general comments.

> --- a/drivers/gpu/drm/bridge/adv7511/Makefile
> +++ b/drivers/gpu/drm/bridge/adv7511/Makefile
> @@ -2,5 +2,5 @@
>  adv7511-y := adv7511_drv.o
>  adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o
>  adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o
> -adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o
> +adv7511-$(CONFIG_DRM_I2C_ADV753x) += adv7533.o
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h 
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index 52b2adfdc877..38288c3c3c53 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -91,6 +91,7 @@
>  #define ADV7511_REG_ARC_CTRL 0xdf
>  #define ADV7511_REG_CEC_I2C_ADDR 0xe1
>  #define ADV7511_REG_CEC_CTRL 0xe2
> +#define ADV7511_REG_SUPPLY_SELECT0xe4
>  #define ADV7511_REG_CHIP_ID_HIGH 0xf5
>  #define ADV7511_REG_CHIP_ID_LOW  0xf6
>  
> @@ -320,6 +321,7 @@ struct adv7511_video_config {
>  enum adv7511_type {
>   ADV7511,
>   ADV7533,
> + ADV7535,
>  };
>  
>  #define ADV7511_MAX_ADDRS 3
> @@ -393,7 +395,7 @@ static inline int adv7511_cec_init(struct device *dev, 
> struct adv7511 *adv7511)
>  }
>  #endif
>  
> -#ifdef CONFIG_DRM_I2C_ADV7533
> +#ifdef CONFIG_DRM_I2C_ADV753x
>  void adv7533_dsi_power_on(struct adv7511 *adv);
>  void adv7533_dsi_power_off(struct adv7511 *adv);
>  void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode 
> *mode);

The else part here define dummy functions.

> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index f6d2681f6927..b1501344df3e 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -367,7 +367,7 @@ static void adv7511_power_on(struct adv7511 *adv7511)
>*/
>   regcache_sync(adv7511->regmap);
>  
> - if (adv7511->type == ADV7533)
> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535)
>   adv7533_dsi_power_on(adv7511);

In the driver we check for adv7511->type - and call
adv7533_dsi_power_on() only for the two types where we have this
function defined.
A simpler approach would be to always call adv7533_dsi_power_on(), and
let the existing logic pick up the right version.
The dummy version should then return 0 to say OK.

Same goes for several places below.


>   adv7511->powered = true;
>  }
> @@ -387,7 +387,7 @@ static void __adv7511_power_off(struct adv7511 *adv7511)
>  static void adv7511_power_off(struct adv7511 *adv7511)
>  {
>   __adv7511_power_off(adv7511);
> - if (adv7511->type == ADV7533)
> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535)
>   adv7533_dsi_power_off(adv7511);
>   adv7511->powered = false;
>  }
> @@ -761,7 +761,7 @@ static void adv7511_mode_set(struct adv7511 *adv7511,
>   regmap_update_bits(adv7511->regmap, 0x17,
>   0x60, (vsync_polarity << 6) | (hsync_polarity << 5));
>  
> - if (adv7511->type == ADV7533)
> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535)
>   adv7533_mode_set(adv7511, adj_mode);
>  
>   drm_mode_copy(>curr_mode, adj_mode);
> @@ -874,7 +874,7 @@ static int adv7511_bridge_attach(struct drm_bridge 
> *bridge)
>_connector_helper_funcs);
>   drm_connector_attach_encoder(>connector, bridge->encoder);
>  
> - if (adv->type == ADV7533)
> + if (adv->type == ADV7533 || adv->type == ADV7535)
>   ret = adv7533_attach_dsi(adv);
>  
>   if (adv->i2c_main->irq)
> @@ -903,6 +903,7 @@ static const char * const adv7511_supply_names[] = {
>   "dvdd-3v",
>  };
>  
> +/* The order of entries is important. If changed update hardcoded indices */
>  static const char * const adv7533_supply_names[] = {
>   "avdd",
>   "dvdd",
> @@ -952,7 +953,7 @@ static bool adv7511_cec_register_volatile(struct device 
> *dev, unsigned int reg)
>   struct i2c_client *i2c = to_i2c_client(dev);
>   struct adv7511 *adv7511 = i2c_get_clientdata(i2c);
>  
> - if (adv7511->type == ADV7533)
> + if (adv7511->type == ADV7533 || adv7511->type == ADV7535)
>   reg -= ADV7533_REG_CEC_OFFSET;
>  
>   switch (reg) {
> @@ -994,7 +995,7 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
>   goto err;
>   }
>  
> - if (adv->type == ADV7533) {
> + if (adv->type == ADV7533 || adv->type == ADV7535) {
>   ret = adv7533_patch_cec_registers(adv);
>   if (ret)
>   goto err;
> @@ -1094,8 +1095,9 @@ static int adv7511_probe(struct i2c_client *i2c, const 
> struct i2c_device_id *id)
>   struct adv7511_link_config link_config;
>   struct adv7511 *adv7511;
>   struct device *dev = >dev;
> + struct regulator 

Re: [PATCH 24/60] drm/panel: Add driver for the Toppology TD043MTEA1 panel

2019-08-09 Thread Sam Ravnborg
Hi Laurent.

> > > +static int td043mtea1_disable(struct drm_panel *panel)
> > > +{
> > > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel);
> > > +
> > > + if (!lcd->spi_suspended)
> > > + td043mtea1_power_off(lcd);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int td043mtea1_enable(struct drm_panel *panel)
> > > +{
> > > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel);
> > > + int ret;
> > > +
> > > + /*
> > > +  * If we are resuming from system suspend, SPI might not be enabled
> > > +  * yet, so we'll program the LCD from SPI PM resume callback.
> > > +  */
> > > + if (lcd->spi_suspended)
> > > + return 0;
> > 
> > I do not recall this is needed in other panel drivers, so look at what
> > other spi based panels do here.
> > I think this is something that today is not required.
> 
> The problem here is that the display controller may be resumed before
> the SPI bus. Has that been solved somewhere in core code ?

I dunno. So the conclusion is to keep it as is, and any change
will wait until someone with HW can step up.

As for all your other feedback to this and the other panel drivers
they did not trigger any repsonse from me.

Looks forward for next iteration of this nice set of patches.
Can we maybe get the panel drivers in before some of the infrastructure
work?
I know the users then may come a bit later, but I think thats OK.

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

Re: [PATCH RFC 01/19] drm: Stop including drm_bridge.h from drm_crtc.h

2019-08-08 Thread Sam Ravnborg
Hi Boris.

Good to see that you kept the alphabetical order in the include files.
One nit below.
With this fixed:
Reviewed-by: Sam Ravnborg 

Sam

> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
> index 982865866a29..c681a9e22484 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.h
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
> @@ -7,6 +7,8 @@
>  #ifndef __HDMI_CONNECTOR_H__
>  #define __HDMI_CONNECTOR_H__
>  
> +#include 
> +
>  #include 
>  #include 
>  #include 

Please follow the same order of include blocks as we see in other files:

#include 

#include 

#include "*.h"

And with an empty line between the blocks, and sorted withint the
blocks.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: drm_panel_get_modes() should take the connector as an argument [Was: drm/bridge: panel: Implement bridge ...]

2019-08-08 Thread Sam Ravnborg
Hi Laurent.

As I said in another mail, you have managed to keep me busy...

> > I took a look at this - it seems simple:
> > - Update drm_panel.get_modes() to take drm_connector as argument, and fix
> >   all callers. All callers already have connector available.
> > - Drop drm_panel_attach(), drm_panel_detach() and update all callers.
> >   In reality just drop all code around attach(), detach().
> >   drm_panel_attach(), drm_panel_detach() will be noops when the
> >   connector stored in drm_panel is no longer used.
> > 
> > The semantic difference is that we supply the connector when we call
> > drm_panel_get_modes() and not at panel creation time with an 
> > drm_panel_attach().
> > 
> > So it should be doable without any migration from one world to the other.
> > 
> > If someone can say "yes it should be that simple", then I will
> > give it a spin.
> 
> Looking forward to that :-)

Almost there
I have all the preparation patches on dri-devel, with positive
feedback on most.

And locally I have updated all get_modes() to take drm_connector as
argument.

A few drivers access drm_panel->connector, still need to look into this.

And then for drm_panel_attach(), drm_panel_detach() - so far they are
kept but changed to take a drm_device*.

Just sharing this so you do not jump at it and duplicate the work.
It will take a little time before I can invest time in this again.
Will post patches when something is ready for review.

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

Re: [Freedreno] [PATCH] drm/msm: Make DRM_MSM default to 'm'

2019-08-07 Thread Sam Ravnborg
Hi Jordan/Rob.

On Wed, Aug 07, 2019 at 12:46:49PM -0600, Jordan Crouse wrote:
> On Wed, Aug 07, 2019 at 11:08:53AM -0700, Rob Clark wrote:
> > On Wed, Aug 7, 2019 at 10:38 AM Sam Ravnborg  wrote:
> > >
> > > Hi Jordan.
> > > On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote:
> > > > Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as
> > > > modules but there are some cases where DRM might be built in for 
> > > > whatever
> > > > reason and in those situations it is preferable to still keep MSM as a
> > > > module by default and let the user decide if they _really_ want to build
> > > > it in.
> > > >
> > > > Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure
> > > > it doesn't get missed when we need it for a6xx tarets.
> > > >
> > > > Signed-off-by: Jordan Crouse 
> > > > ---
> > > >
> > > >  drivers/gpu/drm/msm/Kconfig | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> > > > index 9c37e4d..3b2334b 100644
> > > > --- a/drivers/gpu/drm/msm/Kconfig
> > > > +++ b/drivers/gpu/drm/msm/Kconfig
> > > > @@ -14,11 +14,12 @@ config DRM_MSM
> > > >   select SHMEM
> > > >   select TMPFS
> > > >   select QCOM_SCM if ARCH_QCOM
> > > > + select QCOM_COMMAND_DB if ARCH_QCOM
> > > >   select WANT_DEV_COREDUMP
> > > >   select SND_SOC_HDMI_CODEC if SND_SOC
> > > >   select SYNC_FILE
> > > >   select PM_OPP
> > > > - default y
> > > > + default m
> > >
> > > As a general comment the right thing would be to drop this default.
> > > As it is now the Kconfig says that when DRM is selected then all of the
> > > world would then also get DRM_MSM, which only a small part of this world
> > > you see any benefit in.
> > > So they now have to de-select MSM.
> > 
> > If the default is dropped, it should probably be accompanied by adding
> > CONFIG_DRM_MSM=m to defconfig's, I think
That would be best. So the defconfigs end up with the same config as
before.

> 
> In general I prefer to not use a default but this is the only GPU driver for
> ARCH_QCOM and I think its safe to stay that 99% of ARCH_QCOM users would 
> select
> this module and those that wouldn't will omit DRM entirely.
> 
> I feel it is net negative if we dropped the default but then had to turn 
> around
> and enable it in every defconfig.
"in every" equals three defconfigs:
$ git grep ARCH_QCOM | grep defconfig
arch/arm/configs/multi_v7_defconfig:CONFIG_ARCH_QCOM=y
arch/arm/configs/qcom_defconfig:CONFIG_ARCH_QCOM=y
arch/arm64/configs/defconfig:CONFIG_ARCH_QCOM=y

Sam


Re: [PATCH] drm/msm: Make DRM_MSM default to 'm'

2019-08-07 Thread Sam Ravnborg
Hi Jordan.
On Wed, Aug 07, 2019 at 11:24:27AM -0600, Jordan Crouse wrote:
> Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as
> modules but there are some cases where DRM might be built in for whatever
> reason and in those situations it is preferable to still keep MSM as a
> module by default and let the user decide if they _really_ want to build
> it in.
> 
> Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure
> it doesn't get missed when we need it for a6xx tarets.
> 
> Signed-off-by: Jordan Crouse 
> ---
> 
>  drivers/gpu/drm/msm/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index 9c37e4d..3b2334b 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -14,11 +14,12 @@ config DRM_MSM
>   select SHMEM
>   select TMPFS
>   select QCOM_SCM if ARCH_QCOM
> + select QCOM_COMMAND_DB if ARCH_QCOM
>   select WANT_DEV_COREDUMP
>   select SND_SOC_HDMI_CODEC if SND_SOC
>   select SYNC_FILE
>   select PM_OPP
> - default y
> + default m

As a general comment the right thing would be to drop this default.
As it is now the Kconfig says that when DRM is selected then all of the
world would then also get DRM_MSM, which only a small part of this world
you see any benefit in.
So they now have to de-select MSM.

Kconfig has:
depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)

So maybe not all of the world but all QCOM or IMX5 users. Maybe they are all
interested in MSM. Otherwise the default should rather be dropped.
If there is any good hints then the help text could anyway use some
love, and then add the info there.

The other change with QCOM_COMMAND_DB seems on the other hand to make
sense but then this is another patch.

Sam


Re: [PATCH] drm: Fix kerneldoc warns in connector-related docs

2019-08-07 Thread Sam Ravnborg
Hi Sean.

On Wed, Aug 07, 2019 at 12:28:04PM -0400, Sean Paul wrote:
> From: Sean Paul 
> 
> Fixes the following warnings:
> ../drivers/gpu/drm/drm_connector.c:989: WARNING: Unexpected indentation.
> ../drivers/gpu/drm/drm_connector.c:993: WARNING: Unexpected indentation.
> ../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or 
> phrase reference start-string without end-string.
> ../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or 
> phrase reference start-string without end-string.

Thanks, 4 less warnings..
> 
> Fixes: 1b27fbdde1df ("drm: Add 
> drm_atomic_get_(old|new)_connector_for_encoder() helpers")
> Fixes: bb5a45d40d50 ("drm/hdcp: update content protection property with 
> uevent")
> Cc: Ramalingam C 
> Cc: Daniel Vetter 
> Cc: Pekka Paalanen 
> Cc: Sam Ravnborg 
> Cc: Laurent Pinchart 
> Cc: Jani Nikula 
> Cc: Sean Paul 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/drm_connector.c | 10 ++
>  include/drm/drm_connector.h |  4 ++--
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 354798bad576..4c766624b20d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -986,12 +986,14 @@ static const struct drm_prop_enum_list 
> hdmi_colorspaces[] = {
>   *   - Kernel sends uevent with the connector id and property id through
>   * @drm_hdcp_update_content_protection, upon below kernel triggered
>   * scenarios:
> - *   DESIRED -> ENABLED  (authentication success)
> - *   ENABLED -> DESIRED  (termination of authentication)
> + *
> + *   - DESIRED -> ENABLED (authentication success)
> + *   - ENABLED -> DESIRED (termination of authentication)
>   *   - Please note no uevents for userspace triggered property state changes,
>   * which can't fail such as
> - *   DESIRED/ENABLED -> UNDESIRED
> - *   UNDESIRED -> DESIRED
> + *
> + *   - DESIRED/ENABLED -> UNDESIRED
> + *   - UNDESIRED -> DESIRED
>   *   - Userspace is responsible for polling the property or listen to uevents
>   * to determine when the value transitions from ENABLED to DESIRED.
>   * This signifies the link is no longer protected and userspace should
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 0b9997e27689..e391f9c05f98 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -543,8 +543,8 @@ struct drm_connector_state {
>*
>* This is also used in the atomic helpers to map encoders to their
>* current and previous connectors, see
> -  * _atomic_get_old_connector_for_encoder() and
> -  * _atomic_get_new_connector_for_encoder().
> +  * _atomic_get_old_connector_for_encoder and
> +  * _atomic_get_new_connector_for_encoder.
Please fix this to use () for the functions and drop the "&".
This is what we use in drm kernel-doc for functions.
See for example function references in doc of writeback_job in the same file.

With this fixed:
Reviewed-by: Sam Ravnborg 

>*
>* NOTE: Atomic drivers must fill this out (either themselves or through
>* helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: add gem ttm helpers

2019-08-06 Thread Sam Ravnborg
Hi Gerd.

On Tue, Aug 06, 2019 at 03:34:52PM +0200, Gerd Hoffmann wrote:
> Now with ttm_buffer_object being a subclass of drm_gem_object we can
> easily lookup ttm_buffer_object for a given drm_gem_object, which in
> turm allows to create common helper functions.  This patch starts off
> with dump mmap helpers.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  include/drm/drm_gem_ttm_helper.h | 27 +++
>  drivers/gpu/drm/drm_gem_ttm_helper.c | 52 
>  drivers/gpu/drm/Kconfig  |  7 
>  drivers/gpu/drm/Makefile |  3 ++
>  4 files changed, 89 insertions(+)
>  create mode 100644 include/drm/drm_gem_ttm_helper.h
>  create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c

You have made nice kernel-doc comments for the struct and the public
functions.
Could we plug this into the existing doc too so one can browse around.
This will also allow you to check for any syntax errors using 
"make htmldocs"
(I spotted  one gbo versus bo mismatch)

A small DOC section in the top of drm_gem_ttm_helper.c
would also be nice. Just a few lines that introduce the purpose of this
file would cut it.

Sam


> diff --git a/include/drm/drm_gem_ttm_helper.h 
> b/include/drm/drm_gem_ttm_helper.h
> new file mode 100644
> index ..2c6874190b17
> --- /dev/null
> +++ b/include/drm/drm_gem_ttm_helper.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef DRM_GEM_TTM_HELPER_H
> +#define DRM_GEM_TTM_HELPER_H
> +
> +#include 
> +#include 
> +#include  /* for container_of() */
> +
> +/**
> + * Returns the container of type  ttm_buffer_object
> + * for field base.
> + * @gem: the GEM object
> + * Returns:  The containing GEM VRAM object
> + */
> +static inline struct ttm_buffer_object *drm_gem_ttm_of_gem(
> + struct drm_gem_object *gem)
> +{
> + return container_of(gem, struct ttm_buffer_object, base);
> +}
> +
> +u64 drm_gem_ttm_mmap_offset(struct ttm_buffer_object *bo);
> +int drm_gem_ttm_driver_dumb_mmap_offset(struct drm_file *file,
> + struct drm_device *dev,
> + uint32_t handle, uint64_t *offset);
> +
> +#endif
> diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c 
> b/drivers/gpu/drm/drm_gem_ttm_helper.c
> new file mode 100644
> index ..b069bd0c4c94
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include 
> +
> +/**
> + * drm_gem_ttm_mmap_offset() - Returns a GEM ttm object's mmap offset
> + * @gbo: the GEM ttm object
> + *
> + * See drm_vma_node_offset_addr() for more information.
> + *
> + * Returns:
> + * The buffer object's offset for userspace mappings on success, or
> + * 0 if no offset is allocated.
> + */
> +u64 drm_gem_ttm_mmap_offset(struct ttm_buffer_object *bo)
> +{
> + return drm_vma_node_offset_addr(>base.vma_node);
> +}
> +EXPORT_SYMBOL(drm_gem_ttm_mmap_offset);
> +
> +/**
> + * drm_gem_ttm_driver_dumb_mmap_offset() - \
> + Implements  drm_driver.dumb_mmap_offset
> + * @file:DRM file pointer.
> + * @dev: DRM device.
> + * @handle:  GEM handle
> + * @offset:  Returns the mapping's memory offset on success
> + *
> + * Returns:
> + * 0 on success, or
> + * a negative errno code otherwise.
> + */
> +int drm_gem_ttm_driver_dumb_mmap_offset(struct drm_file *file,
> +  struct drm_device *dev,
> +  uint32_t handle, uint64_t *offset)
> +{
> + struct drm_gem_object *gem;
> + struct ttm_buffer_object *bo;
> +
> + gem = drm_gem_object_lookup(file, handle);
> + if (!gem)
> + return -ENOENT;
> +
> + bo = drm_gem_ttm_of_gem(gem);
> + *offset = drm_gem_ttm_mmap_offset(bo);
> +
> + drm_gem_object_put_unlocked(gem);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_gem_ttm_driver_dumb_mmap_offset);
> +
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index e6f40fb54c9a..f7b25519f95c 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -172,6 +172,13 @@ config DRM_VRAM_HELPER
>   help
> Helpers for VRAM memory management
>  
> +config DRM_TTM_HELPER
> + tristate
> + depends on DRM
> + select DRM_TTM
> + help
> +   Helpers for ttm-based gem objects
> +
>  config DRM_GEM_CMA_HELPER
>   bool
>   depends on DRM
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 10f8329a8b71..545c61d6528b 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -37,6 +37,9 @@ drm_vram_helper-y := drm_gem_vram_helper.o \
>drm_vram_mm_helper.o
>  obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
>  
> +drm_ttm_helper-y := drm_gem_ttm_helper.o
> +obj-$(CONFIG_DRM_TTM_HELPER) += drm_ttm_helper.o
> +
>  drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o 
> drm_probe_helper.o \
>   

Re: [PATCH 1/3] drm: add gem ttm helpers

2019-08-06 Thread Sam Ravnborg
Hi Gerd.

On Tue, Aug 06, 2019 at 03:34:52PM +0200, Gerd Hoffmann wrote:
> Now with ttm_buffer_object being a subclass of drm_gem_object we can
> easily lookup ttm_buffer_object for a given drm_gem_object, which in
> turm allows to create common helper functions.  This patch starts off
> with dump mmap helpers.
> 
> Signed-off-by: Gerd Hoffmann 

One nit below.

> ---
>  include/drm/drm_gem_ttm_helper.h | 27 +++
>  drivers/gpu/drm/drm_gem_ttm_helper.c | 52 
>  drivers/gpu/drm/Kconfig  |  7 
>  drivers/gpu/drm/Makefile |  3 ++
>  4 files changed, 89 insertions(+)
>  create mode 100644 include/drm/drm_gem_ttm_helper.h
>  create mode 100644 drivers/gpu/drm/drm_gem_ttm_helper.c
> 
> diff --git a/include/drm/drm_gem_ttm_helper.h 
> b/include/drm/drm_gem_ttm_helper.h
> new file mode 100644
> index ..2c6874190b17
> --- /dev/null
> +++ b/include/drm/drm_gem_ttm_helper.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef DRM_GEM_TTM_HELPER_H
> +#define DRM_GEM_TTM_HELPER_H
> +
> +#include 
> +#include 
> +#include  /* for container_of() */

The typical order of include files is:

#include 

#include 

So space between each block of includes and sort within each block.

The comment "/* for container_of() */" is not really useful for
anyone.

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

Re: [PATCH v1 1/1] backlight: drop EARLY_EVENT_BLANK support

2019-08-05 Thread Sam Ravnborg
Hi Lee.

On Mon, Aug 05, 2019 at 11:36:00AM +0100, Lee Jones wrote:
> On Fri, 26 Jul 2019, Sam Ravnborg wrote:
> 
> > Hi Daniel.
> > 
> > On Fri, Jul 26, 2019 at 10:50:16AM +0100, Daniel Thompson wrote:
> > > On Thu, Jul 25, 2019 at 04:32:24PM +0200, Sam Ravnborg wrote:
> > > > There was no users left - so drop the code to support EARLY_FB_BLANK.
> > > 
> > > Why are we using a different noun for the subject and description?
> > I fat-fingered the description.
> 
> > Will fix when I apply - or send out a v2 if requested.
> 
> Okay, just apply it then:
> 
> Acked-by: Lee Jones 

Fixed changelog and applied to drm-misc-next.

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

Re: [PATCH 3/4 v2] drm/panel: simple: Support TI nspire panels

2019-08-05 Thread Sam Ravnborg
Hi Linus,

On Mon, Aug 05, 2019 at 10:58:46AM +0200, Linus Walleij wrote:
> This adds support for the TI nspire panels to the simple panel
> roster. This code is based on arch/arm/mach-nspire/clcd.c.
> This includes likely the first grayscale panel supported.
> 
> These panels will be used with the PL11x DRM driver.
> 
> Cc: Daniel Tang 
> Cc: Fabian Vogt 
> Tested-by: Fabian Vogt 
> Signed-off-by: Linus Walleij 

You can now add my:
Reviewed-by: Sam Ravnborg 

I assume this will be applied as one series and you will do it when
ready.

Sam

> ---
> ChangeLog v1->v2:
> - Bump clock frequency to 10 MHz after Fabian's trial-and-error
> - Changed vsymbol names to ti_nspire_*
> - Sorted alphabetically
> - Specify positive edge on the classic display bus
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 64 
>  1 file changed, 64 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c 
> b/drivers/gpu/drm/panel/panel-simple.c
> index 5a93c4edf1e4..96f894b44313 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2578,6 +2578,64 @@ static const struct panel_desc tianma_tm070rvhg71 = {
>   .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
>  };
>  
> +static const struct drm_display_mode ti_nspire_cx_lcd_mode[] = {
> + {
> + .clock = 1,
> + .hdisplay = 320,
> + .hsync_start = 320 + 50,
> + .hsync_end = 320 + 50 + 6,
> + .htotal = 320 + 50 + 6 + 38,
> + .vdisplay = 240,
> + .vsync_start = 240 + 3,
> + .vsync_end = 240 + 3 + 1,
> + .vtotal = 240 + 3 + 1 + 17,
> + .vrefresh = 60,
> + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> + },
> +};
> +
> +static const struct panel_desc ti_nspire_cx_lcd_panel = {
> + .modes = ti_nspire_cx_lcd_mode,
> + .num_modes = 1,
> + .bpc = 8,
> + .size = {
> + .width = 65,
> + .height = 49,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> + .bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE,
> +};
> +
> +static const struct drm_display_mode ti_nspire_classic_lcd_mode[] = {
> + {
> + .clock = 1,
> + .hdisplay = 320,
> + .hsync_start = 320 + 6,
> + .hsync_end = 320 + 6 + 6,
> + .htotal = 320 + 6 + 6 + 6,
> + .vdisplay = 240,
> + .vsync_start = 240 + 0,
> + .vsync_end = 240 + 0 + 1,
> + .vtotal = 240 + 0 + 1 + 0,
> + .vrefresh = 60,
> + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
> + },
> +};
> +
> +static const struct panel_desc ti_nspire_classic_lcd_panel = {
> + .modes = ti_nspire_classic_lcd_mode,
> + .num_modes = 1,
> + /* The grayscale panel has 8 bit for the color .. Y (black) */
> + .bpc = 8,
> + .size = {
> + .width = 71,
> + .height = 53,
> + },
> + /* This is the grayscale bus format */
> + .bus_format = MEDIA_BUS_FMT_Y8_1X8,
> + .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +};
> +
>  static const struct drm_display_mode toshiba_lt089ac29000_mode = {
>   .clock = 79500,
>   .hdisplay = 1280,
> @@ -3029,6 +3087,12 @@ static const struct of_device_id platform_of_match[] = 
> {
>   }, {
>   .compatible = "tianma,tm070rvhg71",
>   .data = _tm070rvhg71,
> + }, {
> + .compatible = "ti,nspire-cx-lcd-panel",
> + .data = _nspire_cx_lcd_panel,
> + }, {
> + .compatible = "ti,nspire-classic-lcd-panel",
> + .data = _nspire_classic_lcd_panel,
>   }, {
>   .compatible = "toshiba,lt089ac29000",
>   .data = _lt089ac29000,
> -- 
> 2.21.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 14/16] drm/panel: call prepare/enable only once

2019-08-05 Thread Sam Ravnborg
Hi Laurent.

> 
> On Sun, Aug 04, 2019 at 10:16:35PM +0200, Sam Ravnborg wrote:
> > Many panel drivers duplicate logic to prevent prepare to be called
> > for a panel that is already prepared.
> > Likewise for enable.
> > 
> > Implement this logic in drm_panel so the individual drivers
> > no longer needs this.
> > A panel is considered prepared/enabled only if the prepare/enable call
> > succeeds.
> > For disable/unprepare it is unconditionally considered
> > disabled/unprepared.
> > 
> > This allows calls to prepare/enable again, even if there were
> > some issue disabling a regulator or similar during disable/unprepare.
> 
> Is this the right place to handle this ? Shouldn't the upper layers
> ensure than enable/disable and prepare/unprepare are correcty balanced,
> and not called multiple times ? Adding enabled and prepared state to
> drm_panel not only doesn't align well with atomic state handling, but
> also would hide issues in upper layers that should really be fixed
> there.

The main rationale behind starting on this was that ~15 panel drivers
already implements logic to prevent the prepare/enable/disable/unprepare
functions to be called out of order.
$ cd drivers/gpu/drm/panel/; git grep enabled | grep bool | wc -l

Several of the panel drivers also implements a
mipi_dsi_driver.shutdown() or platform_driver.shutdown().
To the best of my knowledge we cannot guarantee that the upper layers
have done the proper disable()/unprepare() dance before a shutdown.
So the flags exists to allow the driver to unconditionally call
disable() / unprepare() in the shutdown methods.
Same goes for *_driver.remove()

One improvement could be to detect if the panel is prepare() when
upper layers call enable() and warn/error in this situation.
With the current implementation this is not checked at all.
Likewise for unprepare() (require it was never enabled or disable() was
caled first)

I claim the check exists for the benefit of .remove and .shutdown,
so we could also check if prepare() or enable() is called twice.

Adding logic to call prepare() automagically would hide probems in upper
layers and this was only briefly considered - and discarded as hiding
bugs.

So to sum up:
- Moving the checks from drivers to the core is a good thing
- The core shall check that a panel is prepared when enable is called
  and error out if not (or warn).
- The core shall check that a panel is disabled when unprepare is called
  and error out if not (or warn).
  The core shall check if prepare() and enable() is called out of order.

The patch needs to be extended to cover the last three points.

Laurent / Emil / Thierry - agree/comments?

Note: Did a quick round to see if could spot any wrong use of
drm_panel_* functions.
Most looked good, but then I did not do a throughly check.

bridge/analogix/analogix_dp_core.c looks fishy.
Looks like analogix_dp_prepare_panel() is a nop the way it is called.
I did not look too much on this, maybe I am wrong.

Sam

> 
> > Signed-off-by: Sam Ravnborg 
> > Cc: Maarten Lankhorst 
> > Cc: Maxime Ripard 
> > Cc: Sean Paul 
> > Cc: Thierry Reding 
> > Cc: Sam Ravnborg 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_panel.c | 66 ++---
> >  include/drm/drm_panel.h | 21 
> >  2 files changed, 75 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> > index da19d5b4a2f4..0853764040de 100644
> > --- a/drivers/gpu/drm/drm_panel.c
> > +++ b/drivers/gpu/drm/drm_panel.c
> > @@ -66,10 +66,21 @@ EXPORT_SYMBOL(drm_panel_init);
> >   */
> >  int drm_panel_prepare(struct drm_panel *panel)
> >  {
> > -   if (panel && panel->funcs && panel->funcs->prepare)
> > -   return panel->funcs->prepare(panel);
> > +   int ret = -ENOSYS;
> >  
> > -   return panel ? -ENOSYS : -EINVAL;
> > +   if (!panel)
> > +   return -EINVAL;
> > +
> > +   if (panel->prepared)
> > +   return 0;
> > +
> > +   if (panel->funcs && panel->funcs->prepare)
> > +   ret = panel->funcs->prepare(panel);
> > +
> > +   if (ret >= 0)
> > +   panel->prepared = true;
> > +
> > +   return ret;
> >  }
> >  EXPORT_SYMBOL(drm_panel_prepare);
> >  
> > @@ -85,10 +96,21 @@ EXPORT_SYMBOL(drm_panel_prepare);
> >   */
> >  int drm_panel_enable(struct drm_panel *panel)
> >  {
> > -   if (panel && panel->funcs && panel->funcs->enable)
> > -   return panel->funcs->enable(panel);
>

Re: [PATCH v1 05/16] drm/fsl-dcu: fix opencoded use of drm_panel_*

2019-08-05 Thread Sam Ravnborg
Hi Stefan.

Thanks for the feedback.

On Mon, Aug 05, 2019 at 11:16:26AM +0200, Stefan Agner wrote:
> On 2019-08-04 22:16, Sam Ravnborg wrote:
> > Use drm_panel_get_modes() to access modes.
> > This has a nice side effect to simplify the code.
> > 
> > Signed-off-by: Sam Ravnborg 
> > Cc: Stefan Agner 
> > Cc: Alison Wang 
> > ---
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 10 +-
> >  1 file changed, 1 insertion(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > index 279d83eaffc0..a92fd6c70b09 100644
> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> > @@ -65,17 +65,9 @@ static const struct drm_connector_funcs
> > fsl_dcu_drm_connector_funcs = {
> >  static int fsl_dcu_drm_connector_get_modes(struct drm_connector *connector)
> >  {
> > struct fsl_dcu_drm_connector *fsl_connector;
> > -   int (*get_modes)(struct drm_panel *panel);
> > -   int num_modes = 0;
> >  
> > fsl_connector = to_fsl_dcu_connector(connector);
> > -   if (fsl_connector->panel && fsl_connector->panel->funcs &&
> > -   fsl_connector->panel->funcs->get_modes) {
> > -   get_modes = fsl_connector->panel->funcs->get_modes;
> > -   num_modes = get_modes(fsl_connector->panel);
> > -   }
> > -
> > -   return num_modes;
> > +   return drm_panel_get_modes(fsl_connector->panel);
> 
> Oh, that old code looks rather messy. Thanks for the simplification!
> 
> This behaves slightly different since it now returns -EINVAL or -ENOSYS,
> but that is what we want.

You are right, and I will add this to the changelog when I apply.

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

Re: [PATCH v1 01/16] drm/bridge: tc358767: fix opencoded use of drm_panel_*

2019-08-05 Thread Sam Ravnborg
Hi Philipp.

On Mon, Aug 05, 2019 at 11:35:56AM +0200, Philipp Zabel wrote:
> On Sun, 2019-08-04 at 22:16 +0200, Sam Ravnborg wrote:
> > Replace open coded version with call to drm_panel_get_modes().
> > 
> > Signed-off-by: Sam Ravnborg 
> > Cc: Andrzej Hajda 
> > Cc: Neil Armstrong 
> > Cc: Laurent Pinchart 
> > Cc: Jonas Karlman 
> > Cc: Jernej Skrabec 
> > ---
> >  drivers/gpu/drm/bridge/tc358767.c | 10 --
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/tc358767.c 
> > b/drivers/gpu/drm/bridge/tc358767.c
> > index 42f03a985ac0..cebc8e620820 100644
> > --- a/drivers/gpu/drm/bridge/tc358767.c
> > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > @@ -1312,7 +1312,7 @@ static int tc_connector_get_modes(struct 
> > drm_connector *connector)
> >  {
> > struct tc_data *tc = connector_to_tc(connector);
> > struct edid *edid;
> > -   unsigned int count;
> > +   int count;
> 
> This looks like it also fixes a potential bug ...

get_modes() return either 0 or number of modes.
The negative return values come into play in drm_panel_get_modes() when
panel for example s NULL.

I will add this to changelog before I apply to avoid any
misunderstanding.

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

[PATCH v1 13/16] drm/panel: drop return code from drm_panel_detach()

2019-08-04 Thread Sam Ravnborg
There are no errors that can be reported by this function,
so drop the return code.
Fix the only bridge driver that checked the return result.

Signed-off-by: Sam Ravnborg 
Cc: Thierry Reding 
Cc: Sam Ravnborg 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Laurent Pinchart 
Cc: Andrzej Hajda 
Cc: Gwan-gyeong Mun 
Cc: Thomas Gleixner 
Cc: Linus Walleij 
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 3 +--
 drivers/gpu/drm/drm_panel.c| 6 +-
 include/drm/drm_panel.h| 2 +-
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index f2f7f69d6cc3..22885dceaa17 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1780,8 +1780,7 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
if (dp->plat_data->panel) {
if (drm_panel_unprepare(dp->plat_data->panel))
DRM_ERROR("failed to turnoff the panel\n");
-   if (drm_panel_detach(dp->plat_data->panel))
-   DRM_ERROR("failed to detach the panel\n");
+   drm_panel_detach(dp->plat_data->panel);
}
 
drm_dp_aux_unregister(>aux);
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 9946b8d9bacc..da19d5b4a2f4 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -219,15 +219,11 @@ EXPORT_SYMBOL(drm_panel_attach);
  *
  * This function should not be called by the panel device itself. It
  * is only for the drm device that called drm_panel_attach().
- *
- * Return: 0 on success or a negative error code on failure.
  */
-int drm_panel_detach(struct drm_panel *panel)
+void drm_panel_detach(struct drm_panel *panel)
 {
panel->connector = NULL;
panel->drm = NULL;
-
-   return 0;
 }
 EXPORT_SYMBOL(drm_panel_detach);
 
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 5e62deea49ba..624bd15ecfab 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -153,7 +153,7 @@ int drm_panel_add(struct drm_panel *panel);
 void drm_panel_remove(struct drm_panel *panel);
 
 int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector);
-int drm_panel_detach(struct drm_panel *panel);
+void drm_panel_detach(struct drm_panel *panel);
 
 int drm_panel_prepare(struct drm_panel *panel);
 int drm_panel_unprepare(struct drm_panel *panel);
-- 
2.20.1

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

[PATCH v1 11/16] drm/panel: move drm_panel functions to .c file

2019-08-04 Thread Sam Ravnborg
Move inline functions from include/drm/drm_panel.h to drm_panel.c.
This is in preparation for follow-up patches that will add extra
logic to the functions.
As they are no longer static inline, EXPORT them.

Signed-off-by: Sam Ravnborg 
Cc: Thierry Reding 
Cc: Sam Ravnborg 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/drm_panel.c | 96 +++
 include/drm/drm_panel.h | 99 +++--
 2 files changed, 104 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index dbd5b873e8f2..9946b8d9bacc 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -54,6 +54,102 @@ void drm_panel_init(struct drm_panel *panel)
 }
 EXPORT_SYMBOL(drm_panel_init);
 
+/**
+ * drm_panel_prepare - power on a panel
+ * @panel: DRM panel
+ *
+ * Calling this function will enable power and deassert any reset signals to
+ * the panel. After this has completed it is possible to communicate with any
+ * integrated circuitry via a command bus.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_panel_prepare(struct drm_panel *panel)
+{
+   if (panel && panel->funcs && panel->funcs->prepare)
+   return panel->funcs->prepare(panel);
+
+   return panel ? -ENOSYS : -EINVAL;
+}
+EXPORT_SYMBOL(drm_panel_prepare);
+
+/**
+ * drm_panel_enable - enable a panel
+ * @panel: DRM panel
+ *
+ * Calling this function will cause the panel display drivers to be turned on
+ * and the backlight to be enabled. Content will be visible on screen after
+ * this call completes.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_panel_enable(struct drm_panel *panel)
+{
+   if (panel && panel->funcs && panel->funcs->enable)
+   return panel->funcs->enable(panel);
+
+   return panel ? -ENOSYS : -EINVAL;
+}
+EXPORT_SYMBOL(drm_panel_enable);
+
+/**
+ * drm_panel_disable - disable a panel
+ * @panel: DRM panel
+ *
+ * This will typically turn off the panel's backlight or disable the display
+ * drivers. For smart panels it should still be possible to communicate with
+ * the integrated circuitry via any command bus after this call.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_panel_disable(struct drm_panel *panel)
+{
+   if (panel && panel->funcs && panel->funcs->disable)
+   return panel->funcs->disable(panel);
+
+   return panel ? -ENOSYS : -EINVAL;
+}
+EXPORT_SYMBOL(drm_panel_disable);
+
+/**
+ * drm_panel_unprepare - power off a panel
+ * @panel: DRM panel
+ *
+ * Calling this function will completely power off a panel (assert the panel's
+ * reset, turn off power supplies, ...). After this function has completed, it
+ * is usually no longer possible to communicate with the panel until another
+ * call to drm_panel_prepare().
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_panel_unprepare(struct drm_panel *panel)
+{
+   if (panel && panel->funcs && panel->funcs->unprepare)
+   return panel->funcs->unprepare(panel);
+
+   return panel ? -ENOSYS : -EINVAL;
+}
+EXPORT_SYMBOL(drm_panel_unprepare);
+
+/**
+ * drm_panel_get_modes - probe the available display modes of a panel
+ * @panel: DRM panel
+ *
+ * The modes probed from the panel are automatically added to the connector
+ * that the panel is attached to.
+ *
+ * Return: The number of modes available from the panel on success or a
+ * negative error code on failure.
+ */
+int drm_panel_get_modes(struct drm_panel *panel)
+{
+   if (panel && panel->funcs && panel->funcs->get_modes)
+   return panel->funcs->get_modes(panel);
+
+   return panel ? -ENOSYS : -EINVAL;
+}
+EXPORT_SYMBOL(drm_panel_get_modes);
+
 /**
  * drm_panel_add - add a panel to the global registry
  * @panel: panel to add
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 26377836141c..053d611656b9 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -97,97 +97,6 @@ struct drm_panel {
struct list_head list;
 };
 
-/**
- * drm_disable_unprepare - power off a panel
- * @panel: DRM panel
- *
- * Calling this function will completely power off a panel (assert the panel's
- * reset, turn off power supplies, ...). After this function has completed, it
- * is usually no longer possible to communicate with the panel until another
- * call to drm_panel_prepare().
- *
- * Return: 0 on success or a negative error code on failure.
- */
-static inline int drm_panel_unprepare(struct drm_panel *panel)
-{
-   if (panel && panel->funcs && panel->funcs->unprepare)
-   return panel->funcs->unprepare(panel);

[PATCH v1 01/16] drm/bridge: tc358767: fix opencoded use of drm_panel_*

2019-08-04 Thread Sam Ravnborg
Replace open coded version with call to drm_panel_get_modes().

Signed-off-by: Sam Ravnborg 
Cc: Andrzej Hajda 
Cc: Neil Armstrong 
Cc: Laurent Pinchart 
Cc: Jonas Karlman 
Cc: Jernej Skrabec 
---
 drivers/gpu/drm/bridge/tc358767.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index 42f03a985ac0..cebc8e620820 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1312,7 +1312,7 @@ static int tc_connector_get_modes(struct drm_connector 
*connector)
 {
struct tc_data *tc = connector_to_tc(connector);
struct edid *edid;
-   unsigned int count;
+   int count;
int ret;
 
ret = tc_get_display_props(tc);
@@ -1321,11 +1321,9 @@ static int tc_connector_get_modes(struct drm_connector 
*connector)
return 0;
}
 
-   if (tc->panel && tc->panel->funcs && tc->panel->funcs->get_modes) {
-   count = tc->panel->funcs->get_modes(tc->panel);
-   if (count > 0)
-   return count;
-   }
+   count = drm_panel_get_modes(tc->panel);
+   if (count > 0)
+   return count;
 
edid = drm_get_edid(connector, >aux.ddc);
 
-- 
2.20.1

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

[PATCH v1 15/16] drm/panel: add backlight support

2019-08-04 Thread Sam Ravnborg
Panels often supports backlight as specified in a device tree.
Update the drm_panel infrastructure to support this to
simplify the drivers.

With this the panel driver just needs to add the following to the
probe() function:

err = drm_panel_of_backlight(panel);
if (err)
return err;

Then drm_panel will handle all the rest.

Signed-off-by: Sam Ravnborg 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: Thierry Reding 
Cc: Sam Ravnborg 
Cc: David Airlie 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/drm_panel.c | 41 +
 include/drm/drm_panel.h | 23 +
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 0853764040de..d8139674b883 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -21,6 +21,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 
@@ -110,6 +111,7 @@ int drm_panel_enable(struct drm_panel *panel)
if (ret >= 0)
panel->enabled = true;
 
+   backlight_enable(panel->backlight);
return ret;
 }
 EXPORT_SYMBOL(drm_panel_enable);
@@ -134,6 +136,8 @@ int drm_panel_disable(struct drm_panel *panel)
if (!panel->enabled)
return 0;
 
+   backlight_disable(panel->backlight);
+
if (panel->funcs && panel->funcs->disable)
ret = panel->funcs->disable(panel);
 
@@ -308,6 +312,43 @@ struct drm_panel *of_drm_find_panel(const struct 
device_node *np)
 EXPORT_SYMBOL(of_drm_find_panel);
 #endif
 
+#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
+/**
+ * drm_panel_of_backlight - use backlight device node for backlight
+ * @panel: DRM panel
+ *
+ * Use this function to enable backlight handling if your panel
+ * uses device tree and has a backlight handle.
+ *
+ * When panel is enabled backlight will be enabled after a
+ * successfull call to _panel_funcs.enable()
+ *
+ * When panel is disabled backlight will be disabled before the
+ * call to _panel_funcs.disable().
+ *
+ * A typical implementation for a panel driver supporting device tree
+ * will call this function and then backlight just works.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_panel_of_backlight(struct drm_panel *panel)
+{
+   struct backlight_device *backlight;
+
+   if (!panel || !panel->dev)
+   return -EINVAL;
+
+   backlight = devm_of_find_backlight(panel->dev);
+
+   if (IS_ERR(backlight))
+return PTR_ERR(backlight);
+
+   panel->backlight = backlight;
+   return 0;
+}
+EXPORT_SYMBOL(drm_panel_of_backlight);
+#endif
+
 MODULE_AUTHOR("Thierry Reding ");
 MODULE_DESCRIPTION("DRM panel infrastructure");
 MODULE_LICENSE("GPL and additional rights");
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 7493500fc9bd..31349c2393b7 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -28,6 +28,7 @@
 #include 
 #include 
 
+struct backlight_device;
 struct device_node;
 struct drm_connector;
 struct drm_device;
@@ -59,6 +60,10 @@ struct display_timing;
  *
  * To save power when no video data is transmitted, a driver can power down
  * the panel. This is the job of the .unprepare() function.
+ *
+ * Backlight can be handled automatically if configured using
+ * drm_panel_of_backlight(). Then the driver do not need to implement the
+ * functionality to enable/disable backlight.
  */
 struct drm_panel_funcs {
/**
@@ -139,6 +144,15 @@ struct drm_panel {
 */
struct device *dev;
 
+   /**
+* @backlight:
+*
+* Backlight device, used to turn on backlight after
+* the call to enable(), and to turn off
+* backlight before call to disable().
+*/
+   struct backlight_device *backlight;
+
/**
 * @funcs:
 *
@@ -193,4 +207,13 @@ static inline struct drm_panel *of_drm_find_panel(const 
struct device_node *np)
 }
 #endif
 
+#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) && defined(CONFIG_DRM_PANEL)
+int drm_panel_of_backlight(struct drm_panel *panel);
+#else
+static inline int drm_panel_of_backlight(struct drm_panel *panel)
+{
+   return -EINVAL;
+}
+#endif
+
 #endif
-- 
2.20.1

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

[PATCH v1 12/16] drm/panel: use inline comments in drm_panel.h

2019-08-04 Thread Sam Ravnborg
Inline comments provide better space for additional comments.
Comments was slightly edited to follow the normal style,
but no change to actual content.
Used the opportuniy to change the order in drm_panel_funcs
to follow the order they will be used by a panel.

Signed-off-by: Sam Ravnborg 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: Thierry Reding 
Cc: Sam Ravnborg 
Cc: David Airlie 
Cc: Daniel Vetter 
---
 include/drm/drm_panel.h | 82 +
 1 file changed, 66 insertions(+), 16 deletions(-)

diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 053d611656b9..5e62deea49ba 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -36,14 +36,6 @@ struct display_timing;
 
 /**
  * struct drm_panel_funcs - perform operations on a given panel
- * @disable: disable panel (turn off back light, etc.)
- * @unprepare: turn off panel
- * @prepare: turn on panel and perform set up
- * @enable: enable panel (turn on back light, etc.)
- * @get_modes: add modes to the connector that the panel is attached to and
- * return the number of modes added
- * @get_timings: copy display timings into the provided array and return
- * the number of display timings available
  *
  * The .prepare() function is typically called before the display controller
  * starts to transmit video data. Panel drivers can use this to turn the panel
@@ -69,31 +61,89 @@ struct display_timing;
  * the panel. This is the job of the .unprepare() function.
  */
 struct drm_panel_funcs {
-   int (*disable)(struct drm_panel *panel);
-   int (*unprepare)(struct drm_panel *panel);
+   /**
+* @prepare:
+*
+* Turn on panel and perform set up.
+*/
int (*prepare)(struct drm_panel *panel);
+
+   /**
+* @enable:
+*
+* Enable panel (turn on back light, etc.).
+*/
int (*enable)(struct drm_panel *panel);
+
+   /**
+* @disable:
+*
+* Disable panel (turn off back light, etc.).
+*/
+   int (*disable)(struct drm_panel *panel);
+
+   /**
+* @unprepare:
+*
+* Turn off panel.
+*/
+   int (*unprepare)(struct drm_panel *panel);
+
+   /**
+* @get_modes:
+*
+* Add modes to the connector that the panel is attached to and
+* return the number of modes added.
+*/
int (*get_modes)(struct drm_panel *panel);
+
+   /**
+* @get_timings:
+*
+* Copy display timings into the provided array and return
+* the number of display timings available.
+*/
int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
   struct display_timing *timings);
 };
 
 /**
  * struct drm_panel - DRM panel object
- * @drm: DRM device owning the panel
- * @connector: DRM connector that the panel is attached to
- * @dev: parent device of the panel
- * @link: link from panel device (supplier) to DRM device (consumer)
- * @funcs: operations that can be performed on the panel
- * @list: panel entry in registry
  */
 struct drm_panel {
+   /**
+* @drm:
+*
+* DRM device owning the panel.
+*/
struct drm_device *drm;
+
+   /**
+* @connector:
+*
+* DRM connector that the panel is attached to.
+*/
struct drm_connector *connector;
+
+   /**
+* @dev:
+*
+* Parent device of the panel.
+*/
struct device *dev;
 
+   /**
+* @funcs:
+*
+* Operations that can be performed on the panel.
+*/
const struct drm_panel_funcs *funcs;
 
+   /**
+* @list:
+*
+* Panel entry in registry.
+*/
struct list_head list;
 };
 
-- 
2.20.1

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

[PATCH v1 02/16] drm/exynos: fix opencoded use of drm_panel_*

2019-08-04 Thread Sam Ravnborg
drm_panel_attach() will check if there is a controller
already attached - drop the check in the driver.

Use drm_panel_get_modes() so the driver no longer uses the function
pointer.

Signed-off-by: Sam Ravnborg 
Cc: Inki Dae 
Cc: Joonyoung Shim 
Cc: Seung-Woo Kim 
Cc: Kyungmin Park 
Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
---
 drivers/gpu/drm/exynos/exynos_drm_dpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index 3cebb19ec1c4..5479ff71cbc6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -43,7 +43,7 @@ exynos_dpi_detect(struct drm_connector *connector, bool force)
 {
struct exynos_dpi *ctx = connector_to_dpi(connector);
 
-   if (ctx->panel && !ctx->panel->connector)
+   if (ctx->panel)
drm_panel_attach(ctx->panel, >connector);
 
return connector_status_connected;
@@ -85,7 +85,7 @@ static int exynos_dpi_get_modes(struct drm_connector 
*connector)
}
 
if (ctx->panel)
-   return ctx->panel->funcs->get_modes(ctx->panel);
+   return drm_panel_get_modes(ctx->panel);
 
return 0;
 }
-- 
2.20.1

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

[PATCH v1 14/16] drm/panel: call prepare/enable only once

2019-08-04 Thread Sam Ravnborg
Many panel drivers duplicate logic to prevent prepare to be called
for a panel that is already prepared.
Likewise for enable.

Implement this logic in drm_panel so the individual drivers
no longer needs this.
A panel is considered prepared/enabled only if the prepare/enable call
succeeds.
For disable/unprepare it is unconditionally considered
disabled/unprepared.

This allows calls to prepare/enable again, even if there were
some issue disabling a regulator or similar during disable/unprepare.

Signed-off-by: Sam Ravnborg 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: Thierry Reding 
Cc: Sam Ravnborg 
Cc: David Airlie 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/drm_panel.c | 66 ++---
 include/drm/drm_panel.h | 21 
 2 files changed, 75 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index da19d5b4a2f4..0853764040de 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -66,10 +66,21 @@ EXPORT_SYMBOL(drm_panel_init);
  */
 int drm_panel_prepare(struct drm_panel *panel)
 {
-   if (panel && panel->funcs && panel->funcs->prepare)
-   return panel->funcs->prepare(panel);
+   int ret = -ENOSYS;
 
-   return panel ? -ENOSYS : -EINVAL;
+   if (!panel)
+   return -EINVAL;
+
+   if (panel->prepared)
+   return 0;
+
+   if (panel->funcs && panel->funcs->prepare)
+   ret = panel->funcs->prepare(panel);
+
+   if (ret >= 0)
+   panel->prepared = true;
+
+   return ret;
 }
 EXPORT_SYMBOL(drm_panel_prepare);
 
@@ -85,10 +96,21 @@ EXPORT_SYMBOL(drm_panel_prepare);
  */
 int drm_panel_enable(struct drm_panel *panel)
 {
-   if (panel && panel->funcs && panel->funcs->enable)
-   return panel->funcs->enable(panel);
+   int ret = -ENOSYS;
 
-   return panel ? -ENOSYS : -EINVAL;
+   if (!panel)
+   return -EINVAL;
+
+   if (panel->enabled)
+   return 0;
+
+   if (panel->funcs && panel->funcs->enable)
+   ret = panel->funcs->enable(panel);
+
+   if (ret >= 0)
+   panel->enabled = true;
+
+   return ret;
 }
 EXPORT_SYMBOL(drm_panel_enable);
 
@@ -104,10 +126,20 @@ EXPORT_SYMBOL(drm_panel_enable);
  */
 int drm_panel_disable(struct drm_panel *panel)
 {
-   if (panel && panel->funcs && panel->funcs->disable)
-   return panel->funcs->disable(panel);
+   int ret = -ENOSYS;
 
-   return panel ? -ENOSYS : -EINVAL;
+   if (!panel)
+   return -EINVAL;
+
+   if (!panel->enabled)
+   return 0;
+
+   if (panel->funcs && panel->funcs->disable)
+   ret = panel->funcs->disable(panel);
+
+   panel->enabled = false;
+
+   return ret;
 }
 EXPORT_SYMBOL(drm_panel_disable);
 
@@ -124,10 +156,20 @@ EXPORT_SYMBOL(drm_panel_disable);
  */
 int drm_panel_unprepare(struct drm_panel *panel)
 {
-   if (panel && panel->funcs && panel->funcs->unprepare)
-   return panel->funcs->unprepare(panel);
+   int ret = -ENOSYS;
 
-   return panel ? -ENOSYS : -EINVAL;
+   if (!panel)
+   return -EINVAL;
+
+   if (!panel->prepared)
+   return 0;
+
+   if (panel->funcs && panel->funcs->unprepare)
+   ret = panel->funcs->unprepare(panel);
+
+   panel->prepared = false;
+
+   return ret;
 }
 EXPORT_SYMBOL(drm_panel_unprepare);
 
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 624bd15ecfab..7493500fc9bd 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -65,6 +65,9 @@ struct drm_panel_funcs {
 * @prepare:
 *
 * Turn on panel and perform set up.
+* When the panel is successfully prepared the prepare() function
+* will not be called again until the panel has been unprepared.
+*
 */
int (*prepare)(struct drm_panel *panel);
 
@@ -72,6 +75,8 @@ struct drm_panel_funcs {
 * @enable:
 *
 * Enable panel (turn on back light, etc.).
+* When the panel is successfully enabled the enable() function
+* will not be called again until the panel has been disabled.
 */
int (*enable)(struct drm_panel *panel);
 
@@ -79,6 +84,7 @@ struct drm_panel_funcs {
 * @disable:
 *
 * Disable panel (turn off back light, etc.).
+* If the panel is already disabled the disable() function is not 
called.
 */
int (*disable)(struct drm_panel *panel);
 
@@ -86,6 +92,7 @@ struct drm_panel_funcs {
 * @unprepare:
 *
 * Turn off panel.
+* If the panel

[PATCH v1 09/16] drm/tegra: fix opencoded use of drm_panel_*

2019-08-04 Thread Sam Ravnborg
Use the drm_panel_get_modes function.

Signed-off-by: Sam Ravnborg 
Cc: Thierry Reding 
Cc: Jonathan Hunter 
Cc: linux-te...@vger.kernel.org
---
 drivers/gpu/drm/tegra/output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 274cb955e2e1..52b8396ec2dc 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -23,7 +23,7 @@ int tegra_output_connector_get_modes(struct drm_connector 
*connector)
 * ignore any other means of obtaining a mode.
 */
if (output->panel) {
-   err = output->panel->funcs->get_modes(output->panel);
+   err = drm_panel_get_modes(output->panel);
if (err > 0)
return err;
}
-- 
2.20.1

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

[PATCH v1 03/16] drm/exynos: fix opencoded use of drm_panel_*

2019-08-04 Thread Sam Ravnborg
Call via drm_panel_get_modes().

Signed-off-by: Sam Ravnborg 
Cc: Inki Dae 
Cc: Joonyoung Shim 
Cc: Seung-Woo Kim 
Cc: Kyungmin Park 
Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 6926cee91b36..36b02b456d9c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1460,7 +1460,7 @@ static int exynos_dsi_get_modes(struct drm_connector 
*connector)
struct exynos_dsi *dsi = connector_to_dsi(connector);
 
if (dsi->panel)
-   return dsi->panel->funcs->get_modes(dsi->panel);
+   return drm_panel_get_modes(dsi->panel);
 
return 0;
 }
-- 
2.20.1

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

[PATCH v1 06/16] drm/msm: fix opencoded use of drm_panel_*

2019-08-04 Thread Sam Ravnborg
Use the function drm_panel_get_modes().

Signed-off-by: Sam Ravnborg 
Cc: Alexios Zavras 
Cc: Thomas Gleixner 
Cc: Allison Randal 
Cc: Sam Ravnborg 
Cc: Enrico Weigelt 
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
index ecef4f5b9f26..0e21252fd1d6 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
@@ -55,7 +55,7 @@ static int mdp4_lvds_connector_get_modes(struct drm_connector 
*connector)
if (panel) {
drm_panel_attach(panel, connector);
 
-   ret = panel->funcs->get_modes(panel);
+   ret = drm_panel_get_modes(panel);
 
drm_panel_detach(panel);
}
-- 
2.20.1

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

[PATCH v1 08/16] drm/sti: fix opencoded use of drm_panel_*

2019-08-04 Thread Sam Ravnborg
Use the drm_panel_(enable|disable|get_modes) functions.

Signed-off-by: Sam Ravnborg 
Cc: Benjamin Gaignard 
Cc: Vincent Abriou 
---
 drivers/gpu/drm/sti/sti_dvo.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
index 9e6d5d8b7030..e55870190bf5 100644
--- a/drivers/gpu/drm/sti/sti_dvo.c
+++ b/drivers/gpu/drm/sti/sti_dvo.c
@@ -221,8 +221,7 @@ static void sti_dvo_disable(struct drm_bridge *bridge)
 
writel(0x, dvo->regs + DVO_DOF_CFG);
 
-   if (dvo->panel)
-   dvo->panel->funcs->disable(dvo->panel);
+   drm_panel_disable(dvo->panel);
 
/* Disable/unprepare dvo clock */
clk_disable_unprepare(dvo->clk_pix);
@@ -262,8 +261,7 @@ static void sti_dvo_pre_enable(struct drm_bridge *bridge)
if (clk_prepare_enable(dvo->clk))
DRM_ERROR("Failed to prepare/enable dvo clk\n");
 
-   if (dvo->panel)
-   dvo->panel->funcs->enable(dvo->panel);
+   drm_panel_enable(dvo->panel);
 
/* Set LUT */
writel(config->lowbyte,  dvo->regs + DVO_LUT_PROG_LOW);
@@ -340,7 +338,7 @@ static int sti_dvo_connector_get_modes(struct drm_connector 
*connector)
struct sti_dvo *dvo = dvo_connector->dvo;
 
if (dvo->panel)
-   return dvo->panel->funcs->get_modes(dvo->panel);
+   return drm_panel_get_modes(dvo->panel);
 
return 0;
 }
-- 
2.20.1

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

[PATCH v1 10/16] drm/panel: ili9322: move bus_flags to get_modes()

2019-08-04 Thread Sam Ravnborg
To prepare the driver to receive drm_connector only in the get_modes()
callback, move bus_flags handling to ili9322_get_modes().

Signed-off-by: Sam Ravnborg 
Cc: Thierry Reding 
Cc: Sam Ravnborg 
---
 drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 34 +---
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c 
b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
index 53dd1e128795..3c58f63adbf7 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
@@ -349,7 +349,6 @@ static const struct regmap_config ili9322_regmap_config = {
 
 static int ili9322_init(struct drm_panel *panel, struct ili9322 *ili)
 {
-   struct drm_connector *connector = panel->connector;
u8 reg;
int ret;
int i;
@@ -407,23 +406,11 @@ static int ili9322_init(struct drm_panel *panel, struct 
ili9322 *ili)
 * Polarity and inverted color order for RGB input.
 * None of this applies in the BT.656 mode.
 */
-   if (ili->conf->dclk_active_high) {
+   reg = 0;
+   if (ili->conf->dclk_active_high)
reg = ILI9322_POL_DCLK;
-   connector->display_info.bus_flags |=
-   DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
-   } else {
-   reg = 0;
-   connector->display_info.bus_flags |=
-   DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
-   }
-   if (ili->conf->de_active_high) {
+   if (ili->conf->de_active_high)
reg |= ILI9322_POL_DE;
-   connector->display_info.bus_flags |=
-   DRM_BUS_FLAG_DE_HIGH;
-   } else {
-   connector->display_info.bus_flags |=
-   DRM_BUS_FLAG_DE_LOW;
-   }
if (ili->conf->hsync_active_high)
reg |= ILI9322_POL_HSYNC;
if (ili->conf->vsync_active_high)
@@ -659,9 +646,20 @@ static int ili9322_get_modes(struct drm_panel *panel)
struct drm_connector *connector = panel->connector;
struct ili9322 *ili = panel_to_ili9322(panel);
struct drm_display_mode *mode;
+   struct drm_display_info *info;
+
+   info = >display_info;
+   info->width_mm = ili->conf->width_mm;
+   info->height_mm = ili->conf->height_mm;
+   if (ili->conf->dclk_active_high)
+   info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
+   else
+   info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
 
-   connector->display_info.width_mm = ili->conf->width_mm;
-   connector->display_info.height_mm = ili->conf->height_mm;
+   if (ili->conf->de_active_high)
+   info->bus_flags |= DRM_BUS_FLAG_DE_HIGH;
+   else
+   info->bus_flags |= DRM_BUS_FLAG_DE_LOW;
 
switch (ili->input) {
case ILI9322_INPUT_SRGB_DUMMY_320X240:
-- 
2.20.1

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

[PATCH v1 16/16] drm/panel: simple: use drm_panel infrastructure

2019-08-04 Thread Sam Ravnborg
Use drm_panel infrastrucute:
- drm_panel has guards for calling disable/enable twice
- drm_panel has backlight support

To use the drm_panel infrastructure use the drm_panel_*
variants for prepare/enable/disable/unprepare.

Signed-off-by: Sam Ravnborg 
Cc: Thierry Reding 
Cc: Sam Ravnborg 
---
 drivers/gpu/drm/panel/panel-simple.c | 73 +---
 1 file changed, 11 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index bff7578f84dd..c7eed34f2c9c 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -21,7 +21,6 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -98,13 +97,10 @@ struct panel_desc {
 
 struct panel_simple {
struct drm_panel base;
-   bool prepared;
-   bool enabled;
bool no_hpd;
 
const struct panel_desc *desc;
 
-   struct backlight_device *backlight;
struct regulator *supply;
struct i2c_adapter *ddc;
 
@@ -232,20 +228,9 @@ static int panel_simple_disable(struct drm_panel *panel)
 {
struct panel_simple *p = to_panel_simple(panel);
 
-   if (!p->enabled)
-   return 0;
-
-   if (p->backlight) {
-   p->backlight->props.power = FB_BLANK_POWERDOWN;
-   p->backlight->props.state |= BL_CORE_FBBLANK;
-   backlight_update_status(p->backlight);
-   }
-
if (p->desc->delay.disable)
msleep(p->desc->delay.disable);
 
-   p->enabled = false;
-
return 0;
 }
 
@@ -253,9 +238,6 @@ static int panel_simple_unprepare(struct drm_panel *panel)
 {
struct panel_simple *p = to_panel_simple(panel);
 
-   if (!p->prepared)
-   return 0;
-
gpiod_set_value_cansleep(p->enable_gpio, 0);
 
regulator_disable(p->supply);
@@ -263,8 +245,6 @@ static int panel_simple_unprepare(struct drm_panel *panel)
if (p->desc->delay.unprepare)
msleep(p->desc->delay.unprepare);
 
-   p->prepared = false;
-
return 0;
 }
 
@@ -274,9 +254,6 @@ static int panel_simple_prepare(struct drm_panel *panel)
unsigned int delay;
int err;
 
-   if (p->prepared)
-   return 0;
-
err = regulator_enable(p->supply);
if (err < 0) {
dev_err(panel->dev, "failed to enable supply: %d\n", err);
@@ -291,8 +268,6 @@ static int panel_simple_prepare(struct drm_panel *panel)
if (delay)
msleep(delay);
 
-   p->prepared = true;
-
return 0;
 }
 
@@ -300,20 +275,9 @@ static int panel_simple_enable(struct drm_panel *panel)
 {
struct panel_simple *p = to_panel_simple(panel);
 
-   if (p->enabled)
-   return 0;
-
if (p->desc->delay.enable)
msleep(p->desc->delay.enable);
 
-   if (p->backlight) {
-   p->backlight->props.state &= ~BL_CORE_FBBLANK;
-   p->backlight->props.power = FB_BLANK_UNBLANK;
-   backlight_update_status(p->backlight);
-   }
-
-   p->enabled = true;
-
return 0;
 }
 
@@ -413,7 +377,7 @@ static void panel_simple_parse_panel_timing_node(struct 
device *dev,
 
 static int panel_simple_probe(struct device *dev, const struct panel_desc 
*desc)
 {
-   struct device_node *backlight, *ddc;
+   struct device_node *ddc;
struct panel_simple *panel;
struct display_timing dt;
int err;
@@ -422,8 +386,6 @@ static int panel_simple_probe(struct device *dev, const 
struct panel_desc *desc)
if (!panel)
return -ENOMEM;
 
-   panel->enabled = false;
-   panel->prepared = false;
panel->desc = desc;
 
panel->no_hpd = of_property_read_bool(dev->of_node, "no-hpd");
@@ -441,24 +403,13 @@ static int panel_simple_probe(struct device *dev, const 
struct panel_desc *desc)
return err;
}
 
-   backlight = of_parse_phandle(dev->of_node, "backlight", 0);
-   if (backlight) {
-   panel->backlight = of_find_backlight_by_node(backlight);
-   of_node_put(backlight);
-
-   if (!panel->backlight)
-   return -EPROBE_DEFER;
-   }
-
ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0);
if (ddc) {
panel->ddc = of_find_i2c_adapter_by_node(ddc);
of_node_put(ddc);
 
-   if (!panel->ddc) {
-   err = -EPROBE_DEFER;
-   goto free_backlight;
-   }
+   if (!panel->ddc)
+   return -EPROBE_DEFER;
}
 
if (!of_get_display_timing(dev->of_node, "panel-timing", ))
@@ -468,6 +419,10

  1   2   3   4   5   6   7   8   9   10   >