All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
To: Maxime Ripard <mripard@kernel.org>
Cc: "Harry Wentland" <harry.wentland@amd.com>,
	"Leo Li" <sunpeng.li@amd.com>,
	"Rodrigo Siqueira" <siqueira@igalia.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Andrzej Hajda" <andrzej.hajda@intel.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Robert Foss" <rfoss@kernel.org>,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Jonas Karlman" <jonas@kwiboo.se>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	"Sandy Huang" <hjc@rock-chips.com>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Andy Yan" <andy.yan@rock-chips.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Tvrtko Ursulin" <tursulin@ursulin.net>,
	"Dmitry Baryshkov" <lumag@kernel.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Rob Herring" <robh@kernel.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	kernel@collabora.com, amd-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org,
	intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
	linux-doc@vger.kernel.org, "Andri Yngvason" <andri@yngvason.is>,
	"Werner Sembach" <wse@tuxedocomputers.com>,
	"Marius Vlad" <marius.vlad@collabora.com>
Subject: Re: [PATCH v7 02/22] drm: Add new general DRM property "color format"
Date: Fri, 06 Feb 2026 16:26:56 +0100	[thread overview]
Message-ID: <6318997.lOV4Wx5bFT@workhorse> (raw)
In-Reply-To: <20260206-deft-provocative-perch-6ca9bf@houat>

Hello,

On Friday, 6 February 2026 15:05:08 Central European Standard Time Maxime Ripard wrote:
> Hi,
> 
> On Wed, Jan 21, 2026 at 03:45:09PM +0100, Nicolas Frattaroli wrote:
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 7eaec37ae1c7..b5604dca728a 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -556,6 +556,16 @@ enum drm_colorspace {
> >  	DRM_MODE_COLORIMETRY_COUNT
> >  };
> >  
> > +enum drm_color_format {
> > +	DRM_COLOR_FORMAT_AUTO			= 0,
> > +	DRM_COLOR_FORMAT_RGB444			= BIT(0),
> > +	DRM_COLOR_FORMAT_YCBCR444		= BIT(1),
> > +	DRM_COLOR_FORMAT_YCBCR422		= BIT(2),
> > +	DRM_COLOR_FORMAT_YCBCR420		= BIT(3),
> > +};
> > +
> > +#define DRM_COLOR_FORMAT_COUNT 5
> > +
> 
> I don't really see a reason to expose an enum, with a bunch of values
> that are all mutually exclusive, as a bitmask. It's pretty inconsistent
> with most (all?) the other similar properties we have.
> 
> I appreciate you did that to avoid fixing up every driver using those
> values, but then maybe we don't have to? We could create a userspace
> facing enum, and convert to DRM_COLOR_FORMAT internally.

This is what the series did at v5 and earlier. IMHO it was kind of
counter-productive, because we then had two different things for the
same purpose, and some conversion logic between them. I think it's more
error prone to do it that way (think: mixing up the two), and doesn't
have a clear benefit. Just to give a picture of how bad things get:

1. we have the HDMI color format (aka "HDMI_COLORSPACE")
2. we have driver specific output color formats, e.g. the intel ones
3. we have DRM_COLOR_FORMAT
4. we have the bus formats (multiple per color format)
5. we have the DRM plane formats (again, multiple per color format)

Adding a sixth into the mix feels a bit bad because we'll then need to
justify why we should have another layer of switch-case statements.

Kind regards,
Nicolas Frattaroli

> 
> Maxime
> 





WARNING: multiple messages have this Message-ID (diff)
From: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
To: Maxime Ripard <mripard@kernel.org>
Cc: "Harry Wentland" <harry.wentland@amd.com>,
	"Leo Li" <sunpeng.li@amd.com>,
	"Rodrigo Siqueira" <siqueira@igalia.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Andrzej Hajda" <andrzej.hajda@intel.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Robert Foss" <rfoss@kernel.org>,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Jonas Karlman" <jonas@kwiboo.se>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	"Sandy Huang" <hjc@rock-chips.com>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Andy Yan" <andy.yan@rock-chips.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Tvrtko Ursulin" <tursulin@ursulin.net>,
	"Dmitry Baryshkov" <lumag@kernel.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Rob Herring" <robh@kernel.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	kernel@collabora.com, amd-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org,
	intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
	linux-doc@vger.kernel.org, "Andri Yngvason" <andri@yngvason.is>,
	"Werner Sembach" <wse@tuxedocomputers.com>,
	"Marius Vlad" <marius.vlad@collabora.com>
Subject: Re: [PATCH v7 02/22] drm: Add new general DRM property "color format"
Date: Fri, 06 Feb 2026 16:26:56 +0100	[thread overview]
Message-ID: <6318997.lOV4Wx5bFT@workhorse> (raw)
In-Reply-To: <20260206-deft-provocative-perch-6ca9bf@houat>

Hello,

On Friday, 6 February 2026 15:05:08 Central European Standard Time Maxime Ripard wrote:
> Hi,
> 
> On Wed, Jan 21, 2026 at 03:45:09PM +0100, Nicolas Frattaroli wrote:
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 7eaec37ae1c7..b5604dca728a 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -556,6 +556,16 @@ enum drm_colorspace {
> >  	DRM_MODE_COLORIMETRY_COUNT
> >  };
> >  
> > +enum drm_color_format {
> > +	DRM_COLOR_FORMAT_AUTO			= 0,
> > +	DRM_COLOR_FORMAT_RGB444			= BIT(0),
> > +	DRM_COLOR_FORMAT_YCBCR444		= BIT(1),
> > +	DRM_COLOR_FORMAT_YCBCR422		= BIT(2),
> > +	DRM_COLOR_FORMAT_YCBCR420		= BIT(3),
> > +};
> > +
> > +#define DRM_COLOR_FORMAT_COUNT 5
> > +
> 
> I don't really see a reason to expose an enum, with a bunch of values
> that are all mutually exclusive, as a bitmask. It's pretty inconsistent
> with most (all?) the other similar properties we have.
> 
> I appreciate you did that to avoid fixing up every driver using those
> values, but then maybe we don't have to? We could create a userspace
> facing enum, and convert to DRM_COLOR_FORMAT internally.

This is what the series did at v5 and earlier. IMHO it was kind of
counter-productive, because we then had two different things for the
same purpose, and some conversion logic between them. I think it's more
error prone to do it that way (think: mixing up the two), and doesn't
have a clear benefit. Just to give a picture of how bad things get:

1. we have the HDMI color format (aka "HDMI_COLORSPACE")
2. we have driver specific output color formats, e.g. the intel ones
3. we have DRM_COLOR_FORMAT
4. we have the bus formats (multiple per color format)
5. we have the DRM plane formats (again, multiple per color format)

Adding a sixth into the mix feels a bit bad because we'll then need to
justify why we should have another layer of switch-case statements.

Kind regards,
Nicolas Frattaroli

> 
> Maxime
> 





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  reply	other threads:[~2026-02-07 10:07 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-21 14:45 [PATCH v7 00/22] Add new general DRM property "color format" Nicolas Frattaroli
2026-01-21 14:45 ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 01/22] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 02/22] drm: Add new general DRM property "color format" Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-02-06 14:05   ` Maxime Ripard
2026-02-06 14:05     ` Maxime Ripard
2026-02-06 15:26     ` Nicolas Frattaroli [this message]
2026-02-06 15:26       ` Nicolas Frattaroli
2026-02-10 17:03       ` Maxime Ripard
2026-02-10 17:03         ` Maxime Ripard
2026-01-21 14:45 ` [PATCH v7 03/22] drm: Add enum conversions between DRM_COLOR_FORMAT and HDMI_COLORSPACE Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-02-06 14:08   ` Maxime Ripard
2026-02-06 14:08     ` Maxime Ripard
2026-02-07 19:55     ` Nicolas Frattaroli
2026-02-07 19:55       ` Nicolas Frattaroli
2026-02-10 17:24       ` Maxime Ripard
2026-02-10 17:24         ` Maxime Ripard
2026-02-11 17:10         ` Nicolas Frattaroli
2026-02-11 17:10           ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 04/22] drm/bridge: Act on the DRM color format property Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 05/22] drm/display: hdmi-state-helper: Act on color format DRM property Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-02-06 14:16   ` Maxime Ripard
2026-02-06 14:16     ` Maxime Ripard
2026-01-21 14:45 ` [PATCH v7 06/22] drm/display: hdmi-state-helper: Try subsampling in mode_valid Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 07/22] drm/i915: Implement the "color format" DRM property Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 08/22] drm/amdgpu: Implement " Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 09/22] drm/rockchip: Add YUV422 output mode constants for VOP2 Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-22  6:30   ` Andy Yan
2026-01-22  6:30     ` Andy Yan
2026-01-21 14:45 ` [PATCH v7 10/22] drm/rockchip: vop2: Fix YUV444 output Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-22  8:28   ` Andy Yan
2026-01-22  8:28     ` Andy Yan
2026-01-22 12:59     ` [PATCH " Nicolas Frattaroli
2026-01-22 12:59       ` Nicolas Frattaroli
2026-01-23  1:29       ` Andy Yan
2026-01-23  1:29         ` Andy Yan
2026-02-07 19:31         ` Nicolas Frattaroli
2026-02-07 19:31           ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 11/22] drm/rockchip: vop2: Add RK3576 to the RG swap special case Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-22  8:31   ` Andy Yan
2026-01-22  8:31     ` Andy Yan
2026-01-21 14:45 ` [PATCH v7 12/22] drm/rockchip: vop2: Recognise 10/12-bit YUV422 as YUV formats Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-22  8:42   ` Andy Yan
2026-01-22  8:42     ` Andy Yan
2026-01-21 14:45 ` [PATCH v7 13/22] drm/rockchip: vop2: Set correct output format for RK3576 YUV422 Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-22  8:44   ` Andy Yan
2026-01-22  8:44     ` Andy Yan
2026-01-21 14:45 ` [PATCH v7 14/22] drm/bridge: dw-hdmi-qp: Implement atomic_get_output_bus_fmts Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 15/22] drm/rockchip: dw_hdmi_qp: Implement "color format" DRM property Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 16/22] drm/rockchip: dw_hdmi_qp: Set supported_formats platdata Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 17/22] drm/connector: Register color format property on HDMI connectors Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-01-21 14:45 ` [PATCH v7 18/22] drm/tests: hdmi: Add tests for the color_format property Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-02-10 15:51   ` Maxime Ripard
2026-02-10 15:51     ` Maxime Ripard
2026-01-21 14:45 ` [PATCH v7 19/22] drm/tests: hdmi: Add tests for HDMI helper's mode_valid Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-02-10 15:52   ` Maxime Ripard
2026-02-10 15:52     ` Maxime Ripard
2026-01-21 14:45 ` [PATCH v7 20/22] drm/tests: edid: Add __maybe_unused attribute to EDID definitions Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-02-10 16:00   ` Maxime Ripard
2026-02-10 16:00     ` Maxime Ripard
2026-01-21 14:45 ` [PATCH v7 21/22] drm/tests: bridge: Add KUnit tests for bridge chain format selection Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-02-10 16:11   ` Maxime Ripard
2026-02-10 16:11     ` Maxime Ripard
2026-01-21 14:45 ` [PATCH v7 22/22] drm/bridge: Document " Nicolas Frattaroli
2026-01-21 14:45   ` Nicolas Frattaroli
2026-02-10 16:25   ` Maxime Ripard
2026-02-10 16:25     ` Maxime Ripard
2026-01-21 15:23 ` ✗ CI.checkpatch: warning for Add new general DRM property "color format" (rev4) Patchwork
2026-01-21 15:25 ` ✓ CI.KUnit: success " Patchwork
2026-01-21 15:43 ` ✗ CI.checksparse: warning " Patchwork
2026-01-21 16:07 ` ✓ Xe.CI.BAT: success " Patchwork
2026-01-21 16:34 ` ✓ i915.CI.BAT: " Patchwork
2026-01-21 23:46 ` ✓ Xe.CI.Full: " Patchwork
2026-01-22  3:00 ` ✓ i915.CI.Full: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6318997.lOV4Wx5bFT@workhorse \
    --to=nicolas.frattaroli@collabora.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=andri@yngvason.is \
    --cc=andrzej.hajda@intel.com \
    --cc=andy.yan@rock-chips.com \
    --cc=christian.koenig@amd.com \
    --cc=corbet@lwn.net \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=harry.wentland@amd.com \
    --cc=heiko@sntech.de \
    --cc=hjc@rock-chips.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=kernel@collabora.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=lumag@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=marius.vlad@collabora.com \
    --cc=mripard@kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=rfoss@kernel.org \
    --cc=robh@kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=s.hauer@pengutronix.de \
    --cc=simona@ffwll.ch \
    --cc=siqueira@igalia.com \
    --cc=sunpeng.li@amd.com \
    --cc=tursulin@ursulin.net \
    --cc=tzimmermann@suse.de \
    --cc=wse@tuxedocomputers.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.