Hi Sam,
On Sat, Jun 27, 2020 at 09:55:39PM +0200, Sam Ravnborg wrote:
Hi Liu,
thanks for the notice.
Laurent, I trust you will take a look and resolve this.
I've just reviewed Liu's patches (and CC'ed you on 2/2).
On Thu, Jun 25, 2020 at 04:48:01PM +0800, Liu Ying wrote:
On Tue, 2020-06-23 at 20:55 +0200, Sam Ravnborg wrote:
On Tue, May 26, 2020 at 04:14:38AM +0300, Laurent Pinchart wrote:
Hello,
This patch series converts the R-Car DU driver to use the DRM bridge connector helper drm_bridge_connector_init().
The bulk of the series is conversion of the adv7511, simple-bridge, rcar-lbds and dw-hdmi drivers to make connector creation optional (through the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag).
The series starts with the adv7511 driver, previously posted as "[PATCH 0/4] drm: bridge: adv7511: Enable usage with DRM bridge connector helper" ([1]). Patches 01/27 to 04/27 incorporate all the received review comments.
The next three patches address the simple-bridge driver, previously posted as "[PATCH 0/2] drm: bridge: simple-bridge: Enable usage with DRM bridge connector helper". Patch 05/27 is an additional fix that stems from the review, and patches 06/27 and 07/27 incorporate all the received review comments.
Patch 08/27 is a new patch that addresses the rcar-lvds driver. Instead of implementing direct support for DRM_BRIDGE_ATTACH_NO_CONNECTOR, it simply removes code that shouldn't have been in the driver in the first place by switching to the panel bridge helper.
Patches 09/27 to 22/27 then address the dw-hdmi driver. That's a more sizeable rework, due to the fact that the driver implements a mid- layer for platform-specific glue, with the internal drm_connector being used throughout the whole code. There's no rocket science there, but patch 10/27 should be noted for adding a new argument to the drm_bridge_funcs.mode_valid() function. Please see individual patches for details.
Patch 23/27 adds support to the dw-hdmi driver to attach to a downstream bridge if one is specified in DT. As the DT port number corresponding to the video output differs between platforms that integrate the dw- hdmi (some of them even don't have a video output port, which should probably be fixed, but that's out of scope for this series), the port number has to be specified by the platform glue layer. Patch 24/27 does so for the R-Car dw-hdmi driver.
Patch 25/27 is a drive-by fix for an error path issue in the rcar- du driver. Patch 26/27 finally makes use of the drm_bridge_connector_init() helper.
Unfortunately, this breaks the VGA output on R-Car Gen3 platforms. On those platforms, the VGA DDC lines are not connected, and there is no mean for software to detect the VGA output connection status. When the simple-bridge driver creates a connector, it automatically adds default modes when no DDC is available. This behavious is also present int the DRM probe helper drm_helper_probe_single_connector_modes(), but only when the connector status is connector_status_connected. As the driver (rightfully) reports connector_status_unconnected, no modes are added. Patch 27/27 fixes this issue by extending addition of default modes in drm_helper_probe_single_connector_modes() when the output status is unknown. An alternative approach would be to implement this behaviour in the bridge connector helper (drm_bridge_connector.c).
All the modified drivers have been compile-tested, and the series has been tested on a Renesas R-Car Salvator-XS board with the VGA, HDMI and LVDS outputs.
[1] https://lore.kernel.org/dri-devel/20200409004610.12346-1-laurent.pinchart+re... [2] https://lore.kernel.org/dri-devel/20200409003636.11792-1-laurent.pinchart+re...
As we briefly discussed on IRC the first 21 patches are now applied to drm-misc-next.
I see patch '[22/27] drm: bridge: dw-hdmi: Make connector creation optional' is applied to drm-misc-next. That patch would introduce an uninitialized mutex accessing issue as I mentioned in the patch[1]. And, the patch intends to fix the issue in the first place.
[1] https://patchwork.freedesktop.org/patch/370560/
The rcar-du specific patches was left out and the last patch was likewise not applied in this round- mostly because it was the coming after several rcar-du patches and I was not sure if there was some dependencies to consider.
With this set in we have more examples in the tree how to do proper bridges which is good.
While applying the new r-bs was ofc added.
Laurent Pinchart (27): drm: bridge: adv7511: Split EDID read to a separate function drm: bridge: adv7511: Split connector creation to a separate function drm: bridge: adv7511: Implement bridge connector operations drm: bridge: adv7511: Make connector creation optional drm: bridge: Return NULL on error from drm_bridge_get_edid() drm: bridge: simple-bridge: Delegate operations to next bridge drm: bridge: simple-bridge: Make connector creation optional drm: rcar-du: lvds: Convert to DRM panel bridge helper drm: edid: Constify connector argument to infoframe functions drm: bridge: Pass drm_display_info to drm_bridge_funcs .mode_valid() drm: bridge: dw-hdmi: Pass private data pointer to .mode_valid() drm: bridge: dw-hdmi: Pass private data pointer to .configure_phy() drm: bridge: dw-hdmi: Remove unused field from dw_hdmi_plat_data drm: meson: dw-hdmi: Use dw_hdmi context to replace hack drm: bridge: dw-hdmi: Pass drm_display_info to .mode_valid() drm: bridge: dw-hdmi: Constify mode argument to dw_hdmi_phy_ops .init() drm: bridge: dw-hdmi: Constify mode argument to internal functions drm: bridge: dw-hdmi: Pass drm_display_info to dw_hdmi_support_scdc() drm: bridge: dw-hdmi: Split connector creation to a separate function drm: bridge: dw-hdmi: Store current connector in struct dw_hdmi drm: bridge: dw-hdmi: Pass drm_connector to internal functions as needed drm: bridge: dw-hdmi: Make connector creation optional drm: bridge: dw-hdmi: Attach to next bridge if available drm: rcar-du: dw-hdmi: Set output port number drm: rcar-du: Fix error handling in rcar_du_encoder_init() drm: rcar-du: Use drm_bridge_connector_init() helper drm: Add default modes for connectors in unknown state
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 159 +++++--- .../drm/bridge/analogix/analogix-anx6345.c | 1 + .../drm/bridge/analogix/analogix-anx78xx.c | 1 + drivers/gpu/drm/bridge/cdns-dsi.c | 1 + drivers/gpu/drm/bridge/chrontel-ch7033.c | 1 + drivers/gpu/drm/bridge/nwl-dsi.c | 1 + drivers/gpu/drm/bridge/sii9234.c | 1 + drivers/gpu/drm/bridge/sil-sii8620.c | 1 + drivers/gpu/drm/bridge/simple-bridge.c | 113 +++--- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 357 ++++++++++++------ drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 1 + drivers/gpu/drm/bridge/tc358767.c | 1 + drivers/gpu/drm/bridge/tc358768.c | 1 + drivers/gpu/drm/bridge/thc63lvd1024.c | 1 + drivers/gpu/drm/bridge/ti-tfp410.c | 11 +- drivers/gpu/drm/drm_atomic_helper.c | 3 +- drivers/gpu/drm/drm_bridge.c | 10 +- drivers/gpu/drm/drm_edid.c | 12 +- drivers/gpu/drm/drm_probe_helper.c | 7 +- drivers/gpu/drm/i2c/tda998x_drv.c | 1 + drivers/gpu/drm/imx/dw_hdmi-imx.c | 6 +- drivers/gpu/drm/meson/meson_dw_hdmi.c | 34 +- drivers/gpu/drm/omapdrm/dss/dpi.c | 1 + drivers/gpu/drm/omapdrm/dss/sdi.c | 1 + drivers/gpu/drm/omapdrm/dss/venc.c | 1 + drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 26 +- drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 7 +- drivers/gpu/drm/rcar-du/rcar_lvds.c | 124 +----- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 6 +- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 3 +- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 3 +- include/drm/bridge/dw_hdmi.h | 28 +- include/drm/drm_bridge.h | 3 + include/drm/drm_edid.h | 6 +- include/drm/drm_modeset_helper_vtables.h | 8 +- 36 files changed, 541 insertions(+), 406 deletions(-)