Re: [PATCH 06/10] drm: Conslidate blending properties in drm_blend.[hc]

[Sean Paul <seanpaul@chromium.org>]

On Wed, Aug 31, 2016 at 12:09 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Imo zpos, rotatation, blending eq (once we have it) and all that
> should be in drm_blend.c, since those are all about how exactly the
> pixels are rendered onto the CRTC's visible area. Also noticed that
> one exported function accidentally ended up in drm_crtc_internal.h,
> move it to the right place too.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/drm_blend.c         | 51 ++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/drm_crtc.c          | 49 ------------------------------
>  drivers/gpu/drm/drm_crtc_internal.h |  3 --
>  include/drm/drm_blend.h             | 59 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h              | 27 +----------------
>  5 files changed, 109 insertions(+), 80 deletions(-)
>  create mode 100644 include/drm/drm_blend.h
>
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 0813b7e021be..ab39245a21c4 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -25,12 +25,59 @@
>   */
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic.h>
> -#include <drm/drm_crtc.h>
> +#include <drm/drm_blend.h>
>  #include <linux/export.h>
>  #include <linux/slab.h>
>  #include <linux/sort.h>
>
> -#include "drm_internal.h"
> +struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> +                                                      unsigned int supported_rotations)
> +{
> +       static const struct drm_prop_enum_list props[] = {
> +               { __builtin_ffs(DRM_ROTATE_0) - 1,   "rotate-0" },
> +               { __builtin_ffs(DRM_ROTATE_90) - 1,  "rotate-90" },
> +               { __builtin_ffs(DRM_ROTATE_180) - 1, "rotate-180" },
> +               { __builtin_ffs(DRM_ROTATE_270) - 1, "rotate-270" },
> +               { __builtin_ffs(DRM_REFLECT_X) - 1,  "reflect-x" },
> +               { __builtin_ffs(DRM_REFLECT_Y) - 1,  "reflect-y" },
> +       };
> +
> +       return drm_property_create_bitmask(dev, 0, "rotation",
> +                                          props, ARRAY_SIZE(props),
> +                                          supported_rotations);
> +}
> +EXPORT_SYMBOL(drm_mode_create_rotation_property);
> +
> +/**
> + * drm_rotation_simplify() - Try to simplify the rotation
> + * @rotation: Rotation to be simplified
> + * @supported_rotations: Supported rotations
> + *
> + * Attempt to simplify the rotation to a form that is supported.
> + * Eg. if the hardware supports everything except DRM_REFLECT_X
> + * one could call this function like this:
> + *
> + * drm_rotation_simplify(rotation, DRM_ROTATE_0 |
> + *                       DRM_ROTATE_90 | DRM_ROTATE_180 |
> + *                       DRM_ROTATE_270 | DRM_REFLECT_Y);
> + *
> + * to eliminate the DRM_ROTATE_X flag. Depending on what kind of
> + * transforms the hardware supports, this function may not
> + * be able to produce a supported transform, so the caller should
> + * check the result afterwards.
> + */
> +unsigned int drm_rotation_simplify(unsigned int rotation,
> +                                  unsigned int supported_rotations)
> +{
> +       if (rotation & ~supported_rotations) {
> +               rotation ^= DRM_REFLECT_X | DRM_REFLECT_Y;
> +               rotation = (rotation & DRM_REFLECT_MASK) |
> +                          BIT((ffs(rotation & DRM_ROTATE_MASK) + 1) % 4);
> +       }
> +
> +       return rotation;
> +}
> +EXPORT_SYMBOL(drm_rotation_simplify);
>
>  /**
>   * drm_plane_create_zpos_property - create mutable zpos property
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 9ef7955032db..e5229b48d5d5 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1218,37 +1218,6 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
>  }
>
>  /**
> - * drm_rotation_simplify() - Try to simplify the rotation
> - * @rotation: Rotation to be simplified
> - * @supported_rotations: Supported rotations
> - *
> - * Attempt to simplify the rotation to a form that is supported.
> - * Eg. if the hardware supports everything except DRM_REFLECT_X
> - * one could call this function like this:
> - *
> - * drm_rotation_simplify(rotation, DRM_ROTATE_0 |
> - *                       DRM_ROTATE_90 | DRM_ROTATE_180 |
> - *                       DRM_ROTATE_270 | DRM_REFLECT_Y);
> - *
> - * to eliminate the DRM_ROTATE_X flag. Depending on what kind of
> - * transforms the hardware supports, this function may not
> - * be able to produce a supported transform, so the caller should
> - * check the result afterwards.
> - */
> -unsigned int drm_rotation_simplify(unsigned int rotation,
> -                                  unsigned int supported_rotations)
> -{
> -       if (rotation & ~supported_rotations) {
> -               rotation ^= DRM_REFLECT_X | DRM_REFLECT_Y;
> -               rotation = (rotation & DRM_REFLECT_MASK) |
> -                          BIT((ffs(rotation & DRM_ROTATE_MASK) + 1) % 4);
> -       }
> -
> -       return rotation;
> -}
> -EXPORT_SYMBOL(drm_rotation_simplify);
> -
> -/**
>   * drm_mode_config_init - initialize DRM mode_configuration structure
>   * @dev: DRM device
>   *
> @@ -1364,24 +1333,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_mode_config_cleanup);
>
> -struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> -                                                      unsigned int supported_rotations)
> -{
> -       static const struct drm_prop_enum_list props[] = {
> -               { __builtin_ffs(DRM_ROTATE_0) - 1,   "rotate-0" },
> -               { __builtin_ffs(DRM_ROTATE_90) - 1,  "rotate-90" },
> -               { __builtin_ffs(DRM_ROTATE_180) - 1, "rotate-180" },
> -               { __builtin_ffs(DRM_ROTATE_270) - 1, "rotate-270" },
> -               { __builtin_ffs(DRM_REFLECT_X) - 1,  "reflect-x" },
> -               { __builtin_ffs(DRM_REFLECT_Y) - 1,  "reflect-y" },
> -       };
> -
> -       return drm_property_create_bitmask(dev, 0, "rotation",
> -                                          props, ARRAY_SIZE(props),
> -                                          supported_rotations);
> -}
> -EXPORT_SYMBOL(drm_mode_create_rotation_property);
> -
>  /**
>   * DOC: Tile group
>   *
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index d538c2cc48f3..8caa088daa2d 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -164,9 +164,6 @@ int drm_atomic_remove_fb(struct drm_framebuffer *fb);
>  int drm_modeset_register_all(struct drm_device *dev);
>  void drm_modeset_unregister_all(struct drm_device *dev);
>
> -/* drm_blend.c */
> -int drm_atomic_normalize_zpos(struct drm_device *dev,
> -                             struct drm_atomic_state *state);
>
>  /* drm_plane.c */
>  int drm_plane_register_all(struct drm_device *dev);
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> new file mode 100644
> index 000000000000..868f0364e939
> --- /dev/null
> +++ b/include/drm/drm_blend.h
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright (c) 2016 Intel Corporation
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that copyright
> + * notice and this permission notice appear in supporting documentation, and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no representations
> + * about the suitability of this software for any purpose.  It is provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +#ifndef __DRM_BLEND_H__
> +#define __DRM_BLEND_H__
> +
> +#include <linux/list.h>
> +#include <linux/ctype.h>
> +
> +struct drm_device;
> +struct drm_atomic_state;
> +
> +/*
> + * Rotation property bits. DRM_ROTATE_<degrees> rotates the image by the
> + * specified amount in degrees in counter clockwise direction. DRM_REFLECT_X and
> + * DRM_REFLECT_Y reflects the image along the specified axis prior to rotation
> + */
> +#define DRM_ROTATE_0   BIT(0)
> +#define DRM_ROTATE_90  BIT(1)
> +#define DRM_ROTATE_180 BIT(2)
> +#define DRM_ROTATE_270 BIT(3)
> +#define DRM_ROTATE_MASK (DRM_ROTATE_0   | DRM_ROTATE_90 | \
> +                        DRM_ROTATE_180 | DRM_ROTATE_270)
> +#define DRM_REFLECT_X  BIT(4)
> +#define DRM_REFLECT_Y  BIT(5)
> +#define DRM_REFLECT_MASK (DRM_REFLECT_X | DRM_REFLECT_Y)
> +
> +struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> +                                                      unsigned int supported_rotations);
> +unsigned int drm_rotation_simplify(unsigned int rotation,
> +                                  unsigned int supported_rotations);
> +
> +int drm_plane_create_zpos_property(struct drm_plane *plane,
> +                                  unsigned int zpos,
> +                                  unsigned int min, unsigned int max);
> +int drm_plane_create_zpos_immutable_property(struct drm_plane *plane,
> +                                            unsigned int zpos);
> +int drm_atomic_normalize_zpos(struct drm_device *dev,
> +                             struct drm_atomic_state *state);
> +#endif
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 86e13a590c8f..1facfeeb0138 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -45,6 +45,7 @@
>  #include <drm/drm_bridge.h>
>  #include <drm/drm_edid.h>
>  #include <drm/drm_plane.h>
> +#include <drm/drm_blend.h>
>
>  struct drm_device;
>  struct drm_mode_set;
> @@ -63,21 +64,6 @@ static inline uint64_t I642U64(int64_t val)
>         return (uint64_t)*((uint64_t *)&val);
>  }
>
> -/*
> - * Rotation property bits. DRM_ROTATE_<degrees> rotates the image by the
> - * specified amount in degrees in counter clockwise direction. DRM_REFLECT_X and
> - * DRM_REFLECT_Y reflects the image along the specified axis prior to rotation
> - */
> -#define DRM_ROTATE_0   BIT(0)
> -#define DRM_ROTATE_90  BIT(1)
> -#define DRM_ROTATE_180 BIT(2)
> -#define DRM_ROTATE_270 BIT(3)
> -#define DRM_ROTATE_MASK (DRM_ROTATE_0   | DRM_ROTATE_90 | \
> -                        DRM_ROTATE_180 | DRM_ROTATE_270)
> -#define DRM_REFLECT_X  BIT(4)
> -#define DRM_REFLECT_Y  BIT(5)
> -#define DRM_REFLECT_MASK (DRM_REFLECT_X | DRM_REFLECT_Y)
> -
>  /* data corresponds to displayid vend/prod/serial */
>  struct drm_tile_group {
>         struct kref refcount;
> @@ -1351,22 +1337,11 @@ extern struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
>  extern void drm_mode_put_tile_group(struct drm_device *dev,
>                                    struct drm_tile_group *tg);
>
> -extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> -                                                             unsigned int supported_rotations);
> -extern unsigned int drm_rotation_simplify(unsigned int rotation,
> -                                         unsigned int supported_rotations);
>  extern void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>                                        uint degamma_lut_size,
>                                        bool has_ctm,
>                                        uint gamma_lut_size);
>
> -int drm_plane_create_zpos_property(struct drm_plane *plane,
> -                                  unsigned int zpos,
> -                                  unsigned int min, unsigned int max);
> -
> -int drm_plane_create_zpos_immutable_property(struct drm_plane *plane,
> -                                            unsigned int zpos);
> -
>  /* Helpers */
>  static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
>         uint32_t id)
> --
> 2.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel