[PATCHv4,04/36] drm/gem-fb-helper: Add special version of drm_gem_fb_size_check
james qian wang (Arm Technology China)
james.qian.wang at arm.com
Mon Feb 17 11:34:32 UTC 2020
On Mon, Feb 17, 2020 at 11:55:50AM +0100, Andrzej Pietrasiewicz wrote:
> Hi James,
>
> Thank you for the review.
>
> Did you intentionally review patches from the v4 series or you simply
> didn't notice the v5? There are some differences, the most notable one
> is using proper way of subclassing a drm_framebuffer.
> The v5 series was sent on 17th December 2019.
Hi Andrzej:
Sorry, I did the review based pathwork:
https://patchwork.freedesktop.org/series/70889/#rev35
seems it has problem which only have V4.
I will re-do the review for V5 ASAP.
Sorry again.
James.
> Andrzej
>
> W dniu 17.02.2020 o�09:16, james qian wang (Arm Technology China) pisze:
> > Hi Andrzej:
> >
> > Really a good idea for introducing this custom size check, it's very
> > useful for some Komeda/malidp format, espcially pitch_multiplier, maybe
> > in future we can add it into into the drm_format_info.
> >
> > On Fri, Dec 13, 2019 at 04:58:35PM +0100, Andrzej Pietrasiewicz wrote:
> > > The new version accepts a struct describing deviations from standard way of
> > > doing the size checks. The caller must provide the respective values.
> > >
> > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p at collabora.com>
> > > ---
> > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 47 ++++++++++++++++----
> > > include/drm/drm_gem_framebuffer_helper.h | 16 +++++++
> > > 2 files changed, 55 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> > > index 787edb9a916b..4201dc1f32a5 100644
> > > --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> > > +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> > > @@ -201,8 +201,9 @@ int drm_gem_fb_lookup(struct drm_device *dev,
> > > EXPORT_SYMBOL_GPL(drm_gem_fb_lookup);
> > > /**
> > > - * drm_gem_fb_size_check() - Helper function for use in
> > > - * &drm_mode_config_funcs.fb_create implementations
> > > + * drm_gem_fb_size_check_special() - Helper function for use in
> > > + * &drm_mode_config_funcs.fb_create
> > > + * implementations
> > > * @dev: DRM device
> > > * @mode_cmd: Metadata from the userspace framebuffer creation request
> > > *
> > > @@ -212,9 +213,10 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_lookup);
> > > * Returns:
> > > * Zero on success or a negative error code on failure.
> > > */
> > > -int drm_gem_fb_size_check(struct drm_device *dev,
> > > - const struct drm_mode_fb_cmd2 *mode_cmd,
> > > - struct drm_gem_object **objs)
> > > +int drm_gem_fb_size_check_special(struct drm_device *dev,
> >
> > How about name it to drm_gem_fb_custom_size_check()
> >
> > > + const struct drm_mode_fb_cmd2 *mode_cmd,
> > > + const struct drm_size_check *check,
> > > + struct drm_gem_object **objs)
> > > {
> > > const struct drm_format_info *info;
> > > int i;
> > > @@ -227,10 +229,19 @@ int drm_gem_fb_size_check(struct drm_device *dev,
> > > unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> > > unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> > > unsigned int min_size;
> > > + u32 pitch = mode_cmd->pitches[i];
> > > +
> > > + if (check && check->use_pitch_multiplier)
> > > + if ((pitch * check->pitch_multiplier[i]) %
> > > + check->pitch_modulo)
> > > + return -EINVAL;
> > > - min_size = (height - 1) * mode_cmd->pitches[i]
> > > - + drm_format_info_min_pitch(info, i, width)
> > > - + mode_cmd->offsets[i];
> > > + if (check && check->use_min_size)
> > > + min_size = check->min_size[i];
> > > + else
> > > + min_size = (height - 1) * pitch
> > > + + drm_format_info_min_pitch(info, i, width)
> > > + + mode_cmd->offsets[i];
> > > if (objs[i]->size < min_size)
> > > return -EINVAL;
> > > @@ -239,6 +250,26 @@ int drm_gem_fb_size_check(struct drm_device *dev,
> > > return 0;
> > > }
> > > +EXPORT_SYMBOL_GPL(drm_gem_fb_size_check_special);
> > > +
> > > +/**
> > > + * drm_gem_fb_size_check() - Helper function for use in
> > > + * &drm_mode_config_funcs.fb_create implementations
> > > + * @dev: DRM device
> > > + * @mode_cmd: Metadata from the userspace framebuffer creation request
> > > + *
> > > + * This function can be used to verify buffer sizes for all planes.
> > > + * It is caller's responsibility to put the objects on failure.
> > > + *
> > > + * Returns:
> > > + * Zero on success or a negative error code on failure.
> > > + */
> > > +int drm_gem_fb_size_check(struct drm_device *dev,
> > > + const struct drm_mode_fb_cmd2 *mode_cmd,
> > > + struct drm_gem_object **objs)
> > > +{
> > > + return drm_gem_fb_size_check_special(dev, mode_cmd, NULL, objs);
> > > +}
> > > EXPORT_SYMBOL_GPL(drm_gem_fb_size_check);
> > > /**
> > > diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> > > index c85d4b152e91..74304a268694 100644
> > > --- a/include/drm/drm_gem_framebuffer_helper.h
> > > +++ b/include/drm/drm_gem_framebuffer_helper.h
> > > @@ -11,6 +11,18 @@ struct drm_mode_fb_cmd2;
> > > struct drm_plane;
> > > struct drm_plane_state;
> > > struct drm_simple_display_pipe;
> > > +struct drm_size_check;
> > > +
> > > +/**
> > > + * struct drm_size_check - Description of special requirements for size checks.
> > > + */
> > > +struct drm_size_check {
> > > + unsigned int min_size[4];
> > > + bool use_min_size;
> > > + u32 pitch_multiplier[4];
> > > + u32 pitch_modulo;
> > > + bool use_pitch_multiplier;
> > > +};
> > > struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
> > > unsigned int plane);
> > > @@ -32,6 +44,10 @@ int drm_gem_fb_lookup(struct drm_device *dev,
> > > struct drm_file *file,
> > > const struct drm_mode_fb_cmd2 *mode_cmd,
> > > struct drm_gem_object **objs);
> > > +int drm_gem_fb_size_check_special(struct drm_device *dev,
> > > + const struct drm_mode_fb_cmd2 *mode_cmd,
> > > + const struct drm_size_check *check,
> > > + struct drm_gem_object **objs);
> > > int drm_gem_fb_size_check(struct drm_device *dev,
> > > const struct drm_mode_fb_cmd2 *mode_cmd,
> > > struct drm_gem_object **objs);
> >
> > For this common case can we just define it as a MACRO, or a inline
> > func here in this header.
> >
> > Thanks
> > James
> >
More information about the dri-devel
mailing list