[1/4] drm/fb-dma-helper: Add drm_fb_dma_get_gem_clipped_addr()
Commit Message
drm_fb_dma_get_gem_addr() returns the DMA address to the "unclipped"
framebuffer. However some display drivers want the "clipped" framebuffer
instead, as they are also using the clipped coordinates to program the
hardware.
Some of these drivers are open-coding drm_fb_dma_get_gem_addr() with
the source coordinates replaced, while others have been incorrectly
converted to using drm_fb_dma_get_gem_addr(), which would end up
causing incorrect parts of the framebuffer to be displayed if it were
somehow clipped.
Add drm_fb_dma_get_gem_clipped_addr(), a "clipped" version of
drm_fb_dma_get_gem_addr() for these drivers to use.
Cc: <stable@vger.kernel.org> # dependency for next patch
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
drivers/gpu/drm/drm_fb_dma_helper.c | 63 +++++++++++++++++++++--------
include/drm/drm_fb_dma_helper.h | 4 ++
2 files changed, 51 insertions(+), 16 deletions(-)
Comments
Hi
Am 08.09.26 um 11:07 schrieb Chen-Yu Tsai:
> drm_fb_dma_get_gem_addr() returns the DMA address to the "unclipped"
> framebuffer. However some display drivers want the "clipped" framebuffer
> instead, as they are also using the clipped coordinates to program the
> hardware.
>
> Some of these drivers are open-coding drm_fb_dma_get_gem_addr() with
> the source coordinates replaced, while others have been incorrectly
> converted to using drm_fb_dma_get_gem_addr(), which would end up
> causing incorrect parts of the framebuffer to be displayed if it were
> somehow clipped.
>
> Add drm_fb_dma_get_gem_clipped_addr(), a "clipped" version of
> drm_fb_dma_get_gem_addr() for these drivers to use.
>
> Cc: <stable@vger.kernel.org> # dependency for next patch
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> drivers/gpu/drm/drm_fb_dma_helper.c | 63 +++++++++++++++++++++--------
> include/drm/drm_fb_dma_helper.h | 4 ++
> 2 files changed, 51 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c
> index fd71969d2fb1..a260e7cd5667 100644
> --- a/drivers/gpu/drm/drm_fb_dma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_dma_helper.c
> @@ -59,20 +59,10 @@ struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb,
> }
> EXPORT_SYMBOL_GPL(drm_fb_dma_get_gem_obj);
>
> -/**
> - * drm_fb_dma_get_gem_addr() - Get DMA (bus) address for framebuffer, for pixel
> - * formats where values are grouped in blocks this will get you the beginning of
> - * the block
> - * @fb: The framebuffer
> - * @state: Which state of drm plane
> - * @plane: Which plane
> - * Return the DMA GEM address for given framebuffer.
> - *
> - * This function will usually be called from the PLANE callback functions.
> - */
> -dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
> - struct drm_plane_state *state,
> - unsigned int plane)
> +static dma_addr_t _drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
> + unsigned int plane,
> + unsigned int x,
> + unsigned int y)
> {
> struct drm_gem_dma_object *obj;
> dma_addr_t dma_addr;
> @@ -96,8 +86,8 @@ dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
> v_div = fb->format->vsub;
> }
>
> - sample_x = (state->src_x >> 16) / h_div;
> - sample_y = (state->src_y >> 16) / v_div;
> + sample_x = x / h_div;
> + sample_y = y / v_div;
> block_start_y = (sample_y / block_h) * block_h;
> num_hblocks = sample_x / block_w;
The current code already mixes up responsibilities of the involved
modules. It's a good opportunity to improve that. I think there should
be a block-offset helper for the framebuffer. That function will return
the byte offset of a pixel's block from the beginning of the framebuffer
plane.
/* in drm_framebuffer.{c,h} */
u32 drm_framebuffer_get_block_offset(struct drm_framebuffer *fb, plane,
x, y)
{
/* here goes the current offset calculation from the gem-dma code */
}
In the GEM-DMA code, you can then write your helpers like this
drm_fb_dma_get_gem_addr(...)
{
obj = drm_fb_dma_get_gem_ob(fb)
offset = drm_framebuffer_get_block_offset(obj->base, state->src_x,
state->src_y)
dma_addr = obj->dma_addr + offset;
return dma_addr
}
_get_clipped_gem_addr()
{
/* likewise */
}
This better structures the responsibility. It also works for other GEM
code besides GEM-DMA.
Best regards
Thomas
>
> @@ -106,8 +96,49 @@ dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
>
> return dma_addr;
> }
> +
> +/**
> + * drm_fb_dma_get_gem_addr() - Get DMA (bus) address for unclipped framebuffer,
> + * for pixel formats where values are grouped in blocks this will get you the
> + * beginning of the block
> + * @fb: The framebuffer
> + * @state: Which state of drm plane
> + * @plane: Which plane
> + *
> + * This function will usually be called from the PLANE callback functions.
> + *
> + * Return: GEM DMA address for given framebuffer, unclipped.
> + */
> +dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
> + struct drm_plane_state *state,
> + unsigned int plane)
> +{
> + return _drm_fb_dma_get_gem_addr(fb, plane, state->src_x >> 16,
> + state->src_y >> 16);
> +}
> EXPORT_SYMBOL_GPL(drm_fb_dma_get_gem_addr);
>
> +/**
> + * drm_fb_dma_get_gem_clipped_addr() - Get DMA (bus) address for clipped
> + * framebuffer, for pixel formats where values are grouped in blocks this
> + * will get you the beginning of the block
> + * @fb: The framebuffer
> + * @state: Which state of drm plane
> + * @plane: Which plane
> + *
> + * This function will usually be called from the PLANE callback functions.
> + *
> + * Return: GEM DMA address for given framebuffer, clipped.
> + */
> +dma_addr_t drm_fb_dma_get_gem_clipped_addr(struct drm_framebuffer *fb,
> + struct drm_plane_state *state,
> + unsigned int plane)
> +{
> + return _drm_fb_dma_get_gem_addr(fb, plane, state->src.x1 >> 16,
> + state->src.y1 >> 16);
> +}
> +EXPORT_SYMBOL_GPL(drm_fb_dma_get_gem_clipped_addr);
> +
> /**
> * drm_fb_dma_sync_non_coherent - Sync GEM object to non-coherent backing
> * memory
> diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h
> index c950732c6d36..b2a0bd7ef9d0 100644
> --- a/include/drm/drm_fb_dma_helper.h
> +++ b/include/drm/drm_fb_dma_helper.h
> @@ -17,6 +17,10 @@ dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
> struct drm_plane_state *state,
> unsigned int plane);
>
> +dma_addr_t drm_fb_dma_get_gem_clipped_addr(struct drm_framebuffer *fb,
> + struct drm_plane_state *state,
> + unsigned int plane);
> +
> void drm_fb_dma_sync_non_coherent(struct drm_device *drm,
> struct drm_plane_state *old_state,
> struct drm_plane_state *state);
@@ -59,20 +59,10 @@ struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb,
}
EXPORT_SYMBOL_GPL(drm_fb_dma_get_gem_obj);
-/**
- * drm_fb_dma_get_gem_addr() - Get DMA (bus) address for framebuffer, for pixel
- * formats where values are grouped in blocks this will get you the beginning of
- * the block
- * @fb: The framebuffer
- * @state: Which state of drm plane
- * @plane: Which plane
- * Return the DMA GEM address for given framebuffer.
- *
- * This function will usually be called from the PLANE callback functions.
- */
-dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
- struct drm_plane_state *state,
- unsigned int plane)
+static dma_addr_t _drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
+ unsigned int plane,
+ unsigned int x,
+ unsigned int y)
{
struct drm_gem_dma_object *obj;
dma_addr_t dma_addr;
@@ -96,8 +86,8 @@ dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
v_div = fb->format->vsub;
}
- sample_x = (state->src_x >> 16) / h_div;
- sample_y = (state->src_y >> 16) / v_div;
+ sample_x = x / h_div;
+ sample_y = y / v_div;
block_start_y = (sample_y / block_h) * block_h;
num_hblocks = sample_x / block_w;
@@ -106,8 +96,49 @@ dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
return dma_addr;
}
+
+/**
+ * drm_fb_dma_get_gem_addr() - Get DMA (bus) address for unclipped framebuffer,
+ * for pixel formats where values are grouped in blocks this will get you the
+ * beginning of the block
+ * @fb: The framebuffer
+ * @state: Which state of drm plane
+ * @plane: Which plane
+ *
+ * This function will usually be called from the PLANE callback functions.
+ *
+ * Return: GEM DMA address for given framebuffer, unclipped.
+ */
+dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
+ struct drm_plane_state *state,
+ unsigned int plane)
+{
+ return _drm_fb_dma_get_gem_addr(fb, plane, state->src_x >> 16,
+ state->src_y >> 16);
+}
EXPORT_SYMBOL_GPL(drm_fb_dma_get_gem_addr);
+/**
+ * drm_fb_dma_get_gem_clipped_addr() - Get DMA (bus) address for clipped
+ * framebuffer, for pixel formats where values are grouped in blocks this
+ * will get you the beginning of the block
+ * @fb: The framebuffer
+ * @state: Which state of drm plane
+ * @plane: Which plane
+ *
+ * This function will usually be called from the PLANE callback functions.
+ *
+ * Return: GEM DMA address for given framebuffer, clipped.
+ */
+dma_addr_t drm_fb_dma_get_gem_clipped_addr(struct drm_framebuffer *fb,
+ struct drm_plane_state *state,
+ unsigned int plane)
+{
+ return _drm_fb_dma_get_gem_addr(fb, plane, state->src.x1 >> 16,
+ state->src.y1 >> 16);
+}
+EXPORT_SYMBOL_GPL(drm_fb_dma_get_gem_clipped_addr);
+
/**
* drm_fb_dma_sync_non_coherent - Sync GEM object to non-coherent backing
* memory
@@ -17,6 +17,10 @@ dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
struct drm_plane_state *state,
unsigned int plane);
+dma_addr_t drm_fb_dma_get_gem_clipped_addr(struct drm_framebuffer *fb,
+ struct drm_plane_state *state,
+ unsigned int plane);
+
void drm_fb_dma_sync_non_coherent(struct drm_device *drm,
struct drm_plane_state *old_state,
struct drm_plane_state *state);