[Intel-gfx] [PATCH 2/2] drm/i915: Reuse plane format modifier checks to verify addfb() arguments
Pandiyan, Dhinakaran
dhinakaran.pandiyan at intel.com
Fri Oct 26 18:39:19 UTC 2018
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Friday, October 26, 2018 7:46 AM
> To: Pandiyan, Dhinakaran <dhinakaran.pandiyan at intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/i915: Reuse plane format modifier checks to
> verify addfb() arguments
>
> On Thu, Oct 25, 2018 at 06:32:56PM -0700, Dhinakaran Pandiyan wrote:
> > Currently there is some duplication of pixel format and modifier
> > validation code between the fb creation and plane check paths. We can
> > unify them by checking if any plane supports a pixel format and
> > modifier combination during framebuffer creation.
> >
> > Suggested-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 116
> > ++++++-----------------------------
> > 1 file changed, 19 insertions(+), 97 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index fe045abb6472..1b5d936a93d0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14362,6 +14362,19 @@ u32 intel_fb_pitch_limit(struct
> drm_i915_private *dev_priv,
> > DRM_MODE_ROTATE_0);
> > }
> >
> > +static bool any_plane_supports_format(struct drm_i915_private
> *dev_priv,
> > + uint64_t fb_modifier,
> > + uint32_t pixel_format)
>
> "format first, modifier second" is the typical covention.
Yeah, I did go that way. intel_fb_pitch_limit() that's right above inverts the order,
so I switched it keep it locally consistent.
>
> But I think we could stuff this entire thing into the core, in case someone else
> wants to reuse it. I think I even posted the patches that do it like that.
> Ah yes: https://patchwork.freedesktop.org/series/39700/
Since you posted them first, let's go with it. Patches 1-3 look good to me, can
you rebase and send them to the list? The second patch does not apply.
>
> > +{
> > + struct drm_plane *plane;
> > +
> > + drm_for_each_plane(plane, &dev_priv->drm)
> > + if (!drm_plane_check_pixel_format(plane, pixel_format,
> > + fb_modifier))
> > + return true;
> > + return false;
> > +}
> > +
> > static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> > struct drm_i915_gem_object *obj,
> > struct drm_mode_fb_cmd2 *mode_cmd)
> @@ -14399,40 +14412,12 @@
> > static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> > }
> > }
> >
> > - /* Passed in modifier sanity checking. */
> > - switch (mode_cmd->modifier[0]) {
> > - case I915_FORMAT_MOD_Y_TILED_CCS:
> > - case I915_FORMAT_MOD_Yf_TILED_CCS:
> > - switch (mode_cmd->pixel_format) {
> > - case DRM_FORMAT_XBGR8888:
> > - case DRM_FORMAT_ABGR8888:
> > - case DRM_FORMAT_XRGB8888:
> > - case DRM_FORMAT_ARGB8888:
> > - break;
> > - default:
> > - DRM_DEBUG_KMS("RC supported only with
> RGB8888 formats\n");
> > - goto err;
> > - }
> > - /* fall through */
> > - case I915_FORMAT_MOD_Yf_TILED:
> > - if (mode_cmd->pixel_format == DRM_FORMAT_C8) {
> > - DRM_DEBUG_KMS("Indexed format does not
> support Yf tiling\n");
> > - goto err;
> > - }
> > - /* fall through */
> > - case I915_FORMAT_MOD_Y_TILED:
> > - if (INTEL_GEN(dev_priv) < 9) {
> > - DRM_DEBUG_KMS("Unsupported tiling 0x%llx!\n",
> > - mode_cmd->modifier[0]);
> > - goto err;
> > - }
> > - break;
> > - case DRM_FORMAT_MOD_LINEAR:
> > - case I915_FORMAT_MOD_X_TILED:
> > - break;
> > - default:
> > - DRM_DEBUG_KMS("Unsupported fb modifier 0x%llx!\n",
> > - mode_cmd->modifier[0]);
> > + if (!any_plane_supports_format(dev_priv, mode_cmd->modifier[0],
> > + mode_cmd->pixel_format)) {
> > + DRM_DEBUG_KMS("Unsupported pixel format %s or
> modifier 0x%llx\n",
> > + drm_get_format_name(mode_cmd-
> >pixel_format,
> > + &format_name),
> > + mode_cmd->modifier[0]);
> > goto err;
> > }
> >
> > @@ -14466,69 +14451,6 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> > goto err;
> > }
> >
> > - /* Reject formats not supported by any plane early. */
> > - switch (mode_cmd->pixel_format) {
> > - case DRM_FORMAT_C8:
> > - case DRM_FORMAT_RGB565:
> > - case DRM_FORMAT_XRGB8888:
> > - case DRM_FORMAT_ARGB8888:
> > - break;
> > - case DRM_FORMAT_XRGB1555:
> > - if (INTEL_GEN(dev_priv) > 3) {
> > - DRM_DEBUG_KMS("unsupported pixel format:
> %s\n",
> > - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> > - goto err;
> > - }
> > - break;
> > - case DRM_FORMAT_ABGR8888:
> > - if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)
> &&
> > - INTEL_GEN(dev_priv) < 9) {
> > - DRM_DEBUG_KMS("unsupported pixel format:
> %s\n",
> > - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> > - goto err;
> > - }
> > - break;
> > - case DRM_FORMAT_XBGR8888:
> > - case DRM_FORMAT_XRGB2101010:
> > - case DRM_FORMAT_XBGR2101010:
> > - if (INTEL_GEN(dev_priv) < 4) {
> > - DRM_DEBUG_KMS("unsupported pixel format:
> %s\n",
> > - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> > - goto err;
> > - }
> > - break;
> > - case DRM_FORMAT_ABGR2101010:
> > - if (!IS_VALLEYVIEW(dev_priv) &&
> !IS_CHERRYVIEW(dev_priv)) {
> > - DRM_DEBUG_KMS("unsupported pixel format:
> %s\n",
> > - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> > - goto err;
> > - }
> > - break;
> > - case DRM_FORMAT_YUYV:
> > - case DRM_FORMAT_UYVY:
> > - case DRM_FORMAT_YVYU:
> > - case DRM_FORMAT_VYUY:
> > - if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) {
> > - DRM_DEBUG_KMS("unsupported pixel format:
> %s\n",
> > - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> > - goto err;
> > - }
> > - break;
> > - case DRM_FORMAT_NV12:
> > - if (INTEL_GEN(dev_priv) < 9 || IS_SKYLAKE(dev_priv) ||
> > - IS_BROXTON(dev_priv)) {
> > - DRM_DEBUG_KMS("unsupported pixel format:
> %s\n",
> > - drm_get_format_name(mode_cmd-
> >pixel_format,
> > - &format_name));
> > - goto err;
> > - }
> > - break;
> > - default:
> > - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> > - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> > - goto err;
> > - }
> > -
> > /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> > if (mode_cmd->offsets[0] != 0)
> > goto err;
> > --
> > 2.14.1
>
> --
> Ville Syrjälä
> Intel
More information about the Intel-gfx
mailing list