Re: [PATCH 5/6] drm/i915: Add format modifiers for Intel

Hi Ben,

On 26 July 2017 at 19:08, Ben Widawsky ben@bwidawsk.net wrote:

+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 (!(modifier & DRM_FORMAT_MOD_VENDOR_INTEL) &&
  

•       modifier != DRM_FORMAT_MOD_LINEAR)
  

•           return false;
  

The vendor ID isn't shifted yet, so this comparison is wrong. Even if it was shifted, they're sequential rather than bitmask, so NV/QCOM/BROADCOM all match (vendor & INTEL). To fix both of these: - if (!(modifier & DRM_FORMAT_MOD_VENDOR_INTEL) && - modifier != DRM_FORMAT_MOD_LINEAR) + if ((modifier >> 56) != DRM_FORMAT_MOD_VENDOR_INTEL && + modifier != DRM_FORMAT_MOD_LINEAR)

The practical effect is that this knocked out MOD_Y_TILED and MOD_Y_TILED_CCS from the list, since they're the only two Intel modifiers without the low bit set.

Cheers, Daniel