Re: [PATCH] arm64: dts: rockchip: Drop data-mapping from Engicam PX30 Core

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Removing the `data-mapping` property causes a regression in the Rockchip LVDS controller, silently defaulting the PHY format to 18-bit VESA instead of 24-bit VESA.

Pre-existing issues:
- [High] The `px30_lvds_grf_config` function incorrectly passes a dynamic value as the mask to `regmap_update_bits`, preventing it from clearing format configuration bits.
--

--- Patch [1]: [PATCH] arm64: dts: rockchip: Drop data-mapping from Engicam PX30 Core ---
commit 9e01384e585886fe7dfe0bda8fec7f830eade509
Author: Fabio Estevam <festevam@gmail.com>

arm64: dts: rockchip: Drop data-mapping from Engicam PX30 Core

The commit removes the data-mapping property from the panel node to satisfy
the panel-simple DT schema. The commit message states this has no functional
effect because the DRM OF helpers translate the bus format, falling back to
the descriptor's format when the property is absent.

> diff --git a/arch/arm64/boot/dts/rockchip/px30-engicam-px30-core-ctouch2-of10.dts b/arch/arm64/boot/dts/rockchip/px30-engicam-px30-core-ctouch2-of10.dts
> index 47aa30505a42e..aa636ce45e3a9 100644
> --- a/arch/arm64/boot/dts/rockchip/px30-engicam-px30-core-ctouch2-of10.dts
> +++ b/arch/arm64/boot/dts/rockchip/px30-engicam-px30-core-ctouch2-of10.dts
> @@ -28,7 +28,6 @@ panel {
>  		compatible = "ampire,am-1280800n3tzqw-t00h";
>  		backlight = <&backlight>;
>  		power-supply = <&vcc3v3_lcd>;
> -		data-mapping = "vesa-24";

[Severity: High]
Does removing this property inadvertently break the host LVDS controller
configuration?

The commit message assumes only the panel-simple driver uses this property,
but the rockchip LVDS host controller driver bypasses the framework and
reads it directly from the remote panel device tree node:

drivers/gpu/drm/rockchip/rockchip_lvds.c:rockchip_lvds_bind() {
    ...
    if (of_property_read_string(remote, "data-mapping", &name))
        /* default set it as format vesa 18 */
        lvds->format = LVDS_VESA_18;
    ...
}

By removing the property, will this fallback to LVDS_VESA_18 be triggered,
causing a mismatch with the 24-bit VESA panel and corrupting the display
output?

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the rockchip
LVDS driver's format configuration, I noticed a potential issue with how
the register bits are cleared:

drivers/gpu/drm/rockchip/rockchip_lvds.c:px30_lvds_grf_config() {
    ...
    return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
                              PX30_LVDS_FORMAT(lvds->format),
                              PX30_LVDS_FORMAT(lvds->format));
}

Does this incorrectly pass the dynamic value PX30_LVDS_FORMAT(lvds->format)
as both the mask and the value?

Because regmap_update_bits() uses the mask argument to determine which
bits to clear in the lower 16 bits, passing the value as the mask means any
0-bits in the format will be 0 in the mask and ignored instead of cleared. 

If the bootloader left the format in a different state (e.g., LVDS_JEIDA_18),
will the kernel fail to clear the necessary bits to switch it to LVDS_VESA_24?

>  
>  		port {
>  			panel_in_lvds: endpoint {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727175502.2585290-1-festevam@gmail.com?part=1