Re: [PATCH] squash! drm/i915: Add format modifiers for Intel

On Fri, Mar 31, 2017 at 08:25:23AM -0700, Ben Widawsky wrote:

make sprite and cursor have separate functions

Ville, I think this addresses most of your comments. I'm guessing you're going to ask for separate gen sprite plane functions, but I think this looks pretty decent as is.

Yeah, separate might be nice, but I'm fine with this. I think I want to see if I can rethink some of the format check stuff all over anyway. That might want the separate the functions or it might not. I don't think I'll know until I try and see how it would all turn out. In the meantime we can proceed with this.

But plese drop the plane type BUG_ON()s. We've generally decided to not do BUGs anymore, and we don't have such things in any other plane vfunc either. Oh, and it looks like you have some tabs vs. spaces mess in that sprite funciton.

---

drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++------ drivers/gpu/drm/i915/intel_sprite.c | 51 ++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a5edcc910f4a..3f7ce8c39bc8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13535,18 +13535,16 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier) } }

-static bool intel_plane_format_mod_supported(struct drm_plane *plane,

• 			     uint32_t format,
  

• 			     uint64_t modifier)
  

+static bool intel_primary_plane_format_mod_supported(struct drm_plane *plane,

• 				     uint32_t format,
  

• 				     uint64_t modifier)
  

{ struct drm_i915_private *dev_priv = to_i915(plane->dev);

if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID)) return false;

• if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)

• return modifier == DRM_FORMAT_MOD_LINEAR &&
  

• 	format == DRM_FORMAT_ARGB8888;
  

• BUG_ON(plane->base.type != DRM_PLANE_TYPE_PRIMARY);

  if (INTEL_GEN(dev_priv) >= 9) return skl_mod_supported(format, modifier);

@@ -13558,6 +13556,18 @@ static bool intel_plane_format_mod_supported(struct drm_plane *plane, return false; }

+static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,

• 				    uint32_t format,
  

• 				    uint64_t modifier)
  

+{

• if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))

• return false;
  

• BUG_ON(plane->base.type != DRM_PLANE_TYPE_CURSOR);
• return modifier == DRM_FORMAT_MOD_LINEAR && format == DRM_FORMAT_ARGB8888;

+}

const struct drm_plane_funcs intel_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, @@ -13567,7 +13577,7 @@ const struct drm_plane_funcs intel_plane_funcs = { .atomic_set_property = intel_plane_atomic_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, .atomic_destroy_state = intel_plane_destroy_state,

• .format_mod_supported = intel_plane_format_mod_supported,

• .format_mod_supported = intel_primary_plane_format_mod_supported,

};

static int @@ -13704,7 +13714,7 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = { .atomic_set_property = intel_plane_atomic_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, .atomic_destroy_state = intel_plane_destroy_state,

• .format_mod_supported = intel_plane_format_mod_supported,

• .format_mod_supported = intel_cursor_plane_format_mod_supported,

};

static struct intel_plane * diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index bf18d9edc66f..dae8e8a943b9 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -30,6 +30,7 @@

• support.

*/ #include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> #include <drm/drm_crtc.h> #include <drm/drm_fourcc.h> #include <drm/drm_rect.h> @@ -1102,6 +1103,56 @@ static const uint64_t skl_plane_format_modifiers[] = { DRM_FORMAT_MOD_INVALID };

+static bool intel_sprite_plane_format_mod_supported(struct drm_plane *plane,

•                                                uint32_t format,
  

•                                                uint64_t modifier)
  

+{

•    struct drm_i915_private *dev_priv = to_i915(plane->dev);
  

•    if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
  

•            return false;
  

•    BUG_ON(plane->base.type != DRM_PLANE_TYPE_OVERLAY);
  

• switch (format) {

• case DRM_FORMAT_XBGR2101010:
  

• case DRM_FORMAT_ABGR2101010:
  

• 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
  

• 		return true;
  

• 	break;
  

• case DRM_FORMAT_RGB565:
  

• case DRM_FORMAT_ABGR8888:
  

• case DRM_FORMAT_ARGB8888:
  

• 	if (INTEL_GEN(dev_priv) >= 9 || IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
  

• 		return true;
  

• 	break;
  

• case DRM_FORMAT_XBGR8888:
  

• 	if (INTEL_GEN(dev_priv) >= 6)
  

• 		return true;
  

• 	break;
  

• case DRM_FORMAT_XRGB8888:
  

• case DRM_FORMAT_YUYV:
  

• case DRM_FORMAT_YVYU:
  

• case DRM_FORMAT_UYVY:
  

• case DRM_FORMAT_VYUY:
  

• 	return true;
  

• }

•    return false;
  

+}

+const struct drm_plane_funcs intel_sprite_plane_funcs = {

•    .update_plane = drm_atomic_helper_update_plane,
  

•    .disable_plane = drm_atomic_helper_disable_plane,
  

•    .destroy = intel_plane_destroy,
  

•    .set_property = drm_atomic_helper_plane_set_property,
  

•    .atomic_get_property = intel_plane_atomic_get_property,
  

•    .atomic_set_property = intel_plane_atomic_set_property,
  

•    .atomic_duplicate_state = intel_plane_duplicate_state,
  

•    .atomic_destroy_state = intel_plane_destroy_state,
  

•    .format_mod_supported = intel_sprite_plane_format_mod_supported,
  

+};

struct intel_plane * intel_sprite_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe, int plane) -- 2.12.1