| Message ID | 20180517190714.167312-1-frkoenig@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 17, 2018 at 12:07:14PM -0700, Fritz Koenig wrote: > Planes with an odd width will appear to have an incorrect > stride. When the start position is odd the controller > can lock up. Just remove the strange NV12 check from the %2 checks in intel_check_sprite_plane()? > > Signed-off-by: Fritz Koenig <frkoenig@google.com> > --- > > Hi, > > This appears to be a limitation of the hardware that is not being > checked. Is this supported and am I not enabling it correctly? > > > drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index 7481ce85746b..ca4553592ab9 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -188,6 +188,21 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ > else > crtc_state->active_planes &= ~BIT(intel_plane->id); > > + /* > + * NV12 plane is not allowed to start from an odd position or > + * end on an odd position. > + */ > + if (state->fb && (DRM_FORMAT_NV12 == state->fb->format->format)) { > + if ((intel_state->base.src_w >> 16) & 1) { > + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support odd width\n"); > + return -EINVAL; > + } > + if ((intel_state->base.src_x >> 16) & 1) { > + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support odd x pos\n"); > + return -EINVAL; > + } > + } > + > return intel_plane_atomic_calc_changes(old_crtc_state, > &crtc_state->base, > old_plane_state, > -- > 2.17.0.441.gb46fe60e1d-goog > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, May 18, 2018 at 03:25:26PM +0300, Ville Syrjälä wrote: > On Thu, May 17, 2018 at 12:07:14PM -0700, Fritz Koenig wrote: > > Planes with an odd width will appear to have an incorrect > > stride. When the start position is odd the controller > > can lock up. > > Just remove the strange NV12 check from the %2 checks in > intel_check_sprite_plane()? Hmm. Actually that wouldn't help the "primary" plane. I guess we want to put this check into skl_check_nv12_surface() until we have a better place for it, or until someone fixes the initial phase stuff to actually handle this correctly. > > > > > Signed-off-by: Fritz Koenig <frkoenig@google.com> > > --- > > > > Hi, > > > > This appears to be a limitation of the hardware that is not being > > checked. Is this supported and am I not enabling it correctly? > > > > > > drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > > index 7481ce85746b..ca4553592ab9 100644 > > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > > @@ -188,6 +188,21 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ > > else > > crtc_state->active_planes &= ~BIT(intel_plane->id); > > > > + /* > > + * NV12 plane is not allowed to start from an odd position or > > + * end on an odd position. > > + */ > > + if (state->fb && (DRM_FORMAT_NV12 == state->fb->format->format)) { > > + if ((intel_state->base.src_w >> 16) & 1) { > > + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support odd width\n"); > > + return -EINVAL; > > + } > > + if ((intel_state->base.src_x >> 16) & 1) { > > + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support odd x pos\n"); > > + return -EINVAL; > > + } > > + } > > + > > return intel_plane_atomic_calc_changes(old_crtc_state, > > &crtc_state->base, > > old_plane_state, > > -- > > 2.17.0.441.gb46fe60e1d-goog > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
Quoting Fritz Koenig (2018-05-17 20:07:14) > Planes with an odd width will appear to have an incorrect > stride. When the start position is odd the controller > can lock up. > > Signed-off-by: Fritz Koenig <frkoenig@google.com> > --- > > Hi, > > This appears to be a limitation of the hardware that is not being > checked. Is this supported and am I not enabling it correctly? > > > drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index 7481ce85746b..ca4553592ab9 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -188,6 +188,21 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ > else > crtc_state->active_planes &= ~BIT(intel_plane->id); > > + /* > + * NV12 plane is not allowed to start from an odd position or > + * end on an odd position. > + */ > + if (state->fb && (DRM_FORMAT_NV12 == state->fb->format->format)) { > + if ((intel_state->base.src_w >> 16) & 1) { > + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support odd width\n"); It may seem obvious from the message that src_w is odd, but it's always nice to include as much information as possible for debugging; in this case "%x", src_w. if ((intel_state->base.src_w | intel_state->base.src_x) & BIT(16)) ? I'd leave that question of style to Ville though. -Chris
On Fri, May 18, 2018 at 7:15 AM Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, May 18, 2018 at 03:25:26PM +0300, Ville Syrjälä wrote: > > On Thu, May 17, 2018 at 12:07:14PM -0700, Fritz Koenig wrote: > > > Planes with an odd width will appear to have an incorrect > > > stride. When the start position is odd the controller > > > can lock up. > > > > Just remove the strange NV12 check from the %2 checks in > > intel_check_sprite_plane()? > Hmm. Actually that wouldn't help the "primary" plane. I guess we want to > put this check into skl_check_nv12_surface() until we have a better > place for it, or until someone fixes the initial phase stuff to actually > handle this correctly. Ahh, yes, it looks like we are hitting this now because we are using the NV12 plane as the primary plane, which forgoes all the NV12 checks that occur in intel_check_sprite_plane. This patch is probably the wrong place to do it then. > > > > > > > > Signed-off-by: Fritz Koenig <frkoenig@google.com> > > > --- > > > > > > Hi, > > > > > > This appears to be a limitation of the hardware that is not being > > > checked. Is this supported and am I not enabling it correctly? > > > > > > > > > drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > > > index 7481ce85746b..ca4553592ab9 100644 > > > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > > > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > > > @@ -188,6 +188,21 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ > > > else > > > crtc_state->active_planes &= ~BIT(intel_plane->id); > > > > > > + /* > > > + * NV12 plane is not allowed to start from an odd position or > > > + * end on an odd position. > > > + */ > > > + if (state->fb && (DRM_FORMAT_NV12 == state->fb->format->format)) { > > > + if ((intel_state->base.src_w >> 16) & 1) { > > > + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support odd width\n"); > > > + return -EINVAL; > > > + } > > > + if ((intel_state->base.src_x >> 16) & 1) { > > > + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support odd x pos\n"); > > > + return -EINVAL; > > > + } > > > + } > > > + > > > return intel_plane_atomic_calc_changes(old_crtc_state, > > > &crtc_state->base, > > > old_plane_state, > > > -- > > > 2.17.0.441.gb46fe60e1d-goog > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 7481ce85746b..ca4553592ab9 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -188,6 +188,21 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ else crtc_state->active_planes &= ~BIT(intel_plane->id); + /* + * NV12 plane is not allowed to start from an odd position or + * end on an odd position. + */ + if (state->fb && (DRM_FORMAT_NV12 == state->fb->format->format)) { + if ((intel_state->base.src_w >> 16) & 1) { + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support odd width\n"); + return -EINVAL; + } + if ((intel_state->base.src_x >> 16) & 1) { + DRM_DEBUG_KMS("Invalid Source: Yuv format does not support odd x pos\n"); + return -EINVAL; + } + } + return intel_plane_atomic_calc_changes(old_crtc_state, &crtc_state->base, old_plane_state,
Planes with an odd width will appear to have an incorrect stride. When the start position is odd the controller can lock up. Signed-off-by: Fritz Koenig <frkoenig@google.com> --- Hi, This appears to be a limitation of the hardware that is not being checked. Is this supported and am I not enabling it correctly? drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)