| Message ID | 20181029171419.4512-2-alexandru-cosmin.gheorghe@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add method to describe tile/bit_level_packed formats | expand |
On Mon, Oct 29, 2018 at 05:14:37PM +0000, Alexandru-Cosmin Gheorghe wrote: > For some pixel formats .cpp structure in drm_format info it's not > enough to describe the peculiarities of the pixel layout, for example > tiled formats or packed formats at bit level. > > What's implemented here is to add three new members to drm_format_info > that could describe such formats: > > - char_per_block[3] > - block_w[3] > - block_h[3] > > char_per_block will be put in a union alongside cpp, for transparent > compatibility with the existing format descriptions. > > Regarding, block_w and block_h they are intended to be used through > their equivalent getters drm_format_info_block_width / > drm_format_info_block_height, the reason of the getters is to abstract > the fact that for normal formats block_w and block_h will be unset/0, > but the methods will be returning 1. > > Additionally, convenience function drm_format_info_min_pitch had been > added that computes the minimum required pitch for a given pixel > format and buffer width. > > Using that the following drm core functions had been updated to > generically handle both block and non-block formats: > > - drm_fb_cma_get_gem_addr: for block formats it will just return the > beginning of the block. > - framebuffer_check: Use the newly added drm_format_info_min_pitch. > - drm_gem_fb_create_with_funcs: Use the newly added > drm_format_info_min_pitch. > - In places where is not expecting to handle block formats, like fbdev > helpers I just added some warnings in case the block width/height > are greater than 1. > > Changes since v3: > - Add helper function for computing the minimum required pitch. > - Improve/cleanup documentation > > Reviewed-by: Brian Starkey <brian.starkey@arm.com> > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> Really neat kerneldoc now. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_fb_cma_helper.c | 20 ++++++- > drivers/gpu/drm/drm_fb_helper.c | 6 ++ > drivers/gpu/drm/drm_fourcc.c | 62 ++++++++++++++++++++ > drivers/gpu/drm/drm_framebuffer.c | 6 +- > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 2 +- > include/drm/drm_fourcc.h | 61 ++++++++++++++++++- > 6 files changed, 148 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > index fc2b42dd3dc6..b07ab3f613e0 100644 > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -72,7 +72,9 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, > EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj); > > /** > - * drm_fb_cma_get_gem_addr() - Get physical address for framebuffer > + * drm_fb_cma_get_gem_addr() - Get physical 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 > @@ -87,6 +89,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb, > struct drm_gem_cma_object *obj; > dma_addr_t paddr; > u8 h_div = 1, v_div = 1; > + u32 block_w = drm_format_info_block_width(fb->format, plane); > + u32 block_h = drm_format_info_block_height(fb->format, plane); > + u32 block_size = fb->format->char_per_block[plane]; > + u32 sample_x; > + u32 sample_y; > + u32 block_start_y; > + u32 num_hblocks; > > obj = drm_fb_cma_get_gem_obj(fb, plane); > if (!obj) > @@ -99,8 +108,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb, > v_div = fb->format->vsub; > } > > - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div; > - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div; > + sample_x = (state->src_x >> 16) / h_div; > + sample_y = (state->src_y >> 16) / v_div; > + block_start_y = (sample_y / block_h) * block_h; > + num_hblocks = sample_x / block_w; > + > + paddr += fb->pitches[plane] * block_start_y; > + paddr += block_size * num_hblocks; > > return paddr; > } > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 9b111e846847..8024524f0547 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1614,6 +1614,10 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > if (var->pixclock != 0 || in_dbg_master()) > return -EINVAL; > > + if ((drm_format_info_block_width(fb->format, 0) > 1) || > + (drm_format_info_block_height(fb->format, 0) > 1)) > + return -EINVAL; > + > /* > * Changes struct fb_var_screeninfo are currently not pushed back > * to KMS, hence fail if different settings are requested. > @@ -1988,6 +1992,8 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe > { > struct drm_framebuffer *fb = fb_helper->fb; > > + WARN_ON((drm_format_info_block_width(fb->format, 0) > 1) || > + (drm_format_info_block_height(fb->format, 0) > 1)); > info->pseudo_palette = fb_helper->pseudo_palette; > info->var.xres_virtual = fb->width; > info->var.yres_virtual = fb->height; > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > index e177f6d0d1f4..a843a5fc8dbf 100644 > --- a/drivers/gpu/drm/drm_fourcc.c > +++ b/drivers/gpu/drm/drm_fourcc.c > @@ -403,3 +403,65 @@ int drm_format_plane_height(int height, uint32_t format, int plane) > return height / info->vsub; > } > EXPORT_SYMBOL(drm_format_plane_height); > + > +/** > + * drm_format_info_block_width - width in pixels of block. > + * @info: pixel format info > + * @plane: plane index > + * > + * Returns: > + * The width in pixels of a block, depending on the plane index. > + */ > +unsigned int drm_format_info_block_width(const struct drm_format_info *info, > + int plane) > +{ > + if (!info || plane < 0 || plane >= info->num_planes) > + return 0; > + > + if (!info->block_w[plane]) > + return 1; > + return info->block_w[plane]; > +} > +EXPORT_SYMBOL(drm_format_info_block_width); > + > +/** > + * drm_format_info_block_height - height in pixels of a block > + * @info: pixel format info > + * @plane: plane index > + * > + * Returns: > + * The height in pixels of a block, depending on the plane index. > + */ > +unsigned int drm_format_info_block_height(const struct drm_format_info *info, > + int plane) > +{ > + if (!info || plane < 0 || plane >= info->num_planes) > + return 0; > + > + if (!info->block_h[plane]) > + return 1; > + return info->block_h[plane]; > +} > +EXPORT_SYMBOL(drm_format_info_block_height); > + > +/** > + * drm_format_info_min_pitch - computes the minimum required pitch in bytes > + * @info: pixel format info > + * @plane: plane index > + * @buffer_width: buffer width in pixels > + * > + * Returns: > + * The minimum required pitch in bytes for a buffer by taking into consideration > + * the pixel format information and the buffer width. > + */ > +uint64_t drm_format_info_min_pitch(const struct drm_format_info *info, > + int plane, unsigned int buffer_width) > +{ > + if (!info || plane < 0 || plane >= info->num_planes) > + return 0; > + > + return DIV_ROUND_UP((u64)buffer_width * info->char_per_block[plane], > + drm_format_info_block_width(info, plane) * > + drm_format_info_block_height(info, plane)); > +} > +EXPORT_SYMBOL(drm_format_info_min_pitch); > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > index 3bf729d0aae5..6aca8a1ccdb6 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -195,20 +195,20 @@ static int framebuffer_check(struct drm_device *dev, > for (i = 0; i < info->num_planes; i++) { > unsigned int width = fb_plane_width(r->width, info, i); > unsigned int height = fb_plane_height(r->height, info, i); > - unsigned int cpp = info->cpp[i]; > + u64 min_pitch = drm_format_info_min_pitch(info, i, width); > > if (!r->handles[i]) { > DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i); > return -EINVAL; > } > > - if ((uint64_t) width * cpp > UINT_MAX) > + if (min_pitch > UINT_MAX) > return -ERANGE; > > if ((uint64_t) height * r->pitches[i] + r->offsets[i] > UINT_MAX) > return -ERANGE; > > - if (r->pitches[i] < width * cpp) { > + if (r->pitches[i] < min_pitch) { > DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[i], i); > return -EINVAL; > } > diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c > index ded7a379ac35..acb466d25afc 100644 > --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c > +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c > @@ -171,7 +171,7 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, > } > > min_size = (height - 1) * mode_cmd->pitches[i] > - + width * info->cpp[i] > + + drm_format_info_min_pitch(info, i, width) > + mode_cmd->offsets[i]; > > if (objs[i]->size < min_size) { > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h > index 345f11227e9e..bcb389f04618 100644 > --- a/include/drm/drm_fourcc.h > +++ b/include/drm/drm_fourcc.h > @@ -69,8 +69,59 @@ struct drm_format_info { > /** @num_planes: Number of color planes (1 to 3) */ > u8 num_planes; > > - /** @cpp: Number of bytes per pixel (per plane) */ > - u8 cpp[3]; > + union { > + /** > + * @cpp: > + * > + * Number of bytes per pixel (per plane), this is aliased with > + * @char_per_block. It is deprecated in favour of using the > + * triplet @char_per_block, @block_w, @block_h for better > + * describing the pixel format. > + */ > + u8 cpp[3]; > + > + /** > + * @char_per_block: > + * > + * Number of bytes per block (per plane), where blocks are > + * defined as a rectangle of pixels which are stored next to > + * each other in a byte aligned memory region. Together with > + * @block_w and @block_h this is used to properly describe tiles > + * in tiled formats or to describe groups of pixels in packed > + * formats for which the memory needed for a single pixel is not > + * byte aligned. > + * > + * @cpp has been kept for historical reasons because there are > + * a lot of places in drivers where it's used. In drm core for > + * generic code paths the preferred way is to use > + * @char_per_block, drm_format_info_block_width() and > + * drm_format_info_block_height() which allows handling both > + * block and non-block formats in the same way. > + * > + * For formats that are intended to be used only with non-linear > + * modifiers both @cpp and @char_per_block must be 0 in the > + * generic format table. Drivers could supply accurate > + * information from their drm_mode_config.get_format_info hook > + * if they want the core to be validating the pitch. > + */ > + u8 char_per_block[3]; > + }; > + > + /** > + * @block_w: > + * > + * Block width in pixels, this is intended to be accessed through > + * drm_format_info_block_width() > + */ > + u8 block_w[3]; > + > + /** > + * @block_h: > + * > + * Block height in pixels, this is intended to be accessed through > + * drm_format_info_block_height() > + */ > + u8 block_h[3]; > > /** @hsub: Horizontal chroma subsampling factor */ > u8 hsub; > @@ -106,6 +157,12 @@ int drm_format_horz_chroma_subsampling(uint32_t format); > int drm_format_vert_chroma_subsampling(uint32_t format); > int drm_format_plane_width(int width, uint32_t format, int plane); > int drm_format_plane_height(int height, uint32_t format, int plane); > +unsigned int drm_format_info_block_width(const struct drm_format_info *info, > + int plane); > +unsigned int drm_format_info_block_height(const struct drm_format_info *info, > + int plane); > +uint64_t drm_format_info_min_pitch(const struct drm_format_info *info, > + int plane, unsigned int buffer_width); > const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf); > > #endif /* __DRM_FOURCC_H__ */ > -- > 2.19.1 >
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index fc2b42dd3dc6..b07ab3f613e0 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -72,7 +72,9 @@ struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb, EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj); /** - * drm_fb_cma_get_gem_addr() - Get physical address for framebuffer + * drm_fb_cma_get_gem_addr() - Get physical 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 @@ -87,6 +89,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb, struct drm_gem_cma_object *obj; dma_addr_t paddr; u8 h_div = 1, v_div = 1; + u32 block_w = drm_format_info_block_width(fb->format, plane); + u32 block_h = drm_format_info_block_height(fb->format, plane); + u32 block_size = fb->format->char_per_block[plane]; + u32 sample_x; + u32 sample_y; + u32 block_start_y; + u32 num_hblocks; obj = drm_fb_cma_get_gem_obj(fb, plane); if (!obj) @@ -99,8 +108,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb, v_div = fb->format->vsub; } - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div; - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div; + sample_x = (state->src_x >> 16) / h_div; + sample_y = (state->src_y >> 16) / v_div; + block_start_y = (sample_y / block_h) * block_h; + num_hblocks = sample_x / block_w; + + paddr += fb->pitches[plane] * block_start_y; + paddr += block_size * num_hblocks; return paddr; } diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 9b111e846847..8024524f0547 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1614,6 +1614,10 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, if (var->pixclock != 0 || in_dbg_master()) return -EINVAL; + if ((drm_format_info_block_width(fb->format, 0) > 1) || + (drm_format_info_block_height(fb->format, 0) > 1)) + return -EINVAL; + /* * Changes struct fb_var_screeninfo are currently not pushed back * to KMS, hence fail if different settings are requested. @@ -1988,6 +1992,8 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe { struct drm_framebuffer *fb = fb_helper->fb; + WARN_ON((drm_format_info_block_width(fb->format, 0) > 1) || + (drm_format_info_block_height(fb->format, 0) > 1)); info->pseudo_palette = fb_helper->pseudo_palette; info->var.xres_virtual = fb->width; info->var.yres_virtual = fb->height; diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index e177f6d0d1f4..a843a5fc8dbf 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -403,3 +403,65 @@ int drm_format_plane_height(int height, uint32_t format, int plane) return height / info->vsub; } EXPORT_SYMBOL(drm_format_plane_height); + +/** + * drm_format_info_block_width - width in pixels of block. + * @info: pixel format info + * @plane: plane index + * + * Returns: + * The width in pixels of a block, depending on the plane index. + */ +unsigned int drm_format_info_block_width(const struct drm_format_info *info, + int plane) +{ + if (!info || plane < 0 || plane >= info->num_planes) + return 0; + + if (!info->block_w[plane]) + return 1; + return info->block_w[plane]; +} +EXPORT_SYMBOL(drm_format_info_block_width); + +/** + * drm_format_info_block_height - height in pixels of a block + * @info: pixel format info + * @plane: plane index + * + * Returns: + * The height in pixels of a block, depending on the plane index. + */ +unsigned int drm_format_info_block_height(const struct drm_format_info *info, + int plane) +{ + if (!info || plane < 0 || plane >= info->num_planes) + return 0; + + if (!info->block_h[plane]) + return 1; + return info->block_h[plane]; +} +EXPORT_SYMBOL(drm_format_info_block_height); + +/** + * drm_format_info_min_pitch - computes the minimum required pitch in bytes + * @info: pixel format info + * @plane: plane index + * @buffer_width: buffer width in pixels + * + * Returns: + * The minimum required pitch in bytes for a buffer by taking into consideration + * the pixel format information and the buffer width. + */ +uint64_t drm_format_info_min_pitch(const struct drm_format_info *info, + int plane, unsigned int buffer_width) +{ + if (!info || plane < 0 || plane >= info->num_planes) + return 0; + + return DIV_ROUND_UP((u64)buffer_width * info->char_per_block[plane], + drm_format_info_block_width(info, plane) * + drm_format_info_block_height(info, plane)); +} +EXPORT_SYMBOL(drm_format_info_min_pitch); diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index 3bf729d0aae5..6aca8a1ccdb6 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -195,20 +195,20 @@ static int framebuffer_check(struct drm_device *dev, for (i = 0; i < info->num_planes; i++) { unsigned int width = fb_plane_width(r->width, info, i); unsigned int height = fb_plane_height(r->height, info, i); - unsigned int cpp = info->cpp[i]; + u64 min_pitch = drm_format_info_min_pitch(info, i, width); if (!r->handles[i]) { DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i); return -EINVAL; } - if ((uint64_t) width * cpp > UINT_MAX) + if (min_pitch > UINT_MAX) return -ERANGE; if ((uint64_t) height * r->pitches[i] + r->offsets[i] > UINT_MAX) return -ERANGE; - if (r->pitches[i] < width * cpp) { + if (r->pitches[i] < min_pitch) { DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[i], i); return -EINVAL; } diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index ded7a379ac35..acb466d25afc 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -171,7 +171,7 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, } min_size = (height - 1) * mode_cmd->pitches[i] - + width * info->cpp[i] + + drm_format_info_min_pitch(info, i, width) + mode_cmd->offsets[i]; if (objs[i]->size < min_size) { diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h index 345f11227e9e..bcb389f04618 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -69,8 +69,59 @@ struct drm_format_info { /** @num_planes: Number of color planes (1 to 3) */ u8 num_planes; - /** @cpp: Number of bytes per pixel (per plane) */ - u8 cpp[3]; + union { + /** + * @cpp: + * + * Number of bytes per pixel (per plane), this is aliased with + * @char_per_block. It is deprecated in favour of using the + * triplet @char_per_block, @block_w, @block_h for better + * describing the pixel format. + */ + u8 cpp[3]; + + /** + * @char_per_block: + * + * Number of bytes per block (per plane), where blocks are + * defined as a rectangle of pixels which are stored next to + * each other in a byte aligned memory region. Together with + * @block_w and @block_h this is used to properly describe tiles + * in tiled formats or to describe groups of pixels in packed + * formats for which the memory needed for a single pixel is not + * byte aligned. + * + * @cpp has been kept for historical reasons because there are + * a lot of places in drivers where it's used. In drm core for + * generic code paths the preferred way is to use + * @char_per_block, drm_format_info_block_width() and + * drm_format_info_block_height() which allows handling both + * block and non-block formats in the same way. + * + * For formats that are intended to be used only with non-linear + * modifiers both @cpp and @char_per_block must be 0 in the + * generic format table. Drivers could supply accurate + * information from their drm_mode_config.get_format_info hook + * if they want the core to be validating the pitch. + */ + u8 char_per_block[3]; + }; + + /** + * @block_w: + * + * Block width in pixels, this is intended to be accessed through + * drm_format_info_block_width() + */ + u8 block_w[3]; + + /** + * @block_h: + * + * Block height in pixels, this is intended to be accessed through + * drm_format_info_block_height() + */ + u8 block_h[3]; /** @hsub: Horizontal chroma subsampling factor */ u8 hsub; @@ -106,6 +157,12 @@ int drm_format_horz_chroma_subsampling(uint32_t format); int drm_format_vert_chroma_subsampling(uint32_t format); int drm_format_plane_width(int width, uint32_t format, int plane); int drm_format_plane_height(int height, uint32_t format, int plane); +unsigned int drm_format_info_block_width(const struct drm_format_info *info, + int plane); +unsigned int drm_format_info_block_height(const struct drm_format_info *info, + int plane); +uint64_t drm_format_info_min_pitch(const struct drm_format_info *info, + int plane, unsigned int buffer_width); const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf); #endif /* __DRM_FOURCC_H__ */