diff mbox series

[v6,1/9] drm/omap: add sanity plane state check

Message ID 20211018142842.2511200-2-narmstrong@baylibre.com (mailing list archive)
State New, archived
Headers show
Series drm/omap: Add virtual-planes support | expand

Commit Message

Neil Armstrong Oct. 18, 2021, 2:28 p.m. UTC 
Call drm_atomic_helper_check_plane_state() from the plane
atomic_check() callback in order to add plane state sanity
checking.

It will permit filtering out totally bad scaling factors, even
if the real check are done later in the atomic commit.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Tomi Valkeinen Oct. 27, 2021, 8:29 a.m. UTC | #1 
On 18/10/2021 17:28, Neil Armstrong wrote:
> Call drm_atomic_helper_check_plane_state() from the plane
> atomic_check() callback in order to add plane state sanity
> checking.
> 
> It will permit filtering out totally bad scaling factors, even
> if the real check are done later in the atomic commit.

I think there's more to it: the function sets plane_state->visible, 
which is used in later patches.

  Tomi
Tomi Valkeinen Oct. 27, 2021, 8:30 a.m. UTC | #2 
On 27/10/2021 11:29, Tomi Valkeinen wrote:
> On 18/10/2021 17:28, Neil Armstrong wrote:
>> Call drm_atomic_helper_check_plane_state() from the plane
>> atomic_check() callback in order to add plane state sanity
>> checking.
>>
>> It will permit filtering out totally bad scaling factors, even
>> if the real check are done later in the atomic commit.
> 
> I think there's more to it: the function sets plane_state->visible, 
> which is used in later patches.

Maybe that could be mentioned in a comment. But otherwise:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi
Neil Armstrong Nov. 9, 2021, 1:10 p.m. UTC | #3 
On 27/10/2021 10:30, Tomi Valkeinen wrote:
> On 27/10/2021 11:29, Tomi Valkeinen wrote:
>> On 18/10/2021 17:28, Neil Armstrong wrote:
>>> Call drm_atomic_helper_check_plane_state() from the plane
>>> atomic_check() callback in order to add plane state sanity
>>> checking.
>>>
>>> It will permit filtering out totally bad scaling factors, even
>>> if the real check are done later in the atomic commit.
>>
>> I think there's more to it: the function sets plane_state->visible, which is used in later patches.
> 
> Maybe that could be mentioned in a comment. But otherwise:
> 
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Thanks, I'll update the comment to mention this.

Neil

> 
>  Tomi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 512af976b7e9..c3de4f339387 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -104,12 +104,15 @@  static void omap_plane_atomic_disable(struct drm_plane *plane,
 	dispc_ovl_enable(priv->dispc, omap_plane->id, false);
 }
 
+#define FRAC_16_16(mult, div)    (((mult) << 16) / (div))
+
 static int omap_plane_atomic_check(struct drm_plane *plane,
 				   struct drm_atomic_state *state)
 {
 	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
 										 plane);
 	struct drm_crtc_state *crtc_state;
+	int ret;
 
 	if (!new_plane_state->fb)
 		return 0;
@@ -127,6 +130,18 @@  static int omap_plane_atomic_check(struct drm_plane *plane,
 	if (!crtc_state->enable)
 		return 0;
 
+	/*
+	 * Note: these are just sanity checks to filter out totally bad scaling
+	 * factors. The real limits must be calculated case by case, and
+	 * unfortunately we currently do those checks only at the commit
+	 * phase in dispc.
+	 */
+	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
+						  FRAC_16_16(1, 8), FRAC_16_16(8, 1),
+						  true, true);
+	if (ret)
+		return ret;
+
 	if (new_plane_state->crtc_x < 0 || new_plane_state->crtc_y < 0)
 		return -EINVAL;