Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

Konduru, Chandra
2015-09-30T22:58:08Z

> > @@ -14241,6 +14241,7 @@ static int intel_framebuffer_init(struct
> drm_device *dev,
> > {
> > unsigned int aligned_height;
> > int ret;
> > + int i;
> > u32 pitch_limit, stride_alignment;
> >
> > WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> > @@ -14255,7 +14256,8 @@ static int intel_framebuffer_init(struct
> drm_device *dev,
> > }
> > } else {
> > if (obj->tiling_mode == I915_TILING_X)
> > - mode_cmd->modifier[0] =
> I915_FORMAT_MOD_X_TILED;
> > + for (i = 0; i < drm_format_num_planes(mode_cmd-
> >pixel_format); i++)
> > + mode_cmd->modifier[i] =
> I915_FORMAT_MOD_X_TILED;
>
> The other branch needs updating too so that it will reject the operation
> if the modifier disagrees with the obj tiling mode.

Is below something you meant?

@@ -14223,10 +14223,12 @@ static int intel_framebuffer_init(struct drm_device *d
if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
/* Enforce that fb modifier and tiling mode match, but only for
* X-tiled. This is needed for FBC. */
- if (!!(obj->tiling_mode == I915_TILING_X) !=
- !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) {
- DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
- return -EINVAL;
+ for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i
+ if (!!(obj->tiling_mode == I915_TILING_X) !=
+ !!(mode_cmd->modifier[i] == I915_FORMAT_MOD_X_TILED)) {
+ DRM_DEBUG("tiling_mode doesn't match fb modifier
+ return -EINVAL;
+ }
}
} else {

> > + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED
> &&
> > + (mode_cmd->offsets[1] & 0xFFF)) {
>
> I've been trying to solicit ideas on how we should define the offsets[];
> raw byte offset, or linear offset. I didn't get many opinions yet. So we
> need to figure it out and document it somewhere before we expose it to
> the world. In the meantime we could just reject non tile row aligned
> offsets regardless of the tiling mode.

Above check is simply making sure tile Yf, uv offset starts on a new page.
Is there any issue with above check?