mbox series

[PATCHv2,00/56] drm/omap: Convert DSI code to use drm_mipi_dsi and drm_panel

Message ID 20200224232126.3385250-1-sebastian.reichel@collabora.com (mailing list archive)
Headers show
Series drm/omap: Convert DSI code to use drm_mipi_dsi and drm_panel | expand

Message

Sebastian Reichel Feb. 24, 2020, 11:20 p.m. UTC
This updates the existing omapdrm DSI code, so that it uses
common drm_mipi_dsi API and drm_panel.

The patchset has been tested with Droid 4 using Linux console, X.org and
Weston. The patchset is based on Laurent Pinchartl's patch series [0]
and removes the last custom panel driver, so quite a few cleanups on the
omapdrm codebase were possible.

[0] [PATCH v7 00/54] drm/omap: Replace custom display drivers with drm_bridge and drm_panel
    https://lore.kernel.org/dri-devel/20200222150106.22919-1-laurent.pinchart@ideasonboard.com/
    git://linuxtv.org/pinchartl/media.git omapdrm/bridge/devel

I pushed this patchset into the following branch:

git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-n900.git omapdrm/bridge/devel-with-dsi

The previous version of this patchset has been sent quite some time ago.
This version has been rebased and required cleaning up most of the
hacks. I do not have a detailed changelog, but quite a few things
changed. I decided against doing anything special for the DT change
(adding DSI channel number), since only 3 devices are affected. It is
quite likely, that all developers of those devices update DT together
with kernel for those devices. My suggestion is to merge the first two
patches ASAP and backport to stable, since it does not affect old
kernels and the change is rather small.

RFCv1: https://lore.kernel.org/dri-devel/20191117023946.VjCC3yE08DMx7JIKxNagPoT5et7WTnKGVV6MtOtB9Ro@z/

-- Sebastian

Sebastian Reichel (56):
  ARM: dts: omap: add channel to DSI panels
  ARM: dts: omap4-droid4: add panel compatible
  Revert "drm/omap: dss: Remove unused omap_dss_device operations"
  omap/drm: drop unused dsi.configure_pins
  drm/omap: dsi: use MIPI_DSI_FMT_* instead of OMAP_DSS_DSI_FMT_*
  drm/omap: constify write buffers
  drm/omap: dsi: add generic transfer function
  drm/omap: panel-dsi-cm: convert to transfer API
  drm/omap: dsi: unexport specific data transfer functions
  drm/omap: dsi: drop virtual channel logic
  drm/omap: dsi: simplify write function
  drm/omap: dsi: simplify read functions
  drm/omap: dsi: switch dsi_vc_send_long/short to mipi_dsi_msg
  drm/omap: dsi: introduce mipi_dsi_host
  drm/omap: panel-dsi-cm: use DSI helpers
  drm/omap: dsi: request VC via mipi_dsi_attach
  drm/omap: panel-dsi-cm: drop hardcoded VC
  drm/omap: panel-dsi-cm: use common MIPI DCS 1.3 defines
  drm/omap: dsi: drop unused memory_read()
  drm/omap: dsi: drop unused get_te()
  drm/omap: dsi: drop unused enable_te()
  drm/omap: dsi: drop useless sync()
  drm/omap: dsi: use pixel-format and mode from attach
  drm/omap: panel-dsi-cm: use bulk regulator API
  drm/omap: dsi: lp/hs switching support for transfer()
  drm/omap: dsi: move TE GPIO handling into core
  drm/omap: dsi: drop custom enable_te() API
  drm/omap: dsi: do bus locking in host driver
  drm/omap: dsi: untangle ulps ops from enable/disable
  drm/dsi: add MIPI_DSI_MODE_ULPS_IDLE
  drm/omap: dsi: do ULPS in host driver
  drm/omap: dsi: move panel refresh function to host
  drm/omap: dsi: Reverse direction of the DSS device enable/disable
    operations
  drm/omap: dsi: drop custom panel capability support
  drm/omap: dsi: convert to drm_panel
  drm/omap: drop omapdss-boot-init
  drm/omap: dsi: implement check timings
  drm/omap: panel-dsi-cm: use DEVICE_ATTR_RO
  drm/omap: panel-dsi-cm: support unbinding
  drm/omap: panel-dsi-cm: fix remove()
  drm/omap: dsi: return proper error code from dsi_update_all()
  drm/omap: remove global dss_device variable
  drm/omap: bind components with drm_device argument
  drm/panel: Move OMAP's DSI command mode panel driver
  drm/omap: dsi: Register a drm_bridge
  drm/omap: remove legacy DSS device operations
  drm/omap: remove unused omap_connector
  drm/omap: simplify omap_display_id
  drm/omap: drop unused DSS next pointer
  drm/omap: drop empty omap_encoder helper functions
  drm/omap: drop DSS ops_flags
  drm/omap: drop dssdev display field
  drm/omap: simplify DSI manual update code
  ARM: omap2plus_defconfig: Update for moved DSI command mode panel
  drm/panel/panel-dsi-cm: support rotation property
  ARM: dts: omap4-droid4: add panel orientation

 .../bindings/display/panel/panel-dsi-cm.txt   |    4 +-
 .../boot/dts/motorola-mapphone-common.dtsi    |    6 +-
 arch/arm/boot/dts/omap3-n950.dts              |    3 +-
 arch/arm/boot/dts/omap3.dtsi                  |    3 +
 arch/arm/boot/dts/omap4-sdp.dts               |    6 +-
 arch/arm/boot/dts/omap4.dtsi                  |    6 +
 arch/arm/boot/dts/omap5.dtsi                  |    6 +
 arch/arm/configs/omap2plus_defconfig          |    2 +-
 drivers/gpu/drm/omapdrm/Kconfig               |    1 -
 drivers/gpu/drm/omapdrm/Makefile              |    2 -
 drivers/gpu/drm/omapdrm/displays/Kconfig      |   10 -
 drivers/gpu/drm/omapdrm/displays/Makefile     |    2 -
 .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 1387 -----------------
 drivers/gpu/drm/omapdrm/dss/Kconfig           |    4 +-
 drivers/gpu/drm/omapdrm/dss/Makefile          |    2 -
 drivers/gpu/drm/omapdrm/dss/base.c            |   58 +-
 drivers/gpu/drm/omapdrm/dss/dsi.c             | 1034 +++++++-----
 drivers/gpu/drm/omapdrm/dss/dss.c             |   44 +-
 .../gpu/drm/omapdrm/dss/omapdss-boot-init.c   |  220 ---
 drivers/gpu/drm/omapdrm/dss/omapdss.h         |  141 +-
 drivers/gpu/drm/omapdrm/dss/output.c          |   13 +-
 drivers/gpu/drm/omapdrm/dss/venc.c            |    1 -
 drivers/gpu/drm/omapdrm/omap_connector.c      |  157 --
 drivers/gpu/drm/omapdrm/omap_connector.h      |   28 -
 drivers/gpu/drm/omapdrm/omap_crtc.c           |   35 +-
 drivers/gpu/drm/omapdrm/omap_drv.c            |   49 +-
 drivers/gpu/drm/omapdrm/omap_drv.h            |    1 -
 drivers/gpu/drm/omapdrm/omap_encoder.c        |   59 +-
 drivers/gpu/drm/panel/Kconfig                 |    9 +
 drivers/gpu/drm/panel/Makefile                |    1 +
 drivers/gpu/drm/panel/panel-dsi-cm.c          |  674 ++++++++
 include/drm/drm_mipi_dsi.h                    |    2 +
 32 files changed, 1465 insertions(+), 2505 deletions(-)
 delete mode 100644 drivers/gpu/drm/omapdrm/displays/Kconfig
 delete mode 100644 drivers/gpu/drm/omapdrm/displays/Makefile
 delete mode 100644 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
 delete mode 100644 drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
 delete mode 100644 drivers/gpu/drm/omapdrm/omap_connector.c
 delete mode 100644 drivers/gpu/drm/omapdrm/omap_connector.h
 create mode 100644 drivers/gpu/drm/panel/panel-dsi-cm.c


base-commit: 54ba965bb3873eca6098ddf04e3a8d7bba1b5557

Comments

Tony Lindgren Feb. 25, 2020, 12:10 a.m. UTC | #1
* Sebastian Reichel <sebastian.reichel@collabora.com> [200224 23:22]:
> This updates the existing omapdrm DSI code, so that it uses
> common drm_mipi_dsi API and drm_panel.
> 
> The patchset has been tested with Droid 4 using Linux console, X.org and
> Weston. The patchset is based on Laurent Pinchartl's patch series [0]
> and removes the last custom panel driver, so quite a few cleanups on the
> omapdrm codebase were possible.
> 
> [0] [PATCH v7 00/54] drm/omap: Replace custom display drivers with drm_bridge and drm_panel
>     https://lore.kernel.org/dri-devel/20200222150106.22919-1-laurent.pinchart@ideasonboard.com/
>     git://linuxtv.org/pinchartl/media.git omapdrm/bridge/devel
> 
> I pushed this patchset into the following branch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-n900.git omapdrm/bridge/devel-with-dsi

I gave your omapdrm/bridge/devel-with-dsi branch a quirk test
on droid4, but get the following oops with mostly modular
omap2_defconfig.

BTW, I think there's also some refcount issue in general where
the omapdrm related modules cannot be unloaded any longer?

Regards,

Tony

8< -------------------------
Internal error: Oops: 805 [#1] PREEMPT SMP ARM
Modules linked in: omapdss(+) omapdss_base drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm drm_panel_ors
CPU: 0 PID: 811 Comm: modprobe Not tainted 5.6.0-rc2-00376-g163017c12d62a #1926
Hardware name: Generic OMAP4 (Flattened Device Tree)
PC is at drm_bridge_remove+0x24/0x40 [drm]
LR is at drm_bridge_remove+0x14/0x40 [drm]
pc : [<bf2a0cb4>]    lr : [<bf2a0ca4>]    psr: 60000013
sp : edc6fc88  ip : eda3a100  fp : bf38a5b4
r10: bf38c7d0  r9 : edc164b8  r8 : edc1606c
r7 : edc1647c  r6 : edc93010  r5 : bf2dc428  r4 : edc164b8
r3 : edc164fc  r2 : 00000000  r1 : 00000000  r0 : bf2dc428
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: adc7404a  DAC: 00000051
Process modprobe (pid: 811, stack limit = 0x(ptrval))
Stack: (0xedc6fc88 to 0xedc70000)
fc80:                   edc16040 fffffdfb edc93010 bf37dfd0 00000080 edc2eb00
fca0: edc16040 edc93010 edc93010 00000000 bf392178 00000000 00000000 bf392178
fcc0: 0000001e c0624424 edc93010 c0efafa0 c0efafa4 00000000 00000000 c06222e4
fce0: edc93010 bf392178 bf392178 c0622880 c06243a0 c06243dc c0e93210 c06225cc
fd00: bf38c450 a0000013 bf38a5b4 edc93010 00000000 bf392178 c0622880 c06243a0
fd20: c06243dc c0622878 00000000 bf392178 edc93010 c06228d8 edc94fb4 edc6e000
fd40: bf392178 c062069c edc50eb8 ee8dae58 edc94fb4 d759a07d ee8dae6c bf392178
fd60: edc50e80 c0e93210 00000000 c0621618 bf38f0c0 bf3922c0 00000006 bf392178
fd80: bf3922c0 00000006 c0623db0 c062344c 00000002 bf3922c0 00000006 c06244fc
fda0: bf38a5b0 bf38a5a8 000be666 c0ec0540 bf39a000 00000000 bf392308 00000002
fdc0: edc2c300 c01dc694 edc6ff30 c0102fec c0ebbcf4 00000000 00000cc0 c0d75a70
fde0: edc2c2c0 c02b4538 c0d75a70 a0000013 a0000013 00000008 c01de5bc 00000002
fe00: 00000001 c01dc694 edc6ff30 d759a07d edbe7400 bf3922c0 00000002 d759a07d
fe20: bf392308 bf3922c0 edbe7400 edc2c2c0 bf392308 c01de5f8 00000002 edc2c2c0
fe40: bf3922c0 00000002 edc2c2c0 c01e0b1c bf3922cc 00007fff bf3922c0 c01dd34c
fe60: bf3a44ee 004c1a8f bf3922c0 bf3923d4 bf3924b8 c0a05fa8 00000000 bf38d075
fe80: 00000001 00000000 c0bddd78 c0bc76d8 bf38a03c 00000002 00000000 00000000
fea0: 00000000 00000000 6e72656b 00006c65 00000000 00000000 00000000 00000000
fec0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
fee0: 00000000 d759a07d 7fffffff edc6e000 00000000 00000003 004c1a8f 7fffffff
ff00: 00000000 0000017b 004d56a0 c01e1050 7fffffff 00000000 00000003 c093c724
ff20: 00000002 f1370000 000e3464 00000000 f138b0c6 f138f3c0 f1370000 000e3464
ff40: f1452a3c f14527a4 f1425e58 00022000 00025680 0000a084 00027a0b 00000000
ff60: 00000000 00000000 0000a074 0000003e 0000003f 00000021 00000025 00000018
ff80: 00000000 d759a07d 004dcb80 64a54c46 b6f6a510 0000017b 0000017b c0101204
ffa0: edc6e000 c0101000 64a54c46 b6f6a510 00000003 004c1a8f 00000000 004dcb80
ffc0: 64a54c46 b6f6a510 0000017b 0000017b 004dcb80 00000001 00000000 004d56a0
ffe0: 004dcb80 bea7ba90 0043d1a9 b6f1d0b8 60000030 00000003 00000000 00000000
[<bf2a0cb4>] (drm_bridge_remove [drm]) from [<bf37dfd0>] (dsi_probe+0x3ac/0x55c [omapdss])
[<bf37dfd0>] (dsi_probe [omapdss]) from [<c0624424>] (platform_drv_probe+0x48/0x98)
[<c0624424>] (platform_drv_probe) from [<c06222e4>] (really_probe+0x1dc/0x340)
[<c06222e4>] (really_probe) from [<c06225cc>] (driver_probe_device+0x5c/0x160)
[<c06225cc>] (driver_probe_device) from [<c0622878>] (device_driver_attach+0x58/0x60)
[<c0622878>] (device_driver_attach) from [<c06228d8>] (__driver_attach+0x58/0xcc)
[<c06228d8>] (__driver_attach) from [<c062069c>] (bus_for_each_dev+0x78/0xb8)
[<c062069c>] (bus_for_each_dev) from [<c0621618>] (bus_add_driver+0xf0/0x1d4)
[<c0621618>] (bus_add_driver) from [<c062344c>] (driver_register+0x74/0x108)
[<c062344c>] (driver_register) from [<c06244fc>] (__platform_register_drivers+0x54/0xd0)
[<c06244fc>] (__platform_register_drivers) from [<c0102fec>] (do_one_initcall+0x44/0x2a8)
[<c0102fec>] (do_one_initcall) from [<c01de5f8>] (do_init_module+0x5c/0x234)
[<c01de5f8>] (do_init_module) from [<c01e0b1c>] (load_module+0x22c4/0x2580)
[<c01e0b1c>] (load_module) from [<c01e1050>] (sys_finit_module+0xc4/0xe0)
[<c01e1050>] (sys_finit_module) from [<c0101000>] (ret_fast_syscall+0x0/0x54)
Exception stack(0xedc6ffa8 to 0xedc6fff0)
ffa0:                   64a54c46 b6f6a510 00000003 004c1a8f 00000000 004dcb80
ffc0: 64a54c46 b6f6a510 0000017b 0000017b 004dcb80 00000001 00000000 004d56a0
ffe0: 004dcb80 bea7ba90 0043d1a9 b6f1d0b8
Code: e5942048 e5941044 e2843044 e1a00005 (e5812004)
---[ end trace 6213ab3cbabf9f59 ]---
Sebastian Reichel Feb. 25, 2020, 2:28 a.m. UTC | #2
Hi,

On Mon, Feb 24, 2020 at 04:10:11PM -0800, Tony Lindgren wrote:
> * Sebastian Reichel <sebastian.reichel@collabora.com> [200224 23:22]:
> > This updates the existing omapdrm DSI code, so that it uses
> > common drm_mipi_dsi API and drm_panel.
> > 
> > The patchset has been tested with Droid 4 using Linux console, X.org and
> > Weston. The patchset is based on Laurent Pinchartl's patch series [0]
> > and removes the last custom panel driver, so quite a few cleanups on the
> > omapdrm codebase were possible.
> > 
> > [0] [PATCH v7 00/54] drm/omap: Replace custom display drivers with drm_bridge and drm_panel
> >     https://lore.kernel.org/dri-devel/20200222150106.22919-1-laurent.pinchart@ideasonboard.com/
> >     git://linuxtv.org/pinchartl/media.git omapdrm/bridge/devel
> > 
> > I pushed this patchset into the following branch:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-n900.git omapdrm/bridge/devel-with-dsi
> 
> I gave your omapdrm/bridge/devel-with-dsi branch a quirk test
> on droid4, but get the following oops with mostly modular
> omap2_defconfig.

:( I was testing with a non-modular kernel, since that gives me a
much easier testing workflow. Guess I need to spend some time to get
a good setup for testing modular kernel with Droid4.

> BTW, I think there's also some refcount issue in general where
> the omapdrm related modules cannot be unloaded any longer?

I wouldn't be too surprised. The dependencies are quite interesting
at the moment with omapdss registering omapdrm and then omapdrm
registers the drm_device, which references the encoders from
omapdss. I think this is something to look at once Laurent's and
my branch have been merged to avoid increasing the complexity.
Technically it should be possible to link everything into one
module.

> 8< -------------------------
> Internal error: Oops: 805 [#1] PREEMPT SMP ARM
> Modules linked in: omapdss(+) omapdss_base drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm drm_panel_ors
> CPU: 0 PID: 811 Comm: modprobe Not tainted 5.6.0-rc2-00376-g163017c12d62a #1926
> Hardware name: Generic OMAP4 (Flattened Device Tree)
> PC is at drm_bridge_remove+0x24/0x40 [drm]
> LR is at drm_bridge_remove+0x14/0x40 [drm]
> pc : [<bf2a0cb4>]    lr : [<bf2a0ca4>]    psr: 60000013
> sp : edc6fc88  ip : eda3a100  fp : bf38a5b4
> r10: bf38c7d0  r9 : edc164b8  r8 : edc1606c
> r7 : edc1647c  r6 : edc93010  r5 : bf2dc428  r4 : edc164b8
> r3 : edc164fc  r2 : 00000000  r1 : 00000000  r0 : bf2dc428
> Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: adc7404a  DAC: 00000051
> Process modprobe (pid: 811, stack limit = 0x(ptrval))
> Stack: (0xedc6fc88 to 0xedc70000)
> fc80:                   edc16040 fffffdfb edc93010 bf37dfd0 00000080 edc2eb00
> fca0: edc16040 edc93010 edc93010 00000000 bf392178 00000000 00000000 bf392178
> fcc0: 0000001e c0624424 edc93010 c0efafa0 c0efafa4 00000000 00000000 c06222e4
> fce0: edc93010 bf392178 bf392178 c0622880 c06243a0 c06243dc c0e93210 c06225cc
> fd00: bf38c450 a0000013 bf38a5b4 edc93010 00000000 bf392178 c0622880 c06243a0
> fd20: c06243dc c0622878 00000000 bf392178 edc93010 c06228d8 edc94fb4 edc6e000
> fd40: bf392178 c062069c edc50eb8 ee8dae58 edc94fb4 d759a07d ee8dae6c bf392178
> fd60: edc50e80 c0e93210 00000000 c0621618 bf38f0c0 bf3922c0 00000006 bf392178
> fd80: bf3922c0 00000006 c0623db0 c062344c 00000002 bf3922c0 00000006 c06244fc
> fda0: bf38a5b0 bf38a5a8 000be666 c0ec0540 bf39a000 00000000 bf392308 00000002
> fdc0: edc2c300 c01dc694 edc6ff30 c0102fec c0ebbcf4 00000000 00000cc0 c0d75a70
> fde0: edc2c2c0 c02b4538 c0d75a70 a0000013 a0000013 00000008 c01de5bc 00000002
> fe00: 00000001 c01dc694 edc6ff30 d759a07d edbe7400 bf3922c0 00000002 d759a07d
> fe20: bf392308 bf3922c0 edbe7400 edc2c2c0 bf392308 c01de5f8 00000002 edc2c2c0
> fe40: bf3922c0 00000002 edc2c2c0 c01e0b1c bf3922cc 00007fff bf3922c0 c01dd34c
> fe60: bf3a44ee 004c1a8f bf3922c0 bf3923d4 bf3924b8 c0a05fa8 00000000 bf38d075
> fe80: 00000001 00000000 c0bddd78 c0bc76d8 bf38a03c 00000002 00000000 00000000
> fea0: 00000000 00000000 6e72656b 00006c65 00000000 00000000 00000000 00000000
> fec0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> fee0: 00000000 d759a07d 7fffffff edc6e000 00000000 00000003 004c1a8f 7fffffff
> ff00: 00000000 0000017b 004d56a0 c01e1050 7fffffff 00000000 00000003 c093c724
> ff20: 00000002 f1370000 000e3464 00000000 f138b0c6 f138f3c0 f1370000 000e3464
> ff40: f1452a3c f14527a4 f1425e58 00022000 00025680 0000a084 00027a0b 00000000
> ff60: 00000000 00000000 0000a074 0000003e 0000003f 00000021 00000025 00000018
> ff80: 00000000 d759a07d 004dcb80 64a54c46 b6f6a510 0000017b 0000017b c0101204
> ffa0: edc6e000 c0101000 64a54c46 b6f6a510 00000003 004c1a8f 00000000 004dcb80
> ffc0: 64a54c46 b6f6a510 0000017b 0000017b 004dcb80 00000001 00000000 004d56a0
> ffe0: 004dcb80 bea7ba90 0043d1a9 b6f1d0b8 60000030 00000003 00000000 00000000
> [<bf2a0cb4>] (drm_bridge_remove [drm]) from [<bf37dfd0>] (dsi_probe+0x3ac/0x55c [omapdss])
> [<bf37dfd0>] (dsi_probe [omapdss]) from [<c0624424>] (platform_drv_probe+0x48/0x98)
> [<c0624424>] (platform_drv_probe) from [<c06222e4>] (really_probe+0x1dc/0x340)
> [<c06222e4>] (really_probe) from [<c06225cc>] (driver_probe_device+0x5c/0x160)
> [<c06225cc>] (driver_probe_device) from [<c0622878>] (device_driver_attach+0x58/0x60)
> [<c0622878>] (device_driver_attach) from [<c06228d8>] (__driver_attach+0x58/0xcc)
> [<c06228d8>] (__driver_attach) from [<c062069c>] (bus_for_each_dev+0x78/0xb8)
> [<c062069c>] (bus_for_each_dev) from [<c0621618>] (bus_add_driver+0xf0/0x1d4)
> [<c0621618>] (bus_add_driver) from [<c062344c>] (driver_register+0x74/0x108)
> [<c062344c>] (driver_register) from [<c06244fc>] (__platform_register_drivers+0x54/0xd0)
> [<c06244fc>] (__platform_register_drivers) from [<c0102fec>] (do_one_initcall+0x44/0x2a8)
> [<c0102fec>] (do_one_initcall) from [<c01de5f8>] (do_init_module+0x5c/0x234)
> [<c01de5f8>] (do_init_module) from [<c01e0b1c>] (load_module+0x22c4/0x2580)
> [<c01e0b1c>] (load_module) from [<c01e1050>] (sys_finit_module+0xc4/0xe0)
> [<c01e1050>] (sys_finit_module) from [<c0101000>] (ret_fast_syscall+0x0/0x54)
> Exception stack(0xedc6ffa8 to 0xedc6fff0)
> ffa0:                   64a54c46 b6f6a510 00000003 004c1a8f 00000000 004dcb80
> ffc0: 64a54c46 b6f6a510 0000017b 0000017b 004dcb80 00000001 00000000 004d56a0
> ffe0: 004dcb80 bea7ba90 0043d1a9 b6f1d0b8
> Code: e5942048 e5941044 e2843044 e1a00005 (e5812004)
> ---[ end trace 6213ab3cbabf9f59 ]---

oops, looks like dsi_bridge_init() is missing "drm_bridge_add(&dsi->bridge);"
before returning. Now I wonder why it is working for me without the
bridge being added :) I will send out an updated patch for this tomorrow.

-- Sebastian
Tony Lindgren Feb. 25, 2020, 3:42 p.m. UTC | #3
* Sebastian Reichel <sre@kernel.org> [200225 02:29]:
> On Mon, Feb 24, 2020 at 04:10:11PM -0800, Tony Lindgren wrote:
> > BTW, I think there's also some refcount issue in general where
> > the omapdrm related modules cannot be unloaded any longer?
> 
> I wouldn't be too surprised. The dependencies are quite interesting
> at the moment with omapdss registering omapdrm and then omapdrm
> registers the drm_device, which references the encoders from
> omapdss. I think this is something to look at once Laurent's and
> my branch have been merged to avoid increasing the complexity.
> Technically it should be possible to link everything into one
> module.

Well the DSS is really DOSSI for Display Output SubSystem
Interconnect :) The devices on the interconnect are mostly
independent and ideally the toplevel dss driver would just
provide Linux generic resources to dispc and various output
drivers. So probably not a good idea to try to build it all
into a single module.

Regards,

Tony
Sebastian Reichel Feb. 25, 2020, 11:01 p.m. UTC | #4
Hi,

On Tue, Feb 25, 2020 at 07:42:37AM -0800, Tony Lindgren wrote:
> * Sebastian Reichel <sre@kernel.org> [200225 02:29]:
> > On Mon, Feb 24, 2020 at 04:10:11PM -0800, Tony Lindgren wrote:
> > > BTW, I think there's also some refcount issue in general where
> > > the omapdrm related modules cannot be unloaded any longer?
> > 
> > I wouldn't be too surprised. The dependencies are quite interesting
> > at the moment with omapdss registering omapdrm and then omapdrm
> > registers the drm_device, which references the encoders from
> > omapdss. I think this is something to look at once Laurent's and
> > my branch have been merged to avoid increasing the complexity.
> > Technically it should be possible to link everything into one
> > module.
> 
> Well the DSS is really DOSSI for Display Output SubSystem
> Interconnect :) The devices on the interconnect are mostly
> independent and ideally the toplevel dss driver would just
> provide Linux generic resources to dispc and various output
> drivers. So probably not a good idea to try to build it all
> into a single module.

All the output drivers and dispc are already in a single module:
omapdss.ko. There is omapdss-base.ko, omapdss.ko and omapdrm.ko
module. omapdss-base.ko contains a few helpers, omapdss.ko contains
dispc and all output encoders, omapdrm has the tiler code and
wraps some of the custom DSS APIs to DRM APIs. I think the best
way forward is to eliminate the custom API and use common DRM
APIs directly. Then merge all 3 modules into one module.

In theory one could add modules for each encoder, but practically
this only increases complexity. DRM cannnot hotplug encoders, so
removing any module results in complete loss of DRM. Also during
probe we need to load all modules, since something might be
connected. So having extra modules is not really useful?

-- Sebastian
Laurent Pinchart Feb. 25, 2020, 11:06 p.m. UTC | #5
Hi Sebastian,

On Wed, Feb 26, 2020 at 12:01:24AM +0100, Sebastian Reichel wrote:
> On Tue, Feb 25, 2020 at 07:42:37AM -0800, Tony Lindgren wrote:
> > * Sebastian Reichel <sre@kernel.org> [200225 02:29]:
> > > On Mon, Feb 24, 2020 at 04:10:11PM -0800, Tony Lindgren wrote:
> > > > BTW, I think there's also some refcount issue in general where
> > > > the omapdrm related modules cannot be unloaded any longer?
> > > 
> > > I wouldn't be too surprised. The dependencies are quite interesting
> > > at the moment with omapdss registering omapdrm and then omapdrm
> > > registers the drm_device, which references the encoders from
> > > omapdss. I think this is something to look at once Laurent's and
> > > my branch have been merged to avoid increasing the complexity.
> > > Technically it should be possible to link everything into one
> > > module.
> > 
> > Well the DSS is really DOSSI for Display Output SubSystem
> > Interconnect :) The devices on the interconnect are mostly
> > independent and ideally the toplevel dss driver would just
> > provide Linux generic resources to dispc and various output
> > drivers. So probably not a good idea to try to build it all
> > into a single module.
> 
> All the output drivers and dispc are already in a single module:
> omapdss.ko. There is omapdss-base.ko, omapdss.ko and omapdrm.ko
> module. omapdss-base.ko contains a few helpers, omapdss.ko contains
> dispc and all output encoders, omapdrm has the tiler code and
> wraps some of the custom DSS APIs to DRM APIs. I think the best
> way forward is to eliminate the custom API and use common DRM
> APIs directly. Then merge all 3 modules into one module.
> 
> In theory one could add modules for each encoder, but practically
> this only increases complexity. DRM cannnot hotplug encoders, so
> removing any module results in complete loss of DRM. Also during
> probe we need to load all modules, since something might be
> connected. So having extra modules is not really useful?

This was considered previously, but blocked (IIRC) because it introduced
circular dependencies with the omapdrm-specific display drivers in
drivers/gpu/drm/omapdrm/displays/. Once panel-dsi-cm goes away, I think
we can give it another try.
Tony Lindgren Feb. 25, 2020, 11:09 p.m. UTC | #6
* Sebastian Reichel <sre@kernel.org> [200225 23:03]:
> Hi,
> 
> On Tue, Feb 25, 2020 at 07:42:37AM -0800, Tony Lindgren wrote:
> > * Sebastian Reichel <sre@kernel.org> [200225 02:29]:
> > > On Mon, Feb 24, 2020 at 04:10:11PM -0800, Tony Lindgren wrote:
> > > > BTW, I think there's also some refcount issue in general where
> > > > the omapdrm related modules cannot be unloaded any longer?
> > > 
> > > I wouldn't be too surprised. The dependencies are quite interesting
> > > at the moment with omapdss registering omapdrm and then omapdrm
> > > registers the drm_device, which references the encoders from
> > > omapdss. I think this is something to look at once Laurent's and
> > > my branch have been merged to avoid increasing the complexity.
> > > Technically it should be possible to link everything into one
> > > module.
> > 
> > Well the DSS is really DOSSI for Display Output SubSystem
> > Interconnect :) The devices on the interconnect are mostly
> > independent and ideally the toplevel dss driver would just
> > provide Linux generic resources to dispc and various output
> > drivers. So probably not a good idea to try to build it all
> > into a single module.
> 
> All the output drivers and dispc are already in a single module:
> omapdss.ko. There is omapdss-base.ko, omapdss.ko and omapdrm.ko
> module. omapdss-base.ko contains a few helpers, omapdss.ko contains
> dispc and all output encoders, omapdrm has the tiler code and
> wraps some of the custom DSS APIs to DRM APIs. I think the best
> way forward is to eliminate the custom API and use common DRM
> APIs directly. Then merge all 3 modules into one module.
> 
> In theory one could add modules for each encoder, but practically
> this only increases complexity. DRM cannnot hotplug encoders, so
> removing any module results in complete loss of DRM. Also during
> probe we need to load all modules, since something might be
> connected. So having extra modules is not really useful?

Well my main concern here is that we should use generic
Linux frameworks between the devices within DSS where possible.
I can see a single driver pile of code quickly turn into a
spaghetti of internal calls instead..

Also each devices on the DSS interconnect needs to do
pm_runtime_get for it's struct device naturally.

If you can avoid the issues above, then I have no objections
of just having one module.

Regards,

Tony
Sebastian Reichel Feb. 25, 2020, 11:52 p.m. UTC | #7
Hi,

On Tue, Feb 25, 2020 at 03:09:37PM -0800, Tony Lindgren wrote:
> * Sebastian Reichel <sre@kernel.org> [200225 23:03]:
> > Hi,
> > 
> > On Tue, Feb 25, 2020 at 07:42:37AM -0800, Tony Lindgren wrote:
> > > * Sebastian Reichel <sre@kernel.org> [200225 02:29]:
> > > > On Mon, Feb 24, 2020 at 04:10:11PM -0800, Tony Lindgren wrote:
> > > > > BTW, I think there's also some refcount issue in general where
> > > > > the omapdrm related modules cannot be unloaded any longer?
> > > > 
> > > > I wouldn't be too surprised. The dependencies are quite interesting
> > > > at the moment with omapdss registering omapdrm and then omapdrm
> > > > registers the drm_device, which references the encoders from
> > > > omapdss. I think this is something to look at once Laurent's and
> > > > my branch have been merged to avoid increasing the complexity.
> > > > Technically it should be possible to link everything into one
> > > > module.
> > > 
> > > Well the DSS is really DOSSI for Display Output SubSystem
> > > Interconnect :) The devices on the interconnect are mostly
> > > independent and ideally the toplevel dss driver would just
> > > provide Linux generic resources to dispc and various output
> > > drivers. So probably not a good idea to try to build it all
> > > into a single module.
> > 
> > All the output drivers and dispc are already in a single module:
> > omapdss.ko. There is omapdss-base.ko, omapdss.ko and omapdrm.ko
> > module. omapdss-base.ko contains a few helpers, omapdss.ko contains
> > dispc and all output encoders, omapdrm has the tiler code and
> > wraps some of the custom DSS APIs to DRM APIs. I think the best
> > way forward is to eliminate the custom API and use common DRM
> > APIs directly. Then merge all 3 modules into one module.
> > 
> > In theory one could add modules for each encoder, but practically
> > this only increases complexity. DRM cannnot hotplug encoders, so
> > removing any module results in complete loss of DRM. Also during
> > probe we need to load all modules, since something might be
> > connected. So having extra modules is not really useful?
> 
> Well my main concern here is that we should use generic
> Linux frameworks between the devices within DSS where possible.

Ack.

> I can see a single driver pile of code quickly turn into a
> spaghetti of internal calls instead.

I was only talking about generating a single module, it contains
multiple drivers. And we already have the spaghetti of internal
calls between omapdrm and omapdss. With the modules it only means,
that functions are either exported or accessed via callbacks.

> Also each devices on the DSS interconnect needs to do
> pm_runtime_get for it's struct device naturally.

Sure, but you are talking about things that are already part of a
single module (omapdss.ko). omapdss-base.ko and omapdrm.ko do not
directly access hardware.

> If you can avoid the issues above, then I have no objections
> of just having one module.

Well for now let's get Laurent's and my series forward. I think
the next step would be to get rid of omap_encoder by moving the
encoder init into the DSS output code. Technically we are already
merging omapdrm and omapdss, e.g. omap_connector is gone from
omapdrm after the series.

-- Sebastian
Tomi Valkeinen Feb. 26, 2020, 12:20 p.m. UTC | #8
On 26/02/2020 01:52, Sebastian Reichel wrote:

> Well for now let's get Laurent's and my series forward. I think
> the next step would be to get rid of omap_encoder by moving the
> encoder init into the DSS output code. Technically we are already
> merging omapdrm and omapdss, e.g. omap_connector is gone from
> omapdrm after the series.

After Laurent's series (now pushed) and this one, we can turn omapdrm into a single module, as it's 
supposed to be.

I don't see a point in trying to change DSS internal encoders into separate modules, as they're all 
under DSS, and at times connected in ways which are not easily represented with generic APIs. In 
theory it should be possible, but I don't see any real benefit in such work.

  Tomi
Tomi Valkeinen Feb. 26, 2020, 12:28 p.m. UTC | #9
On 25/02/2020 01:20, Sebastian Reichel wrote:
> This updates the existing omapdrm DSI code, so that it uses
> common drm_mipi_dsi API and drm_panel.
> 
> The patchset has been tested with Droid 4 using Linux console, X.org and
> Weston. The patchset is based on Laurent Pinchartl's patch series [0]
> and removes the last custom panel driver, so quite a few cleanups on the
> omapdrm codebase were possible.

I haven't done any reviews yet, but applied these (and the one v2.1 patch) on top of Laurent's. I booted up AM5 EVM and loaded the modules:

[   17.261560] WARNING: CPU: 0 PID: 419 at drivers/base/component.c:636 component_bind_all+0x1f4/0x258
[   17.270811] Modules linked in: omapdrm(+) omapdss omapdss_base panel_osd_osd101t2587_53ts panel_simple simple_bridge ti_tpd12s015 display_connec
tor tc358767 tc358768 sii902x ti_tfp410 drm_kms_helper drm drm_panel_orientation_quirks cfbfillrect cfbimgblt cfbcopyarea cec
[   17.295521] CPU: 0 PID: 419 Comm: insmod Not tainted 5.6.0-rc2-00375-g66c4203ed9d4 #2
[   17.303387] Hardware name: Generic DRA74X (Flattened Device Tree)
[   17.309520] [<c011431c>] (unwind_backtrace) from [<c010dc20>] (show_stack+0x10/0x14)
[   17.317306] [<c010dc20>] (show_stack) from [<c09acca4>] (dump_stack+0xb4/0xd0)
[   17.324567] [<c09acca4>] (dump_stack) from [<c013a33c>] (__warn+0xc0/0xf8)
[   17.331477] [<c013a33c>] (__warn) from [<c013a700>] (warn_slowpath_fmt+0x58/0xb8)
[   17.338998] [<c013a700>] (warn_slowpath_fmt) from [<c0630698>] (component_bind_all+0x1f4/0x258)
[   17.347768] [<c0630698>] (component_bind_all) from [<bf11b52c>] (pdev_probe+0xe0/0x7a0 [omapdrm])
[   17.356709] [<bf11b52c>] (pdev_probe [omapdrm]) from [<c063a198>] (platform_drv_probe+0x48/0x98)
[   17.365537] [<c063a198>] (platform_drv_probe) from [<c0637c1c>] (really_probe+0x200/0x478)
[   17.373841] [<c0637c1c>] (really_probe) from [<c0638060>] (driver_probe_device+0x6c/0x1b4)
[   17.382145] [<c0638060>] (driver_probe_device) from [<c06383f0>] (device_driver_attach+0x58/0x60)
[   17.391060] [<c06383f0>] (device_driver_attach) from [<c063849c>] (__driver_attach+0xa4/0x148)
[   17.399713] [<c063849c>] (__driver_attach) from [<c0635af8>] (bus_for_each_dev+0x70/0xb4)
[   17.407930] [<c0635af8>] (bus_for_each_dev) from [<c0636d60>] (bus_add_driver+0x100/0x204)
[   17.416233] [<c0636d60>] (bus_add_driver) from [<c0639078>] (driver_register+0x74/0x108)
[   17.424361] [<c0639078>] (driver_register) from [<c063a2a8>] (__platform_register_drivers+0x58/0x150)
[   17.433628] [<c063a2a8>] (__platform_register_drivers) from [<c0102ff8>] (do_one_initcall+0x48/0x2a0)
[   17.442893] [<c0102ff8>] (do_one_initcall) from [<c01e2efc>] (do_init_module+0x5c/0x234)
[   17.451022] [<c01e2efc>] (do_init_module) from [<c01e5668>] (load_module+0x250c/0x28a4)
[   17.459064] [<c01e5668>] (load_module) from [<c01e5cb8>] (sys_finit_module+0xcc/0x110)
[   17.467020] [<c01e5cb8>] (sys_finit_module) from [<c0101000>] (ret_fast_syscall+0x0/0x54)
[   17.475233] Exception stack(0xea84bfa8 to 0xea84bff0)
[   17.480307] bfa0:                   00000002 00000000 00000003 0002a894 00000000 bebd2d74
[   17.488524] bfc0: 00000002 00000000 00028424 0000017b 0003eeb0 00000002 b6ffe000 00000000
[   17.496738] bfe0: bebd2bb8 bebd2ba8 00020d7c b6edb0b0
[   17.502365] ---[ end trace c20802296d6b2775 ]---
[   17.508567] omapdss_dss 58000000.dss: bound 58001000.dispc (ops dsi_framedone_timeout_work_callback [omapdss])
[   17.520358] omapdss_dss 58000000.dss: bound 58040000.encoder (ops dsi_framedone_timeout_work_callback [omapdss])
[   17.559728] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[   17.574544] [drm] Enabling DMM ywrap scrolling
[   17.580606] omapdrm omapdrm.0: fb0: omapdrmdrmfb frame buffer device
[   17.625099] [drm] Initialized omapdrm 1.0.0 20110917 for omapdrm.0 on minor 0

Similar warning on module unload. The display works, though.

 Tomi
Sebastian Reichel Feb. 27, 2020, 12:28 a.m. UTC | #10
Hi,

On Wed, Feb 26, 2020 at 02:28:23PM +0200, Tomi Valkeinen wrote:
> On 25/02/2020 01:20, Sebastian Reichel wrote:
> > This updates the existing omapdrm DSI code, so that it uses
> > common drm_mipi_dsi API and drm_panel.
> > 
> > The patchset has been tested with Droid 4 using Linux console, X.org and
> > Weston. The patchset is based on Laurent Pinchartl's patch series [0]
> > and removes the last custom panel driver, so quite a few cleanups on the
> > omapdrm codebase were possible.
> 
> I haven't done any reviews yet, but applied these (and the one
> v2.1 patch) on top of Laurent's. I booted up AM5 EVM and loaded
> the modules:
> 
> [   17.261560] WARNING: CPU: 0 PID: 419 at drivers/base/component.c:636 component_bind_all+0x1f4/0x258

I did not see that warning for some reason. I guess I was always
lucky, since its a race condition introduced by "drm/omap: bind
components with drm_device argument". That patch moves
component_bind_all from dss_bind() to omapdrm's probe() to get
access to drm_dev. Currently omapdrm is the only DRM driver not
supplying that as parameter and its required to move encoder init
into the DSS output drivers. I assumed that it would be ok, since
it just postpones the call a little bit. Unfortunately it also
moves it out of the lock protection from dss_bind().

The only fix, that I see is to register the drm_dev is dss_bind
and then supply it to omapdrm via pdata. But I think we could
actually get rid of the omapdrm platform device completly and
just use its parent "omapdss_dss". I suppose a follow-up patchset
could merge the code into one module and do this merge.

Probably it's the best to drop that patch for now. It's not yet
used in the patches, that I sent.

> [   17.270811] Modules linked in: omapdrm(+) omapdss omapdss_base panel_osd_osd101t2587_53ts panel_simple simple_bridge ti_tpd12s015 display_connec
> tor tc358767 tc358768 sii902x ti_tfp410 drm_kms_helper drm drm_panel_orientation_quirks cfbfillrect cfbimgblt cfbcopyarea cec
> [   17.295521] CPU: 0 PID: 419 Comm: insmod Not tainted 5.6.0-rc2-00375-g66c4203ed9d4 #2
> [   17.303387] Hardware name: Generic DRA74X (Flattened Device Tree)
> [   17.309520] [<c011431c>] (unwind_backtrace) from [<c010dc20>] (show_stack+0x10/0x14)
> [   17.317306] [<c010dc20>] (show_stack) from [<c09acca4>] (dump_stack+0xb4/0xd0)
> [   17.324567] [<c09acca4>] (dump_stack) from [<c013a33c>] (__warn+0xc0/0xf8)
> [   17.331477] [<c013a33c>] (__warn) from [<c013a700>] (warn_slowpath_fmt+0x58/0xb8)
> [   17.338998] [<c013a700>] (warn_slowpath_fmt) from [<c0630698>] (component_bind_all+0x1f4/0x258)
> [   17.347768] [<c0630698>] (component_bind_all) from [<bf11b52c>] (pdev_probe+0xe0/0x7a0 [omapdrm])
> [   17.356709] [<bf11b52c>] (pdev_probe [omapdrm]) from [<c063a198>] (platform_drv_probe+0x48/0x98)
> [   17.365537] [<c063a198>] (platform_drv_probe) from [<c0637c1c>] (really_probe+0x200/0x478)
> [   17.373841] [<c0637c1c>] (really_probe) from [<c0638060>] (driver_probe_device+0x6c/0x1b4)
> [   17.382145] [<c0638060>] (driver_probe_device) from [<c06383f0>] (device_driver_attach+0x58/0x60)
> [   17.391060] [<c06383f0>] (device_driver_attach) from [<c063849c>] (__driver_attach+0xa4/0x148)
> [   17.399713] [<c063849c>] (__driver_attach) from [<c0635af8>] (bus_for_each_dev+0x70/0xb4)
> [   17.407930] [<c0635af8>] (bus_for_each_dev) from [<c0636d60>] (bus_add_driver+0x100/0x204)
> [   17.416233] [<c0636d60>] (bus_add_driver) from [<c0639078>] (driver_register+0x74/0x108)
> [   17.424361] [<c0639078>] (driver_register) from [<c063a2a8>] (__platform_register_drivers+0x58/0x150)
> [   17.433628] [<c063a2a8>] (__platform_register_drivers) from [<c0102ff8>] (do_one_initcall+0x48/0x2a0)
> [   17.442893] [<c0102ff8>] (do_one_initcall) from [<c01e2efc>] (do_init_module+0x5c/0x234)
> [   17.451022] [<c01e2efc>] (do_init_module) from [<c01e5668>] (load_module+0x250c/0x28a4)
> [   17.459064] [<c01e5668>] (load_module) from [<c01e5cb8>] (sys_finit_module+0xcc/0x110)
> [   17.467020] [<c01e5cb8>] (sys_finit_module) from [<c0101000>] (ret_fast_syscall+0x0/0x54)
> [   17.475233] Exception stack(0xea84bfa8 to 0xea84bff0)
> [   17.480307] bfa0:                   00000002 00000000 00000003 0002a894 00000000 bebd2d74
> [   17.488524] bfc0: 00000002 00000000 00028424 0000017b 0003eeb0 00000002 b6ffe000 00000000
> [   17.496738] bfe0: bebd2bb8 bebd2ba8 00020d7c b6edb0b0
> [   17.502365] ---[ end trace c20802296d6b2775 ]---
> [   17.508567] omapdss_dss 58000000.dss: bound 58001000.dispc (ops dsi_framedone_timeout_work_callback [omapdss])
> [   17.520358] omapdss_dss 58000000.dss: bound 58040000.encoder (ops dsi_framedone_timeout_work_callback [omapdss])
> [   17.559728] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [   17.574544] [drm] Enabling DMM ywrap scrolling
> [   17.580606] omapdrm omapdrm.0: fb0: omapdrmdrmfb frame buffer device
> [   17.625099] [drm] Initialized omapdrm 1.0.0 20110917 for omapdrm.0 on minor 0
> 
> Similar warning on module unload. The display works, though.

same problem, with dss_unbind() and omapdrm's remove function.

-- Sebastian
Tomi Valkeinen March 25, 2020, 12:47 p.m. UTC | #11
On 25/02/2020 01:20, Sebastian Reichel wrote:
> This updates the existing omapdrm DSI code, so that it uses
> common drm_mipi_dsi API and drm_panel.
> 
> The patchset has been tested with Droid 4 using Linux console, X.org and
> Weston. The patchset is based on Laurent Pinchartl's patch series [0]
> and removes the last custom panel driver, so quite a few cleanups on the
> omapdrm codebase were possible.

This is a big series, and I suggest to keep the cleanups to minimum. Things can be cleaned up 
afterwards after the functional parts of this series have been merged.

  Tomi
Laurent Pinchart March 25, 2020, 1:03 p.m. UTC | #12
Hi Tomi,

On Wed, Mar 25, 2020 at 02:47:48PM +0200, Tomi Valkeinen wrote:
> On 25/02/2020 01:20, Sebastian Reichel wrote:
> > This updates the existing omapdrm DSI code, so that it uses
> > common drm_mipi_dsi API and drm_panel.
> > 
> > The patchset has been tested with Droid 4 using Linux console, X.org and
> > Weston. The patchset is based on Laurent Pinchartl's patch series [0]
> > and removes the last custom panel driver, so quite a few cleanups on the
> > omapdrm codebase were possible.
> 
> This is a big series, and I suggest to keep the cleanups to minimum.
> Things can be cleaned up afterwards after the functional parts of this
> series have been merged.

There are a few cleanups at the bottom of the series that could be
merged without waiting for the rest though :-)
Tomi Valkeinen April 1, 2020, 12:43 p.m. UTC | #13
On 25/03/2020 15:03, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Wed, Mar 25, 2020 at 02:47:48PM +0200, Tomi Valkeinen wrote:
>> On 25/02/2020 01:20, Sebastian Reichel wrote:
>>> This updates the existing omapdrm DSI code, so that it uses
>>> common drm_mipi_dsi API and drm_panel.
>>>
>>> The patchset has been tested with Droid 4 using Linux console, X.org and
>>> Weston. The patchset is based on Laurent Pinchartl's patch series [0]
>>> and removes the last custom panel driver, so quite a few cleanups on the
>>> omapdrm codebase were possible.
>>
>> This is a big series, and I suggest to keep the cleanups to minimum.
>> Things can be cleaned up afterwards after the functional parts of this
>> series have been merged.
> 
> There are a few cleanups at the bottom of the series that could be
> merged without waiting for the rest though :-)

Yep. So, Sebastian, if there are patches that can be applied independently of this series, can you 
send those separately or move them to the beginning of the series.

There was also the change to include/drm/drm_mipi_dsi.h which is outside the OMAP context. Maybe 
send that separately to the relevant maintainers. Or at least cc them, as now it's kind of hidden 
between all the omap changes.

And I need to try to wake up one of my boards with a DSI video mode display for testing...

  Tomi
H. Nikolaus Schaller June 25, 2020, 1:07 p.m. UTC | #14
Hi Sebastian,

> Am 25.02.2020 um 00:20 schrieb Sebastian Reichel <sebastian.reichel@collabora.com>:
> 
> This updates the existing omapdrm DSI code, so that it uses
> common drm_mipi_dsi API and drm_panel.
> 
> The patchset has been tested with Droid 4 using Linux console, X.org and
> Weston. The patchset is based on Laurent Pinchartl's patch series [0]
> and removes the last custom panel driver, so quite a few cleanups on the
> omapdrm codebase were possible.
> 
> [0] [PATCH v7 00/54] drm/omap: Replace custom display drivers with drm_bridge and drm_panel
>    https://lore.kernel.org/dri-devel/20200222150106.22919-1-laurent.pinchart@ideasonboard.com/
>    git://linuxtv.org/pinchartl/media.git omapdrm/bridge/devel
> 
> I pushed this patchset into the following branch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-n900.git omapdrm/bridge/devel-with-dsi
> 
> The previous version of this patchset has been sent quite some time ago.
> This version has been rebased and required cleaning up most of the
> hacks. I do not have a detailed changelog, but quite a few things
> changed. I decided against doing anything special for the DT change
> (adding DSI channel number), since only 3 devices are affected. It is
> quite likely, that all developers of those devices update DT together
> with kernel for those devices. My suggestion is to merge the first two
> patches ASAP and backport to stable, since it does not affect old
> kernels and the change is rather small.
> 
> RFCv1: https://lore.kernel.org/dri-devel/20191117023946.VjCC3yE08DMx7JIKxNagPoT5et7WTnKGVV6MtOtB9Ro@z/

is there something new for this series? Is there a repo where it has been rebased to v5.8-rc1 (and tested)?

Our Pyra MIPI/DSI omap5 panel setup is broken by upstream changes since v5.7-rc1 and I am not sure if it is better to search in the old omapdrm code what is missing now. Or to wait for your DSI code and just update/fix our panel driver (and have a chance to get that one upstream).

BR and thanks,
Nikolaus


> 
> -- Sebastian
> 
> Sebastian Reichel (56):
>  ARM: dts: omap: add channel to DSI panels
>  ARM: dts: omap4-droid4: add panel compatible
>  Revert "drm/omap: dss: Remove unused omap_dss_device operations"
>  omap/drm: drop unused dsi.configure_pins
>  drm/omap: dsi: use MIPI_DSI_FMT_* instead of OMAP_DSS_DSI_FMT_*
>  drm/omap: constify write buffers
>  drm/omap: dsi: add generic transfer function
>  drm/omap: panel-dsi-cm: convert to transfer API
>  drm/omap: dsi: unexport specific data transfer functions
>  drm/omap: dsi: drop virtual channel logic
>  drm/omap: dsi: simplify write function
>  drm/omap: dsi: simplify read functions
>  drm/omap: dsi: switch dsi_vc_send_long/short to mipi_dsi_msg
>  drm/omap: dsi: introduce mipi_dsi_host
>  drm/omap: panel-dsi-cm: use DSI helpers
>  drm/omap: dsi: request VC via mipi_dsi_attach
>  drm/omap: panel-dsi-cm: drop hardcoded VC
>  drm/omap: panel-dsi-cm: use common MIPI DCS 1.3 defines
>  drm/omap: dsi: drop unused memory_read()
>  drm/omap: dsi: drop unused get_te()
>  drm/omap: dsi: drop unused enable_te()
>  drm/omap: dsi: drop useless sync()
>  drm/omap: dsi: use pixel-format and mode from attach
>  drm/omap: panel-dsi-cm: use bulk regulator API
>  drm/omap: dsi: lp/hs switching support for transfer()
>  drm/omap: dsi: move TE GPIO handling into core
>  drm/omap: dsi: drop custom enable_te() API
>  drm/omap: dsi: do bus locking in host driver
>  drm/omap: dsi: untangle ulps ops from enable/disable
>  drm/dsi: add MIPI_DSI_MODE_ULPS_IDLE
>  drm/omap: dsi: do ULPS in host driver
>  drm/omap: dsi: move panel refresh function to host
>  drm/omap: dsi: Reverse direction of the DSS device enable/disable
>    operations
>  drm/omap: dsi: drop custom panel capability support
>  drm/omap: dsi: convert to drm_panel
>  drm/omap: drop omapdss-boot-init
>  drm/omap: dsi: implement check timings
>  drm/omap: panel-dsi-cm: use DEVICE_ATTR_RO
>  drm/omap: panel-dsi-cm: support unbinding
>  drm/omap: panel-dsi-cm: fix remove()
>  drm/omap: dsi: return proper error code from dsi_update_all()
>  drm/omap: remove global dss_device variable
>  drm/omap: bind components with drm_device argument
>  drm/panel: Move OMAP's DSI command mode panel driver
>  drm/omap: dsi: Register a drm_bridge
>  drm/omap: remove legacy DSS device operations
>  drm/omap: remove unused omap_connector
>  drm/omap: simplify omap_display_id
>  drm/omap: drop unused DSS next pointer
>  drm/omap: drop empty omap_encoder helper functions
>  drm/omap: drop DSS ops_flags
>  drm/omap: drop dssdev display field
>  drm/omap: simplify DSI manual update code
>  ARM: omap2plus_defconfig: Update for moved DSI command mode panel
>  drm/panel/panel-dsi-cm: support rotation property
>  ARM: dts: omap4-droid4: add panel orientation
> 
> .../bindings/display/panel/panel-dsi-cm.txt   |    4 +-
> .../boot/dts/motorola-mapphone-common.dtsi    |    6 +-
> arch/arm/boot/dts/omap3-n950.dts              |    3 +-
> arch/arm/boot/dts/omap3.dtsi                  |    3 +
> arch/arm/boot/dts/omap4-sdp.dts               |    6 +-
> arch/arm/boot/dts/omap4.dtsi                  |    6 +
> arch/arm/boot/dts/omap5.dtsi                  |    6 +
> arch/arm/configs/omap2plus_defconfig          |    2 +-
> drivers/gpu/drm/omapdrm/Kconfig               |    1 -
> drivers/gpu/drm/omapdrm/Makefile              |    2 -
> drivers/gpu/drm/omapdrm/displays/Kconfig      |   10 -
> drivers/gpu/drm/omapdrm/displays/Makefile     |    2 -
> .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 1387 -----------------
> drivers/gpu/drm/omapdrm/dss/Kconfig           |    4 +-
> drivers/gpu/drm/omapdrm/dss/Makefile          |    2 -
> drivers/gpu/drm/omapdrm/dss/base.c            |   58 +-
> drivers/gpu/drm/omapdrm/dss/dsi.c             | 1034 +++++++-----
> drivers/gpu/drm/omapdrm/dss/dss.c             |   44 +-
> .../gpu/drm/omapdrm/dss/omapdss-boot-init.c   |  220 ---
> drivers/gpu/drm/omapdrm/dss/omapdss.h         |  141 +-
> drivers/gpu/drm/omapdrm/dss/output.c          |   13 +-
> drivers/gpu/drm/omapdrm/dss/venc.c            |    1 -
> drivers/gpu/drm/omapdrm/omap_connector.c      |  157 --
> drivers/gpu/drm/omapdrm/omap_connector.h      |   28 -
> drivers/gpu/drm/omapdrm/omap_crtc.c           |   35 +-
> drivers/gpu/drm/omapdrm/omap_drv.c            |   49 +-
> drivers/gpu/drm/omapdrm/omap_drv.h            |    1 -
> drivers/gpu/drm/omapdrm/omap_encoder.c        |   59 +-
> drivers/gpu/drm/panel/Kconfig                 |    9 +
> drivers/gpu/drm/panel/Makefile                |    1 +
> drivers/gpu/drm/panel/panel-dsi-cm.c          |  674 ++++++++
> include/drm/drm_mipi_dsi.h                    |    2 +
> 32 files changed, 1465 insertions(+), 2505 deletions(-)
> delete mode 100644 drivers/gpu/drm/omapdrm/displays/Kconfig
> delete mode 100644 drivers/gpu/drm/omapdrm/displays/Makefile
> delete mode 100644 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> delete mode 100644 drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> delete mode 100644 drivers/gpu/drm/omapdrm/omap_connector.c
> delete mode 100644 drivers/gpu/drm/omapdrm/omap_connector.h
> create mode 100644 drivers/gpu/drm/panel/panel-dsi-cm.c
> 
> 
> base-commit: 54ba965bb3873eca6098ddf04e3a8d7bba1b5557
> -- 
> 2.25.0
>