mbox series

[v2,00/12] drm/imx/ipuv3: switch LDB and parallel-display driver to use drm_bridge_connector

Message ID 20240331-drm-imx-cleanup-v2-0-d81c1d1c1026@linaro.org
Headers show
Series drm/imx/ipuv3: switch LDB and parallel-display driver to use drm_bridge_connector | expand

Message

Dmitry Baryshkov March 31, 2024, 8:28 p.m. UTC
The IPUv3 DRM i.MX driver contains several codepaths for different
usescases: both LDB and paralllel-display drivers handle next-bridge,
panel and the legacy display-timings DT node on their own.

Drop unused ddc-i2c-bus and edid handling (none of the DT files merged
upstream ever used these features), switch to panel-bridge driver,
removing the need to handle drm_panel codepaths separately and finally
switch to drm_bridge_connector, removing requirement for the downstream
bridges to create drm_connector on their own.

This has been tested on the iMX53 with the DPI panel attached to LDB via
LVDS decoder, using all possible usecases (lvds-codec + panel, panel
linked directly to LDB node and the display-timings node).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Changes in v2:
- Fixed drm_bridge_attach flags in imx/parallel-display driver.
- Moved the legacy bridge to drivers/gpu/drm/bridge
- Added missing EXPORT_SYMBOL_GPL to the iMX legacy bridge
- Link to v1: https://lore.kernel.org/r/20240311-drm-imx-cleanup-v1-0-e104f05caa51@linaro.org

---
Dmitry Baryshkov (12):
      dt-bindings: display: fsl-imx-drm: drop edid property support
      dt-bindings: display: imx/ldb: drop ddc-i2c-bus property
      drm/imx: cleanup the imx-drm header
      drm/imx: parallel-display: drop edid override support
      drm/imx: ldb: drop custom EDID support
      drm/imx: ldb: drop custom DDC bus support
      drm/imx: ldb: switch to drm_panel_bridge
      drm/imx: parallel-display: switch to drm_panel_bridge
      drm/imx: add internal bridge handling display-timings DT node
      drm/imx: ldb: switch to imx_legacy_bridge / drm_bridge_connector
      drm/imx: parallel-display: switch to imx_legacy_bridge / drm_bridge_connector
      drm/imx: move imx_drm_connector_destroy to imx-tve

 .../bindings/display/imx/fsl-imx-drm.txt           |   2 -
 .../devicetree/bindings/display/imx/ldb.txt        |   1 -
 drivers/gpu/drm/bridge/imx/Kconfig                 |  10 +
 drivers/gpu/drm/bridge/imx/Makefile                |   1 +
 drivers/gpu/drm/bridge/imx/imx-legacy-bridge.c     |  85 +++++++++
 drivers/gpu/drm/imx/ipuv3/Kconfig                  |   5 +
 drivers/gpu/drm/imx/ipuv3/imx-drm-core.c           |   7 -
 drivers/gpu/drm/imx/ipuv3/imx-drm.h                |  14 --
 drivers/gpu/drm/imx/ipuv3/imx-ldb.c                | 202 +++++----------------
 drivers/gpu/drm/imx/ipuv3/imx-tve.c                |   8 +-
 drivers/gpu/drm/imx/ipuv3/parallel-display.c       | 137 +++-----------
 include/drm/bridge/imx.h                           |  13 ++
 12 files changed, 184 insertions(+), 301 deletions(-)
---
base-commit: 13ee4a7161b6fd938aef6688ff43b163f6d83e37
change-id: 20240310-drm-imx-cleanup-10746a9b71f5

Best regards,

Comments

Dmitry Baryshkov April 16, 2024, 9:58 p.m. UTC | #1
On Sun, Mar 31, 2024 at 11:28:57PM +0300, Dmitry Baryshkov wrote:
> The IPUv3 DRM i.MX driver contains several codepaths for different
> usescases: both LDB and paralllel-display drivers handle next-bridge,
> panel and the legacy display-timings DT node on their own.
> 
> Drop unused ddc-i2c-bus and edid handling (none of the DT files merged
> upstream ever used these features), switch to panel-bridge driver,
> removing the need to handle drm_panel codepaths separately and finally
> switch to drm_bridge_connector, removing requirement for the downstream
> bridges to create drm_connector on their own.
> 
> This has been tested on the iMX53 with the DPI panel attached to LDB via
> LVDS decoder, using all possible usecases (lvds-codec + panel, panel
> linked directly to LDB node and the display-timings node).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> Changes in v2:
> - Fixed drm_bridge_attach flags in imx/parallel-display driver.
> - Moved the legacy bridge to drivers/gpu/drm/bridge
> - Added missing EXPORT_SYMBOL_GPL to the iMX legacy bridge
> - Link to v1: https://lore.kernel.org/r/20240311-drm-imx-cleanup-v1-0-e104f05caa51@linaro.org

Phillip, Shawn, Sascha, any comments on this patchset?
Philipp Zabel May 27, 2024, 3:40 p.m. UTC | #2
Hi Dmitry,

On Mi, 2024-04-17 at 00:58 +0300, Dmitry Baryshkov wrote:
> On Sun, Mar 31, 2024 at 11:28:57PM +0300, Dmitry Baryshkov wrote:
> > The IPUv3 DRM i.MX driver contains several codepaths for different
> > usescases: both LDB and paralllel-display drivers handle next-bridge,
> > panel and the legacy display-timings DT node on their own.
> > 
> > Drop unused ddc-i2c-bus and edid handling (none of the DT files merged
> > upstream ever used these features), switch to panel-bridge driver,
> > removing the need to handle drm_panel codepaths separately and finally
> > switch to drm_bridge_connector, removing requirement for the downstream
> > bridges to create drm_connector on their own.
> > 
> > This has been tested on the iMX53 with the DPI panel attached to LDB via
> > LVDS decoder, using all possible usecases (lvds-codec + panel, panel
> > linked directly to LDB node and the display-timings node).
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> > Changes in v2:
> > - Fixed drm_bridge_attach flags in imx/parallel-display driver.
> > - Moved the legacy bridge to drivers/gpu/drm/bridge
> > - Added missing EXPORT_SYMBOL_GPL to the iMX legacy bridge
> > - Link to v1: https://lore.kernel.org/r/20240311-drm-imx-cleanup-v1-0-e104f05caa51@linaro.org
> 
> Phillip, Shawn, Sascha, any comments on this patchset?

Sorry for the delay, this all looks sane to me. I can't find any users
of the "edid" and "ddc-i2c-bus" properties either. But let me test on
i.MX6 and with parallel-display as well.

regards
Philipp
Dmitry Baryshkov May 28, 2024, 12:51 a.m. UTC | #3
On Mon, May 27, 2024 at 05:40:03PM +0200, Philipp Zabel wrote:
> Hi Dmitry,
> 
> On Mi, 2024-04-17 at 00:58 +0300, Dmitry Baryshkov wrote:
> > On Sun, Mar 31, 2024 at 11:28:57PM +0300, Dmitry Baryshkov wrote:
> > > The IPUv3 DRM i.MX driver contains several codepaths for different
> > > usescases: both LDB and paralllel-display drivers handle next-bridge,
> > > panel and the legacy display-timings DT node on their own.
> > > 
> > > Drop unused ddc-i2c-bus and edid handling (none of the DT files merged
> > > upstream ever used these features), switch to panel-bridge driver,
> > > removing the need to handle drm_panel codepaths separately and finally
> > > switch to drm_bridge_connector, removing requirement for the downstream
> > > bridges to create drm_connector on their own.
> > > 
> > > This has been tested on the iMX53 with the DPI panel attached to LDB via
> > > LVDS decoder, using all possible usecases (lvds-codec + panel, panel
> > > linked directly to LDB node and the display-timings node).
> > > 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > > Changes in v2:
> > > - Fixed drm_bridge_attach flags in imx/parallel-display driver.
> > > - Moved the legacy bridge to drivers/gpu/drm/bridge
> > > - Added missing EXPORT_SYMBOL_GPL to the iMX legacy bridge
> > > - Link to v1: https://lore.kernel.org/r/20240311-drm-imx-cleanup-v1-0-e104f05caa51@linaro.org
> > 
> > Phillip, Shawn, Sascha, any comments on this patchset?
> 
> Sorry for the delay, this all looks sane to me. I can't find any users
> of the "edid" and "ddc-i2c-bus" properties either. But let me test on
> i.MX6 and with parallel-display as well.

At some point Chris dropped me a note that he had issues with i.MX5 /
sii902x driver, but because of the Connect I wasn't able to fully triage
it. I hope to have time this week.
Philipp Zabel May 29, 2024, 2:58 p.m. UTC | #4
On So, 2024-03-31 at 23:29 +0300, Dmitry Baryshkov wrote:
> Defer panel handling to drm_panel_bridge, unifying codepaths for the
> panel and bridge cases.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/imx/ipuv3/Kconfig            |  2 ++
>  drivers/gpu/drm/imx/ipuv3/parallel-display.c | 35 +++++++---------------------
>  2 files changed, 10 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3/Kconfig b/drivers/gpu/drm/imx/ipuv3/Kconfig
> index 4e41611c8532..8aaf2441bcef 100644
> --- a/drivers/gpu/drm/imx/ipuv3/Kconfig
> +++ b/drivers/gpu/drm/imx/ipuv3/Kconfig
> @@ -13,6 +13,8 @@ config DRM_IMX_PARALLEL_DISPLAY
>  	tristate "Support for parallel displays"
>  	select DRM_PANEL

This shouldn't be necessary anymore either.

>  	depends on DRM_IMX
> +	depends on DRM_BRIDGE

I'd prefer this to select DRM_BRIDGE.

> +	select DRM_PANEL_BRIDGE
>  	select VIDEOMODE_HELPERS
>  
>  config DRM_IMX_TVE
> diff --git a/drivers/gpu/drm/imx/ipuv3/parallel-display.c b/drivers/gpu/drm/imx/ipuv3/parallel-display.c
> index 4d17fb96e77c..b7743b30475a 100644
> --- a/drivers/gpu/drm/imx/ipuv3/parallel-display.c
> +++ b/drivers/gpu/drm/imx/ipuv3/parallel-display.c

You can drop the "#include <drm/panel.h>".


regards
Philipp
Philipp Zabel May 29, 2024, 2:58 p.m. UTC | #5
On So, 2024-03-31 at 23:29 +0300, Dmitry Baryshkov wrote:
> Defer panel handling to drm_panel_bridge, unifying codepaths for the
> panel and bridge cases.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/imx/ipuv3/Kconfig   |  2 ++
>  drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 44 ++++++++++++-------------------------
>  2 files changed, 16 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3/Kconfig b/drivers/gpu/drm/imx/ipuv3/Kconfig
> index bacf0655ebaf..4e41611c8532 100644
> --- a/drivers/gpu/drm/imx/ipuv3/Kconfig
> +++ b/drivers/gpu/drm/imx/ipuv3/Kconfig
> @@ -28,7 +28,9 @@ config DRM_IMX_LDB
>  	tristate "Support for LVDS displays"
>  	depends on DRM_IMX && MFD_SYSCON
>  	depends on COMMON_CLK
> +	depends on DRM_BRIDGE

To avoid hiding the DRM_IMX_LDB option for legacy configurations with
DRM_BRIDGE disabled, it would be better to select DRM_BRIDGE here
instead.

>  	select DRM_PANEL

And this shouldn't be necessary anymore.

regards
Philipp
Philipp Zabel May 29, 2024, 2:58 p.m. UTC | #6
On So, 2024-03-31 at 23:29 +0300, Dmitry Baryshkov wrote:
> Use the imx_legacy bridge driver instead of handlign display modes via
> the connector node.
> 
> All existing usecases already support attaching using
> the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, while the imx_legacy bridge
> doesn't support creating connector at all. Switch to
> drm_bridge_connector at the same time.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/imx/ipuv3/parallel-display.c | 99 ++++++----------------------
>  1 file changed, 19 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3/parallel-display.c b/drivers/gpu/drm/imx/ipuv3/parallel-display.c
> index b7743b30475a..cf29021f497e 100644
> --- a/drivers/gpu/drm/imx/ipuv3/parallel-display.c
> +++ b/drivers/gpu/drm/imx/ipuv3/parallel-display.c
> @@ -12,10 +12,9 @@
>  #include <linux/platform_device.h>
>  #include <linux/videodev2.h>
>  
> -#include <video/of_display_timing.h>
> -
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/drm_managed.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_panel.h>

This is missing "#include <drm/bridge/imx.h>".

regards
Philipp
Philipp Zabel May 29, 2024, 2:59 p.m. UTC | #7
On So, 2024-03-31 at 23:29 +0300, Dmitry Baryshkov wrote:
> Drop unused defines and obsolete prototypes from the imx-drm.h header.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
Philipp Zabel May 29, 2024, 2:59 p.m. UTC | #8
On So, 2024-03-31 at 23:29 +0300, Dmitry Baryshkov wrote:
> None of the in-kernel DT files ever used edid override with the
> fsl-imx-drm driver. In case the EDID needs to be specified manually, DRM
> core allows one to either override it via the debugfs or to load it via
> request_firmware by using DRM_LOAD_EDID_FIRMWARE. In all other cases
> EDID and/or modes are to be provided as a part of the panel driver.
> 
> Drop support for the edid property.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
Philipp Zabel May 29, 2024, 2:59 p.m. UTC | #9
On So, 2024-03-31 at 23:29 +0300, Dmitry Baryshkov wrote:
> Bindings for the imx-ldb never allowed specifying the EDID in DT. None
> of the existing DT files use it. Drop it now in favour of using debugfs
> overrides or the drm.edid_firmware support.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
Philipp Zabel May 29, 2024, 3 p.m. UTC | #10
On So, 2024-03-31 at 23:29 +0300, Dmitry Baryshkov wrote:
> None of the boards ever supported by the upstream kernel used the custom
> DDC bus support with the LDB connector. If a need arises to do so, one
> should use panel-simple and its DDC bus code. Drop ddc-i2c-bus support
> from the imx-ldb driver.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp