Hi
Am 05.03.26 um 09:09 schrieb Icenowy Zheng:
Move the conversion from drm_format to vs_format to atomic_check, which
is before the point of no return and can properly bail out.
Signed-off-by: Icenowy Zheng <[email protected]>
---
Changes in v2:
- Add non-NULL check for drm_plane_state->fb.
drivers/gpu/drm/verisilicon/vs_plane.h | 2 ++
.../gpu/drm/verisilicon/vs_primary_plane.c | 21 ++++++++++++-------
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/verisilicon/vs_plane.h
b/drivers/gpu/drm/verisilicon/vs_plane.h
index 12848a72af576..df4b248b52954 100644
--- a/drivers/gpu/drm/verisilicon/vs_plane.h
+++ b/drivers/gpu/drm/verisilicon/vs_plane.h
@@ -65,6 +65,8 @@ struct vs_format {
struct vs_plane_state {
struct drm_plane_state base;
+
+ struct vs_format format;
In case you implement my comment about kzalloc vs kmemdup, don't forget
to assign it in the plane-duplicate helper.
};
static inline struct vs_plane_state *state_to_vs_plane_state(struct drm_plane_state *state)
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index bad0bc5e3242d..ca41c8053ae12 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -25,12 +25,20 @@ static int vs_primary_plane_atomic_check(struct drm_plane
*plane,
{
struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state,
plane);
+ struct vs_plane_state *new_vs_plane_state =
state_to_vs_plane_state(new_plane_state);
struct drm_crtc *crtc = new_plane_state->crtc;
+ struct drm_framebuffer *fb = new_plane_state->fb;
struct drm_crtc_state *crtc_state;
+ int ret;
- if (!crtc)
+ if (!crtc || !fb)
return 0;
I see that even the original code was not correct.
You have to call drm_atomic_helper_check_plane_state() in any case. If
there's no CRTC, use NULL for the CRTC state. See [1] for an example.
+ ret = drm_format_to_vs_format(fb->format->format,
+ &new_vs_plane_state->format);
+ if (ret)
+ return ret;
+
That should also only happen after
drm_atomic_helper_check_plane_state(). You can again take the code
around [1] for an example.
[1]
https://elixir.bootlin.com/linux/v7.0-rc1/source/drivers/gpu/drm/ast/ast_mode.c#L504
I think it makes sense to warn once if the conversion fails. After all,
the driver should only export DRM formats that it can program to hardware.
if (drm_WARN_ON_ONCE(plane->dev, ret))
return ret;
crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
if (WARN_ON(!crtc_state))
return -EINVAL;
@@ -88,11 +96,11 @@ static void vs_primary_plane_atomic_update(struct drm_plane
*plane,
{
struct drm_plane_state *state =
drm_atomic_get_new_plane_state(atomic_state,
plane);
+ struct vs_plane_state *vs_state = state_to_vs_plane_state(state);
struct drm_framebuffer *fb = state->fb;
struct drm_crtc *crtc = state->crtc;
struct vs_dc *dc;
struct vs_crtc *vcrtc;
- struct vs_format fmt;
unsigned int output;
dma_addr_t dma_addr;
@@ -105,16 +113,15 @@ static void vs_primary_plane_atomic_update(struct drm_plane *plane,
output = vcrtc->id;
dc = vcrtc->dc;
- drm_format_to_vs_format(state->fb->format->format, &fmt);
Removing another possible failure from the atomic commit is really nice.
Best regards
Thomas
-
regmap_update_bits(dc->regs, VSDC_FB_CONFIG(output),
VSDC_FB_CONFIG_FMT_MASK,
- VSDC_FB_CONFIG_FMT(fmt.color));
+ VSDC_FB_CONFIG_FMT(vs_state->format.color));
regmap_update_bits(dc->regs, VSDC_FB_CONFIG(output),
VSDC_FB_CONFIG_SWIZZLE_MASK,
- VSDC_FB_CONFIG_SWIZZLE(fmt.swizzle));
+ VSDC_FB_CONFIG_SWIZZLE(vs_state->format.swizzle));
regmap_assign_bits(dc->regs, VSDC_FB_CONFIG(output),
- VSDC_FB_CONFIG_UV_SWIZZLE_EN, fmt.uv_swizzle);
+ VSDC_FB_CONFIG_UV_SWIZZLE_EN,
+ vs_state->format.uv_swizzle);
dma_addr = vs_fb_get_dma_addr(fb, &state->src);
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
