All of lore.kernel.org
 help / color / mirror / Atom feed
From: "José Expósito" <jose.exposito89@gmail.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Simon Ser <contact@emersion.fr>,
	airlied@linux.ie, alexandre.torgue@foss.st.com,
	benjamin.gaignard@linaro.org,
	linux-stm32@st-md-mailman.stormreply.com, marex@denx.de,
	linux-imx@nxp.com, intel-gfx@lists.freedesktop.org,
	tzimmermann@suse.de, s.hauer@pengutronix.de,
	rodrigo.vivi@intel.com, kernel@pengutronix.de,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org, yannick.fertre@foss.st.com,
	linux-kernel@vger.kernel.org, philippe.cornu@foss.st.com,
	mcoquelin.stm32@gmail.com, dmitry.baryshkov@linaro.org,
	shawnguo@kernel.org
Subject: Re: [PATCH v2 1/6] drm/plane: Make format_mod_supported truly optional
Date: Thu, 23 Dec 2021 17:57:06 +0100	[thread overview]
Message-ID: <20211223165706.GA11019@elementary> (raw)
In-Reply-To: <YcSPt+81fuzteeCu@intel.com>

Thanks for your reviews :) I'll wait a couple of days to see
if somebody else wants to comment and I'll send v3 adding the
reviewed by tags and fixing the compiler warning.

On Thu, Dec 23, 2021 at 05:03:19PM +0200, Ville Syrjälä wrote:
> Another related thing that might be worth checking is whether
> drivers generally do anything to validate the modifiers in
> the addfb2 ioctl. Looks like i915 and amdgpu are the only ones
> to use drm_any_plane_has_format() for that, so all the other
> drivers must either be checking it manually (or they're just
> potentially broken when handed unexpected modifiers by evil
> userspace).

I'm pretty new to this subsystem, so please correct me if I'm 
wrong, but after looking into a couple of drivers I think you
are right, this check is missing in some drivers.

This possible bug reminds me of this ToDo task [1]:

> Many drivers wrap drm_gem_fb_create() only to check for valid formats. For
> atomic drivers we could check for valid formats by calling
> drm_plane_check_pixel_format() against all planes, and pass if any plane
> supports the format. For non-atomic that's not possible since like the format
> list for the primary plane is fake and we'd therefor reject valid formats.

I had a look to the Raspberry Pi driver (mainly because I'm trying
to understand it) and it looks like the check is missing. Other
drivers, for example Mali, are checking the format modifier manually.

I'll try to do some actual testing during Christmas and see
how it goes.

José Expósito

[1] https://www.kernel.org/doc/html/latest/gpu/todo.html#drm-framebuffer-funcs-and-drm-mode-config-funcs-fb-create-cleanup

WARNING: multiple messages have this Message-ID (diff)
From: "José Expósito" <jose.exposito89@gmail.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: marex@denx.de, mcoquelin.stm32@gmail.com, kernel@pengutronix.de,
	s.hauer@pengutronix.de, tzimmermann@suse.de, airlied@linux.ie,
	intel-gfx@lists.freedesktop.org, alexandre.torgue@foss.st.com,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	yannick.fertre@foss.st.com, linux-imx@nxp.com,
	benjamin.gaignard@linaro.org, rodrigo.vivi@intel.com,
	dmitry.baryshkov@linaro.org, shawnguo@kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org, philippe.cornu@foss.st.com
Subject: Re: [PATCH v2 1/6] drm/plane: Make format_mod_supported truly optional
Date: Thu, 23 Dec 2021 17:57:06 +0100	[thread overview]
Message-ID: <20211223165706.GA11019@elementary> (raw)
In-Reply-To: <YcSPt+81fuzteeCu@intel.com>

Thanks for your reviews :) I'll wait a couple of days to see
if somebody else wants to comment and I'll send v3 adding the
reviewed by tags and fixing the compiler warning.

On Thu, Dec 23, 2021 at 05:03:19PM +0200, Ville Syrjälä wrote:
> Another related thing that might be worth checking is whether
> drivers generally do anything to validate the modifiers in
> the addfb2 ioctl. Looks like i915 and amdgpu are the only ones
> to use drm_any_plane_has_format() for that, so all the other
> drivers must either be checking it manually (or they're just
> potentially broken when handed unexpected modifiers by evil
> userspace).

I'm pretty new to this subsystem, so please correct me if I'm 
wrong, but after looking into a couple of drivers I think you
are right, this check is missing in some drivers.

This possible bug reminds me of this ToDo task [1]:

> Many drivers wrap drm_gem_fb_create() only to check for valid formats. For
> atomic drivers we could check for valid formats by calling
> drm_plane_check_pixel_format() against all planes, and pass if any plane
> supports the format. For non-atomic that's not possible since like the format
> list for the primary plane is fake and we'd therefor reject valid formats.

I had a look to the Raspberry Pi driver (mainly because I'm trying
to understand it) and it looks like the check is missing. Other
drivers, for example Mali, are checking the format modifier manually.

I'll try to do some actual testing during Christmas and see
how it goes.

José Expósito

[1] https://www.kernel.org/doc/html/latest/gpu/todo.html#drm-framebuffer-funcs-and-drm-mode-config-funcs-fb-create-cleanup

WARNING: multiple messages have this Message-ID (diff)
From: "José Expósito" <jose.exposito89@gmail.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Simon Ser <contact@emersion.fr>,
	airlied@linux.ie, alexandre.torgue@foss.st.com,
	benjamin.gaignard@linaro.org,
	linux-stm32@st-md-mailman.stormreply.com, marex@denx.de,
	linux-imx@nxp.com, intel-gfx@lists.freedesktop.org,
	tzimmermann@suse.de, s.hauer@pengutronix.de,
	rodrigo.vivi@intel.com, kernel@pengutronix.de,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org, yannick.fertre@foss.st.com,
	linux-kernel@vger.kernel.org, philippe.cornu@foss.st.com,
	mcoquelin.stm32@gmail.com, dmitry.baryshkov@linaro.org,
	shawnguo@kernel.org
Subject: Re: [PATCH v2 1/6] drm/plane: Make format_mod_supported truly optional
Date: Thu, 23 Dec 2021 17:57:06 +0100	[thread overview]
Message-ID: <20211223165706.GA11019@elementary> (raw)
In-Reply-To: <YcSPt+81fuzteeCu@intel.com>

Thanks for your reviews :) I'll wait a couple of days to see
if somebody else wants to comment and I'll send v3 adding the
reviewed by tags and fixing the compiler warning.

On Thu, Dec 23, 2021 at 05:03:19PM +0200, Ville Syrjälä wrote:
> Another related thing that might be worth checking is whether
> drivers generally do anything to validate the modifiers in
> the addfb2 ioctl. Looks like i915 and amdgpu are the only ones
> to use drm_any_plane_has_format() for that, so all the other
> drivers must either be checking it manually (or they're just
> potentially broken when handed unexpected modifiers by evil
> userspace).

I'm pretty new to this subsystem, so please correct me if I'm 
wrong, but after looking into a couple of drivers I think you
are right, this check is missing in some drivers.

This possible bug reminds me of this ToDo task [1]:

> Many drivers wrap drm_gem_fb_create() only to check for valid formats. For
> atomic drivers we could check for valid formats by calling
> drm_plane_check_pixel_format() against all planes, and pass if any plane
> supports the format. For non-atomic that's not possible since like the format
> list for the primary plane is fake and we'd therefor reject valid formats.

I had a look to the Raspberry Pi driver (mainly because I'm trying
to understand it) and it looks like the check is missing. Other
drivers, for example Mali, are checking the format modifier manually.

I'll try to do some actual testing during Christmas and see
how it goes.

José Expósito

[1] https://www.kernel.org/doc/html/latest/gpu/todo.html#drm-framebuffer-funcs-and-drm-mode-config-funcs-fb-create-cleanup

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: "José Expósito" <jose.exposito89@gmail.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: marex@denx.de, mcoquelin.stm32@gmail.com, kernel@pengutronix.de,
	s.hauer@pengutronix.de, tzimmermann@suse.de, airlied@linux.ie,
	Simon Ser <contact@emersion.fr>,
	intel-gfx@lists.freedesktop.org, alexandre.torgue@foss.st.com,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	yannick.fertre@foss.st.com, linux-imx@nxp.com,
	benjamin.gaignard@linaro.org, dmitry.baryshkov@linaro.org,
	shawnguo@kernel.org, linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org, philippe.cornu@foss.st.com
Subject: Re: [Intel-gfx]  [PATCH v2 1/6] drm/plane: Make format_mod_supported truly optional
Date: Thu, 23 Dec 2021 17:57:06 +0100	[thread overview]
Message-ID: <20211223165706.GA11019@elementary> (raw)
In-Reply-To: <YcSPt+81fuzteeCu@intel.com>

Thanks for your reviews :) I'll wait a couple of days to see
if somebody else wants to comment and I'll send v3 adding the
reviewed by tags and fixing the compiler warning.

On Thu, Dec 23, 2021 at 05:03:19PM +0200, Ville Syrjälä wrote:
> Another related thing that might be worth checking is whether
> drivers generally do anything to validate the modifiers in
> the addfb2 ioctl. Looks like i915 and amdgpu are the only ones
> to use drm_any_plane_has_format() for that, so all the other
> drivers must either be checking it manually (or they're just
> potentially broken when handed unexpected modifiers by evil
> userspace).

I'm pretty new to this subsystem, so please correct me if I'm 
wrong, but after looking into a couple of drivers I think you
are right, this check is missing in some drivers.

This possible bug reminds me of this ToDo task [1]:

> Many drivers wrap drm_gem_fb_create() only to check for valid formats. For
> atomic drivers we could check for valid formats by calling
> drm_plane_check_pixel_format() against all planes, and pass if any plane
> supports the format. For non-atomic that's not possible since like the format
> list for the primary plane is fake and we'd therefor reject valid formats.

I had a look to the Raspberry Pi driver (mainly because I'm trying
to understand it) and it looks like the check is missing. Other
drivers, for example Mali, are checking the format modifier manually.

I'll try to do some actual testing during Christmas and see
how it goes.

José Expósito

[1] https://www.kernel.org/doc/html/latest/gpu/todo.html#drm-framebuffer-funcs-and-drm-mode-config-funcs-fb-create-cleanup

  reply	other threads:[~2021-12-23 16:57 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-22  9:05 [PATCH v2 0/6] Add missing format_mod_supported functions José Expósito
2021-12-22  9:05 ` [Intel-gfx] " José Expósito
2021-12-22  9:05 ` José Expósito
2021-12-22  9:05 ` José Expósito
2021-12-22  9:05 ` [PATCH v2 1/6] drm/plane: Make format_mod_supported truly optional José Expósito
2021-12-22  9:05   ` [Intel-gfx] " José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22 13:10   ` kernel test robot
2021-12-22 13:10     ` kernel test robot
2021-12-23 10:10   ` Simon Ser
2021-12-23 10:10     ` Simon Ser
2021-12-23 10:10     ` [Intel-gfx] " Simon Ser
2021-12-23 10:10     ` Simon Ser
2021-12-23 11:56   ` Ville Syrjälä
2021-12-23 11:56     ` Ville Syrjälä
2021-12-23 11:56     ` [Intel-gfx] " Ville Syrjälä
2021-12-23 11:56     ` Ville Syrjälä
2021-12-23 13:42     ` Simon Ser
2021-12-23 13:42       ` Simon Ser
2021-12-23 13:42       ` [Intel-gfx] " Simon Ser
2021-12-23 13:42       ` Simon Ser
2021-12-23 15:03       ` Ville Syrjälä
2021-12-23 15:03         ` Ville Syrjälä
2021-12-23 15:03         ` [Intel-gfx] " Ville Syrjälä
2021-12-23 15:03         ` Ville Syrjälä
2021-12-23 16:57         ` José Expósito [this message]
2021-12-23 16:57           ` [Intel-gfx] " José Expósito
2021-12-23 16:57           ` José Expósito
2021-12-23 16:57           ` José Expósito
2021-12-22  9:05 ` [PATCH v2 2/6] drm/plane: Fix typo in format_mod_supported documentation José Expósito
2021-12-22  9:05   ` [Intel-gfx] " José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-23 10:07   ` Simon Ser
2021-12-23 10:07     ` Simon Ser
2021-12-23 10:07     ` [Intel-gfx] " Simon Ser
2021-12-23 10:07     ` Simon Ser
2021-12-22  9:05 ` [PATCH v2 3/6] drm/simple-kms: Drop format_mod_supported function José Expósito
2021-12-22  9:05   ` [Intel-gfx] " José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22  9:05 ` [PATCH v2 4/6] drm/i915/display: " José Expósito
2021-12-22  9:05   ` [Intel-gfx] " José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22  9:05 ` [PATCH v2 5/6] drm: mxsfb: " José Expósito
2021-12-22  9:05   ` [Intel-gfx] " José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22  9:05 ` [PATCH v2 6/6] drm/stm: ltdc: " José Expósito
2021-12-22  9:05   ` [Intel-gfx] " José Expósito
2021-12-22  9:05   ` José Expósito
2021-12-22  9:05   ` José Expósito
2022-01-12  9:44   ` yannick Fertre
2022-01-12  9:44     ` [Intel-gfx] " yannick Fertre
2022-01-12  9:44     ` yannick Fertre
2022-01-12  9:44     ` yannick Fertre
2022-01-12 10:01   ` Jagan Teki
2022-01-12 10:01     ` Jagan Teki
2022-01-12 10:01     ` [Intel-gfx] " Jagan Teki
2022-01-12 10:01     ` Jagan Teki
2022-01-10 19:14 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for Add missing format_mod_supported functions (rev2) 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=20211223165706.GA11019@elementary \
    --to=jose.exposito89@gmail.com \
    --cc=airlied@linux.ie \
    --cc=alexandre.torgue@foss.st.com \
    --cc=benjamin.gaignard@linaro.org \
    --cc=contact@emersion.fr \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=marex@denx.de \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=philippe.cornu@foss.st.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=tzimmermann@suse.de \
    --cc=ville.syrjala@linux.intel.com \
    --cc=yannick.fertre@foss.st.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.