On Thursday 19 July 2012 14:46:38 Laurent Pinchart wrote:
> On Monday 02 July 2012 16:37:47 Lars-Peter Clausen wrote:
> > This patchset introduces a set of helper function for implementing the KMS
> > framebuffer layer for drivers which use the drm gem CMA helper function.
> >
> > Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> >
> > ---
> > Note: This patch depends on Sascha's "DRM: add drm gem CMA helper" patch
> >
> > Changes since v2:
> > * Adapt to changes in the GEM CMA helper
> > * Add basic buffer size checking in drm_fb_cma_create
> >
> > Changes since v1:
> > * Some spelling fixes
> > * Add missing kfree in drm_fb_cma_alloc error path
> > * Add multi-plane support
> >
> > ---
> >
> > drivers/gpu/drm/Kconfig | 10 +
> > drivers/gpu/drm/Makefile | 1 +
> > drivers/gpu/drm/drm_fb_cma_helper.c | 393
> > ++++++++++++++++++++++++++++++++
> > include/drm/drm_fb_cma_helper.h | 27 +++
> > 4 files changed, 431 insertions(+)
> > create mode 100644 drivers/gpu/drm/drm_fb_cma_helper.c
> > create mode 100644 include/drm/drm_fb_cma_helper.h
>
> [snip]
>
> > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > b/drivers/gpu/drm/drm_fb_cma_helper.c new file mode 100644
> > index 0000000..9042233
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
>
> [snip]
>
> > +/**
> > + * drm_fb_cma_create() - (struct drm_mode_config_funcs *)->fb_create
> > callback function
> > + *
> > + * If your hardware has special alignment or pitch requirements these
> > should be
> > + * checked before calling this function.
> > + */
> > +struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
> > + struct drm_file *file_priv, struct drm_mode_fb_cmd2 *mode_cmd)
> > +{
> > + struct drm_fb_cma *fb_cma;
> > + struct drm_gem_cma_object *objs[4];
> > + struct drm_gem_object *obj;
> > + int ret;
> > + int i;
> > +
> > + for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
> > + obj = drm_gem_object_lookup(dev, file_priv, mode_cmd->handles[i]);
> > + if (!obj) {
> > + dev_err(dev->dev, "Failed to lookup GEM object\n");
> > + ret = -ENXIO;
> > + goto err_gem_object_unreference;
> > + }
> > +
> > + if (obj->size < mode_cmd->height * mode_cmd->pitches[i]) {
>
> Shouldn't this be
>
> if (obj->size < mode_cmd->height * mode_cmd->pitches[i]
> + mode_cmd->offsets[i])
>
> ?
And, for multiplanar formats, we need to divide height by the plane vertical
subsampling factor. What about something like this ?
struct drm_fb_cma *fb_cma;
struct drm_gem_cma_object *objs[4];
struct drm_gem_object *obj;
int ret;
int i;
for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
unsigned int min_size;
obj = drm_gem_object_lookup(dev, file_priv, mode_cmd->handles[i]);
if (!obj) {
dev_err(dev->dev, "Failed to lookup GEM object\n");
ret = -ENXIO;
goto err_gem_object_unreference;
}
min_size = mode_cmd->pitches[i] * mode_cmd->height;
if (i)
min_size /= drm_format_vert_chroma_subsampling(mode_cmd->pixel_format);
min_size += mode_cmd->offsets[i];
if (obj->size < min_size) {
drm_gem_object_unreference_unlocked(obj);
ret = -EINVAL;
goto err_gem_object_unreference;
}
objs[i] = to_drm_gem_cma_obj(obj);
}
On a totally unrelated note, I'm wondering whether we should replace the
various DRM format information calls (drm_format_num_planes,
drm_fb_get_bpp_depth, ...) by a single drm_get_format_info() function that
would return a const pointer to format information. We could keep the existing
API as static inline functions. This would save CPU time when we need to
repeatedly access various information about a single format.
> > + drm_gem_object_unreference_unlocked(obj);
> > + ret = -EINVAL;
> > + goto err_gem_object_unreference;
> > + }
> > + objs[i] = to_drm_gem_cma_obj(obj);
> > + }
> > +
> > + fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i);
> > + if (IS_ERR(fb_cma)) {
> > + ret = PTR_ERR(fb_cma);
> > + goto err_gem_object_unreference;
> > + }
> > +
> > + return &fb_cma->fb;
> > +
> > +err_gem_object_unreference:
> > + for (i--; i >= 0; i--)
> > + drm_gem_object_unreference_unlocked(&objs[i]->base);
> > + return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL_GPL(drm_fb_cma_create);
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
