* [PATCH v2] drm/i915: correct the pitch check for NV12 framebuffer
@ 2018-12-12 0:39 Dongseong Hwang
2018-12-12 20:24 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Dongseong Hwang @ 2018-12-12 0:39 UTC (permalink / raw)
To: dri-devel, intel-gfx
Cc: Vidya Srinivas, Juha-Pekka Heikkila, airlied, rodrigo.vivi,
Chandra Konduru
framebuffer for NV12 requires the pitch to the multiplier of 4, instead
of the width. This patch corrects it.
For instance, a 480p video, whose width and pitch are 854 and 896
respectively, is excluded for NV12 plane so far.
Signed-off-by: Dongseong Hwang <dongseong.hwang@intel.com>
Cc: Chandra Konduru <chandra.konduru@intel.com>
Cc: Vidya Srinivas <vidya.srinivas@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 13e5650..8a3de12 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14600,7 +14600,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
if (fb->format->format == DRM_FORMAT_NV12 &&
(fb->width < SKL_MIN_YUV_420_SRC_W ||
fb->height < SKL_MIN_YUV_420_SRC_H ||
- (fb->width % 4) != 0 || (fb->height % 4) != 0)) {
+ (fb->pitches[0] % 4) != 0 || (fb->height % 4) != 0)) {
DRM_DEBUG_KMS("src dimensions not correct for NV12\n");
goto err;
}
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915: correct the pitch check for NV12 framebuffer
2018-12-12 0:39 [PATCH v2] drm/i915: correct the pitch check for NV12 framebuffer Dongseong Hwang
@ 2018-12-12 20:24 ` Ville Syrjälä
2018-12-12 20:33 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2018-12-12 20:24 UTC (permalink / raw)
To: Dongseong Hwang
Cc: Chandra Konduru, Vidya Srinivas, Juha-Pekka Heikkila, airlied,
intel-gfx, dri-devel, rodrigo.vivi
On Tue, Dec 11, 2018 at 04:39:05PM -0800, Dongseong Hwang wrote:
> framebuffer for NV12 requires the pitch to the multiplier of 4, instead
> of the width. This patch corrects it.
>
> For instance, a 480p video, whose width and pitch are 854 and 896
> respectively, is excluded for NV12 plane so far.
>
> Signed-off-by: Dongseong Hwang <dongseong.hwang@intel.com>
> Cc: Chandra Konduru <chandra.konduru@intel.com>
> Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 13e5650..8a3de12 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14600,7 +14600,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> if (fb->format->format == DRM_FORMAT_NV12 &&
> (fb->width < SKL_MIN_YUV_420_SRC_W ||
> fb->height < SKL_MIN_YUV_420_SRC_H ||
> - (fb->width % 4) != 0 || (fb->height % 4) != 0)) {
> + (fb->pitches[0] % 4) != 0 || (fb->height % 4) != 0)) {
The stride can never be misaligned like that. It'll be at least tile
aligned, or 64 byte aligned with linear buffers.
Anyways this entire piece of code doesn't make too much sense. The fb
size doesn't really matter for us, only the src viewport size matters.
That one we limit to a minimum of 2x2 pixels w/o scaling, and 16x16
pixems w/ scaling. So looks like this code can be just ripped out.
> DRM_DEBUG_KMS("src dimensions not correct for NV12\n");
> goto err;
> }
> --
> 2.7.4
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915: correct the pitch check for NV12 framebuffer
2018-12-12 20:24 ` Ville Syrjälä
@ 2018-12-12 20:33 ` Daniel Vetter
2018-12-12 21:15 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2018-12-12 20:33 UTC (permalink / raw)
To: Syrjala, Ville; +Cc: Dave Airlie, intel-gfx, dri-devel, Rodrigo Vivi
On Wed, Dec 12, 2018 at 9:24 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Tue, Dec 11, 2018 at 04:39:05PM -0800, Dongseong Hwang wrote:
> > framebuffer for NV12 requires the pitch to the multiplier of 4, instead
> > of the width. This patch corrects it.
> >
> > For instance, a 480p video, whose width and pitch are 854 and 896
> > respectively, is excluded for NV12 plane so far.
> >
> > Signed-off-by: Dongseong Hwang <dongseong.hwang@intel.com>
> > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 13e5650..8a3de12 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14600,7 +14600,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> > if (fb->format->format == DRM_FORMAT_NV12 &&
> > (fb->width < SKL_MIN_YUV_420_SRC_W ||
> > fb->height < SKL_MIN_YUV_420_SRC_H ||
> > - (fb->width % 4) != 0 || (fb->height % 4) != 0)) {
> > + (fb->pitches[0] % 4) != 0 || (fb->height % 4) != 0)) {
>
> The stride can never be misaligned like that. It'll be at least tile
> aligned, or 64 byte aligned with linear buffers.
>
> Anyways this entire piece of code doesn't make too much sense. The fb
> size doesn't really matter for us, only the src viewport size matters.
> That one we limit to a minimum of 2x2 pixels w/o scaling, and 16x16
> pixems w/ scaling. So looks like this code can be just ripped out.
Do we have igt testcases for these cornercases in igt? Obviously would
need to be intel specific subtests ...
-Daniel
>
> > DRM_DEBUG_KMS("src dimensions not correct for NV12\n");
> > goto err;
> > }
> > --
> > 2.7.4
>
> --
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915: correct the pitch check for NV12 framebuffer
2018-12-12 20:33 ` Daniel Vetter
@ 2018-12-12 21:15 ` Ville Syrjälä
2018-12-12 21:18 ` [Intel-gfx] " Singaiah Chintalapudi
2018-12-13 0:51 ` Hwang, Dongseong
0 siblings, 2 replies; 6+ messages in thread
From: Ville Syrjälä @ 2018-12-12 21:15 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Dave Airlie, intel-gfx, dri-devel, Rodrigo Vivi
On Wed, Dec 12, 2018 at 09:33:49PM +0100, Daniel Vetter wrote:
> On Wed, Dec 12, 2018 at 9:24 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Tue, Dec 11, 2018 at 04:39:05PM -0800, Dongseong Hwang wrote:
> > > framebuffer for NV12 requires the pitch to the multiplier of 4, instead
> > > of the width. This patch corrects it.
> > >
> > > For instance, a 480p video, whose width and pitch are 854 and 896
> > > respectively, is excluded for NV12 plane so far.
> > >
> > > Signed-off-by: Dongseong Hwang <dongseong.hwang@intel.com>
> > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 13e5650..8a3de12 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -14600,7 +14600,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> > > if (fb->format->format == DRM_FORMAT_NV12 &&
> > > (fb->width < SKL_MIN_YUV_420_SRC_W ||
> > > fb->height < SKL_MIN_YUV_420_SRC_H ||
> > > - (fb->width % 4) != 0 || (fb->height % 4) != 0)) {
> > > + (fb->pitches[0] % 4) != 0 || (fb->height % 4) != 0)) {
> >
> > The stride can never be misaligned like that. It'll be at least tile
> > aligned, or 64 byte aligned with linear buffers.
> >
> > Anyways this entire piece of code doesn't make too much sense. The fb
> > size doesn't really matter for us, only the src viewport size matters.
> > That one we limit to a minimum of 2x2 pixels w/o scaling, and 16x16
> > pixems w/ scaling. So looks like this code can be just ripped out.
>
> Do we have igt testcases for these cornercases in igt? Obviously would
> need to be intel specific subtests ...
Can't spot anything quite that specific. Someone would need to write
one I suppose. Also Imre has a test somewhere on the list for testing
the plane clipping underrun fails which tests small src viewport sizes,
and JP has been working on a rotation vs. clipping test that is also
somewhat related. Not sure if we could combine any of these somehow
to avoid having too many similar tests.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: correct the pitch check for NV12 framebuffer
2018-12-12 21:15 ` Ville Syrjälä
@ 2018-12-12 21:18 ` Singaiah Chintalapudi
2018-12-13 0:51 ` Hwang, Dongseong
1 sibling, 0 replies; 6+ messages in thread
From: Singaiah Chintalapudi @ 2018-12-12 21:18 UTC (permalink / raw)
To: Ville Syrjälä, Daniel Vetter
Cc: Dave Airlie, intel-gfx, dri-devel, Rodrigo Vivi
Hello,
I am not sure why I am receiving these emails. Please remove me.
Thanks.
On 12/12/18, 3:15 PM, "dri-devel on behalf of Ville Syrjälä" <dri-devel-bounces@lists.freedesktop.org on behalf of ville.syrjala@linux.intel.com> wrote:
On Wed, Dec 12, 2018 at 09:33:49PM +0100, Daniel Vetter wrote:
> On Wed, Dec 12, 2018 at 9:24 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Tue, Dec 11, 2018 at 04:39:05PM -0800, Dongseong Hwang wrote:
> > > framebuffer for NV12 requires the pitch to the multiplier of 4, instead
> > > of the width. This patch corrects it.
> > >
> > > For instance, a 480p video, whose width and pitch are 854 and 896
> > > respectively, is excluded for NV12 plane so far.
> > >
> > > Signed-off-by: Dongseong Hwang <dongseong.hwang@intel.com>
> > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 13e5650..8a3de12 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -14600,7 +14600,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> > > if (fb->format->format == DRM_FORMAT_NV12 &&
> > > (fb->width < SKL_MIN_YUV_420_SRC_W ||
> > > fb->height < SKL_MIN_YUV_420_SRC_H ||
> > > - (fb->width % 4) != 0 || (fb->height % 4) != 0)) {
> > > + (fb->pitches[0] % 4) != 0 || (fb->height % 4) != 0)) {
> >
> > The stride can never be misaligned like that. It'll be at least tile
> > aligned, or 64 byte aligned with linear buffers.
> >
> > Anyways this entire piece of code doesn't make too much sense. The fb
> > size doesn't really matter for us, only the src viewport size matters.
> > That one we limit to a minimum of 2x2 pixels w/o scaling, and 16x16
> > pixems w/ scaling. So looks like this code can be just ripped out.
>
> Do we have igt testcases for these cornercases in igt? Obviously would
> need to be intel specific subtests ...
Can't spot anything quite that specific. Someone would need to write
one I suppose. Also Imre has a test somewhere on the list for testing
the plane clipping underrun fails which tests small src viewport sizes,
and JP has been working on a rotation vs. clipping test that is also
somewhat related. Not sure if we could combine any of these somehow
to avoid having too many similar tests.
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=DwIGaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=AqbVQX0MMoes-vp18k-9nce8K2Yg4CXmWwlCoNEEl4k&m=kffYCLL5-FFzZkD5uXpdCQgXMa3WtXfkAByKQlZ0ovM&s=gEd_OrHeidWz5ViCEWQMKjVQOHqxVUhrfz0RsNrOZOc&e=
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915: correct the pitch check for NV12 framebuffer
2018-12-12 21:15 ` Ville Syrjälä
2018-12-12 21:18 ` [Intel-gfx] " Singaiah Chintalapudi
@ 2018-12-13 0:51 ` Hwang, Dongseong
1 sibling, 0 replies; 6+ messages in thread
From: Hwang, Dongseong @ 2018-12-13 0:51 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: airlied, intel-gfx, ML dri-devel, Vivi, Rodrigo
On Wed, Dec 12, 2018 at 1:15 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Wed, Dec 12, 2018 at 09:33:49PM +0100, Daniel Vetter wrote:
> > On Wed, Dec 12, 2018 at 9:24 PM Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > On Tue, Dec 11, 2018 at 04:39:05PM -0800, Dongseong Hwang wrote:
> > > > framebuffer for NV12 requires the pitch to the multiplier of 4, instead
> > > > of the width. This patch corrects it.
> > > >
> > > > For instance, a 480p video, whose width and pitch are 854 and 896
> > > > respectively, is excluded for NV12 plane so far.
> > > >
> > > > Signed-off-by: Dongseong Hwang <dongseong.hwang@intel.com>
> > > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > > Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index 13e5650..8a3de12 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -14600,7 +14600,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> > > > if (fb->format->format == DRM_FORMAT_NV12 &&
> > > > (fb->width < SKL_MIN_YUV_420_SRC_W ||
> > > > fb->height < SKL_MIN_YUV_420_SRC_H ||
> > > > - (fb->width % 4) != 0 || (fb->height % 4) != 0)) {
> > > > + (fb->pitches[0] % 4) != 0 || (fb->height % 4) != 0)) {
> > >
> > > The stride can never be misaligned like that. It'll be at least tile
> > > aligned, or 64 byte aligned with linear buffers.
> > >
> > > Anyways this entire piece of code doesn't make too much sense. The fb
> > > size doesn't really matter for us, only the src viewport size matters.
> > > That one we limit to a minimum of 2x2 pixels w/o scaling, and 16x16
> > > pixems w/ scaling. So looks like this code can be just ripped out.
> >
> > Do we have igt testcases for these cornercases in igt? Obviously would
> > need to be intel specific subtests ...
>
> Can't spot anything quite that specific. Someone would need to write
> one I suppose. Also Imre has a test somewhere on the list for testing
> the plane clipping underrun fails which tests small src viewport sizes,
> and JP has been working on a rotation vs. clipping test that is also
> somewhat related. Not sure if we could combine any of these somehow
> to avoid having too many similar tests.
Ap pointed out there is i915 workaround
1106 Display NV12, Rotation, Horizontal flip
Display corruption/color shift observed when using NV12 with 270
rotation or 90 rotation + horizontal flip.
WA: NV12 with 270 rotation or 90 rotation + horizontal flip requires
the programmed plane height to be a multiple of 4.
GEN9:ALL
GLK:ALL
GLV:ALL
CNL:*:A
CNL:*:B
I think this condition was introduced to deal with the workaround, and
I think the stride restriction is enough for the above workaround.
Ville, if I add the igt covering this change, is it good to land?
Best regards,
Dongseong
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-13 0:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-12 0:39 [PATCH v2] drm/i915: correct the pitch check for NV12 framebuffer Dongseong Hwang
2018-12-12 20:24 ` Ville Syrjälä
2018-12-12 20:33 ` Daniel Vetter
2018-12-12 21:15 ` Ville Syrjälä
2018-12-12 21:18 ` [Intel-gfx] " Singaiah Chintalapudi
2018-12-13 0:51 ` Hwang, Dongseong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox