* Re: [PATCH v13 12/27] drm/i915/dp: Add YCBCR444 handling for sink formats
From: Ville Syrjälä @ 2026-04-13 19:08 UTC (permalink / raw)
To: Nicolas Frattaroli
Cc: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan, kernel, amd-gfx, dri-devel,
linux-kernel, linux-arm-kernel, linux-rockchip, intel-gfx,
intel-xe, linux-doc
In-Reply-To: <20260413-color-format-v13-12-ab37d4dfba48@collabora.com>
On Mon, Apr 13, 2026 at 12:07:26PM +0200, Nicolas Frattaroli wrote:
> In anticipation of userspace being able to explicitly select supported
> sink formats, add handling of the YCBCR444 sink format. The AUTO path
> does not choose this format, but with explicit format selection added to
> the driver, it becomes a possibility.
>
> Check for sink support of YCBCR444 to intel_dp_sink_format_valid.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 35b8fb5740aa..47bd3d59ea93 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1364,6 +1364,11 @@ intel_dp_sink_format_valid(struct intel_connector *connector,
>
> return MODE_OK;
> case INTEL_OUTPUT_FORMAT_RGB:
> + return MODE_OK;
> + case INTEL_OUTPUT_FORMAT_YCBCR444:
> + if (!(info->color_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444)))
> + return MODE_BAD;
The DP situation is a lot more more fuzzy than the HDMI situation
due to the PCON stuff. So I'm not quite sure what we should do here.
At the very least I think we want the equivalent of
intel_dp_can_ycbcr420() for 444, and the same intel_dp_has_hdmi_sink()
check that we have for 420.
> +
> return MODE_OK;
> default:
> MISSING_CASE(sink_format);
>
> --
> 2.53.0
--
Ville Syrjälä
Intel
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH V3 2/9] iio: imu: inv_icm42607: Add Core for inv_icm42607 Driver
From: Jonathan Cameron @ 2026-04-13 19:06 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
andriy.shevchenko, Chris Morgan
In-Reply-To: <20260330195853.392877-3-macroalpha82@gmail.com>
On Mon, 30 Mar 2026 14:58:46 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add the core component of a new inv_icm42607 driver. This includes
> a few setup functions and the full register definition in the
> header file.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Hi Chris,
I've avoided repeating a few things David raised, but there may
well be overlap on some things. I'm not particularly keen on code
that doesn't build at the end of a patch or even provide the Kconfig
and makefile stuff to do so. Maybe it's too much effort given how you have
this structured.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_icm42607/inv_icm42607.h | 424 ++++++++++++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_core.c | 300 +++++++++++++
> 2 files changed, 724 insertions(+)
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607.h
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> new file mode 100644
> index 000000000000..609188c40ffc
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> @@ -0,0 +1,424 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#ifndef INV_ICM42607_H_
> +#define INV_ICM42607_H_
> +
> +#include <linux/bitops.h>
> +#include <linux/bitfield.h>
> +#include <linux/regmap.h>
> +#include <linux/mutex.h>
> +#include <linux/regulator/consumer.h>
Alphabetical order. Though fine to have an IIO specific block at the end.
> +#include <linux/iio/iio.h>
> +#include <linux/iio/common/inv_sensors_timestamp.h>
> +/* ODR values */
> +enum inv_icm42607_odr {
> + INV_ICM42607_ODR_1600HZ = 5,
> + INV_ICM42607_ODR_800HZ,
> + INV_ICM42607_ODR_400HZ,
> + INV_ICM42607_ODR_200HZ,
> + INV_ICM42607_ODR_100HZ,
> + INV_ICM42607_ODR_50HZ,
> + INV_ICM42607_ODR_25HZ,
> + INV_ICM42607_ODR_12_5HZ,
> + INV_ICM42607_ODR_6_25HZ_LP,
> + INV_ICM42607_ODR_3_125HZ_LP,
> + INV_ICM42607_ODR_1_5625HZ_LP,
> + INV_ICM42607_ODR_NB,
If just here as number, then no terminating comma as
we don't want to imply something might come after it.
Same for all the other enums.
> +};
> +
> +enum inv_icm42607_filter {
> + /* Low-Noise mode sensor data filter */
> + INV_ICM42607_FILTER_BYPASS,
> + INV_ICM42607_FILTER_BW_180HZ,
> + INV_ICM42607_FILTER_BW_121HZ,
> + INV_ICM42607_FILTER_BW_73HZ,
> + INV_ICM42607_FILTER_BW_53HZ,
> + INV_ICM42607_FILTER_BW_34HZ,
> + INV_ICM42607_FILTER_BW_25HZ,
> + INV_ICM42607_FILTER_BW_16HZ,
> +
> + /* Low-Power mode sensor data filter (averaging) */
> + INV_ICM42607_FILTER_AVG_2X = 0,
> + INV_ICM42607_FILTER_AVG_4X,
> + INV_ICM42607_FILTER_AVG_8X,
> + INV_ICM42607_FILTER_AVG_16X,
> + INV_ICM42607_FILTER_AVG_32X,
> + INV_ICM42607_FILTER_AVG_64X,
Bring these in when used. For now these to me look like they should be broken
into two separate enums. I can't see how the will be used though so hard
to tell.
> +};
>
> +#define INV_ICM42607_INTF_CONFIG1_CLKSEL_MASK GENMASK(1, 0)
> +#define INV_ICM42607_INTF_CONFIG1_CLKSEL_PLL \
> + FIELD_PREP(INV_ICM42607_INTF_CONFIG1_CLKSEL_MASK, 1)
Define the value of the field to give that a name and use FIELD_PREP() inline.
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> new file mode 100644
> index 000000000000..6b7078387568
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -0,0 +1,300 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
Most likely this should be more specific headers like
dev_printk.h etc It is pretty rare it makes sense to include
device.h if following IWYU approach (which we do for IIO drivers
and is generally accepted across the kernel).
> +#include <linux/module.h>
Alphabetical order.
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/mutex.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/iio/iio.h>
> +u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr)
> +{
> + static u32 odr_periods[INV_ICM42607_ODR_NB] = {
Hopefully the compiler can figure it out, but better
as explicit const.
> + /* Reserved values */
> + 0, 0, 0, 0, 0,
> + /* 1600Hz */
> + 625000,
> + /* 800Hz */
> + 1250000,
> + /* 400Hz */
> + 2500000,
> + /* 200Hz */
> + 5000000,
> + /* 100 Hz */
> + 10000000,
> + /* 50Hz */
> + 20000000,
> + /* 25Hz */
> + 40000000,
> + /* 12.5Hz */
> + 80000000,
> + /* 6.25Hz */
> + 160000000,
> + /* 3.125Hz */
> + 320000000,
> + /* 1.5625Hz */
> + 640000000,
> + };
> +
> + return odr_periods[odr];
> +}
> +
> +static int inv_icm42607_set_conf(struct inv_icm42607_state *st,
> + const struct inv_icm42607_conf *conf)
> +{
> + unsigned int val;
> + int ret;
> +
> + val = INV_ICM42607_PWR_MGMT0_GYRO(conf->gyro.mode) |
> + INV_ICM42607_PWR_MGMT0_ACCEL(conf->accel.mode);
Align as
val = INV_ICM42607_PWR_MGMT0_GYRO(conf->gyro.mode) |
INV_ICM42607_PWR_MGMT0_ACCEL(conf->accel.mode);
Though as mentioned in David's review, prefer to see the FIELD_PREP()
inline.
> + /*
> + * No temperature enable reg in datasheet, but BSP driver
> + * selected RC oscillator clock in LP mode when temperature
> + * was disabled.
> + */
> + if (!conf->temp_en)
> + val |= INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL;
Could make this
val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL,
!conf->temp_en);
Not particularly important though if you prefer the if.
> + ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> + if (ret)
> + return ret;
> +
> + val = INV_ICM42607_GYRO_CONFIG0_FS_SEL(conf->gyro.fs) |
> + INV_ICM42607_GYRO_CONFIG0_ODR(conf->gyro.odr);
As above. If it's the second line of a statement, it must be indented
to avoid readability issues. Fix all examples of this.
> + ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
> + if (ret)
> + return ret;
> +
> + val = INV_ICM42607_ACCEL_CONFIG0_FS_SEL(conf->accel.fs) |
> + INV_ICM42607_ACCEL_CONFIG0_ODR(conf->accel.odr);
> + ret = regmap_write(st->map, INV_ICM42607_REG_ACCEL_CONFIG0, val);
> + if (ret)
> + return ret;
> +
> + val = INV_ICM42607_GYRO_CONFIG1_FILTER(conf->gyro.filter);
> + ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG1, val);
> + if (ret)
> + return ret;
> +
> + val = INV_ICM42607_ACCEL_CONFIG1_FILTER(conf->accel.filter);
> + ret = regmap_write(st->map, INV_ICM42607_REG_ACCEL_CONFIG1, val);
> + if (ret)
> + return ret;
> +
> + st->conf = *conf;
> +
> + return 0;
> +}
> +
> +/**
> + * inv_icm42607_setup() - check and setup chip
> + * @st: driver internal state
> + * @bus_setup: callback for setting up bus specific registers
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + */
> +static int inv_icm42607_setup(struct inv_icm42607_state *st,
> + inv_icm42607_bus_setup bus_setup)
> +{
> + const struct inv_icm42607_hw *hw = &inv_icm42607_hw[st->chip];
> + const struct device *dev = regmap_get_device(st->map);
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(st->map, INV_ICM42607_REG_WHOAMI, &val);
> + if (ret)
> + return ret;
> +
> + if (val != hw->whoami)
> + dev_warn_probe(dev, -ENODEV,
> + "invalid whoami %#02x expected %#02x (%s)\n",
> + val, hw->whoami, hw->name);
> +
> + st->name = hw->name;
> +
> + ret = regmap_write(st->map, INV_ICM42607_REG_SIGNAL_PATH_RESET,
> + INV_ICM42607_SIGNAL_PATH_RESET_SOFT_RESET);
> + if (ret)
> + return ret;
> + msleep(INV_ICM42607_RESET_TIME_MS);
fsleep() here as sleeping longer is probably fine and that function will
apply appropriate slack on the times.
> +
> + ret = regmap_read(st->map, INV_ICM42607_REG_INT_STATUS, &val);
> + if (ret)
> + return ret;
> + if (!(val & INV_ICM42607_INT_STATUS_RESET_DONE))
> + return dev_err_probe(dev, -ENODEV,
> + "reset error, reset done bit not set\n");
> +
> + ret = bus_setup(st);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG0,
> + INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN,
> + INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG1,
> + INV_ICM42607_INTF_CONFIG1_CLKSEL_MASK,
> + INV_ICM42607_INTF_CONFIG1_CLKSEL_PLL);
> + if (ret)
> + return ret;
> +
> + return inv_icm42607_set_conf(st, hw->conf);
> +}
> +
> +static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
> +{
> + int ret;
> +
> + ret = regulator_enable(st->vddio_supply);
> + if (ret)
> + return ret;
> +
> + usleep_range(3000, 4000);
David covered this, fsleep() preferred.
> +
> + return 0;
> +}
> +
> +int inv_icm42607_core_probe(struct regmap *regmap, int chip,
> + inv_icm42607_bus_setup bus_setup)
> +{
> + struct device *dev = regmap_get_device(regmap);
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + struct inv_icm42607_state *st;
> + int irq, irq_type;
> + bool open_drain;
> + int ret;
> +
> + if (chip < INV_CHIP_INVALID || chip >= INV_CHIP_NB)
> + dev_warn_probe(dev, -ENODEV,
> + "Invalid chip = %d\n", chip);
Easily fits on one line shorter than 80 chars.
> +
> + /* get INT1 only supported interrupt or fallback to first interrupt */
Why the fallback? I suspect this is copied from a driver
that didn't (bug) support named interrupts from the start. Given you are
doing so here, just insist on named one to simplify things. I'd imagine
that if INT2 is ever supported, the device configuration will need to be
different.
> + irq = fwnode_irq_get_byname(fwnode, "INT1");
> + if (irq < 0 && irq != -EPROBE_DEFER) {
> + dev_info(dev, "no INT1 interrupt defined, fallback to first interrupt\n");
> + irq = fwnode_irq_get(fwnode, 0);
> + }
> + if (irq < 0)
> + return dev_err_probe(dev, irq, "error missing INT1 interrupt\n");
> +
> + irq_type = irq_get_trigger_type(irq);
> + if (!irq_type)
> + irq_type = IRQF_TRIGGER_FALLING;
Not used at this point. Bring it in when you use it so we can understand
the default setting.
> +
> + open_drain = device_property_read_bool(dev, "drive-open-drain");
> +
> + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> +
> + dev_set_drvdata(dev, st);
> + mutex_init(&st->lock);
For new code
ret = devm_mutex_init(&st->lock);
if (ret)
return ret;
Brings a very small advantage if someone is debugging locks.
> + st->chip = chip;
> + st->map = regmap;
> + st->irq = irq;
> +
> + ret = iio_read_mount_matrix(dev, &st->orientation);
> + if (ret) {
> + dev_err(dev, "failed to retrieve mounting matrix %d\n", ret);
> + return ret;
return dev_err_probe();
> + }
> +
> + ret = devm_regulator_get_enable(dev, "vdd");
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to get vdd regulator\n");
> +
> + msleep(INV_ICM42607_POWER_UP_TIME_MS);
> +
> + st->vddio_supply = devm_regulator_get(dev, "vddio");
> + if (IS_ERR(st->vddio_supply))
> + return PTR_ERR(st->vddio_supply);
> +
> + ret = inv_icm42607_enable_vddio_reg(st);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev, inv_icm42607_disable_vddio_reg, st);
> + if (ret)
> + return ret;
> +
> + /* Setup chip registers (includes WHOAMI check, reset check, bus setup) */
> + ret = inv_icm42607_setup(st, bus_setup);
> +
> + return ret;
Looking forwards I would instead do:
if (ret)
return ret;
return 0;
I think that makes it easier to add the additional pm stuff in later patches.
> +}
> +EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH v13 11/27] drm/i915/hdmi: Add YCBCR444 handling for sink formats
From: Ville Syrjälä @ 2026-04-13 19:04 UTC (permalink / raw)
To: Nicolas Frattaroli
Cc: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan, kernel, amd-gfx, dri-devel,
linux-kernel, linux-arm-kernel, linux-rockchip, intel-gfx,
intel-xe, linux-doc
In-Reply-To: <ad08zqpKbyF--Br3@intel.com>
On Mon, Apr 13, 2026 at 09:58:22PM +0300, Ville Syrjälä wrote:
> On Mon, Apr 13, 2026 at 12:07:25PM +0200, Nicolas Frattaroli wrote:
> > In anticipation of userspace being able to explicitly select supported
> > sink formats, add handling of the YCBCR444 sink format. The AUTO path
> > does not choose this format, but with explicit format selection added to
> > the driver, it becomes a possibility.
> >
> > Check for YCBCR444 support on the sink in both sink_bpc_possible, and
> > sink_format_valid.
> >
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_hdmi.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index 874076a29da4..5ab5b5f85cde 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -1966,6 +1966,8 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *_connector,
> >
> > if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> > return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36;
> > + else if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444)
> > + return info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_36;
> > else
> > return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36;
> > case 10:
> > @@ -1974,6 +1976,8 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *_connector,
> >
> > if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> > return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_30;
> > + else if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444)
> > + return info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_30;
> > else
> > return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30;
> > case 8:
> > @@ -2038,6 +2042,11 @@ intel_hdmi_sink_format_valid(struct intel_connector *connector,
> >
> > return MODE_OK;
> > case INTEL_OUTPUT_FORMAT_RGB:
> > + return MODE_OK;
> > + case INTEL_OUTPUT_FORMAT_YCBCR444:
>
> We'll also want the !has_hdmi_sink check here like for 4:2:0.
>
> And I think we also want something to mirror the ycbcr_420_allowed
> flag. I guess you could just make it something like:
>
> intel_hdmi_ycbcr_444_allowed(display)
> {
> return DISPLAY_VER(display) >= 5 && !HAS_GMCH(display);
Actually the display version check is redundant there.
!HAS_GMCH alone is sufficient.
> }
>
> That can also be reused when setting up the allowed property values.
>
> > + if (!(info->color_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444)))
> > + return MODE_BAD;
> > +
> > return MODE_OK;
> > default:
> > MISSING_CASE(sink_format);
> >
> > --
> > 2.53.0
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH v13 11/27] drm/i915/hdmi: Add YCBCR444 handling for sink formats
From: Ville Syrjälä @ 2026-04-13 18:58 UTC (permalink / raw)
To: Nicolas Frattaroli
Cc: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan, kernel, amd-gfx, dri-devel,
linux-kernel, linux-arm-kernel, linux-rockchip, intel-gfx,
intel-xe, linux-doc
In-Reply-To: <20260413-color-format-v13-11-ab37d4dfba48@collabora.com>
On Mon, Apr 13, 2026 at 12:07:25PM +0200, Nicolas Frattaroli wrote:
> In anticipation of userspace being able to explicitly select supported
> sink formats, add handling of the YCBCR444 sink format. The AUTO path
> does not choose this format, but with explicit format selection added to
> the driver, it becomes a possibility.
>
> Check for YCBCR444 support on the sink in both sink_bpc_possible, and
> sink_format_valid.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
> drivers/gpu/drm/i915/display/intel_hdmi.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 874076a29da4..5ab5b5f85cde 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1966,6 +1966,8 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *_connector,
>
> if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36;
> + else if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444)
> + return info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_36;
> else
> return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36;
> case 10:
> @@ -1974,6 +1976,8 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *_connector,
>
> if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> return hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_30;
> + else if (sink_format == INTEL_OUTPUT_FORMAT_YCBCR444)
> + return info->edid_hdmi_ycbcr444_dc_modes & DRM_EDID_HDMI_DC_30;
> else
> return info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30;
> case 8:
> @@ -2038,6 +2042,11 @@ intel_hdmi_sink_format_valid(struct intel_connector *connector,
>
> return MODE_OK;
> case INTEL_OUTPUT_FORMAT_RGB:
> + return MODE_OK;
> + case INTEL_OUTPUT_FORMAT_YCBCR444:
We'll also want the !has_hdmi_sink check here like for 4:2:0.
And I think we also want something to mirror the ycbcr_420_allowed
flag. I guess you could just make it something like:
intel_hdmi_ycbcr_444_allowed(display)
{
return DISPLAY_VER(display) >= 5 && !HAS_GMCH(display);
}
That can also be reused when setting up the allowed property values.
> + if (!(info->color_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444)))
> + return MODE_BAD;
> +
> return MODE_OK;
> default:
> MISSING_CASE(sink_format);
>
> --
> 2.53.0
--
Ville Syrjälä
Intel
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH v13 00/27] Add new general DRM property "color format"
From: Ville Syrjälä @ 2026-04-13 18:45 UTC (permalink / raw)
To: Nicolas Frattaroli
Cc: Heiko Stübner, linux-doc, Joonas Lahtinen, dri-devel,
linux-kernel, Andri Yngvason, Laurent Pinchart, Andrzej Hajda,
Shuah Khan, kernel, David Airlie, Marius Vlad, Simona Vetter,
Rob Herring, Robert Foss, Jonathan Corbet, Andy Yan,
Jernej Skrabec, Tvrtko Ursulin, linux-rockchip, Harry Wentland,
intel-gfx, Rodrigo Siqueira, Jonas Karlman, Leo Li, Sascha Hauer,
Maarten Lankhorst, Maxime Ripard, Jani Nikula, Rodrigo Vivi,
intel-xe, linux-arm-kernel, Dmitry Baryshkov, Neil Armstrong,
amd-gfx, Dmitry Baryshkov, Werner Sembach, Sandy Huang,
Thomas Zimmermann, Alex Deucher, Andy Yan, Christian König
In-Reply-To: <20260413-color-format-v13-0-ab37d4dfba48@collabora.com>
On Mon, Apr 13, 2026 at 12:07:14PM +0200, Nicolas Frattaroli wrote:
> Hello,
>
> this is a follow-up to
> https://lore.kernel.org/all/20250911130739.4936-1-marius.vlad@collabora.com/
> which in of itself is a follow-up to
> https://lore.kernel.org/dri-devel/20240115160554.720247-1-andri@yngvason.is/ where
> a new DRM connector property has been added allowing users to
> force a particular color format.
Looks like we're still missing the wayland folks in the cc. But I was
told that everyone should just cc wayland-devel@lists.freedesktop.org
on all relevant uapi stuff. So please add that on the next version.
The i915 rework is now merged so you should even get a buildable
series next time.
I'll go read the i915 parts now...
--
Ville Syrjälä
Intel
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH v7 0/4] PCI: Add support for resetting the Root Ports in a platform specific way
From: Manivannan Sadhasivam @ 2026-04-13 16:35 UTC (permalink / raw)
To: Brian Norris, Hongxing Zhu
Cc: Hongxing Zhu, manivannan.sadhasivam@oss.qualcomm.com,
Bjorn Helgaas, Mahesh J Salgaonkar, Oliver O'Halloran,
Will Deacon, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Heiko Stuebner, Philipp Zabel,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org, linux-rockchip@lists.infradead.org,
Niklas Cassel, Wilfred Mallawa, Krishna Chaitanya Chundru,
Lukas Wunner, Wilson Ding, Miles Chen
In-Reply-To: <adcHylFjFjhHT-tP@google.com>
Hi Brian,
On Wed, Apr 08, 2026 at 06:58:34PM -0700, Brian Norris wrote:
> Hi Richard and Mani,
>
> For the record, I've been using a form of an earlier version of this
> patchset in my environment for some time now, and I've run across
> problems that *might* relate to what Richard is reporting, but I'm not
> quite sure at the moment. Details below.
>
> On Wed, Mar 25, 2026 at 07:06:49AM +0000, Hongxing Zhu wrote:
> > Hi Mani:
> > I've accidentally encountered a new issue based on the reset root port patch-set.
> > After performing a few hot-reset operations, the PCIe link enters a continuous up/down cycling pattern.
> >
> > I found that calling pci_reset_secondary_bus() first in pcibios_reset_secondary_bus() appears to resolve this issue.
> > Have you experienced a similar problem?
> >
> > "
> > ...
> > [ 141.897701] imx6q-pcie 4c300000.pcie: PCIe(LNK_STS:0x00000700) link up detected
> > [ 142.086341] imx6q-pcie 4c300000.pcie: PCIe Gen.3 x1 link up
> > [ 142.092038] imx6q-pcie 4c300000.pcie: PCIe(LNK_STS:0x00000c00) link down detected
> > ...
> > "
> >
> > Platform: i.MX95 EVK board plus local Root Ports reset supports based on the #1 and #2 patches of v7 patch-set.
> > Notes of the logs:
> > - One Gen3 NVME device is connected.
> > - "./memtool 4c341058=0;./memtool 4c341058=1;" is used to toggle the LTSSM_EN bit to trigger the link down.
> > - Toggle BIT6 of Bridge Control Register to trigger hot reset by "./memtool 4c30003c=004001ff; ./memtool 4c30003c=000001ff;"
> > - The Root Port reset patches works correctly at first.
> > However, after several hot-reset triggers, the link enters a repeated down/up cycling state.
> >
> > Logs:
> > [ 3.553188] imx6q-pcie 4c300000.pcie: host bridge /soc/pcie@4c300000 ranges:
> > [ 3.560308] imx6q-pcie 4c300000.pcie: IO 0x006ff00000..0x006fffffff -> 0x0000000000
> > [ 3.568525] imx6q-pcie 4c300000.pcie: MEM 0x0910000000..0x091fffffff -> 0x0010000000
> > [ 3.577314] imx6q-pcie 4c300000.pcie: config reg[1] 0x60100000 == cpu 0x60100000
> > [ 3.796029] imx6q-pcie 4c300000.pcie: iATU: unroll T, 128 ob, 128 ib, align 4K, limit 1024G
> > [ 4.003746] imx6q-pcie 4c300000.pcie: PCIe Gen.3 x1 link up
> > [ 4.009553] imx6q-pcie 4c300000.pcie: PCI host bridge to bus 0000:00
> > root@imx95evk:~#
> > root@imx95evk:~#
> > root@imx95evk:~# ./memtool 4c341058=0;./memtool 4c341058=1; Writing 32-bit value 0x0 to address 0x4C341058
> > Writing 32-bit v
> > [ 87.265348] imx6q-pcie 4c300000.pcie: PCIe(LNK_STS:0x00000d01) link down detected
> > alue 0x1 to adder
> > [ 87.273106] imx6q-pcie 4c300000.pcie: Stop root bus and handle link down
> > ss 0x4C341058
> > [ 87.281264] pcieport 0000:00:00.0: Recovering Root Port due to Link Down
> > [ 87.289245] pci 0000:01:00.0: AER: can't recover (no error_detected callback)
> > root@imx95evk:~# [ 87.514216] imx6q-pcie 4c300000.pcie: PCIe(LNK_STS:0x00000700) link up detected
> > [ 87.702968] imx6q-pcie 4c300000.pcie: PCIe Gen.3 x1 link up
> > [ 87.834983] pcieport 0000:00:00.0: Root Port has been reset
> > [ 87.840714] pcieport 0000:00:00.0: AER: device recovery failed
> > [ 87.846592] imx6q-pcie 4c300000.pcie: Rescan bus after link up is detected
> > [ 87.855947] pcieport 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>
> I've seen this same line ("bridge configuration invalid") before, and I
> believe that's because the saved state (pci_save_state(); more about
> this below) is invalid -- it contains 0 values in places where they
> should be non-zero. So when those values are restored
> (pci_restore_state()), we get confused.
>
> I believe we've pinned down one reason this invalid state occurs -- it's
> because of an automatic (mis)feature in the DesignWare PCIe hardware.
> Specifically, it's because of what the controller does during a surprise
> link-down error.
>
> From the Designware docs:
>
> "[...] during normal operation, the link might fail and go down. After
> this link-down event, the controller requests the DWC_pcie_clkrst.v
> module to hot-reset the controller. There is no difference in the
> handling of a link-down reset or a hot reset; the controller asserts
> the link_req_rst_not output requesting the DWC_pcie_clkrst.v module to
> reset the controller."
>
> In some of the adjacent documentation (and confirmed in local testing),
> it suggests that this automatic reset will also reset various DBI (i.e.,
> PCIe config space) registers. It also seems as if there's not really a
> good way to completely stop this automatic reset -- the docs mention
> some SW methods prevent the reset, but they all seem racy or incomplete.
>
> Anyway, I think this implies that patch 1 is somewhat wrong [1]. It
> includes some code like this:
>
> pci_save_state(dev);
> ret = host->reset_root_port(host, dev);
> if (ret)
> pci_err(dev, "Failed to reset Root Port: %d\n", ret);
> else
> /* Now restore it on success */
> pci_restore_state(dev);
>
> That first line (pci_save_state()) is prone to saving invalid state,
> depending on whether the link-down event has finished flushing and
> resetting the controller yet or not. The resulting impact is a bit hard
> to judge, depending on what (mis)configuration you end up with.
>
Thanks a lot for your investigation. I think your observation makes sense and
could be the culprit in saving the corrupted state. Even on non-DWC controllers,
there is no guarantee that the Root Port config registers state will be
preserved after LDn (before Root Port reset).
> I also noticed commit a2f1e22390ac ("PCI/ERR: Ensure error
> recoverability at all times") was merged recently. With that change, I
> believe it is now safe to perform pci_restore_state() even without
> pci_save_state() here.
>
> So ... can we remove pci_save_state() from
> pcibios_reset_secondary_bus()? Might that help?
I think so. I will also test it locally and report back soon.
> It sounds like my above
> observations *may* match Richard's reports, but I'm not sure. And
> anyway, the documented hardware behavior is racy, so it's hard to
> propose a foolproof solution.
>
@Richard: Can you confirm if removing 'pci_save_state(dev);' from
pcibios_reset_secondary_bus() fixes your issue?
- Mani
--
மணிவண்ணன் சதாசிவம்
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH v14 3/8] drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()
From: Luca Ceresoli @ 2026-04-13 16:07 UTC (permalink / raw)
To: Damon Ding, andrzej.hajda, neil.armstrong, rfoss,
maarten.lankhorst, mripard, tzimmermann, airlied, simona,
inki.dae, sw0312.kim, kyungmin.park, krzk, jingoohan1, hjc, heiko,
andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, nicolas.frattaroli, dianders, m.szyprowski,
linux-kernel, dri-devel, linux-arm-kernel, linux-samsung-soc,
linux-rockchip
In-Reply-To: <20260413132551.1049307-4-damon.ding@rock-chips.com>
Hello Damon,
On Mon Apr 13, 2026 at 3:25 PM CEST, Damon Ding wrote:
> Since the panel/bridge should logically be positioned behind the
> Analogix bridge in the display pipeline, it makes sense to handle
> the panel/bridge parsing on the Analogix side. Therefore, we add
> a new API analogix_dp_finish_probe(), which combines the panel/bridge
> parsing with component addition, to do it.
>
> In order to process component binding right after the probe completes,
> the &analogix_dp_plat_data.ops is newly added to pass &component_ops,
> for which the &dp_aux_ep_device_with_data.done_probing() of DP AUX bus
> only supports passing &drm_dp_aux.
>
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de> # rk3588
>
> ---
>
> Changes in v4:
> - Rename the &analogix_dp_plat_data.bridge to
> &analogix_dp_plat_data.next_bridge.
> - Remame API analogix_dp_find_panel_or_bridge() to
> analogix_dp_finish_probe().
>
> Changes in v5:
> - Select DRM_DISPLAY_DP_AUX_BUS for DRM_ANALOGIX_DP.
>
> Changes in v9:
> - Add Tested-by tag.
>
> Changes in v10:
> - Fix to use dev_err_probe() in analogix_dp_finish_probe().
> - Expand the commit message.
>
> Changes in v13:
> - Modify '(on rk3588)' to '# rk3588' for Tested-by tag.
> ---
> drivers/gpu/drm/bridge/analogix/Kconfig | 1 +
> .../drm/bridge/analogix/analogix_dp_core.c | 46 +++++++++++++++++++
> include/drm/bridge/analogix_dp.h | 2 +
> 3 files changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
> index 03dc7ffe824a..8a6136cd675f 100644
> --- a/drivers/gpu/drm/bridge/analogix/Kconfig
> +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
> @@ -29,6 +29,7 @@ config DRM_ANALOGIX_ANX78XX
> config DRM_ANALOGIX_DP
> tristate
> depends on DRM
> + select DRM_DISPLAY_DP_AUX_BUS
While applying, sparse noticed an issue here: DRM_DISPLAY_DP_AUX_BUS
depends on OF, so you need to propagate the 'depends on OF' to
DRM_ANALOGIX_DP and its reverse dependencies.
I fixed it while applying the patch.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH v14 0/8] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver
From: Luca Ceresoli @ 2026-04-13 16:05 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan, Damon Ding
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, nicolas.frattaroli, dianders, m.szyprowski,
linux-kernel, dri-devel, linux-arm-kernel, linux-samsung-soc,
linux-rockchip
In-Reply-To: <20260413132551.1049307-1-damon.ding@rock-chips.com>
On Mon, 13 Apr 2026 21:25:43 +0800, Damon Ding wrote:
> Picked from:
> https://lore.kernel.org/all/20260409065301.446670-1-damon.ding@rock-chips.com/
>
> PATCH 1 is the preparation for apply drm_bridge_connector helper.
> PATCH 2 is to apply the drm_bridge_connector helper.
> PATCH 3-5 are to move the panel/bridge parsing to the Analogix side.
> PATCH 6 is to attach the next bridge on Analogix side uniformly.
> PATCH 7-8 are to apply the panel_bridge helper.
>
> [...]
Applied, thanks!
[1/8] drm/bridge: analogix_dp: Pass struct drm_atomic_state* for analogix_dp_bridge_mode_set()
commit: 3be024d26a576519ce75fabfbdf6972731c19900
[2/8] drm/bridge: analogix_dp: Apply drm_bridge_connector helper
commit: 99a49ff5ef7a5f01e28e724b888d94b6735a88c1
[3/8] drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()
commit: e9c897c898f9ff32d93380ee4ebc778a6b787aaa
[4/8] drm/rockchip: analogix_dp: Apply analogix_dp_finish_probe()
commit: d35ac0973463f67162d9ee73bf0c828ad5d4d2f9
[5/8] drm/exynos: exynos_dp: Apply analogix_dp_finish_probe()
commit: 02b8a4f240abdc4e99efd6cf95c47378a1015903
[6/8] drm/bridge: analogix_dp: Attach the next bridge in analogix_dp_bridge_attach()
commit: 3076510af7cd01a7fb0ae5103116a39ad35eb5cb
[7/8] drm/bridge: analogix_dp: Remove bridge disabing and panel unpreparing in analogix_dp_unbind()
commit: 2bfc4e192f04260c2eead9b79274d39984f5a143
[8/8] drm/bridge: analogix_dp: Apply panel_bridge helper
commit: 1b86a69b61df411354da70d9528f022833bee4d7
Best regards,
--
Luca Ceresoli <luca.ceresoli@bootlin.com>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* [PATCH v14 8/8] drm/bridge: analogix_dp: Apply panel_bridge helper
From: Damon Ding @ 2026-04-13 13:25 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, luca.ceresoli, nicolas.frattaroli, dianders,
m.szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-rockchip, Damon Ding
In-Reply-To: <20260413132551.1049307-1-damon.ding@rock-chips.com>
In order to unify the handling of the panel and bridge, apply
panel_bridge helpers for Analogix DP driver. With this patch, the
bridge support will also become available.
The following changes have ben made:
- Apply plane_bridge helper to wrap the panel as the bridge.
- Remove the explicit panel APIs calls, which can be replaced with
the automic bridge APIs calls wrapped by the panel.
- Remove the unnecessary analogix_dp_bridge_get_modes().
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de> # rk3588
---
Changes in v4:
- Rename the &analogix_dp_plat_data.bridge to
&analogix_dp_plat_data.next_bridge.
Changes in v5:
- Move panel_bridge addition a little forward.
- Move next_bridge attachment from Analogix side to Rockchip/Exynos
side.
Changes in v6
- Remove the unnecessary analogix_dp_bridge_get_modes().
- Not to set DRM_BRIDGE_OP_MODES if the next is a panel.
- Squash [PATCH v5 15/17]drm/bridge: analogix_dp: Remove panel
disabling and enabling in analogix_dp_set_bridge() into this
commit.
- Fix the &drm_bridge->ops to DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT.
Changes in v9:
- Add Tested-by tag.
Changes in v13:
- Modify '(on rk3588)' to '# rk3588' for Tested-by tag.
---
.../drm/bridge/analogix/analogix_dp_core.c | 41 +++++--------------
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index ec56d900f899..460729fdcecd 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -750,9 +750,6 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
{
int ret;
- /* Keep the panel disabled while we configure video */
- drm_panel_disable(dp->plat_data->panel);
-
ret = analogix_dp_train_link(dp);
if (ret) {
dev_err(dp->dev, "unable to do link train, ret=%d\n", ret);
@@ -772,9 +769,6 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
return ret;
}
- /* Safe to enable the panel now */
- drm_panel_enable(dp->plat_data->panel);
-
/* Check whether panel supports fast training */
ret = analogix_dp_fast_link_train_detection(dp);
if (ret)
@@ -859,17 +853,6 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
}
-static int analogix_dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
-{
- struct analogix_dp_device *dp = to_dp(bridge);
- int num_modes = 0;
-
- if (dp->plat_data->panel)
- num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
-
- return num_modes;
-}
-
static const struct drm_edid *analogix_dp_bridge_edid_read(struct drm_bridge *bridge,
struct drm_connector *connector)
{
@@ -910,7 +893,7 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
struct analogix_dp_device *dp = to_dp(bridge);
enum drm_connector_status status = connector_status_disconnected;
- if (dp->plat_data->panel || dp->plat_data->next_bridge)
+ if (dp->plat_data->next_bridge)
return connector_status_connected;
if (!analogix_dp_detect_hpd(dp))
@@ -996,8 +979,6 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
/* Don't touch the panel if we're coming back from PSR */
if (old_crtc_state && old_crtc_state->self_refresh_active)
return;
-
- drm_panel_prepare(dp->plat_data->panel);
}
static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
@@ -1169,16 +1150,12 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
if (dp->dpms_mode != DRM_MODE_DPMS_ON)
return;
- drm_panel_disable(dp->plat_data->panel);
-
disable_irq(dp->irq);
analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
pm_runtime_put_sync(dp->dev);
- drm_panel_unprepare(dp->plat_data->panel);
-
dp->fast_train_enable = false;
dp->psr_supported = false;
dp->dpms_mode = DRM_MODE_DPMS_OFF;
@@ -1253,7 +1230,6 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
.atomic_post_disable = analogix_dp_bridge_atomic_post_disable,
.atomic_check = analogix_dp_bridge_atomic_check,
.attach = analogix_dp_bridge_attach,
- .get_modes = analogix_dp_bridge_get_modes,
.edid_read = analogix_dp_bridge_edid_read,
.detect = analogix_dp_bridge_detect,
};
@@ -1499,17 +1475,22 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
return ret;
}
- if (dp->plat_data->panel)
- bridge->ops = DRM_BRIDGE_OP_MODES | DRM_BRIDGE_OP_DETECT;
- else
- bridge->ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT;
-
+ bridge->ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT;
bridge->of_node = dp->dev->of_node;
bridge->type = DRM_MODE_CONNECTOR_eDP;
ret = devm_drm_bridge_add(dp->dev, &dp->bridge);
if (ret)
goto err_unregister_aux;
+ if (dp->plat_data->panel) {
+ dp->plat_data->next_bridge = devm_drm_panel_bridge_add(dp->dev,
+ dp->plat_data->panel);
+ if (IS_ERR(dp->plat_data->next_bridge)) {
+ ret = PTR_ERR(bridge);
+ goto err_unregister_aux;
+ }
+ }
+
ret = drm_bridge_attach(dp->encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret) {
DRM_ERROR("failed to create bridge (%d)\n", ret);
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v14 7/8] drm/bridge: analogix_dp: Remove bridge disabing and panel unpreparing in analogix_dp_unbind()
From: Damon Ding @ 2026-04-13 13:25 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, luca.ceresoli, nicolas.frattaroli, dianders,
m.szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-rockchip, Damon Ding
In-Reply-To: <20260413132551.1049307-1-damon.ding@rock-chips.com>
The analogix_dp_unbind() should be balanced with analogix_dp_bind().
There are no bridge enabling and panel preparing in analogix_dp_bind(),
so it should be reasonable to remove the bridge disabing and panel
unpreparing in analogix_dp_unbind().
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de> # rk3588
---
Changes in v9:
- Add Tested-by tag.
Changes in v11:
- Add Reviewed-by tag.
Changes in v13:
- Modify '(on rk3588)' to '# rk3588' for Tested-by tag.
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 85033a8ab146..ec56d900f899 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1527,10 +1527,6 @@ EXPORT_SYMBOL_GPL(analogix_dp_bind);
void analogix_dp_unbind(struct analogix_dp_device *dp)
{
- analogix_dp_bridge_disable(&dp->bridge);
-
- drm_panel_unprepare(dp->plat_data->panel);
-
drm_dp_aux_unregister(&dp->aux);
}
EXPORT_SYMBOL_GPL(analogix_dp_unbind);
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v14 2/8] drm/bridge: analogix_dp: Apply drm_bridge_connector helper
From: Damon Ding @ 2026-04-13 13:25 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, luca.ceresoli, nicolas.frattaroli, dianders,
m.szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-rockchip, Damon Ding
In-Reply-To: <20260413132551.1049307-1-damon.ding@rock-chips.com>
Initialize bridge_connector for both Rockchip and Exynos encoder sides.
Then, make DRM_BRIDGE_ATTACH_NO_CONNECTOR mandatory for Analogix bridge
side, as the private &drm_connector is no longer created.
The previous &drm_connector_funcs and &drm_connector_helper_funcs APIs
are replaced by the corresponding &drm_bridge_funcs APIs:
analogix_dp_atomic_check() -> analogix_dp_bridge_atomic_check()
analogix_dp_detect() -> analogix_dp_bridge_detect()
analogix_dp_get_modes() -> analogix_dp_bridge_get_modes()
analogix_dp_bridge_edid_read()
Additionally, the compatibilities of Analogix DP bridge based on whether
the next bridge is a 'panel'. If it is, OP_MODES and OP_DETECT are
supported; If not (the next bridge is a 'monitor' or a bridge chip),
OP_EDID and OP_DETECT are supported.
The devm_drm_bridge_add() is placed in analogix_dp_bind() instead of
analogix_dp_probe(), because the type of next bridge (the panel, monitor
or bridge chip) can only be determined after the probe process has fully
completed.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de> # rk3588
---
Changes in v2:
- For &drm_bridge.ops, remove DRM_BRIDGE_OP_HPD and add
DRM_BRIDGE_OP_EDID.
- Add analogix_dp_bridge_edid_read().
- Move &analogix_dp_plat_data.skip_connector deletion to the previous
patches.
Changes in v3:
- Rebase with the new devm_drm_bridge_alloc() related commit
48f05c3b4b70 ("drm/bridge: analogix_dp: Use devm_drm_bridge_alloc()
API").
- Expand the commit message.
- Call drm_bridge_get_modes() in analogix_dp_bridge_get_modes() if the
bridge is available.
- Remove unnecessary parameter struct drm_connector* for callback
&analogix_dp_plat_data.attach.
- In order to decouple the connector driver and the bridge driver, move
the bridge connector initilization to the Rockchip and Exynos sides.
Changes in v4:
- Expand analogix_dp_bridge_detect() parameters to &drm_bridge and
&drm_connector.
- Rename the &analogix_dp_plat_data.bridge to
&analogix_dp_plat_data.next_bridge.
Changes in v5:
- Set the flag fo drm_bridge_attach() to DRM_BRIDGE_ATTACH_NO_CONNECTOR
for next bridge attachment of Exynos side.
- Distinguish the &drm_bridge->ops of Analogix bridge based on whether
the downstream device is a panel, a bridge or neither.
- Remove the calls to &analogix_dp_plat_data.get_modes().
Changes in v6:
- Select DRM_BRIDGE_CONNECTOR for both Rockchip and Exynos sides.
- Remove unnecessary drm_bridge_get_modes() in
analogix_dp_bridge_get_modes().
- Simplify analogix_dp_bridge_edid_read().
- If the next is a bridge, set DRM_BRIDGE_OP_DETECT and return
connector_status_connected in analogix_dp_bridge_detect().
- Set flag DRM_BRIDGE_ATTACH_NO_CONNECTOR for bridge attachment while
binding. Meanwhile, make DRM_BRIDGE_ATTACH_NO_CONNECTOR unsuppported
in analogix_dp_bridge_attach().
- Simplify the check of bridge capabilities.
Changes in v7:
- Remove temporary flag &exynos_dp_device.has_of_bridge.
Changes in v9
- Add Tested-by tag.
Changes in v10:
- Split this commit into serval smaller ones.
- Simplify the commit message.
Changes in v11:
- Move the removal of &analogix_dp_device.connector to this commit.
- In exynos_dp_bridge_attach(), set the bridge flag to 'flags |
DRM_BRIDGE_ATTACH_NO_CONNECTOR' instead of fixed
'DRM_BRIDGE_ATTACH_NO_CONNECTOR' for extensibility.
Changes in v12:
- Restore accidentally removed DRM_BRIDGE_CONNECTOR Kconfig in v10.
Changes in v13:
- Rebase after commit 01962a191242 ("drm/rockchip: analogix: Convert
to drm_output_color_format")
- Modify '(on rk3588)' to '# rk3588' for Tested-by tag.
Changes in v14:
- Add Reviewed-by tags.
- Apply __free() to call drm_bridge_put() in CRC ralted functions.
---
.../drm/bridge/analogix/analogix_dp_core.c | 136 +++++++-----------
.../drm/bridge/analogix/analogix_dp_core.h | 1 -
drivers/gpu/drm/exynos/Kconfig | 1 +
drivers/gpu/drm/exynos/exynos_dp.c | 25 ++--
drivers/gpu/drm/rockchip/Kconfig | 1 +
.../gpu/drm/rockchip/analogix_dp-rockchip.c | 11 +-
6 files changed, 79 insertions(+), 96 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 84b994cce900..3c6eed9279d2 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -6,6 +6,7 @@
* Author: Jingoo Han <jg1.han@samsung.com>
*/
+#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/component.h>
#include <linux/err.h>
@@ -856,44 +857,32 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
}
-static int analogix_dp_get_modes(struct drm_connector *connector)
+static int analogix_dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
{
- struct analogix_dp_device *dp = to_dp(connector);
- const struct drm_edid *drm_edid;
+ struct analogix_dp_device *dp = to_dp(bridge);
int num_modes = 0;
- if (dp->plat_data->panel) {
+ if (dp->plat_data->panel)
num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
- } else {
- drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
-
- drm_edid_connector_update(&dp->connector, drm_edid);
-
- if (drm_edid) {
- num_modes += drm_edid_connector_add_modes(&dp->connector);
- drm_edid_free(drm_edid);
- }
- }
return num_modes;
}
-static struct drm_encoder *
-analogix_dp_best_encoder(struct drm_connector *connector)
+static const struct drm_edid *analogix_dp_bridge_edid_read(struct drm_bridge *bridge,
+ struct drm_connector *connector)
{
- struct analogix_dp_device *dp = to_dp(connector);
+ struct analogix_dp_device *dp = to_dp(bridge);
- return dp->encoder;
+ return drm_edid_read_ddc(connector, &dp->aux.ddc);
}
-
-static int analogix_dp_atomic_check(struct drm_connector *connector,
- struct drm_atomic_state *state)
+static int analogix_dp_bridge_atomic_check(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
{
- struct analogix_dp_device *dp = to_dp(connector);
- struct drm_display_info *di = &connector->display_info;
- struct drm_connector_state *conn_state;
- struct drm_crtc_state *crtc_state;
+ struct analogix_dp_device *dp = to_dp(bridge);
+ struct drm_display_info *di = &conn_state->connector->display_info;
u32 mask = BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) | BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422);
if (is_rockchip(dp->plat_data->dev_type)) {
@@ -905,38 +894,21 @@ static int analogix_dp_atomic_check(struct drm_connector *connector,
}
}
- conn_state = drm_atomic_get_new_connector_state(state, connector);
- if (WARN_ON(!conn_state))
- return -ENODEV;
-
conn_state->self_refresh_aware = true;
- if (!conn_state->crtc)
- return 0;
-
- crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
- if (!crtc_state)
- return 0;
-
if (crtc_state->self_refresh_active && !dp->psr_supported)
return -EINVAL;
return 0;
}
-static const struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = {
- .get_modes = analogix_dp_get_modes,
- .best_encoder = analogix_dp_best_encoder,
- .atomic_check = analogix_dp_atomic_check,
-};
-
static enum drm_connector_status
-analogix_dp_detect(struct drm_connector *connector, bool force)
+analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
{
- struct analogix_dp_device *dp = to_dp(connector);
+ struct analogix_dp_device *dp = to_dp(bridge);
enum drm_connector_status status = connector_status_disconnected;
- if (dp->plat_data->panel)
+ if (dp->plat_data->panel || dp->plat_data->next_bridge)
return connector_status_connected;
if (!analogix_dp_detect_hpd(dp))
@@ -945,51 +917,18 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
return status;
}
-static const struct drm_connector_funcs analogix_dp_connector_funcs = {
- .fill_modes = drm_helper_probe_single_connector_modes,
- .detect = analogix_dp_detect,
- .destroy = drm_connector_cleanup,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
static int analogix_dp_bridge_attach(struct drm_bridge *bridge,
struct drm_encoder *encoder,
enum drm_bridge_attach_flags flags)
{
struct analogix_dp_device *dp = to_dp(bridge);
- struct drm_connector *connector = NULL;
int ret = 0;
- if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
- DRM_ERROR("Fix bridge driver to make connector optional!");
+ if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
+ DRM_ERROR("Unsupported connector creation\n");
return -EINVAL;
}
- if (!dp->plat_data->next_bridge) {
- connector = &dp->connector;
- connector->polled = DRM_CONNECTOR_POLL_HPD;
-
- ret = drm_connector_init(dp->drm_dev, connector,
- &analogix_dp_connector_funcs,
- DRM_MODE_CONNECTOR_eDP);
- if (ret) {
- DRM_ERROR("Failed to initialize connector with drm\n");
- return ret;
- }
-
- drm_connector_helper_add(connector,
- &analogix_dp_connector_helper_funcs);
- drm_connector_attach_encoder(connector, encoder);
- }
-
- /*
- * NOTE: the connector registration is implemented in analogix
- * platform driver, that to say connector would be exist after
- * plat_data->attch return, that's why we record the connector
- * point after plat attached.
- */
if (dp->plat_data->attach) {
ret = dp->plat_data->attach(dp->plat_data, bridge);
if (ret) {
@@ -1309,7 +1248,11 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
.atomic_enable = analogix_dp_bridge_atomic_enable,
.atomic_disable = analogix_dp_bridge_atomic_disable,
.atomic_post_disable = analogix_dp_bridge_atomic_post_disable,
+ .atomic_check = analogix_dp_bridge_atomic_check,
.attach = analogix_dp_bridge_attach,
+ .get_modes = analogix_dp_bridge_get_modes,
+ .edid_read = analogix_dp_bridge_edid_read,
+ .detect = analogix_dp_bridge_detect,
};
static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
@@ -1539,6 +1482,7 @@ EXPORT_SYMBOL_GPL(analogix_dp_resume);
int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
{
+ struct drm_bridge *bridge = &dp->bridge;
int ret;
dp->drm_dev = drm_dev;
@@ -1552,7 +1496,18 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
return ret;
}
- ret = drm_bridge_attach(dp->encoder, &dp->bridge, NULL, 0);
+ if (dp->plat_data->panel)
+ bridge->ops = DRM_BRIDGE_OP_MODES | DRM_BRIDGE_OP_DETECT;
+ else
+ bridge->ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT;
+
+ bridge->of_node = dp->dev->of_node;
+ bridge->type = DRM_MODE_CONNECTOR_eDP;
+ ret = devm_drm_bridge_add(dp->dev, &dp->bridge);
+ if (ret)
+ goto err_unregister_aux;
+
+ ret = drm_bridge_attach(dp->encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret) {
DRM_ERROR("failed to create bridge (%d)\n", ret);
goto err_unregister_aux;
@@ -1570,7 +1525,6 @@ EXPORT_SYMBOL_GPL(analogix_dp_bind);
void analogix_dp_unbind(struct analogix_dp_device *dp)
{
analogix_dp_bridge_disable(&dp->bridge);
- dp->connector.funcs->destroy(&dp->connector);
drm_panel_unprepare(dp->plat_data->panel);
@@ -1580,7 +1534,9 @@ EXPORT_SYMBOL_GPL(analogix_dp_unbind);
int analogix_dp_start_crc(struct drm_connector *connector)
{
- struct analogix_dp_device *dp = to_dp(connector);
+ struct analogix_dp_device *dp;
+ struct drm_bridge *bridge __free(drm_bridge_put) =
+ drm_bridge_chain_get_first_bridge(connector->encoder);
if (!connector->state->crtc) {
DRM_ERROR("Connector %s doesn't currently have a CRTC.\n",
@@ -1588,13 +1544,25 @@ int analogix_dp_start_crc(struct drm_connector *connector)
return -EINVAL;
}
+ if (!bridge || bridge->type != DRM_MODE_CONNECTOR_eDP)
+ return -EINVAL;
+
+ dp = to_dp(bridge);
+
return drm_dp_start_crc(&dp->aux, connector->state->crtc);
}
EXPORT_SYMBOL_GPL(analogix_dp_start_crc);
int analogix_dp_stop_crc(struct drm_connector *connector)
{
- struct analogix_dp_device *dp = to_dp(connector);
+ struct analogix_dp_device *dp;
+ struct drm_bridge *bridge __free(drm_bridge_put) =
+ drm_bridge_chain_get_first_bridge(connector->encoder);
+
+ if (!bridge || bridge->type != DRM_MODE_CONNECTOR_eDP)
+ return -EINVAL;
+
+ dp = to_dp(bridge);
return drm_dp_stop_crc(&dp->aux);
}
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 91b215c6a0cf..17347448c6b0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -154,7 +154,6 @@ struct analogix_dp_device {
struct drm_encoder *encoder;
struct device *dev;
struct drm_device *drm_dev;
- struct drm_connector connector;
struct drm_bridge bridge;
struct drm_dp_aux aux;
struct clk *clock;
diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 380d9a8ce259..38bf070866f6 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -70,6 +70,7 @@ config DRM_EXYNOS_DP
bool "Exynos specific extensions for Analogix DP driver"
depends on DRM_EXYNOS_FIMD || DRM_EXYNOS7_DECON
select DRM_ANALOGIX_DP
+ select DRM_BRIDGE_CONNECTOR
select DRM_DISPLAY_DP_HELPER
default DRM_EXYNOS
select DRM_OF_DISPLAY_MODE_BRIDGE
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 71a00ee97782..b2597fafd73d 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -22,6 +22,7 @@
#include <drm/bridge/of-display-mode-bridge.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
#include <drm/drm_crtc.h>
#include <drm/drm_of.h>
#include <drm/drm_panel.h>
@@ -41,8 +42,6 @@ struct exynos_dp_device {
struct analogix_dp_device *adp;
struct analogix_dp_plat_data plat_data;
-
- bool has_of_bridge;
};
static int exynos_dp_crtc_clock_enable(struct analogix_dp_plat_data *plat_data,
@@ -78,10 +77,8 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
/* Pre-empt DP connector creation if there's a bridge */
if (plat_data->next_bridge) {
- if (dp->has_of_bridge)
- flags = DRM_BRIDGE_ATTACH_NO_CONNECTOR;
-
- ret = drm_bridge_attach(&dp->encoder, plat_data->next_bridge, bridge, flags);
+ ret = drm_bridge_attach(&dp->encoder, plat_data->next_bridge, bridge,
+ flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret)
return ret;
}
@@ -111,6 +108,7 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
struct exynos_dp_device *dp = dev_get_drvdata(dev);
struct drm_encoder *encoder = &dp->encoder;
struct drm_device *drm_dev = data;
+ struct drm_connector *connector;
int ret;
dp->drm_dev = drm_dev;
@@ -126,10 +124,19 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
dp->plat_data.encoder = encoder;
ret = analogix_dp_bind(dp->adp, dp->drm_dev);
- if (ret)
+ if (ret) {
dp->encoder.funcs->destroy(&dp->encoder);
+ return ret;
+ }
+
+ connector = drm_bridge_connector_init(dp->drm_dev, dp->plat_data.encoder);
+ if (IS_ERR(connector)) {
+ ret = PTR_ERR(connector);
+ dev_err(dp->dev, "Failed to initialize bridge_connector\n");
+ return ret;
+ }
- return ret;
+ return drm_connector_attach_encoder(connector, dp->plat_data.encoder);
}
static void exynos_dp_unbind(struct device *dev, struct device *master,
@@ -186,8 +193,6 @@ static int exynos_dp_probe(struct platform_device *pdev)
dp->dev->of_node,
DRM_MODE_CONNECTOR_eDP);
ret = IS_ERR(dp->plat_data.next_bridge) ? PTR_ERR(dp->plat_data.next_bridge) : 0;
- if (!ret)
- dp->has_of_bridge = true;
}
if (ret)
return ret;
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 1479b8c4ed40..e7f49fe845ea 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -46,6 +46,7 @@ config ROCKCHIP_VOP2
config ROCKCHIP_ANALOGIX_DP
bool "Rockchip specific extensions for Analogix DP driver"
depends on ROCKCHIP_VOP
+ select DRM_BRIDGE_CONNECTOR
select DRM_DISPLAY_HELPER
select DRM_DISPLAY_DP_HELPER
help
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 48206a7b767f..99e739f4f583 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -25,6 +25,7 @@
#include <drm/display/drm_dp_helper.h>
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge_connector.h>
#include <drm/bridge/analogix_dp.h>
#include <drm/drm_of.h>
#include <drm/drm_panel.h>
@@ -369,6 +370,7 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
{
struct rockchip_dp_device *dp = dev_get_drvdata(dev);
struct drm_device *drm_dev = data;
+ struct drm_connector *connector;
int ret;
dp->drm_dev = drm_dev;
@@ -388,7 +390,14 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
if (ret)
goto err_cleanup_encoder;
- return 0;
+ connector = drm_bridge_connector_init(dp->drm_dev, dp->plat_data.encoder);
+ if (IS_ERR(connector)) {
+ ret = PTR_ERR(connector);
+ dev_err(dp->dev, "Failed to initialize bridge_connector\n");
+ goto err_cleanup_encoder;
+ }
+
+ return drm_connector_attach_encoder(connector, dp->plat_data.encoder);
err_cleanup_encoder:
dp->encoder.encoder.funcs->destroy(&dp->encoder.encoder);
return ret;
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v14 6/8] drm/bridge: analogix_dp: Attach the next bridge in analogix_dp_bridge_attach()
From: Damon Ding @ 2026-04-13 13:25 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, luca.ceresoli, nicolas.frattaroli, dianders,
m.szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-rockchip, Damon Ding
In-Reply-To: <20260413132551.1049307-1-damon.ding@rock-chips.com>
Uniformly, move the next bridge attachment to the Analogix side
rather than scattered on Rockchip and Exynos sides. It can also
help get rid of the callback &analogix_dp_plat_data.attach() and
make codes more concise.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de> # rk3588
---
Changes in v6:
- Move the next bridge attachment to the Analogix side rather than
scattered on Rockchip and Exynos sides.
Changes in v9:
- Add Tested-by tag.
Changes in v11:
- Add Reviewed-by tag.
Changes in v13:
- Modify '(on rk3588)' to '# rk3588' for Tested-by tag.
---
.../drm/bridge/analogix/analogix_dp_core.c | 7 ++++---
drivers/gpu/drm/exynos/exynos_dp.c | 19 -------------------
include/drm/bridge/analogix_dp.h | 1 -
3 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 50415a98acb7..85033a8ab146 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -931,10 +931,11 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge,
return -EINVAL;
}
- if (dp->plat_data->attach) {
- ret = dp->plat_data->attach(dp->plat_data, bridge);
+ if (dp->plat_data->next_bridge) {
+ ret = drm_bridge_attach(dp->encoder, dp->plat_data->next_bridge, bridge,
+ DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret) {
- DRM_ERROR("Failed at platform attach func\n");
+ dev_err(dp->dev, "failed to attach following panel or bridge (%d)\n", ret);
return ret;
}
}
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 9f3d4d4c7352..6884ea6d04eb 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -68,24 +68,6 @@ static int exynos_dp_poweroff(struct analogix_dp_plat_data *plat_data)
return exynos_dp_crtc_clock_enable(plat_data, false);
}
-static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
- struct drm_bridge *bridge)
-{
- struct exynos_dp_device *dp = to_dp(plat_data);
- enum drm_bridge_attach_flags flags = 0;
- int ret;
-
- /* Pre-empt DP connector creation if there's a bridge */
- if (plat_data->next_bridge) {
- ret = drm_bridge_attach(&dp->encoder, plat_data->next_bridge, bridge,
- flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR);
- if (ret)
- return ret;
- }
-
- return 0;
-}
-
static void exynos_dp_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
@@ -196,7 +178,6 @@ static int exynos_dp_probe(struct platform_device *pdev)
dp->plat_data.dev_type = EXYNOS_DP;
dp->plat_data.power_on = exynos_dp_poweron;
dp->plat_data.power_off = exynos_dp_poweroff;
- dp->plat_data.attach = exynos_dp_bridge_attach;
dp->plat_data.ops = &exynos_dp_ops;
out:
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index bae969dec63a..854af692229b 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -34,7 +34,6 @@ struct analogix_dp_plat_data {
int (*power_on)(struct analogix_dp_plat_data *);
int (*power_off)(struct analogix_dp_plat_data *);
- int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *);
};
int analogix_dp_resume(struct analogix_dp_device *dp);
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v14 1/8] drm/bridge: analogix_dp: Pass struct drm_atomic_state* for analogix_dp_bridge_mode_set()
From: Damon Ding @ 2026-04-13 13:25 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, luca.ceresoli, nicolas.frattaroli, dianders,
m.szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-rockchip, Damon Ding
In-Reply-To: <20260413132551.1049307-1-damon.ding@rock-chips.com>
To avoid using &analogix_dp_device.connector for compatibility
with the bridge connector framework, get &drm_connector from
&drm_atomic_state instead.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v14:
- Add Reviewed-by tags.
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 840c1963e60e..84b994cce900 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1095,14 +1095,21 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
}
static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
+ struct drm_atomic_state *state,
const struct drm_display_mode *mode)
{
struct analogix_dp_device *dp = to_dp(bridge);
- struct drm_display_info *display_info = &dp->connector.display_info;
struct video_info *video = &dp->video_info;
struct device_node *dp_node = dp->dev->of_node;
+ struct drm_connector *connector;
+ struct drm_display_info *display_info;
int vic;
+ connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
+ if (!connector)
+ return;
+ display_info = &connector->display_info;
+
/* Input video interlaces & hsync pol & vsync pol */
video->interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
@@ -1186,7 +1193,7 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
new_crtc_state = drm_atomic_get_new_crtc_state(old_state, crtc);
if (!new_crtc_state)
return;
- analogix_dp_bridge_mode_set(bridge, &new_crtc_state->adjusted_mode);
+ analogix_dp_bridge_mode_set(bridge, old_state, &new_crtc_state->adjusted_mode);
old_crtc_state = drm_atomic_get_old_crtc_state(old_state, crtc);
/* Not a full enable, just disable PSR and continue */
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v14 5/8] drm/exynos: exynos_dp: Apply analogix_dp_finish_probe()
From: Damon Ding @ 2026-04-13 13:25 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, luca.ceresoli, nicolas.frattaroli, dianders,
m.szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-rockchip, Damon Ding
In-Reply-To: <20260413132551.1049307-1-damon.ding@rock-chips.com>
Apply analogix_dp_finish_probe() in order to move the panel/bridge
parsing from Exynos side to the Analogix side.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de> # rk3588
---
Changes in v4:
- Rename analogix_dp_find_panel_or_bridge() to
analogix_dp_finish_probe().
Changes in v7:
- Remove exynos_dp_legacy_bridge_init() and inline API
devm_drm_of_display_mode_bridge().
- If the panel or the next_bridge is parsed from DT, use ptr validity
to check whether to call component_add() directly at the end of
probing.
Changes in v9:
- Add Tested-by tag.
Changes in v13:
- Modify '(on rk3588)' to '# rk3588' for Tested-by tag.
---
drivers/gpu/drm/exynos/exynos_dp.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index b2597fafd73d..9f3d4d4c7352 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -158,9 +158,6 @@ static int exynos_dp_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *np;
struct exynos_dp_device *dp;
- struct drm_panel *panel;
- struct drm_bridge *bridge;
- int ret;
dp = devm_kzalloc(&pdev->dev, sizeof(struct exynos_dp_device),
GFP_KERNEL);
@@ -187,30 +184,30 @@ static int exynos_dp_probe(struct platform_device *pdev)
goto out;
}
- ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, &bridge);
- if (ret == -ENODEV) {
+ if (of_get_display_timings(dev->of_node)) {
dp->plat_data.next_bridge = devm_drm_of_display_mode_bridge(dp->dev,
dp->dev->of_node,
DRM_MODE_CONNECTOR_eDP);
- ret = IS_ERR(dp->plat_data.next_bridge) ? PTR_ERR(dp->plat_data.next_bridge) : 0;
+ if (IS_ERR(dp->plat_data.next_bridge))
+ return PTR_ERR(dp->plat_data.next_bridge);
}
- if (ret)
- return ret;
/* The remote port can be either a panel or a bridge */
- dp->plat_data.panel = panel;
- dp->plat_data.next_bridge = bridge;
dp->plat_data.dev_type = EXYNOS_DP;
dp->plat_data.power_on = exynos_dp_poweron;
dp->plat_data.power_off = exynos_dp_poweroff;
dp->plat_data.attach = exynos_dp_bridge_attach;
+ dp->plat_data.ops = &exynos_dp_ops;
out:
dp->adp = analogix_dp_probe(dev, &dp->plat_data);
if (IS_ERR(dp->adp))
return PTR_ERR(dp->adp);
- return component_add(&pdev->dev, &exynos_dp_ops);
+ if (dp->plat_data.panel || dp->plat_data.next_bridge)
+ return component_add(&pdev->dev, &exynos_dp_ops);
+ else
+ return analogix_dp_finish_probe(dp->adp);
}
static void exynos_dp_remove(struct platform_device *pdev)
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v14 3/8] drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()
From: Damon Ding @ 2026-04-13 13:25 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, luca.ceresoli, nicolas.frattaroli, dianders,
m.szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-rockchip, Damon Ding
In-Reply-To: <20260413132551.1049307-1-damon.ding@rock-chips.com>
Since the panel/bridge should logically be positioned behind the
Analogix bridge in the display pipeline, it makes sense to handle
the panel/bridge parsing on the Analogix side. Therefore, we add
a new API analogix_dp_finish_probe(), which combines the panel/bridge
parsing with component addition, to do it.
In order to process component binding right after the probe completes,
the &analogix_dp_plat_data.ops is newly added to pass &component_ops,
for which the &dp_aux_ep_device_with_data.done_probing() of DP AUX bus
only supports passing &drm_dp_aux.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de> # rk3588
---
Changes in v4:
- Rename the &analogix_dp_plat_data.bridge to
&analogix_dp_plat_data.next_bridge.
- Remame API analogix_dp_find_panel_or_bridge() to
analogix_dp_finish_probe().
Changes in v5:
- Select DRM_DISPLAY_DP_AUX_BUS for DRM_ANALOGIX_DP.
Changes in v9:
- Add Tested-by tag.
Changes in v10:
- Fix to use dev_err_probe() in analogix_dp_finish_probe().
- Expand the commit message.
Changes in v13:
- Modify '(on rk3588)' to '# rk3588' for Tested-by tag.
---
drivers/gpu/drm/bridge/analogix/Kconfig | 1 +
.../drm/bridge/analogix/analogix_dp_core.c | 46 +++++++++++++++++++
include/drm/bridge/analogix_dp.h | 2 +
3 files changed, 49 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
index 03dc7ffe824a..8a6136cd675f 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -29,6 +29,7 @@ config DRM_ANALOGIX_ANX78XX
config DRM_ANALOGIX_DP
tristate
depends on DRM
+ select DRM_DISPLAY_DP_AUX_BUS
config DRM_ANALOGIX_ANX7625
tristate "Analogix Anx7625 MIPI to DP interface support"
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 3c6eed9279d2..50415a98acb7 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -21,12 +21,14 @@
#include <linux/platform_device.h>
#include <drm/bridge/analogix_dp.h>
+#include <drm/display/drm_dp_aux_bus.h>
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_bridge.h>
#include <drm/drm_crtc.h>
#include <drm/drm_device.h>
#include <drm/drm_edid.h>
+#include <drm/drm_of.h>
#include <drm/drm_panel.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -1582,6 +1584,50 @@ struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp)
}
EXPORT_SYMBOL_GPL(analogix_dp_get_aux);
+static int analogix_dp_aux_done_probing(struct drm_dp_aux *aux)
+{
+ struct analogix_dp_device *dp = to_dp(aux);
+ struct analogix_dp_plat_data *plat_data = dp->plat_data;
+ int port = plat_data->dev_type == EXYNOS_DP ? 0 : 1;
+ int ret;
+
+ /*
+ * If drm_of_find_panel_or_bridge() returns -ENODEV, there may be no valid panel
+ * or bridge nodes. The driver should go on for the driver-free bridge or the DP
+ * mode applications.
+ */
+ ret = drm_of_find_panel_or_bridge(dp->dev->of_node, port, 0,
+ &plat_data->panel, &plat_data->next_bridge);
+ if (ret && ret != -ENODEV)
+ return ret;
+
+ return component_add(dp->dev, plat_data->ops);
+}
+
+int analogix_dp_finish_probe(struct analogix_dp_device *dp)
+{
+ int ret;
+
+ ret = devm_of_dp_aux_populate_bus(&dp->aux, analogix_dp_aux_done_probing);
+ if (ret) {
+ /*
+ * If devm_of_dp_aux_populate_bus() returns -ENODEV, the done_probing() will
+ * not be called because there are no EP devices. Then the callback function
+ * analogix_dp_aux_done_probing() will be called directly in order to support
+ * the other valid DT configurations.
+ *
+ * NOTE: The devm_of_dp_aux_populate_bus() is allowed to return -EPROBE_DEFER.
+ */
+ if (ret != -ENODEV)
+ return dev_err_probe(dp->dev, ret, "failed to populate aux bus\n");
+
+ return analogix_dp_aux_done_probing(&dp->aux);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_finish_probe);
+
MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
MODULE_DESCRIPTION("Analogix DP Core Driver");
MODULE_LICENSE("GPL v2");
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 3428ffff24c5..bae969dec63a 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -30,6 +30,7 @@ struct analogix_dp_plat_data {
struct drm_bridge *next_bridge;
struct drm_encoder *encoder;
struct drm_connector *connector;
+ const struct component_ops *ops;
int (*power_on)(struct analogix_dp_plat_data *);
int (*power_off)(struct analogix_dp_plat_data *);
@@ -49,5 +50,6 @@ int analogix_dp_stop_crc(struct drm_connector *connector);
struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct drm_dp_aux *aux);
struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp);
+int analogix_dp_finish_probe(struct analogix_dp_device *dp);
#endif /* _ANALOGIX_DP_H_ */
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v14 0/8] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver
From: Damon Ding @ 2026-04-13 13:25 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, luca.ceresoli, nicolas.frattaroli, dianders,
m.szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-rockchip, Damon Ding
Picked from:
https://lore.kernel.org/all/20260409065301.446670-1-damon.ding@rock-chips.com/
PATCH 1 is the preparation for apply drm_bridge_connector helper.
PATCH 2 is to apply the drm_bridge_connector helper.
PATCH 3-5 are to move the panel/bridge parsing to the Analogix side.
PATCH 6 is to attach the next bridge on Analogix side uniformly.
PATCH 7-8 are to apply the panel_bridge helper.
Damon Ding (8):
drm/bridge: analogix_dp: Pass struct drm_atomic_state* for
analogix_dp_bridge_mode_set()
drm/bridge: analogix_dp: Apply drm_bridge_connector helper
drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()
drm/rockchip: analogix_dp: Apply analogix_dp_finish_probe()
drm/exynos: exynos_dp: Apply analogix_dp_finish_probe()
drm/bridge: analogix_dp: Attach the next bridge in
analogix_dp_bridge_attach()
drm/bridge: analogix_dp: Remove bridge disabing and panel unpreparing
in analogix_dp_unbind()
drm/bridge: analogix_dp: Apply panel_bridge helper
drivers/gpu/drm/bridge/analogix/Kconfig | 1 +
.../drm/bridge/analogix/analogix_dp_core.c | 225 +++++++++---------
.../drm/bridge/analogix/analogix_dp_core.h | 1 -
drivers/gpu/drm/exynos/Kconfig | 1 +
drivers/gpu/drm/exynos/exynos_dp.c | 59 ++---
drivers/gpu/drm/rockchip/Kconfig | 1 +
.../gpu/drm/rockchip/analogix_dp-rockchip.c | 49 +---
include/drm/bridge/analogix_dp.h | 3 +-
8 files changed, 150 insertions(+), 190 deletions(-)
---
Changes in v2:
- Update Exynos DP driver synchronously.
- Move the panel/bridge parsing to the Analogix side.
Changes in v3:
- Rebase for the existing devm_drm_bridge_alloc() applying commit.
- Fix the typographical error of panel/bridge check in exynos_dp_bind().
- Squash all commits related to skip_connector deletion in both Exynos and
Analogix code into one.
- Apply panel_bridge helper to make the codes more concise.
- Fix the handing of bridge in analogix_dp_bridge_get_modes().
- Remove unnecessary parameter struct drm_connector* for callback
&analogix_dp_plat_data.attach().
- In order to decouple the connector driver and the bridge driver, move
the bridge connector initilization to the Rockchip and Exynos sides.
Changes in v4:
- Rebase for the applied &drm_bridge_funcs.detect() modification commit.
- Rename analogix_dp_find_panel_or_bridge() to analogix_dp_finish_probe().
- Drop the drmm_encoder_init() modification commit.
- Rename the &analogix_dp_plat_data.bridge to
&analogix_dp_plat_data.next_bridge.
Changes in v5:
- Add legacy bridge to parse the display-timings node under the dp node
for Exynos side.
- Move color format check to &drm_connector_helper_funcs.atomic_check()
in order to get rid of &analogix_dp_plat_data.get_modes().
- Remove unused callback &analogix_dp_plat_data.get_modes().
- Distinguish the &drm_bridge->ops of Analogix bridge based on whether
the downstream device is a panel, a bridge or neither.
- Select DRM_DISPLAY_DP_AUX_BUS for DRM_ANALOGIX_DP, and remove it for
ROCKCHIP_ANALOGIX_DP.
- Apply rockchip_dp_attach() to support the next bridge attachment for
the Rockchip side.
- Move next_bridge attachment from Analogix side to Rockchip/Exynos sides.
Changes in v6:
- Move legacy bridge driver out of imx directory for multi-platform use.
- Apply DRM legacy bridge to parse display timings intead of implementing
the same codes only for Exynos DP.
- Ensure last bridge determines EDID/modes detection capabilities in DRM
bridge_connector driver.
- Remove unnecessary drm_bridge_get_modes() in
analogix_dp_bridge_get_modes().
- Simplify analogix_dp_bridge_edid_read().
- If the next is a bridge, set DRM_BRIDGE_OP_DETECT and return
connector_status_connected in analogix_dp_bridge_detect().
- Set flag DRM_BRIDGE_ATTACH_NO_CONNECTOR for bridge attachment while
binding. Meanwhile, make DRM_BRIDGE_ATTACH_NO_CONNECTOR unsuppported
in analogix_dp_bridge_attach().
- Move the next bridge attachment to the Analogix side rather than
scattered on Rockchip and Exynos sides.
- Remove the unnecessary analogix_dp_bridge_get_modes().
- Squash [PATCH v5 15/17] into [PATCH v5 17/17].
- Fix the &drm_bridge->ops to DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT.
Changes in v7:
- As Luca suggested, simplify the code and related comment for bridge_connector
modifications. Additionally, move the commit related to bridge_connector to
the top of this patch series.
- Rename legacy-bridge driver to of-display-mode-bridge driver.
- Remove unnecessary API drm_bridge_is_legacy() and apply a temporary flag
&exynos_dp_device.has_of_bridge instead, which will be removed finally.
- Remove exynos_dp_legacy_bridge_init() and inline API
devm_drm_of_display_mode_bridge().
Changes in v8:
- Adapt the related modifications to the newest bridge_connector driver.
Changes in v9:
- Fix the Kconfig help text for CONFIG_DRM_OF_DISPLAY_MODE_BRIDGE.
- Add Tested-by tag from Heiko.
Changes in v10:
- Fix to use dev_err_probe() in newly added API analogix_dp_finish_probe().
- Expaned commit message for [PATCH v9 9/15] and [PATCH v9 10/15].
- Split [PATCH v9 9/15] into serval smaller commits.
- Add Reviewed-by tags from Luca.
Changes in v11:
- Merge [PATCH v10 12/18] into [PATCH v10 11/18].
- Fix the bridge flag to 'flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR' in
[PATCH v10 11/18].
- Add Reviewed-by tags from Luca.
Changes in v12:
- Restore accidentally removed DRM_BRIDGE_CONNECTOR Kconfig in v10.
Changes in v13:
- Modify '(on rk3588)' to '# rk3588' for Tested-by tag as discussed in
https://lore.kernel.org/all/571cc85a-3310-4b56-a3ef-3aab698192f6@rock-chips.com/
- Rebase [PATCH v12 2/17] after commit 02df94d98ff8 ("drm/imx: parallel-display:
add DRM_DISPLAY_HELPER for DRM_IMX_PARALLEL_DISPLAY")
- Rebase [PATCH v12 7/17] and [PATCH v12 11/17] after commit 01962a191242
("drm/rockchip: analogix: Convert to drm_output_color_format")
Changes in v14:
- Picked from
https://lore.kernel.org/all/20260409065301.446670-1-damon.ding@rock-chips.com/
- Add Reviewed-by tags.
- Apply __free() to call drm_bridge_put() in CRC ralted functions of Analogix
side.
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* [PATCH v14 4/8] drm/rockchip: analogix_dp: Apply analogix_dp_finish_probe()
From: Damon Ding @ 2026-04-13 13:25 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, inki.dae, sw0312.kim, kyungmin.park,
krzk, jingoohan1, hjc, heiko, andy.yan
Cc: Laurent.pinchart, jonas, jernej.skrabec, alim.akhtar,
dmitry.baryshkov, luca.ceresoli, nicolas.frattaroli, dianders,
m.szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-rockchip, Damon Ding
In-Reply-To: <20260413132551.1049307-1-damon.ding@rock-chips.com>
Apply analogix_dp_finish_probe() in order to move the panel/bridge
parsing from Rockchip side to the Analogix side.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Heiko Stuebner <heiko@sntech.de> # rk3588
---
Changes in v4:
- Rename analogix_dp_find_panel_or_bridge() to
analogix_dp_finish_probe().
Changes in v5:
- Remove DRM_DISPLAY_DP_AUX_BUS for ROCKCHIP_ANALOGIX_DP
Changes in v9:
- Add Tested-by tag.
Changes in v13:
- Modify '(on rk3588)' to '# rk3588' for Tested-by tag.
---
.../gpu/drm/rockchip/analogix_dp-rockchip.c | 38 +------------------
1 file changed, 2 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 99e739f4f583..eea230f0227a 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -21,14 +21,12 @@
#include <video/of_videomode.h>
#include <video/videomode.h>
-#include <drm/display/drm_dp_aux_bus.h>
#include <drm/display/drm_dp_helper.h>
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_bridge_connector.h>
#include <drm/bridge/analogix_dp.h>
#include <drm/drm_of.h>
-#include <drm/drm_panel.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
#include <drm/drm_simple_kms_helper.h>
@@ -417,24 +415,6 @@ static const struct component_ops rockchip_dp_component_ops = {
.unbind = rockchip_dp_unbind,
};
-static int rockchip_dp_link_panel(struct drm_dp_aux *aux)
-{
- struct analogix_dp_plat_data *plat_data = analogix_dp_aux_to_plat_data(aux);
- struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
- int ret;
-
- /*
- * If drm_of_find_panel_or_bridge() returns -ENODEV, there may be no valid panel
- * or bridge nodes. The driver should go on for the driver-free bridge or the DP
- * mode applications.
- */
- ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 1, 0, &plat_data->panel, NULL);
- if (ret && ret != -ENODEV)
- return ret;
-
- return component_add(dp->dev, &rockchip_dp_component_ops);
-}
-
static int rockchip_dp_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -475,6 +455,7 @@ static int rockchip_dp_probe(struct platform_device *pdev)
dp->plat_data.dev_type = dp->data->chip_type;
dp->plat_data.power_on = rockchip_dp_poweron;
dp->plat_data.power_off = rockchip_dp_powerdown;
+ dp->plat_data.ops = &rockchip_dp_component_ops;
ret = rockchip_dp_of_probe(dp);
if (ret < 0)
@@ -486,22 +467,7 @@ static int rockchip_dp_probe(struct platform_device *pdev)
if (IS_ERR(dp->adp))
return PTR_ERR(dp->adp);
- ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
- if (ret) {
- /*
- * If devm_of_dp_aux_populate_bus() returns -ENODEV, the done_probing() will not
- * be called because there are no EP devices. Then the rockchip_dp_link_panel()
- * will be called directly in order to support the other valid DT configurations.
- *
- * NOTE: The devm_of_dp_aux_populate_bus() is allowed to return -EPROBE_DEFER.
- */
- if (ret != -ENODEV)
- return dev_err_probe(dp->dev, ret, "failed to populate aux bus\n");
-
- return rockchip_dp_link_panel(analogix_dp_get_aux(dp->adp));
- }
-
- return 0;
+ return analogix_dp_finish_probe(dp->adp);
}
static void rockchip_dp_remove(struct platform_device *pdev)
--
2.34.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* Re: [PATCH v13 00/27] Add new general DRM property "color format"
From: Nicolas Frattaroli @ 2026-04-13 10:26 UTC (permalink / raw)
To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan
Cc: Dmitry Baryshkov, linux-doc, Andy Yan, intel-gfx, linux-kernel,
dri-devel, Marius Vlad, linux-rockchip, Andri Yngvason, amd-gfx,
Werner Sembach, kernel, intel-xe, linux-arm-kernel
In-Reply-To: <20260413-color-format-v13-0-ab37d4dfba48@collabora.com>
On Monday, 13 April 2026 12:07:14 Central European Summer Time Nicolas Frattaroli wrote:
> Hello,
>
> [snip mucho texto]
>
> ---
> Nicolas Frattaroli (26):
> drm/display: hdmi-state-helper: Use default case for unsupported formats
> drm: Add new general DRM property "color format"
> drm/connector: Let connectors have a say in their color format
> drm/display: bridge_connector: Use HDMI color format for HDMI conns
> drm/bridge: Act on the DRM color format property
> drm/atomic-helper: Add HDMI bridge output bus formats helper
> drm/display: hdmi-state-helper: Act on color format DRM property
> drm/display: hdmi-state-helper: Try subsampling in mode_valid
> drm/amdgpu: Implement "color format" DRM property
> drm/i915/hdmi: Add YCBCR444 handling for sink formats
> drm/i915/dp: Add YCBCR444 handling for sink formats
> drm/i915: Implement the "color format" DRM property
> drm/rockchip: Add YUV422 output mode constants for VOP2
> drm/rockchip: vop2: Add RK3576 to the RG swap special case
> drm/rockchip: vop2: Recognise 10-bit YUV422 as YUV format
> drm/rockchip: vop2: Set correct output format for RK3576 YUV422
> drm/bridge: dw-hdmi-qp: Use common HDMI output bus fmts helper
> drm/rockchip: dw_hdmi_qp: Implement "color format" DRM property
> drm/rockchip: dw_hdmi_qp: Set supported_formats platdata
> drm/connector: Register color format property on HDMI connectors
> drm/tests: hdmi: Add tests for the color_format property
> drm/tests: hdmi: Add tests for HDMI helper's mode_valid
> drm/tests: bridge: Add KUnit tests for bridge chain format selection
> drm/tests: bridge: Add test for HDMI output bus formats helper
> drm/bridge: Document bridge chain format selection
> drm/connector: Update docs of "colorspace" for color format prop
>
> Werner Sembach (1):
> drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check
>
> Documentation/gpu/drm-kms-helpers.rst | 6 +
> Documentation/gpu/drm-kms.rst | 6 +
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 91 +-
> drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 1 +
> drivers/gpu/drm/display/drm_bridge_connector.c | 24 +
> drivers/gpu/drm/display/drm_hdmi_state_helper.c | 53 +-
> drivers/gpu/drm/drm_atomic_helper.c | 86 ++
> drivers/gpu/drm/drm_atomic_uapi.c | 11 +
> drivers/gpu/drm/drm_bridge.c | 104 ++-
> drivers/gpu/drm/drm_connector.c | 178 +++-
> drivers/gpu/drm/i915/display/intel_connector.c | 10 +
> drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 43 +-
> drivers/gpu/drm/i915/display/intel_hdmi.c | 47 +-
> drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 111 ++-
> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 4 +
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 21 +-
> drivers/gpu/drm/tests/drm_bridge_test.c | 971 +++++++++++++++++++++
> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 345 ++++++++
> include/drm/drm_atomic_helper.h | 7 +
> include/drm/drm_connector.h | 120 +++
> 21 files changed, 2203 insertions(+), 37 deletions(-)
> ---
> base-commit: e3dc6072028a594d5e9bfc6476165842602d5307
> change-id: 20251028-color-format-49fd202b7183
> prerequisite-message-id: <20260409101539.22032-1-ville.syrjala@linux.intel.com>
> prerequisite-patch-id: f382aeb5da5f2b8f6e2cb22b88eb47f490f2c724
> prerequisite-patch-id: 20570aeb28e3c31353e6f697b193b23d8b47c47c
> prerequisite-patch-id: 1b7e24034883b22cd82be025a1cf82ae77170fd0
> prerequisite-patch-id: 6ec2dc2c05a75391b67cb12d93168f5e8da8ec55
> prerequisite-patch-id: 32e84581998ef4eef05e1681c7ec36b90f2a6bb7
> prerequisite-patch-id: 99c2187a846b0c9ac2ea1a892c17483120cb7da1
> prerequisite-patch-id: fb41b4668a3b7c8c375c67ffd6b178fa3273e86a
> prerequisite-patch-id: 4c115a36eea0d5f80643dc34310690894ac80e0e
> prerequisite-patch-id: 6fdec0832cd6062de3cc5c2f363c5d624d8a00f9
>
> Best regards,
> --
> Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>
>
Oops, forgot to apply the review trailers from last revision. (I did
actually remember, but didn't remember to explicitly confirm applying
them, and it defaults to not doing so.)
So these two:
- drm/atomic-helper: Add HDMI bridge output bus formats helper
- drm/connector: Register color format property on HDMI connectors
Lack Dmitry Baryshkov's R-b, but I assume it'll get picked up
on the next `b4 trailers -u` anyway. Just mentioning it though
to ruin the surprise for any reviewer who feels gaslit otherwise.
Kind regards,
Nicolas Frattaroli
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* [PATCH v13 26/27] drm/bridge: Document bridge chain format selection
From: Nicolas Frattaroli @ 2026-04-13 10:07 UTC (permalink / raw)
To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan
Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, intel-gfx, intel-xe, linux-doc,
Nicolas Frattaroli
In-Reply-To: <20260413-color-format-v13-0-ab37d4dfba48@collabora.com>
The bridge chain format selection behaviour was, until now,
undocumented. With the addition of the "color format" DRM property, it's
not sufficiently complex enough that documentation is warranted,
especially for driver authors trying to do the right thing.
Add a high-level overview of how the process is supposed to work, and
mention what the display driver is supposed to do if it wants to make
use of this functionality.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Documentation/gpu/drm-kms-helpers.rst | 6 ++++++
drivers/gpu/drm/drm_bridge.c | 40 +++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index b4a9e5ae81f6..bf5a9d909cf3 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -169,6 +169,12 @@ Bridge Operations
.. kernel-doc:: drivers/gpu/drm/drm_bridge.c
:doc: bridge operations
+Bridge Chain Format Selection
+-----------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_bridge.c
+ :doc: bridge chain format selection
+
Bridge Connector Helper
-----------------------
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 5acd6bf84ae2..4cdc3c944f89 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -198,6 +198,46 @@
* driver.
*/
+/**
+ * DOC: bridge chain format selection
+ *
+ * A bridge chain, from display output processor to connector, may contain
+ * bridges capable of converting between bus formats on their inputs, and
+ * output formats on their outputs. For example, a bridge may be able to convert
+ * from RGB to YCbCr 4:4:4, and pass through YCbCr 4:2:0 as-is, but not convert
+ * from RGB to YCbCr 4:2:0. This means not all input formats map to all output
+ * formats.
+ *
+ * Further adding to this, a desired output color format, as specified with the
+ * "color format" DRM property, might not correspond 1:1 to what the display
+ * driver should set at its output. The bridge chain it feeds into may only be
+ * able to reach the desired output format, if a conversion from a different
+ * starting format is performed.
+ *
+ * To deal with this complexity, the recursive bridge chain bus format selection
+ * logic starts with the last bridge in the chain, usually the connector, and
+ * then recursively walks the chain of bridges backwards to the first bridge,
+ * trying to find a path.
+ *
+ * For a display driver to work in such a scenario, it should read the first
+ * bridge's bridge state to figure out which bus format the chain resolved to.
+ * If the first bridge's input format resolved to %MEDIA_BUS_FMT_FIXED, then its
+ * output format should be used.
+ *
+ * Special handling is done for HDMI as it relates to format selection. Instead
+ * of directly using the "color format" DRM property for bridge chains that end
+ * in HDMI bridges, the bridge chain format selection logic will trust the logic
+ * that set the HDMI output format. For the common HDMI state helper
+ * functionality, this means that %DRM_CONNECTOR_COLOR_FORMAT_AUTO will allow
+ * fallbacks to YCBCr 4:2:0 if the bandwidth requirements would otherwise be too
+ * high but the mode and connector allow it.
+ *
+ * For bridge chains that do not end in an HDMI bridge,
+ * %DRM_CONNECTOR_COLOR_FORMAT_AUTO will be satisfied with the first output
+ * format on the last bridge for which it can find a path back to the first
+ * bridge.
+ */
+
/* Protect bridge_list and bridge_lingering_list */
static DEFINE_MUTEX(bridge_lock);
static LIST_HEAD(bridge_list);
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v13 27/27] drm/connector: Update docs of "colorspace" for color format prop
From: Nicolas Frattaroli @ 2026-04-13 10:07 UTC (permalink / raw)
To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan
Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, intel-gfx, intel-xe, linux-doc,
Nicolas Frattaroli
In-Reply-To: <20260413-color-format-v13-0-ab37d4dfba48@collabora.com>
The colorspace property's documentation states that BT2020_RGB and
BT2020_YCC are equivalent, and the output format depends on the driver.
Now that there is a "color format" property that userspace can use to
explicitly set a format, update the colorspace docs to mention this.
The behaviour here is not changed for userspace that doesn't know about
the color format property yet, as the color format property defaults to
"AUTO", where the choice of output format is left up to drivers.
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
drivers/gpu/drm/drm_connector.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 048789032971..88185a53d940 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2573,7 +2573,8 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
* conversion matrix and convert to the appropriate quantization
* range.
* The variants BT2020_RGB and BT2020_YCC are equivalent and the
- * driver chooses between RGB and YCbCr on its own.
+ * driver chooses between RGB and YCbCr based on the color format
+ * property.
*
* SMPTE_170M_YCC:
* BT709_YCC:
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v13 24/27] drm/tests: bridge: Add KUnit tests for bridge chain format selection
From: Nicolas Frattaroli @ 2026-04-13 10:07 UTC (permalink / raw)
To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan
Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, intel-gfx, intel-xe, linux-doc,
Nicolas Frattaroli
In-Reply-To: <20260413-color-format-v13-0-ab37d4dfba48@collabora.com>
With the "color format" property, the bridge chain format selection has
gained increased complexity. Instead of simply finding any sequence of
bus formats that works, the bridge chain format selection needs to pick
a sequence that results in the requested color format.
Add KUnit tests for this new logic. These take the form of some pleasant
preprocessor macros to make it less cumbersome to define test bridges
with a set of possible input and output formats.
The input and output formats are defined for bridges in the form of
tuples, where the first member defines the input format, and the second
member defines the output format that can be produced from this input
format. This means the tests can construct scenarios in which not all
inputs can be converted to all outputs.
Some tests are added to test interesting scenarios to exercise the bus
format selection in the presence of a specific color format request.
Furthermore, tests are added to verify that bridge chains that end in an
HDMI connector will always prefer RGB when the color format is
DRM_CONNECTOR_COLOR_FORMAT_AUTO, as is the behaviour in the HDMI state
helpers.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
drivers/gpu/drm/tests/drm_bridge_test.c | 787 ++++++++++++++++++++++++++++++++
1 file changed, 787 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index 887020141c7f..cb821c606070 100644
--- a/drivers/gpu/drm/tests/drm_bridge_test.c
+++ b/drivers/gpu/drm/tests/drm_bridge_test.c
@@ -2,15 +2,23 @@
/*
* Kunit test for drm_bridge functions
*/
+#include <linux/cleanup.h>
+#include <linux/media-bus-format.h>
+
#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_atomic_uapi.h>
#include <drm/drm_bridge.h>
#include <drm/drm_bridge_connector.h>
#include <drm/drm_bridge_helper.h>
+#include <drm/drm_edid.h>
#include <drm/drm_kunit_helpers.h>
+#include <drm/drm_managed.h>
#include <kunit/device.h>
#include <kunit/test.h>
+#include "drm_kunit_edid.h"
+
/*
* Mimick the typical "private" struct defined by a bridge driver, which
* embeds a bridge plus other fields.
@@ -37,6 +45,21 @@ struct drm_bridge_init_priv {
bool destroyed;
};
+struct drm_bridge_chain_priv {
+ struct drm_device drm;
+ struct drm_encoder encoder;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_connector *connector;
+ unsigned int num_bridges;
+
+ /**
+ * @test_bridges: array of pointers to &struct drm_bridge_priv entries
+ * of which the first @num_bridges entries are valid.
+ */
+ struct drm_bridge_priv **test_bridges;
+};
+
static struct drm_bridge_priv *bridge_to_priv(struct drm_bridge *bridge)
{
return container_of(bridge, struct drm_bridge_priv, bridge);
@@ -95,6 +118,229 @@ static const struct drm_bridge_funcs drm_test_bridge_atomic_funcs = {
.atomic_reset = drm_atomic_helper_bridge_reset,
};
+/**
+ * struct fmt_tuple - a tuple of input/output MEDIA_BUS_FMT_*
+ */
+struct fmt_tuple {
+ u32 in_fmt;
+ u32 out_fmt;
+};
+
+/*
+ * Format mapping that only accepts RGB888, and outputs only RGB888
+ */
+static const struct fmt_tuple rgb8_passthrough[] = {
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+};
+
+/*
+ * Format mapping that only accepts YUV444, and outputs only YUV444
+ */
+static const struct fmt_tuple yuv8_passthrough[] = {
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+};
+
+/*
+ * Format mapping where 8bpc RGB -> 8bpc YUV444, or ID(RGB) or ID(YUV444)
+ */
+static const struct fmt_tuple rgb8_to_yuv8_or_id[] = {
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+};
+
+static const struct fmt_tuple rgb8_to_id_yuv8_or_yuv8_to_yuv422_yuv420[] = {
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_UYVY8_1X16 },
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_UYYVYY8_0_5X24 },
+};
+
+/*
+ * Format mapping where 8bpc YUV444 -> 8bpc RGB, or ID(YUV444)
+ */
+static const struct fmt_tuple yuv8_to_rgb8_or_id[] = {
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+};
+
+/*
+ * A format mapping that acts like a video processor that generates an RGB signal
+ */
+static const struct fmt_tuple rgb_producer[] = {
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB101010_1X30 },
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB121212_1X36 },
+};
+
+/*
+ * A format mapping that acts like a video processor that generates an 8-bit RGB,
+ * YUV444 or YUV420 signal
+ */
+static const struct fmt_tuple rgb_yuv444_yuv420_producer[] = {
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_YUV8_1X24 },
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_UYYVYY8_0_5X24 },
+};
+
+static const struct fmt_tuple rgb8_yuv444_yuv422_passthrough[] = {
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+ { MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_UYVY8_1X16 },
+};
+
+static const struct fmt_tuple yuv444_yuv422_rgb8_passthrough[] = {
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+ { MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_UYVY8_1X16 },
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+};
+
+static bool fmt_in_list(const u32 fmt, const u32 *out_fmts, const size_t num_fmts)
+{
+ size_t i;
+
+ for (i = 0; i < num_fmts; i++)
+ if (out_fmts[i] == fmt)
+ return true;
+
+ return false;
+}
+
+/**
+ * get_tuples_out_fmts - Get unique output formats of a &struct fmt_tuple list
+ * @fmt_tuples: array of &struct fmt_tuple
+ * @num_fmt_tuples: number of entries in @fmt_tuples
+ * @out_fmts: target array to store the unique output bus formats
+ *
+ * Returns the number of unique output formats, i.e. the number of entries in
+ * @out_fmts that were populated with sensible values.
+ */
+static size_t get_tuples_out_fmts(const struct fmt_tuple *fmt_tuples,
+ const size_t num_fmt_tuples, u32 *out_fmts)
+{
+ size_t num_unique = 0;
+ size_t i;
+
+ for (i = 0; i < num_fmt_tuples; i++)
+ if (!fmt_in_list(fmt_tuples[i].out_fmt, out_fmts, num_unique))
+ out_fmts[num_unique++] = fmt_tuples[i].out_fmt;
+
+ return num_unique;
+}
+
+#define DEFINE_FMT_FUNCS_FROM_TUPLES(name) \
+static u32 *drm_test_bridge_ ## name ## _out_fmts(struct drm_bridge *bridge, \
+ struct drm_bridge_state *bridge_state, \
+ struct drm_crtc_state *crtc_state, \
+ struct drm_connector_state *conn_state, \
+ unsigned int *num_output_fmts) \
+{ \
+ u32 *out_fmts = kcalloc(ARRAY_SIZE((name)), sizeof(u32), GFP_KERNEL); \
+ \
+ if (out_fmts) \
+ *num_output_fmts = get_tuples_out_fmts((name), ARRAY_SIZE((name)), out_fmts); \
+ else \
+ *num_output_fmts = 0; \
+ \
+ return out_fmts; \
+} \
+ \
+static u32 *drm_test_bridge_ ## name ## _in_fmts(struct drm_bridge *bridge, \
+ struct drm_bridge_state *bridge_state, \
+ struct drm_crtc_state *crtc_state, \
+ struct drm_connector_state *conn_state, \
+ u32 output_fmt, \
+ unsigned int *num_input_fmts) \
+{ \
+ u32 *in_fmts = kcalloc(ARRAY_SIZE((name)), sizeof(u32), GFP_KERNEL); \
+ unsigned int num_fmts = 0; \
+ size_t i; \
+ \
+ if (!in_fmts) { \
+ *num_input_fmts = 0; \
+ return NULL; \
+ } \
+ \
+ for (i = 0; i < ARRAY_SIZE((name)); i++) \
+ if ((name)[i].out_fmt == output_fmt) \
+ in_fmts[num_fmts++] = (name)[i].in_fmt; \
+ \
+ *num_input_fmts = num_fmts; \
+ \
+ return in_fmts; \
+}
+
+#define DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI_FUNC(ident, input_fmts_func, output_fmts_func, \
+ hdmi_write_infoframe_func, \
+ hdmi_clear_infoframe_func) \
+static const struct drm_bridge_funcs (ident) = { \
+ .atomic_enable = drm_test_bridge_atomic_enable, \
+ .atomic_disable = drm_test_bridge_atomic_disable, \
+ .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, \
+ .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, \
+ .atomic_reset = drm_atomic_helper_bridge_reset, \
+ .atomic_get_input_bus_fmts = (input_fmts_func), \
+ .atomic_get_output_bus_fmts = (output_fmts_func), \
+ .hdmi_write_avi_infoframe = (hdmi_write_infoframe_func), \
+ .hdmi_clear_avi_infoframe = (hdmi_clear_infoframe_func), \
+ .hdmi_write_hdmi_infoframe = (hdmi_write_infoframe_func), \
+ .hdmi_clear_hdmi_infoframe = (hdmi_clear_infoframe_func), \
+}
+
+#define DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_FUNC(ident, input_fmts_func, output_fmts_func) \
+ DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI_FUNC(ident, input_fmts_func, output_fmts_func, \
+ NULL, NULL)
+
+#define DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(_name) \
+ DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_FUNC(_name ## _funcs, \
+ drm_test_bridge_ ## _name ## _in_fmts, \
+ drm_test_bridge_ ## _name ## _out_fmts)
+
+static int drm_test_bridge_write_infoframe_stub(struct drm_bridge *bridge,
+ const u8 *buffer, size_t len)
+{
+ return 0;
+}
+
+static int drm_test_bridge_clear_infoframe_stub(struct drm_bridge *bridge)
+{
+ return 0;
+}
+
+#define DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI(_name) \
+ DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI_FUNC(_name ## _hdmi ## _funcs, \
+ drm_test_bridge_ ## _name ## _in_fmts, \
+ drm_test_bridge_ ## _name ## _out_fmts, \
+ drm_test_bridge_write_infoframe_stub, \
+ drm_test_bridge_clear_infoframe_stub)
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb8_passthrough)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb8_passthrough);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(yuv8_passthrough)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(yuv8_passthrough);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb8_to_yuv8_or_id)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb8_to_yuv8_or_id);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(yuv8_to_rgb8_or_id)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(yuv8_to_rgb8_or_id);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb_producer)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb_producer);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb_yuv444_yuv420_producer)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb_yuv444_yuv420_producer);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb8_to_id_yuv8_or_yuv8_to_yuv422_yuv420)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb8_to_id_yuv8_or_yuv8_to_yuv422_yuv420);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(rgb8_yuv444_yuv422_passthrough)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(rgb8_yuv444_yuv422_passthrough);
+
+DEFINE_FMT_FUNCS_FROM_TUPLES(yuv444_yuv422_rgb8_passthrough)
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT(yuv444_yuv422_rgb8_passthrough);
+DRM_BRIDGE_ATOMIC_WITH_BUS_FMT_HDMI(yuv444_yuv422_rgb8_passthrough);
+
KUNIT_DEFINE_ACTION_WRAPPER(drm_bridge_remove_wrapper,
drm_bridge_remove,
struct drm_bridge *);
@@ -180,6 +426,119 @@ drm_test_bridge_init(struct kunit *test, const struct drm_bridge_funcs *funcs)
return priv;
}
+static struct drm_bridge_chain_priv *
+drm_test_bridge_chain_init(struct kunit *test, unsigned int num_bridges,
+ const struct drm_bridge_funcs **funcs)
+{
+ struct drm_bridge_chain_priv *priv;
+ const struct drm_edid *edid;
+ struct drm_bridge *prev;
+ struct drm_encoder *enc;
+ struct drm_bridge *bridge;
+ struct drm_device *drm;
+ bool has_hdmi = false;
+ struct device *dev;
+ unsigned int i;
+ int ret;
+
+ dev = drm_kunit_helper_alloc_device(test);
+ if (IS_ERR(dev))
+ return ERR_CAST(dev);
+
+ priv = drm_kunit_helper_alloc_drm_device(test, dev, struct drm_bridge_chain_priv,
+ drm, DRIVER_MODESET | DRIVER_ATOMIC);
+ if (IS_ERR(priv))
+ return ERR_CAST(priv);
+
+ drm = &priv->drm;
+
+ priv->test_bridges = drmm_kmalloc_array(drm, num_bridges, sizeof(*priv->test_bridges),
+ GFP_KERNEL);
+ if (!priv->test_bridges)
+ return ERR_PTR(-ENOMEM);
+
+ priv->num_bridges = num_bridges;
+
+ for (i = 0; i < num_bridges; i++) {
+ priv->test_bridges[i] = devm_drm_bridge_alloc(dev, struct drm_bridge_priv,
+ bridge, funcs[i]);
+ if (IS_ERR(priv->test_bridges[i]))
+ return ERR_CAST(priv->test_bridges[i]);
+
+ priv->test_bridges[i]->data = priv;
+ }
+
+ priv->plane = drm_kunit_helper_create_primary_plane(test, drm, NULL, NULL,
+ NULL, 0, NULL);
+ if (IS_ERR(priv->plane))
+ return ERR_CAST(priv->plane);
+
+ priv->crtc = drm_kunit_helper_create_crtc(test, drm, priv->plane, NULL,
+ NULL, NULL);
+ if (IS_ERR(priv->crtc))
+ return ERR_CAST(priv->crtc);
+
+ enc = &priv->encoder;
+ ret = drmm_encoder_init(drm, enc, NULL, DRM_MODE_ENCODER_TMDS, NULL);
+ if (ret)
+ return ERR_PTR(ret);
+
+ enc->possible_crtcs = drm_crtc_mask(priv->crtc);
+
+ prev = NULL;
+ for (i = 0; i < num_bridges; i++) {
+ bridge = &priv->test_bridges[i]->bridge;
+ bridge->type = DRM_MODE_CONNECTOR_VIRTUAL;
+
+ if (bridge->funcs->hdmi_write_hdmi_infoframe) {
+ has_hdmi = true;
+ bridge->ops |= DRM_BRIDGE_OP_HDMI;
+ bridge->type = DRM_MODE_CONNECTOR_HDMIA;
+ bridge->vendor = "LNX";
+ bridge->product = "KUnit";
+ bridge->supported_formats = (BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420));
+ }
+
+ ret = drm_kunit_bridge_add(test, bridge);
+ if (ret)
+ return ERR_PTR(ret);
+
+ ret = drm_bridge_attach(enc, bridge, prev, 0);
+ if (ret)
+ return ERR_PTR(ret);
+
+ prev = bridge;
+ }
+
+ priv->connector = drm_bridge_connector_init(drm, enc);
+ if (IS_ERR(priv->connector))
+ return ERR_CAST(priv->connector);
+
+ drm_connector_attach_encoder(priv->connector, enc);
+
+ drm_mode_config_reset(drm);
+
+ if (!has_hdmi)
+ return priv;
+
+ scoped_guard(mutex, &drm->mode_config.mutex) {
+ edid = drm_edid_alloc(test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz,
+ ARRAY_SIZE(test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz));
+ if (!edid)
+ return ERR_PTR(-EINVAL);
+
+ drm_edid_connector_update(priv->connector, edid);
+ KUNIT_ASSERT_GT(test, drm_edid_connector_add_modes(priv->connector), 0);
+
+ ret = priv->connector->funcs->fill_modes(priv->connector, 4096, 4096);
+ }
+
+ return priv;
+}
+
/*
* Test that drm_bridge_get_current_state() returns the last committed
* state for an atomic bridge.
@@ -508,14 +867,442 @@ static struct kunit_suite drm_bridge_alloc_test_suite = {
.test_cases = drm_bridge_alloc_tests,
};
+/**
+ * drm_test_bridge_chain_verify_fmt - Verify bridge chain format selection
+ * @test: pointer to KUnit test object
+ * @priv: pointer to a &struct drm_bridge_chain_priv for this chain
+ * @expected: constant array of &struct fmt_tuple describing the expected
+ * input and output bus formats
+ * @num_expected: number of entries in @expected
+ *
+ * Runs the KUNIT_EXPECT clauses to verify the bridge chain format selection
+ * resulted in the expected formats. If %0 is given as a format in a
+ * &struct fmt_tuple, then it is understood to mean "any".
+ *
+ * Must be called with the modeset lock held.
+ */
+static void drm_test_bridge_chain_verify_fmt(struct kunit *test,
+ struct drm_bridge_chain_priv *priv,
+ const struct fmt_tuple *const expected,
+ const unsigned int num_expected)
+{
+ struct drm_bridge_state *bstate;
+ unsigned int i = 0;
+
+ drm_for_each_bridge_in_chain_scoped(&priv->encoder, bridge) {
+ KUNIT_ASSERT_LT(test, i, num_expected);
+
+ bstate = drm_bridge_get_current_state(bridge);
+ if (expected[i].in_fmt)
+ KUNIT_EXPECT_EQ(test, bstate->input_bus_cfg.format,
+ expected[i].in_fmt);
+ if (expected[i].out_fmt)
+ KUNIT_EXPECT_EQ(test, bstate->output_bus_cfg.format,
+ expected[i].out_fmt);
+
+ i++;
+ }
+
+ KUNIT_ASSERT_EQ_MSG(test, i, num_expected,
+ "Fewer bridges (%u) than expected (%u)\n", i, num_expected);
+}
+
+/*
+ * Test that constructs a bridge chain in which an RGB888 producer is chained to
+ * two bridges that will convert from RGB to YUV and from YUV to RGB respectively.
+ *
+ * The test requests an output color_format of RGB using the color_format property,
+ * so to satisfy this request, the bridge chain must take a detour over YUV.
+ */
+static void drm_test_bridge_rgb_yuv_rgb(struct kunit *test)
+{
+ static const struct drm_bridge_funcs *funcs[] = {
+ &rgb_producer_funcs,
+ &rgb8_to_yuv8_or_id_funcs,
+ &yuv8_to_rgb8_or_id_funcs,
+ };
+ static const struct fmt_tuple expected[] = {
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+ };
+ struct drm_connector_state *conn_state;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_bridge_chain_priv *priv;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *mode;
+ int ret;
+
+ priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+ conn_state = drm_atomic_get_connector_state(state, priv->connector);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+ conn_state->color_format = DRM_CONNECTOR_COLOR_FORMAT_RGB444;
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+ ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state->enable = true;
+ crtc_state->active = true;
+
+ ret = drm_atomic_commit(state);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ drm_test_bridge_chain_verify_fmt(test, priv, expected, ARRAY_SIZE(expected));
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
+/*
+ * Test in which a bridge capable of producing RGB, YUV444 and YUV420 has to
+ * produce RGB and convert with a downstream bridge in the chain to reach the
+ * requested YUV444 color format, as no direct path exists between its YUV444
+ * and the last bridge.
+ *
+ * The rationale behind this test is to devise a scenario in which naively
+ * assuming any format the video processor can output, and the connector
+ * requests, is the right format to pick, does not work.
+ */
+static void drm_test_bridge_must_convert_to_yuv444(struct kunit *test)
+{
+ static const struct drm_bridge_funcs *funcs[] = {
+ &rgb_yuv444_yuv420_producer_funcs,
+ &rgb8_passthrough_funcs,
+ &rgb8_to_id_yuv8_or_yuv8_to_yuv422_yuv420_funcs,
+ &rgb8_yuv444_yuv422_passthrough_funcs,
+ };
+ static const struct fmt_tuple expected[] = {
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+ };
+ struct drm_connector_state *conn_state;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_bridge_chain_priv *priv;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *mode;
+ int ret;
+
+ priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+ conn_state = drm_atomic_get_connector_state(state, priv->connector);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+ conn_state->color_format = DRM_CONNECTOR_COLOR_FORMAT_YCBCR444;
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+ ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state->enable = true;
+ crtc_state->active = true;
+
+ ret = drm_atomic_commit(state);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ drm_test_bridge_chain_verify_fmt(test, priv, expected, ARRAY_SIZE(expected));
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
+/*
+ * Test which checks that no matter the order of bus formats returned by an
+ * HDMI bridge, RGB is preferred on DRM_CONNECTOR_COLOR_FORMAT_AUTO if it's
+ * available.
+ */
+static void drm_test_bridge_hdmi_auto_rgb(struct kunit *test)
+{
+ static const struct drm_bridge_funcs *funcs[] = {
+ &rgb_yuv444_yuv420_producer_funcs,
+ &yuv444_yuv422_rgb8_passthrough_hdmi_funcs,
+ };
+ static const struct fmt_tuple expected[] = {
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_RGB888_1X24 },
+ };
+ struct drm_connector_state *conn_state;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_bridge_chain_priv *priv;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *mode;
+ int ret;
+
+ priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+ conn_state = drm_atomic_get_connector_state(state, priv->connector);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+ KUNIT_ASSERT_EQ(test, conn_state->color_format, DRM_CONNECTOR_COLOR_FORMAT_AUTO);
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+ ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state->enable = true;
+ crtc_state->active = true;
+
+ ret = drm_atomic_commit(state);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, DRM_OUTPUT_COLOR_FORMAT_RGB444);
+
+ drm_test_bridge_chain_verify_fmt(test, priv, expected, ARRAY_SIZE(expected));
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
+/*
+ * Test which checks that DRM_CONNECTOR_COLOR_FORMAT_AUTO on non-HDMI connectors
+ * will result in the first bus format on the output to be picked.
+ */
+static void drm_test_bridge_auto_first(struct kunit *test)
+{
+ static const struct drm_bridge_funcs *funcs[] = {
+ &rgb_yuv444_yuv420_producer_funcs,
+ &yuv444_yuv422_rgb8_passthrough_funcs,
+ };
+ static const struct fmt_tuple expected[] = {
+ { MEDIA_BUS_FMT_FIXED, MEDIA_BUS_FMT_YUV8_1X24 },
+ { MEDIA_BUS_FMT_YUV8_1X24, MEDIA_BUS_FMT_YUV8_1X24 },
+ };
+ struct drm_connector_state *conn_state;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_bridge_chain_priv *priv;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *mode;
+ int ret;
+
+ priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+ conn_state = drm_atomic_get_connector_state(state, priv->connector);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+ KUNIT_ASSERT_EQ(test, conn_state->color_format, DRM_CONNECTOR_COLOR_FORMAT_AUTO);
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+ ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state->enable = true;
+ crtc_state->active = true;
+
+ ret = drm_atomic_commit(state);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ drm_test_bridge_chain_verify_fmt(test, priv, expected, ARRAY_SIZE(expected));
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
+/*
+ * Test which checks that in a configuration of bridge chains where an RGB
+ * producer is hooked to a YUV444-only pass-through, the atomic commit fails as
+ * the bridge format selection cannot find a valid sequence of bus formats.
+ */
+static void drm_test_bridge_rgb_yuv_no_path(struct kunit *test)
+{
+ static const struct drm_bridge_funcs *funcs[] = {
+ &rgb_producer_funcs,
+ &yuv8_passthrough_funcs,
+ &rgb8_yuv444_yuv422_passthrough_funcs,
+ };
+ struct drm_connector_state *conn_state;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_bridge_chain_priv *priv;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *mode;
+ int ret;
+
+ priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+ conn_state = drm_atomic_get_connector_state(state, priv->connector);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+ ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state->enable = true;
+ crtc_state->active = true;
+
+ ret = drm_atomic_commit(state);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_EXPECT_EQ(test, ret, -ENOTSUPP);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
+static struct kunit_case drm_bridge_bus_fmt_tests[] = {
+ KUNIT_CASE(drm_test_bridge_rgb_yuv_rgb),
+ KUNIT_CASE(drm_test_bridge_must_convert_to_yuv444),
+ KUNIT_CASE(drm_test_bridge_hdmi_auto_rgb),
+ KUNIT_CASE(drm_test_bridge_auto_first),
+ KUNIT_CASE(drm_test_bridge_rgb_yuv_no_path),
+ { }
+};
+
+static struct kunit_suite drm_bridge_bus_fmt_test_suite = {
+ .name = "drm_bridge_bus_fmt",
+ .test_cases = drm_bridge_bus_fmt_tests,
+};
+
kunit_test_suites(
&drm_bridge_get_current_state_test_suite,
&drm_bridge_helper_reset_crtc_test_suite,
&drm_bridge_alloc_test_suite,
+ &drm_bridge_bus_fmt_test_suite,
);
MODULE_AUTHOR("Maxime Ripard <mripard@kernel.org>");
MODULE_AUTHOR("Luca Ceresoli <luca.ceresoli@bootlin.com>");
+MODULE_AUTHOR("Nicolas Frattaroli <nicolas.frattaroli@collabora.com>");
MODULE_DESCRIPTION("Kunit test for drm_bridge functions");
MODULE_LICENSE("GPL");
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v13 25/27] drm/tests: bridge: Add test for HDMI output bus formats helper
From: Nicolas Frattaroli @ 2026-04-13 10:07 UTC (permalink / raw)
To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan
Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, intel-gfx, intel-xe, linux-doc,
Nicolas Frattaroli
In-Reply-To: <20260413-color-format-v13-0-ab37d4dfba48@collabora.com>
The common atomic_get_output_bus_fmts helper for HDMI bridge connectors,
called drm_atomic_helper_bridge_get_hdmi_output_bus_fmts, should return
an array of output bus formats depending on the supported formats of the
connector, and the current output BPC.
Add a test to exercise some of this helper.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
drivers/gpu/drm/tests/drm_bridge_test.c | 184 ++++++++++++++++++++++++++++++++
1 file changed, 184 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index cb821c606070..d9bd930b1197 100644
--- a/drivers/gpu/drm/tests/drm_bridge_test.c
+++ b/drivers/gpu/drm/tests/drm_bridge_test.c
@@ -5,6 +5,7 @@
#include <linux/cleanup.h>
#include <linux/media-bus-format.h>
+#include <drm/drm_atomic_helper.h>
#include <drm/drm_atomic_state_helper.h>
#include <drm/drm_atomic_uapi.h>
#include <drm/drm_bridge.h>
@@ -118,6 +119,28 @@ static const struct drm_bridge_funcs drm_test_bridge_atomic_funcs = {
.atomic_reset = drm_atomic_helper_bridge_reset,
};
+static int dummy_clear_infoframe(struct drm_bridge *bridge)
+{
+ return 0;
+}
+
+static int dummy_write_infoframe(struct drm_bridge *bridge, const u8 *buffer,
+ size_t len)
+{
+ return 0;
+}
+
+static const struct drm_bridge_funcs drm_test_bridge_bus_fmts_funcs = {
+ .atomic_get_output_bus_fmts = drm_atomic_helper_bridge_get_hdmi_output_bus_fmts,
+ .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+ .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+ .atomic_reset = drm_atomic_helper_bridge_reset,
+ .hdmi_write_avi_infoframe = dummy_write_infoframe,
+ .hdmi_write_hdmi_infoframe = dummy_write_infoframe,
+ .hdmi_clear_avi_infoframe = dummy_clear_infoframe,
+ .hdmi_clear_hdmi_infoframe = dummy_clear_infoframe,
+};
+
/**
* struct fmt_tuple - a tuple of input/output MEDIA_BUS_FMT_*
*/
@@ -539,6 +562,83 @@ drm_test_bridge_chain_init(struct kunit *test, unsigned int num_bridges,
return priv;
}
+static struct drm_bridge_init_priv *
+drm_test_bridge_hdmi_init(struct kunit *test, const struct drm_bridge_funcs *funcs,
+ unsigned int supported_formats, int max_bpc)
+{
+ struct drm_bridge_init_priv *priv;
+ struct drm_encoder *enc;
+ struct drm_bridge *bridge;
+ struct drm_device *drm;
+ struct device *dev;
+ int ret;
+
+ dev = drm_kunit_helper_alloc_device(test);
+ if (IS_ERR(dev))
+ return ERR_CAST(dev);
+
+ priv = drm_kunit_helper_alloc_drm_device(test, dev,
+ struct drm_bridge_init_priv, drm,
+ DRIVER_MODESET | DRIVER_ATOMIC);
+ if (IS_ERR(priv))
+ return ERR_CAST(priv);
+
+ priv->test_bridge = devm_drm_bridge_alloc(dev, struct drm_bridge_priv, bridge, funcs);
+ if (IS_ERR(priv->test_bridge))
+ return ERR_CAST(priv->test_bridge);
+
+ priv->test_bridge->data = priv;
+
+ drm = &priv->drm;
+ priv->plane = drm_kunit_helper_create_primary_plane(test, drm,
+ NULL,
+ NULL,
+ NULL, 0,
+ NULL);
+ if (IS_ERR(priv->plane))
+ return ERR_CAST(priv->plane);
+
+ priv->crtc = drm_kunit_helper_create_crtc(test, drm,
+ priv->plane, NULL,
+ NULL,
+ NULL);
+ if (IS_ERR(priv->crtc))
+ return ERR_CAST(priv->crtc);
+
+ enc = &priv->encoder;
+ ret = drmm_encoder_init(drm, enc, NULL, DRM_MODE_ENCODER_TMDS, NULL);
+ if (ret)
+ return ERR_PTR(ret);
+
+ enc->possible_crtcs = drm_crtc_mask(priv->crtc);
+
+ bridge = &priv->test_bridge->bridge;
+ bridge->type = DRM_MODE_CONNECTOR_HDMIA;
+ bridge->supported_formats = supported_formats;
+ bridge->max_bpc = max_bpc;
+ bridge->ops |= DRM_BRIDGE_OP_HDMI;
+ bridge->vendor = "LNX";
+ bridge->product = "KUnit";
+
+ ret = drm_kunit_bridge_add(test, bridge);
+ if (ret)
+ return ERR_PTR(ret);
+
+ ret = drm_bridge_attach(enc, bridge, NULL, 0);
+ if (ret)
+ return ERR_PTR(ret);
+
+ priv->connector = drm_bridge_connector_init(drm, enc);
+ if (IS_ERR(priv->connector))
+ return ERR_CAST(priv->connector);
+
+ drm_connector_attach_encoder(priv->connector, enc);
+
+ drm_mode_config_reset(drm);
+
+ return priv;
+}
+
/*
* Test that drm_bridge_get_current_state() returns the last committed
* state for an atomic bridge.
@@ -786,10 +886,94 @@ static void drm_test_drm_bridge_helper_reset_crtc_legacy(struct kunit *test)
KUNIT_EXPECT_EQ(test, bridge_priv->disable_count, 1);
}
+/*
+ * Test that a bridge using the drm_atomic_helper_bridge_get_hdmi_output_bus_fmts()
+ * function for &drm_bridge_funcs.atomic_get_output_bus_fmts behaves as expected
+ * for an HDMI connector bridge. Does so by creating an HDMI bridge connector
+ * with RGB444, YCBCR444, and YCBCR420 (but not YCBCR422) as supported formats,
+ * sets the output depth to 8 bits per component, and then validates the returned
+ * list of bus formats.
+ */
+static void drm_test_drm_bridge_helper_hdmi_output_bus_fmts(struct kunit *test)
+{
+ struct drm_connector_state *conn_state;
+ struct drm_bridge_state *bridge_state;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_bridge_init_priv *priv;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *mode;
+ unsigned int num_output_fmts;
+ struct drm_bridge *bridge;
+ u32 *out_bus_fmts;
+ int ret;
+
+ priv = drm_test_bridge_hdmi_init(test, &drm_test_bridge_bus_fmts_funcs,
+ BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420),
+ 12);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+ bridge = &priv->test_bridge->bridge;
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_commit:
+ conn_state = drm_atomic_get_connector_state(state, priv->connector);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ conn_state->hdmi.output_bpc = 8;
+
+ mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+ ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ goto retry_commit;
+ }
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state->enable = true;
+ crtc_state->active = true;
+
+ bridge_state = drm_atomic_get_bridge_state(state, bridge);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bridge_state);
+
+ out_bus_fmts = bridge->funcs->atomic_get_output_bus_fmts(
+ bridge, bridge_state, crtc_state, conn_state, &num_output_fmts);
+ KUNIT_EXPECT_NOT_NULL(test, out_bus_fmts);
+ KUNIT_EXPECT_EQ(test, num_output_fmts, 3);
+
+ KUNIT_EXPECT_EQ(test, out_bus_fmts[0], MEDIA_BUS_FMT_RGB888_1X24);
+ KUNIT_EXPECT_EQ(test, out_bus_fmts[1], MEDIA_BUS_FMT_YUV8_1X24);
+ KUNIT_EXPECT_EQ(test, out_bus_fmts[2], MEDIA_BUS_FMT_UYYVYY8_0_5X24);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+
+ kfree(out_bus_fmts);
+}
+
static struct kunit_case drm_bridge_helper_reset_crtc_tests[] = {
KUNIT_CASE(drm_test_drm_bridge_helper_reset_crtc_atomic),
KUNIT_CASE(drm_test_drm_bridge_helper_reset_crtc_atomic_disabled),
KUNIT_CASE(drm_test_drm_bridge_helper_reset_crtc_legacy),
+ KUNIT_CASE(drm_test_drm_bridge_helper_hdmi_output_bus_fmts),
{ }
};
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v13 22/27] drm/tests: hdmi: Add tests for the color_format property
From: Nicolas Frattaroli @ 2026-04-13 10:07 UTC (permalink / raw)
To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan
Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, intel-gfx, intel-xe, linux-doc,
Nicolas Frattaroli
In-Reply-To: <20260413-color-format-v13-0-ab37d4dfba48@collabora.com>
Add some KUnit tests to check the color_format property is working as
expected with the HDMI state helper.
Existing tests are extended to also test the
DRM_CONNECTOR_COLOR_FORMAT_AUTO case, in order to avoid duplicating test
cases. For the explicitly selected color format cases, parameterized
tests are added.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 236 +++++++++++++++++++++
1 file changed, 236 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index a4357efaa983..3444c93c615f 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -60,6 +60,23 @@ static struct drm_display_mode *find_preferred_mode(struct drm_connector *connec
return preferred;
}
+static struct drm_display_mode *find_420_only_mode(struct drm_connector *connector)
+{
+ struct drm_device *drm = connector->dev;
+ struct drm_display_mode *mode;
+
+ mutex_lock(&drm->mode_config.mutex);
+ list_for_each_entry(mode, &connector->modes, head) {
+ if (drm_mode_is_420_only(&connector->display_info, mode)) {
+ mutex_unlock(&drm->mode_config.mutex);
+ return mode;
+ }
+ }
+ mutex_unlock(&drm->mode_config.mutex);
+
+ return NULL;
+}
+
static int set_connector_edid(struct kunit *test, struct drm_connector *connector,
const void *edid, size_t edid_len)
{
@@ -1547,6 +1564,7 @@ static void drm_test_check_max_tmds_rate_bpc_fallback_yuv420(struct kunit *test)
* RGB/10bpc
* - The chosen mode has a TMDS character rate lower than the display
* supports in YUV422/12bpc.
+ * - The HDMI connector state's color format property is unset (i.e. AUTO)
*
* Then we will prefer to keep the RGB format with a lower bpc over
* picking YUV422.
@@ -1609,6 +1627,7 @@ static void drm_test_check_max_tmds_rate_bpc_fallback_ignore_yuv422(struct kunit
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
+ KUNIT_ASSERT_EQ(test, conn_state->color_format, DRM_CONNECTOR_COLOR_FORMAT_AUTO);
KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 10);
KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, DRM_OUTPUT_COLOR_FORMAT_RGB444);
@@ -1626,6 +1645,7 @@ static void drm_test_check_max_tmds_rate_bpc_fallback_ignore_yuv422(struct kunit
* RGB/8bpc
* - The chosen mode has a TMDS character rate lower than the display
* supports in YUV420/12bpc.
+ * - The HDMI connector state's color format property is unset (i.e. AUTO)
*
* Then we will prefer to keep the RGB format with a lower bpc over
* picking YUV420.
@@ -1687,6 +1707,7 @@ static void drm_test_check_max_tmds_rate_bpc_fallback_ignore_yuv420(struct kunit
conn_state = conn->state;
KUNIT_ASSERT_NOT_NULL(test, conn_state);
+ KUNIT_ASSERT_EQ(test, conn_state->color_format, DRM_CONNECTOR_COLOR_FORMAT_AUTO);
KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 8);
KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, DRM_OUTPUT_COLOR_FORMAT_RGB444);
@@ -2198,6 +2219,217 @@ static void drm_test_check_disable_connector(struct kunit *test)
drm_modeset_acquire_fini(&ctx);
}
+struct color_format_test_param {
+ enum drm_connector_color_format fmt;
+ enum drm_output_color_format expected;
+ int expected_ret;
+ const char *desc;
+};
+
+/* Test that if:
+ * - an HDMI connector supports RGB, YUV444, YUV422, and YUV420
+ * - the display supports RGB, YUV444, YUV422, and YUV420
+ * - the "color format" property is set
+ * then, for the preferred mode, for a given "color format" option:
+ * - DRM_CONNECTOR_COLOR_FORMAT_AUTO results in an output format of RGB
+ * - DRM_CONNECTOR_COLOR_FORMAT_YCBCR422 results in an output format of YUV422
+ * - DRM_CONNECTOR_COLOR_FORMAT_YCBCR420 results in an output format of YUV420
+ * - DRM_CONNECTOR_COLOR_FORMAT_YCBCR444 results in an output format of YUV444
+ * - DRM_CONNECTOR_COLOR_FORMAT_RGB results in an HDMI output format of RGB
+ */
+static void drm_test_check_hdmi_color_format(struct kunit *test)
+{
+ const struct color_format_test_param *param = test->param_value;
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_connector_state *conn_state;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_info *info;
+ struct drm_display_mode *preferred;
+ int ret;
+
+ priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+ BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444),
+ 12,
+ &dummy_connector_hdmi_funcs,
+ test_edid_hdmi_4k_rgb_yuv420_dc_max_340mhz);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ KUNIT_ASSERT_TRUE(test, priv->connector.ycbcr_420_allowed);
+
+ info = &priv->connector.display_info;
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, info);
+ preferred = find_preferred_mode(&priv->connector);
+ KUNIT_ASSERT_TRUE(test, drm_mode_is_420(info, preferred));
+
+ state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+ conn_state = drm_atomic_get_connector_state(state, &priv->connector);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ conn_state->color_format = param->fmt;
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+ ret = drm_atomic_set_mode_for_crtc(crtc_state, preferred);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state->enable = true;
+ crtc_state->active = true;
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_EXPECT_EQ(test, ret, param->expected_ret);
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, param->expected);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
+static const struct color_format_test_param hdmi_color_format_params[] = {
+ {
+ .fmt = DRM_CONNECTOR_COLOR_FORMAT_AUTO,
+ .expected = DRM_OUTPUT_COLOR_FORMAT_RGB444,
+ .expected_ret = 0,
+ .desc = "AUTO -> RGB"
+ },
+ {
+ .fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
+ .expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR422,
+ .expected_ret = 0,
+ .desc = "YCBCR422 -> YUV422"
+ },
+ {
+ .fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
+ .expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR420,
+ .expected_ret = 0,
+ .desc = "YCBCR420 -> YUV420"
+ },
+ {
+ .fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
+ .expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR444,
+ .expected_ret = 0,
+ .desc = "YCBCR444 -> YUV444"
+ },
+ {
+ .fmt = DRM_CONNECTOR_COLOR_FORMAT_RGB444,
+ .expected = DRM_OUTPUT_COLOR_FORMAT_RGB444,
+ .expected_ret = 0,
+ .desc = "RGB -> RGB"
+ },
+};
+
+KUNIT_ARRAY_PARAM_DESC(check_hdmi_color_format, hdmi_color_format_params, desc);
+
+/* Test that if:
+ * - the HDMI connector supports RGB, YUV422, YUV420, and YUV444
+ * - the display has a YUV420-only mode
+ * - the "color format" property is explicitly set (i.e. !AUTO)
+ * then:
+ * - color format DRM_CONNECTOR_COLOR_FORMAT_RGB444 will fail
+ * drm_atomic_check_only for the YUV420-only mode with -EINVAL
+ * - color format DRM_CONNECTOR_COLOR_FORMAT_YCBCR444 will fail
+ * drm_atomic_check_only for the YUV420-only mode with -EINVAL
+ * - color format DRM_CONNECTOR_COLOR_FORMAT_YCBCR422 will fail
+ * drm_atomic_check_only for the YUV420-only mode with -EINVAL
+ * - color format DRM_CONNECTOR_COLOR_FORMAT_YCBCR420 passes
+ * drm_atomic_check_only for the YUV420-only mode
+ */
+static void drm_test_check_hdmi_color_format_420_only(struct kunit *test)
+{
+ const struct color_format_test_param *param = test->param_value;
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_connector_state *conn_state;
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_crtc_state *crtc_state;
+ struct drm_atomic_state *state;
+ struct drm_display_mode *dank;
+ int ret;
+
+ priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+ BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420) |
+ BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444),
+ 12,
+ &dummy_connector_hdmi_funcs,
+ test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ dank = find_420_only_mode(&priv->connector);
+ KUNIT_ASSERT_NOT_NULL(test, dank);
+
+ state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+ conn_state = drm_atomic_get_connector_state(state, &priv->connector);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+ conn_state->color_format = param->fmt;
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, priv->crtc);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state = drm_atomic_get_crtc_state(state, priv->crtc);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
+
+ ret = drm_atomic_set_mode_for_crtc(crtc_state, dank);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ crtc_state->enable = true;
+ crtc_state->active = true;
+
+ ret = drm_atomic_check_only(state);
+ KUNIT_EXPECT_EQ(test, ret, param->expected_ret);
+ if (!param->expected_ret)
+ KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_format, param->expected);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+};
+
+static const struct color_format_test_param hdmi_color_format_420_only_params[] = {
+ {
+ .fmt = DRM_CONNECTOR_COLOR_FORMAT_RGB444,
+ .expected = DRM_OUTPUT_COLOR_FORMAT_RGB444,
+ .expected_ret = -EINVAL,
+ .desc = "RGB should fail"
+ },
+ {
+ .fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
+ .expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR444,
+ .expected_ret = -EINVAL,
+ .desc = "YUV444 should fail"
+ },
+ {
+ .fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
+ .expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR422,
+ .expected_ret = -EINVAL,
+ .desc = "YUV422 should fail"
+ },
+ {
+ .fmt = DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
+ .expected = DRM_OUTPUT_COLOR_FORMAT_YCBCR420,
+ .expected_ret = 0,
+ .desc = "YUV420 should work"
+ },
+};
+
+KUNIT_ARRAY_PARAM_DESC(check_hdmi_color_format_420_only,
+ hdmi_color_format_420_only_params, desc);
+
static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
KUNIT_CASE(drm_test_check_broadcast_rgb_auto_cea_mode),
KUNIT_CASE(drm_test_check_broadcast_rgb_auto_cea_mode_vic_1),
@@ -2227,6 +2459,10 @@ static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_8bpc),
KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_10bpc),
KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_12bpc),
+ KUNIT_CASE_PARAM(drm_test_check_hdmi_color_format,
+ check_hdmi_color_format_gen_params),
+ KUNIT_CASE_PARAM(drm_test_check_hdmi_color_format_420_only,
+ check_hdmi_color_format_420_only_gen_params),
/*
* TODO: We should have tests to check that a change in the
* format triggers a CRTC mode change just like we do for the
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v13 23/27] drm/tests: hdmi: Add tests for HDMI helper's mode_valid
From: Nicolas Frattaroli @ 2026-04-13 10:07 UTC (permalink / raw)
To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan
Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, intel-gfx, intel-xe, linux-doc,
Nicolas Frattaroli
In-Reply-To: <20260413-color-format-v13-0-ab37d4dfba48@collabora.com>
Add some KUnit tests to verify that the HDMI state helper's mode_valid
implementation does not improperly reject chroma subsampled modes on the
basis of their clock rate not being satisfiable in RGB.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 109 +++++++++++++++++++++
1 file changed, 109 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index 3444c93c615f..74c9933eabfc 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -77,6 +77,23 @@ static struct drm_display_mode *find_420_only_mode(struct drm_connector *connect
return NULL;
}
+static struct drm_display_mode *find_420_also_mode(struct drm_connector *connector)
+{
+ struct drm_device *drm = connector->dev;
+ struct drm_display_mode *mode;
+
+ mutex_lock(&drm->mode_config.mutex);
+ list_for_each_entry(mode, &connector->modes, head) {
+ if (drm_mode_is_420_also(&connector->display_info, mode)) {
+ mutex_unlock(&drm->mode_config.mutex);
+ return mode;
+ }
+ }
+ mutex_unlock(&drm->mode_config.mutex);
+
+ return NULL;
+}
+
static int set_connector_edid(struct kunit *test, struct drm_connector *connector,
const void *edid, size_t edid_len)
{
@@ -2745,11 +2762,103 @@ static void drm_test_check_mode_valid_reject_max_clock(struct kunit *test)
KUNIT_EXPECT_EQ(test, preferred->clock, 25200);
}
+/*
+ * Test that drm_hdmi_connector_mode_valid() will accept modes that require a
+ * 4:2:0 chroma subsampling, even if said mode would violate maximum clock
+ * constraints if it used RGB 4:4:4.
+ */
+static void drm_test_check_mode_valid_yuv420_only_max_clock(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_display_mode *dank;
+ struct drm_connector *conn;
+
+ priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+ BIT(HDMI_COLORSPACE_RGB) |
+ BIT(HDMI_COLORSPACE_YUV420),
+ 8,
+ &dummy_connector_hdmi_funcs,
+ test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ KUNIT_ASSERT_EQ(test, conn->display_info.max_tmds_clock, 200 * 1000);
+
+ dank = find_420_only_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, dank);
+ KUNIT_EXPECT_EQ(test, dank->hdisplay, 3840);
+ KUNIT_EXPECT_EQ(test, dank->vdisplay, 2160);
+
+ /*
+ * Note: The mode's "clock" here is not accurate to the actual TMDS
+ * clock that HDMI will use for a subsampled mode. Hence, why the mode's
+ * clock is above the .max_tmds_clock of 200MHz.
+ */
+ KUNIT_EXPECT_EQ(test, dank->clock, 297000);
+}
+
+/*
+ * Test that drm_hdmi_connector_mode_valid() will reject modes that require
+ * 4:2:0 chroma subsampling, if the connector does not support 4:2:0.
+ */
+static void
+drm_test_check_mode_valid_reject_yuv420_only_connector(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_display_mode *dank;
+ struct drm_connector *conn;
+
+ priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8,
+ &dummy_connector_hdmi_funcs,
+ test_edid_hdmi_1080p_rgb_yuv_4k_yuv420_dc_max_200mhz);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ KUNIT_ASSERT_EQ(test, conn->display_info.max_tmds_clock, 200 * 1000);
+
+ dank = find_420_only_mode(conn);
+ KUNIT_EXPECT_NULL(test, dank);
+}
+
+/*
+ * Test that drm_hdmi_connector_mode_valid() will accept modes that allow (among
+ * other color formats) 4:2:0 chroma subsampling, even if the connector does not
+ * support 4:2:0, but the mode's clock works for RGB 4:4:4.
+ */
+static void
+drm_test_check_mode_valid_accept_yuv420_also_connector_rgb(struct kunit *test)
+{
+ struct drm_atomic_helper_connector_hdmi_priv *priv;
+ struct drm_display_mode *mode;
+ struct drm_connector *conn;
+
+ priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8,
+ &dummy_connector_hdmi_funcs,
+ test_edid_hdmi_4k_rgb_yuv420_dc_max_340mhz);
+ KUNIT_ASSERT_NOT_NULL(test, priv);
+
+ conn = &priv->connector;
+ KUNIT_ASSERT_EQ(test, conn->display_info.max_tmds_clock, 340 * 1000);
+
+ mode = find_420_also_mode(conn);
+ KUNIT_ASSERT_NOT_NULL(test, mode);
+ KUNIT_EXPECT_EQ(test, mode->hdisplay, 3840);
+ KUNIT_EXPECT_EQ(test, mode->vdisplay, 2160);
+ KUNIT_EXPECT_EQ(test, mode->clock, 297000);
+}
+
static struct kunit_case drm_atomic_helper_connector_hdmi_mode_valid_tests[] = {
KUNIT_CASE(drm_test_check_mode_valid),
KUNIT_CASE(drm_test_check_mode_valid_reject),
KUNIT_CASE(drm_test_check_mode_valid_reject_rate),
KUNIT_CASE(drm_test_check_mode_valid_reject_max_clock),
+ KUNIT_CASE(drm_test_check_mode_valid_yuv420_only_max_clock),
+ KUNIT_CASE(drm_test_check_mode_valid_reject_yuv420_only_connector),
+ KUNIT_CASE(drm_test_check_mode_valid_accept_yuv420_also_connector_rgb),
{ }
};
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
* [PATCH v13 21/27] drm/connector: Register color format property on HDMI connectors
From: Nicolas Frattaroli @ 2026-04-13 10:07 UTC (permalink / raw)
To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan
Cc: kernel, amd-gfx, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, intel-gfx, intel-xe, linux-doc,
Nicolas Frattaroli
In-Reply-To: <20260413-color-format-v13-0-ab37d4dfba48@collabora.com>
The drmm_connector_hdmi_init function can figure out what DRM color
formats are supported by a particular connector based on the supported
HDMI format bitmask that's passed in.
Use it to register the drm color format property.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
drivers/gpu/drm/drm_connector.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index d354ab77d3b8..048789032971 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -627,6 +627,10 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
if (max_bpc > 8)
drm_connector_attach_hdr_output_metadata_property(connector);
+ ret = drm_connector_attach_color_format_property(connector, supported_formats);
+ if (ret)
+ return ret;
+
connector->hdmi.funcs = hdmi_funcs;
return 0;
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox