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 <andrze...@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 > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
