Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-10-16 Thread Tvrtko Ursulin 


On 08/10/15 09:58, Tvrtko Ursulin wrote:

On 07/10/15 15:19, Daniel Vetter wrote:

On Tue, Oct 06, 2015 at 07:28:10PM +0300, Ville Syrjälä wrote:

On Tue, Oct 06, 2015 at 08:16:19AM -0700, Matt Roper wrote:

On Tue, Oct 06, 2015 at 05:42:42PM +0300, Ville Syrjälä wrote:

On Tue, Oct 06, 2015 at 07:29:54AM -0700, Matt Roper wrote:

On Tue, Oct 06, 2015 at 02:32:47PM +0100, Tvrtko Ursulin wrote:


On 10/04/15 10:07, Sonika Jindal wrote:

Signed-off-by: Sonika Jindal 
Reviewed-by: Matt Roper 
---
  drivers/gpu/drm/i915/intel_display.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index ceb2e61..f0bbc22 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct
drm_plane *plane,
  struct drm_rect *dest = >dst;
  struct drm_rect *src = >src;
  const struct drm_rect *clip = >clip;
+bool can_position = false;
  int ret;

  crtc = crtc ? crtc : plane->crtc;
  intel_crtc = to_intel_crtc(crtc);

+if (INTEL_INFO(dev)->gen >= 9)
+can_position = true;
+
  ret = drm_plane_helper_check_update(plane, crtc, fb,
  src, dest, clip,
  DRM_PLANE_HELPER_NO_SCALING,
  DRM_PLANE_HELPER_NO_SCALING,
-false, true, >visible);
+can_position, true,
+>visible);
  if (ret)
  return ret;




I have discovered today that, while this allows SetCrtc and SetPlane
ioctls to work with frame buffers which do not cover the plane, page
flips are not that lucky and fail roughly with:

[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC
viewport 1920x1080+0+0.


Maybe I'm misunderstanding your explanation, but a framebuffer is
always
required to fill/cover the plane scanning out of it.  What this
patch is
supposed to be allowing is for the primary plane to not cover the
entire
CRTC (since that's something that only became possible for Intel
hardware on the gen9+ platforms).  I.e., the primary plane is now
allowed to positioned and resized to cover a subset of the CRTC area,
just like "sprite" planes have always been able to.

If you've got a 1080x1080 framebuffer, then it's legal to have a
1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT.
However it is not legal to size the primary plane as 1920x1080 and
use
this same 1080x1080 framebuffer with any of our interfaces (setplane,
setcrtc, pageflip, or atomic).

Are you using ioctls/libdrm directly or are you using igt_kms
helpers?
IIRC, the IGT helpers will try to be extra helpful and automatically
size the plane to match the framebuffer (unless you override that
behavior), so that might be what's causing the confusion here.


The problem is clear as day in drm_mode_page_flip_ioctl():
ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, >mode,
fb);
if (ret)
goto out;

The fix should be easy; just extract the current src coordinates from
the plane state and check those against the new fb size. And then hope
that the plane state is really up to date.


Yep, that's the conclusion we came to once Tvrtko explained what he was
seeing on IRC.  I'm not sure whether non-atomic drivers have enough
state setup by the default helpers to work properly.  Worst case we'll
just assume that a non-atomic driver won't support primary plane
windowing (since none have in the past) and fall back to looking at the
mode for legacy non-atomic drivers.



And I'm sure rotated cases will go boom in some other ways. Probably
we should just switch over to using the full plane update for mmio
flips to fix it.


Yeah; the core looks at a drm_plane->invert_dimensions field that's
only
set by omap.  That should probably be updated to look at the state's
rotation on atomic-capable drivers.


We can just look at the src coordinates. Those always match the fb
orientation.


Can we just not bother with legacy pageflips on rotated planes? setplane
works and once you rotate it kinda gets nasty anyway.


I don't know - thought it is simple enough to make it work so why not?
Just " [PATCH] drm/i915: Consider plane rotation when calculating stride
in skl_do_mmio_flip" I posted, plus Matt's "[PATCH] drm: Check fb
against plane size rather than CRTC mode for pageflip​" to allow smaller
than mode planes.


The problem I see is that with legacy pageflip we also need to hack up
something that doesn't look at plane->state for legacy and all that for a
grand total of about 2 drivers, both getting converted to atomic.


I'll leave the legacy/atomic/etc considerations to the experts. :)


Are we sure any efforts to support rotation in legacy page flips is not 
worth it?


So far there were three patches for this: Plane programming fix (very 
simple) and an IGT test case

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-10-08 Thread Tvrtko Ursulin 


On 07/10/15 15:19, Daniel Vetter wrote:

On Tue, Oct 06, 2015 at 07:28:10PM +0300, Ville Syrjälä wrote:

On Tue, Oct 06, 2015 at 08:16:19AM -0700, Matt Roper wrote:

On Tue, Oct 06, 2015 at 05:42:42PM +0300, Ville Syrjälä wrote:

On Tue, Oct 06, 2015 at 07:29:54AM -0700, Matt Roper wrote:

On Tue, Oct 06, 2015 at 02:32:47PM +0100, Tvrtko Ursulin wrote:


On 10/04/15 10:07, Sonika Jindal wrote:

Signed-off-by: Sonika Jindal 
Reviewed-by: Matt Roper 
---
  drivers/gpu/drm/i915/intel_display.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ceb2e61..f0bbc22 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,
struct drm_rect *dest = >dst;
struct drm_rect *src = >src;
const struct drm_rect *clip = >clip;
+   bool can_position = false;
int ret;

crtc = crtc ? crtc : plane->crtc;
intel_crtc = to_intel_crtc(crtc);

+   if (INTEL_INFO(dev)->gen >= 9)
+   can_position = true;
+
ret = drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
-   false, true, >visible);
+   can_position, true,
+   >visible);
if (ret)
return ret;




I have discovered today that, while this allows SetCrtc and SetPlane
ioctls to work with frame buffers which do not cover the plane, page
flips are not that lucky and fail roughly with:

[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC
viewport 1920x1080+0+0.


Maybe I'm misunderstanding your explanation, but a framebuffer is always
required to fill/cover the plane scanning out of it.  What this patch is
supposed to be allowing is for the primary plane to not cover the entire
CRTC (since that's something that only became possible for Intel
hardware on the gen9+ platforms).  I.e., the primary plane is now
allowed to positioned and resized to cover a subset of the CRTC area,
just like "sprite" planes have always been able to.

If you've got a 1080x1080 framebuffer, then it's legal to have a
1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT.
However it is not legal to size the primary plane as 1920x1080 and use
this same 1080x1080 framebuffer with any of our interfaces (setplane,
setcrtc, pageflip, or atomic).

Are you using ioctls/libdrm directly or are you using igt_kms helpers?
IIRC, the IGT helpers will try to be extra helpful and automatically
size the plane to match the framebuffer (unless you override that
behavior), so that might be what's causing the confusion here.


The problem is clear as day in drm_mode_page_flip_ioctl():
ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, >mode, fb);
if (ret)
goto out;

The fix should be easy; just extract the current src coordinates from
the plane state and check those against the new fb size. And then hope
that the plane state is really up to date.


Yep, that's the conclusion we came to once Tvrtko explained what he was
seeing on IRC.  I'm not sure whether non-atomic drivers have enough
state setup by the default helpers to work properly.  Worst case we'll
just assume that a non-atomic driver won't support primary plane
windowing (since none have in the past) and fall back to looking at the
mode for legacy non-atomic drivers.



And I'm sure rotated cases will go boom in some other ways. Probably
we should just switch over to using the full plane update for mmio
flips to fix it.


Yeah; the core looks at a drm_plane->invert_dimensions field that's only
set by omap.  That should probably be updated to look at the state's
rotation on atomic-capable drivers.


We can just look at the src coordinates. Those always match the fb
orientation.


Can we just not bother with legacy pageflips on rotated planes? setplane
works and once you rotate it kinda gets nasty anyway.


I don't know - thought it is simple enough to make it work so why not? 
Just " [PATCH] drm/i915: Consider plane rotation when calculating stride 
in skl_do_mmio_flip" I posted, plus Matt's "[PATCH] drm: Check fb 
against plane size rather than CRTC mode for pageflip​" to allow smaller 
than mode planes.



The problem I see is that with legacy pageflip we also need to hack up
something that doesn't look at plane->state for legacy and all that for a
grand total of about 2 drivers, both getting converted to atomic.


I'll leave the legacy/atomic/etc considerations to the experts. :)

Regards,

Tvrtko



___

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-10-07 Thread Daniel Vetter 
On Tue, Oct 06, 2015 at 07:28:10PM +0300, Ville Syrjälä wrote:
> On Tue, Oct 06, 2015 at 08:16:19AM -0700, Matt Roper wrote:
> > On Tue, Oct 06, 2015 at 05:42:42PM +0300, Ville Syrjälä wrote:
> > > On Tue, Oct 06, 2015 at 07:29:54AM -0700, Matt Roper wrote:
> > > > On Tue, Oct 06, 2015 at 02:32:47PM +0100, Tvrtko Ursulin wrote:
> > > > > 
> > > > > On 10/04/15 10:07, Sonika Jindal wrote:
> > > > > >Signed-off-by: Sonika Jindal 
> > > > > >Reviewed-by: Matt Roper 
> > > > > >---
> > > > > >  drivers/gpu/drm/i915/intel_display.c |7 ++-
> > > > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > > >
> > > > > >diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > > >b/drivers/gpu/drm/i915/intel_display.c
> > > > > >index ceb2e61..f0bbc22 100644
> > > > > >--- a/drivers/gpu/drm/i915/intel_display.c
> > > > > >+++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > >@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane 
> > > > > >*plane,
> > > > > > struct drm_rect *dest = >dst;
> > > > > > struct drm_rect *src = >src;
> > > > > > const struct drm_rect *clip = >clip;
> > > > > >+bool can_position = false;
> > > > > > int ret;
> > > > > >
> > > > > > crtc = crtc ? crtc : plane->crtc;
> > > > > > intel_crtc = to_intel_crtc(crtc);
> > > > > >
> > > > > >+if (INTEL_INFO(dev)->gen >= 9)
> > > > > >+can_position = true;
> > > > > >+
> > > > > > ret = drm_plane_helper_check_update(plane, crtc, fb,
> > > > > > src, dest, clip,
> > > > > > DRM_PLANE_HELPER_NO_SCALING,
> > > > > > DRM_PLANE_HELPER_NO_SCALING,
> > > > > >-false, true, 
> > > > > >>visible);
> > > > > >+can_position, true,
> > > > > >+>visible);
> > > > > > if (ret)
> > > > > > return ret;
> > > > > >
> > > > > >
> > > > > 
> > > > > I have discovered today that, while this allows SetCrtc and SetPlane
> > > > > ioctls to work with frame buffers which do not cover the plane, page
> > > > > flips are not that lucky and fail roughly with:
> > > > > 
> > > > > [drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC
> > > > > viewport 1920x1080+0+0.
> > > > 
> > > > Maybe I'm misunderstanding your explanation, but a framebuffer is always
> > > > required to fill/cover the plane scanning out of it.  What this patch is
> > > > supposed to be allowing is for the primary plane to not cover the entire
> > > > CRTC (since that's something that only became possible for Intel
> > > > hardware on the gen9+ platforms).  I.e., the primary plane is now
> > > > allowed to positioned and resized to cover a subset of the CRTC area,
> > > > just like "sprite" planes have always been able to.
> > > > 
> > > > If you've got a 1080x1080 framebuffer, then it's legal to have a
> > > > 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT.
> > > > However it is not legal to size the primary plane as 1920x1080 and use
> > > > this same 1080x1080 framebuffer with any of our interfaces (setplane,
> > > > setcrtc, pageflip, or atomic).
> > > > 
> > > > Are you using ioctls/libdrm directly or are you using igt_kms helpers?
> > > > IIRC, the IGT helpers will try to be extra helpful and automatically
> > > > size the plane to match the framebuffer (unless you override that
> > > > behavior), so that might be what's causing the confusion here.
> > > 
> > > The problem is clear as day in drm_mode_page_flip_ioctl():
> > > ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, >mode, fb);
> > > if (ret)
> > >   goto out;
> > > 
> > > The fix should be easy; just extract the current src coordinates from
> > > the plane state and check those against the new fb size. And then hope
> > > that the plane state is really up to date.
> > 
> > Yep, that's the conclusion we came to once Tvrtko explained what he was
> > seeing on IRC.  I'm not sure whether non-atomic drivers have enough
> > state setup by the default helpers to work properly.  Worst case we'll
> > just assume that a non-atomic driver won't support primary plane
> > windowing (since none have in the past) and fall back to looking at the
> > mode for legacy non-atomic drivers.
> > 
> > > 
> > > And I'm sure rotated cases will go boom in some other ways. Probably
> > > we should just switch over to using the full plane update for mmio
> > > flips to fix it.
> > 
> > Yeah; the core looks at a drm_plane->invert_dimensions field that's only
> > set by omap.  That should probably be updated to look at the state's
> > rotation on atomic-capable drivers.
> 
> We can just look at the src coordinates. Those always match the fb
> orientation.

Can we just not bother with legacy pageflips on rotated planes? setplane
works and once you rotate it

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-10-06 Thread Matt Roper 
On Tue, Oct 06, 2015 at 02:32:47PM +0100, Tvrtko Ursulin wrote:
> 
> On 10/04/15 10:07, Sonika Jindal wrote:
> >Signed-off-by: Sonika Jindal 
> >Reviewed-by: Matt Roper 
> >---
> >  drivers/gpu/drm/i915/intel_display.c |7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c 
> >b/drivers/gpu/drm/i915/intel_display.c
> >index ceb2e61..f0bbc22 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,
> > struct drm_rect *dest = >dst;
> > struct drm_rect *src = >src;
> > const struct drm_rect *clip = >clip;
> >+bool can_position = false;
> > int ret;
> >
> > crtc = crtc ? crtc : plane->crtc;
> > intel_crtc = to_intel_crtc(crtc);
> >
> >+if (INTEL_INFO(dev)->gen >= 9)
> >+can_position = true;
> >+
> > ret = drm_plane_helper_check_update(plane, crtc, fb,
> > src, dest, clip,
> > DRM_PLANE_HELPER_NO_SCALING,
> > DRM_PLANE_HELPER_NO_SCALING,
> >-false, true, >visible);
> >+can_position, true,
> >+>visible);
> > if (ret)
> > return ret;
> >
> >
> 
> I have discovered today that, while this allows SetCrtc and SetPlane
> ioctls to work with frame buffers which do not cover the plane, page
> flips are not that lucky and fail roughly with:
> 
> [drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC
> viewport 1920x1080+0+0.

Maybe I'm misunderstanding your explanation, but a framebuffer is always
required to fill/cover the plane scanning out of it.  What this patch is
supposed to be allowing is for the primary plane to not cover the entire
CRTC (since that's something that only became possible for Intel
hardware on the gen9+ platforms).  I.e., the primary plane is now
allowed to positioned and resized to cover a subset of the CRTC area,
just like "sprite" planes have always been able to.

If you've got a 1080x1080 framebuffer, then it's legal to have a
1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT.
However it is not legal to size the primary plane as 1920x1080 and use
this same 1080x1080 framebuffer with any of our interfaces (setplane,
setcrtc, pageflip, or atomic).

Are you using ioctls/libdrm directly or are you using igt_kms helpers?
IIRC, the IGT helpers will try to be extra helpful and automatically
size the plane to match the framebuffer (unless you override that
behavior), so that might be what's causing the confusion here.


Matt

> 
> I have posted a quick IGT exerciser for this as "kms_rotation_crc:
> Excercise page flips with 90 degree rotation". May not be that great
> but shows the failure.
> 
> I am not that hot on meddling with this code, nor do I feel
> competent to even try on my own at least. :/ Maybe just because the
> atomic and plane related rewrites have been going on for so long,
> and have multiple people involved, it all sounds pretty scary and
> fragile.
> 
> But I think some sort of plan on how to fix this could be in order?
> 
> Regards,
> 
> Tvrtko

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-10-06 Thread Ville Syrjälä 
On Tue, Oct 06, 2015 at 07:29:54AM -0700, Matt Roper wrote:
> On Tue, Oct 06, 2015 at 02:32:47PM +0100, Tvrtko Ursulin wrote:
> > 
> > On 10/04/15 10:07, Sonika Jindal wrote:
> > >Signed-off-by: Sonika Jindal 
> > >Reviewed-by: Matt Roper 
> > >---
> > >  drivers/gpu/drm/i915/intel_display.c |7 ++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > >b/drivers/gpu/drm/i915/intel_display.c
> > >index ceb2e61..f0bbc22 100644
> > >--- a/drivers/gpu/drm/i915/intel_display.c
> > >+++ b/drivers/gpu/drm/i915/intel_display.c
> > >@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane 
> > >*plane,
> > >   struct drm_rect *dest = >dst;
> > >   struct drm_rect *src = >src;
> > >   const struct drm_rect *clip = >clip;
> > >+  bool can_position = false;
> > >   int ret;
> > >
> > >   crtc = crtc ? crtc : plane->crtc;
> > >   intel_crtc = to_intel_crtc(crtc);
> > >
> > >+  if (INTEL_INFO(dev)->gen >= 9)
> > >+  can_position = true;
> > >+
> > >   ret = drm_plane_helper_check_update(plane, crtc, fb,
> > >   src, dest, clip,
> > >   DRM_PLANE_HELPER_NO_SCALING,
> > >   DRM_PLANE_HELPER_NO_SCALING,
> > >-  false, true, >visible);
> > >+  can_position, true,
> > >+  >visible);
> > >   if (ret)
> > >   return ret;
> > >
> > >
> > 
> > I have discovered today that, while this allows SetCrtc and SetPlane
> > ioctls to work with frame buffers which do not cover the plane, page
> > flips are not that lucky and fail roughly with:
> > 
> > [drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC
> > viewport 1920x1080+0+0.
> 
> Maybe I'm misunderstanding your explanation, but a framebuffer is always
> required to fill/cover the plane scanning out of it.  What this patch is
> supposed to be allowing is for the primary plane to not cover the entire
> CRTC (since that's something that only became possible for Intel
> hardware on the gen9+ platforms).  I.e., the primary plane is now
> allowed to positioned and resized to cover a subset of the CRTC area,
> just like "sprite" planes have always been able to.
> 
> If you've got a 1080x1080 framebuffer, then it's legal to have a
> 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT.
> However it is not legal to size the primary plane as 1920x1080 and use
> this same 1080x1080 framebuffer with any of our interfaces (setplane,
> setcrtc, pageflip, or atomic).
> 
> Are you using ioctls/libdrm directly or are you using igt_kms helpers?
> IIRC, the IGT helpers will try to be extra helpful and automatically
> size the plane to match the framebuffer (unless you override that
> behavior), so that might be what's causing the confusion here.

The problem is clear as day in drm_mode_page_flip_ioctl():
ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, >mode, fb);
if (ret)
goto out;

The fix should be easy; just extract the current src coordinates from
the plane state and check those against the new fb size. And then hope
that the plane state is really up to date.

And I'm sure rotated cases will go boom in some other ways. Probably
we should just switch over to using the full plane update for mmio
flips to fix it.

> 
> 
> Matt
> 
> > 
> > I have posted a quick IGT exerciser for this as "kms_rotation_crc:
> > Excercise page flips with 90 degree rotation". May not be that great
> > but shows the failure.
> > 
> > I am not that hot on meddling with this code, nor do I feel
> > competent to even try on my own at least. :/ Maybe just because the
> > atomic and plane related rewrites have been going on for so long,
> > and have multiple people involved, it all sounds pretty scary and
> > fragile.
> > 
> > But I think some sort of plan on how to fix this could be in order?
> > 
> > Regards,
> > 
> > Tvrtko
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-10-06 Thread Matt Roper 
On Tue, Oct 06, 2015 at 05:42:42PM +0300, Ville Syrjälä wrote:
> On Tue, Oct 06, 2015 at 07:29:54AM -0700, Matt Roper wrote:
> > On Tue, Oct 06, 2015 at 02:32:47PM +0100, Tvrtko Ursulin wrote:
> > > 
> > > On 10/04/15 10:07, Sonika Jindal wrote:
> > > >Signed-off-by: Sonika Jindal 
> > > >Reviewed-by: Matt Roper 
> > > >---
> > > >  drivers/gpu/drm/i915/intel_display.c |7 ++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > >diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > >b/drivers/gpu/drm/i915/intel_display.c
> > > >index ceb2e61..f0bbc22 100644
> > > >--- a/drivers/gpu/drm/i915/intel_display.c
> > > >+++ b/drivers/gpu/drm/i915/intel_display.c
> > > >@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane 
> > > >*plane,
> > > > struct drm_rect *dest = >dst;
> > > > struct drm_rect *src = >src;
> > > > const struct drm_rect *clip = >clip;
> > > >+bool can_position = false;
> > > > int ret;
> > > >
> > > > crtc = crtc ? crtc : plane->crtc;
> > > > intel_crtc = to_intel_crtc(crtc);
> > > >
> > > >+if (INTEL_INFO(dev)->gen >= 9)
> > > >+can_position = true;
> > > >+
> > > > ret = drm_plane_helper_check_update(plane, crtc, fb,
> > > > src, dest, clip,
> > > > DRM_PLANE_HELPER_NO_SCALING,
> > > > DRM_PLANE_HELPER_NO_SCALING,
> > > >-false, true, 
> > > >>visible);
> > > >+can_position, true,
> > > >+>visible);
> > > > if (ret)
> > > > return ret;
> > > >
> > > >
> > > 
> > > I have discovered today that, while this allows SetCrtc and SetPlane
> > > ioctls to work with frame buffers which do not cover the plane, page
> > > flips are not that lucky and fail roughly with:
> > > 
> > > [drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC
> > > viewport 1920x1080+0+0.
> > 
> > Maybe I'm misunderstanding your explanation, but a framebuffer is always
> > required to fill/cover the plane scanning out of it.  What this patch is
> > supposed to be allowing is for the primary plane to not cover the entire
> > CRTC (since that's something that only became possible for Intel
> > hardware on the gen9+ platforms).  I.e., the primary plane is now
> > allowed to positioned and resized to cover a subset of the CRTC area,
> > just like "sprite" planes have always been able to.
> > 
> > If you've got a 1080x1080 framebuffer, then it's legal to have a
> > 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT.
> > However it is not legal to size the primary plane as 1920x1080 and use
> > this same 1080x1080 framebuffer with any of our interfaces (setplane,
> > setcrtc, pageflip, or atomic).
> > 
> > Are you using ioctls/libdrm directly or are you using igt_kms helpers?
> > IIRC, the IGT helpers will try to be extra helpful and automatically
> > size the plane to match the framebuffer (unless you override that
> > behavior), so that might be what's causing the confusion here.
> 
> The problem is clear as day in drm_mode_page_flip_ioctl():
> ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, >mode, fb);
> if (ret)
>   goto out;
> 
> The fix should be easy; just extract the current src coordinates from
> the plane state and check those against the new fb size. And then hope
> that the plane state is really up to date.

Yep, that's the conclusion we came to once Tvrtko explained what he was
seeing on IRC.  I'm not sure whether non-atomic drivers have enough
state setup by the default helpers to work properly.  Worst case we'll
just assume that a non-atomic driver won't support primary plane
windowing (since none have in the past) and fall back to looking at the
mode for legacy non-atomic drivers.

> 
> And I'm sure rotated cases will go boom in some other ways. Probably
> we should just switch over to using the full plane update for mmio
> flips to fix it.

Yeah; the core looks at a drm_plane->invert_dimensions field that's only
set by omap.  That should probably be updated to look at the state's
rotation on atomic-capable drivers.


Matt

> 
> > 
> > 
> > Matt
> > 
> > > 
> > > I have posted a quick IGT exerciser for this as "kms_rotation_crc:
> > > Excercise page flips with 90 degree rotation". May not be that great
> > > but shows the failure.
> > > 
> > > I am not that hot on meddling with this code, nor do I feel
> > > competent to even try on my own at least. :/ Maybe just because the
> > > atomic and plane related rewrites have been going on for so long,
> > > and have multiple people involved, it all sounds pretty scary and
> > > fragile.
> > > 
> > > But I think some sort of plan on how to fix this could be in order?
> > >

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-10-06 Thread Tvrtko Ursulin 


On 10/04/15 10:07, Sonika Jindal wrote:

Signed-off-by: Sonika Jindal 
Reviewed-by: Matt Roper 
---
  drivers/gpu/drm/i915/intel_display.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ceb2e61..f0bbc22 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,
struct drm_rect *dest = >dst;
struct drm_rect *src = >src;
const struct drm_rect *clip = >clip;
+   bool can_position = false;
int ret;

crtc = crtc ? crtc : plane->crtc;
intel_crtc = to_intel_crtc(crtc);

+   if (INTEL_INFO(dev)->gen >= 9)
+   can_position = true;
+
ret = drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
-   false, true, >visible);
+   can_position, true,
+   >visible);
if (ret)
return ret;




I have discovered today that, while this allows SetCrtc and SetPlane 
ioctls to work with frame buffers which do not cover the plane, page 
flips are not that lucky and fail roughly with:


[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC 
viewport 1920x1080+0+0.


I have posted a quick IGT exerciser for this as "kms_rotation_crc: 
Excercise page flips with 90 degree rotation". May not be that great but 
shows the failure.


I am not that hot on meddling with this code, nor do I feel competent to 
even try on my own at least. :/ Maybe just because the atomic and plane 
related rewrites have been going on for so long, and have multiple 
people involved, it all sounds pretty scary and fragile.


But I think some sort of plan on how to fix this could be in order?

Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-10-06 Thread Ville Syrjälä 
On Tue, Oct 06, 2015 at 08:16:19AM -0700, Matt Roper wrote:
> On Tue, Oct 06, 2015 at 05:42:42PM +0300, Ville Syrjälä wrote:
> > On Tue, Oct 06, 2015 at 07:29:54AM -0700, Matt Roper wrote:
> > > On Tue, Oct 06, 2015 at 02:32:47PM +0100, Tvrtko Ursulin wrote:
> > > > 
> > > > On 10/04/15 10:07, Sonika Jindal wrote:
> > > > >Signed-off-by: Sonika Jindal 
> > > > >Reviewed-by: Matt Roper 
> > > > >---
> > > > >  drivers/gpu/drm/i915/intel_display.c |7 ++-
> > > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > >
> > > > >diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > >b/drivers/gpu/drm/i915/intel_display.c
> > > > >index ceb2e61..f0bbc22 100644
> > > > >--- a/drivers/gpu/drm/i915/intel_display.c
> > > > >+++ b/drivers/gpu/drm/i915/intel_display.c
> > > > >@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane 
> > > > >*plane,
> > > > >   struct drm_rect *dest = >dst;
> > > > >   struct drm_rect *src = >src;
> > > > >   const struct drm_rect *clip = >clip;
> > > > >+  bool can_position = false;
> > > > >   int ret;
> > > > >
> > > > >   crtc = crtc ? crtc : plane->crtc;
> > > > >   intel_crtc = to_intel_crtc(crtc);
> > > > >
> > > > >+  if (INTEL_INFO(dev)->gen >= 9)
> > > > >+  can_position = true;
> > > > >+
> > > > >   ret = drm_plane_helper_check_update(plane, crtc, fb,
> > > > >   src, dest, clip,
> > > > >   DRM_PLANE_HELPER_NO_SCALING,
> > > > >   DRM_PLANE_HELPER_NO_SCALING,
> > > > >-  false, true, 
> > > > >>visible);
> > > > >+  can_position, true,
> > > > >+  >visible);
> > > > >   if (ret)
> > > > >   return ret;
> > > > >
> > > > >
> > > > 
> > > > I have discovered today that, while this allows SetCrtc and SetPlane
> > > > ioctls to work with frame buffers which do not cover the plane, page
> > > > flips are not that lucky and fail roughly with:
> > > > 
> > > > [drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC
> > > > viewport 1920x1080+0+0.
> > > 
> > > Maybe I'm misunderstanding your explanation, but a framebuffer is always
> > > required to fill/cover the plane scanning out of it.  What this patch is
> > > supposed to be allowing is for the primary plane to not cover the entire
> > > CRTC (since that's something that only became possible for Intel
> > > hardware on the gen9+ platforms).  I.e., the primary plane is now
> > > allowed to positioned and resized to cover a subset of the CRTC area,
> > > just like "sprite" planes have always been able to.
> > > 
> > > If you've got a 1080x1080 framebuffer, then it's legal to have a
> > > 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT.
> > > However it is not legal to size the primary plane as 1920x1080 and use
> > > this same 1080x1080 framebuffer with any of our interfaces (setplane,
> > > setcrtc, pageflip, or atomic).
> > > 
> > > Are you using ioctls/libdrm directly or are you using igt_kms helpers?
> > > IIRC, the IGT helpers will try to be extra helpful and automatically
> > > size the plane to match the framebuffer (unless you override that
> > > behavior), so that might be what's causing the confusion here.
> > 
> > The problem is clear as day in drm_mode_page_flip_ioctl():
> > ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, >mode, fb);
> > if (ret)
> > goto out;
> > 
> > The fix should be easy; just extract the current src coordinates from
> > the plane state and check those against the new fb size. And then hope
> > that the plane state is really up to date.
> 
> Yep, that's the conclusion we came to once Tvrtko explained what he was
> seeing on IRC.  I'm not sure whether non-atomic drivers have enough
> state setup by the default helpers to work properly.  Worst case we'll
> just assume that a non-atomic driver won't support primary plane
> windowing (since none have in the past) and fall back to looking at the
> mode for legacy non-atomic drivers.
> 
> > 
> > And I'm sure rotated cases will go boom in some other ways. Probably
> > we should just switch over to using the full plane update for mmio
> > flips to fix it.
> 
> Yeah; the core looks at a drm_plane->invert_dimensions field that's only
> set by omap.  That should probably be updated to look at the state's
> rotation on atomic-capable drivers.

We can just look at the src coordinates. Those always match the fb
orientation.

> 
> 
> Matt
> 
> > 
> > > 
> > > 
> > > Matt
> > > 
> > > > 
> > > > I have posted a quick IGT exerciser for this as "kms_rotation_crc:
> > > > Excercise page flips with 90 degree rotation". May not be that great
> > > > but shows the failure.
> > > > 
> > > > I am not that hot on meddling with this code, nor do I feel

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-04-07 Thread Jindal, Sonika 



On 4/7/2015 1:46 PM, Daniel Vetter wrote:

On Mon, Apr 06, 2015 at 10:50:51AM +0530, Jindal, Sonika wrote:



On 4/2/2015 9:18 PM, Matt Roper wrote:

On Thu, Apr 02, 2015 at 10:08:27AM +0530, Jindal, Sonika wrote:



On 4/1/2015 11:51 PM, Matt Roper wrote:

On Mon, Mar 30, 2015 at 02:04:56PM +0530, Sonika Jindal wrote:

Signed-off-by: Sonika Jindal sonika.jin...@intel.com


It looks like this is dependent on Ville's patch

   [PATCH v2 6/9] drm/i915: Pass the primary plane position to 
.update_primary_plane()

to actually let us do something sensible with the destination rectangle
at the hardware level.  Looks like that patch has a r-b, but hasn't made
it into di-nightly yet.


Right now, can_position is used to check for the scenarios where the
primary plane is not covering the complete crtc. This could be due
to positioning or a smaller fb on primary plane.
With Ville's patch, we would be able to allow positioning to happen.
But I need it here, to create a smaller fb for 90/270 rotation.

I agree that, until Ville's patch is there, we won't be entertaining
any positioning requests on the primary plane and we will not be
throwing any error also.


Right...and I think failing to throw an error would be seen as a bug,
which is why I think Ville's patch needs to go in first.  Since it's
already been reviewed, I'm not aware of anything holding that up from
happening.


Agree, will check with Daniel on this.


But for the 90/270 testcase in kms_rotation_crc to go through, we
would need this to create a smaller fb so that we can rotate it.


So is your worry here that drm_plane_helper_check_update() doesn't
understand rotation and winds up mixing up width/height?  If so, I think
the proper course of action is to write a patch for the helper function
that makes it rotation-aware.


No, the worry is that it rejects a smaller fb for primary plane for all the
platforms. I mentioned 90/270 rotation, because I create a smaller fb
(rather than the full screen fb), so that the rotated plane fits into the
screen. If it is lets say 1920x1080, and the pipe is set at 1920x1080, after
rotation the plane becomes 1080x1920 and the height of the plane surpasses
that of crtc.
For gen =9 , we can have smaller fb for primary plane which might not cover
the entire crtc.


That sounds like a bug in the helper though if it rejects such a
framebuffer.
-Daniel
Till now we used to have primary plane covering the crtc, so its not a 
bug. For gen  9 we can have smaller primary planes (or with other 
platforms as well?)

Anyways, we need to add this patch to get past that restriction.

Regards,
Sonika



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-04-07 Thread Daniel Vetter 
On Mon, Apr 06, 2015 at 10:50:51AM +0530, Jindal, Sonika wrote:
 
 
 On 4/2/2015 9:18 PM, Matt Roper wrote:
 On Thu, Apr 02, 2015 at 10:08:27AM +0530, Jindal, Sonika wrote:
 
 
 On 4/1/2015 11:51 PM, Matt Roper wrote:
 On Mon, Mar 30, 2015 at 02:04:56PM +0530, Sonika Jindal wrote:
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com
 
 It looks like this is dependent on Ville's patch
 
[PATCH v2 6/9] drm/i915: Pass the primary plane position to 
  .update_primary_plane()
 
 to actually let us do something sensible with the destination rectangle
 at the hardware level.  Looks like that patch has a r-b, but hasn't made
 it into di-nightly yet.
 
 Right now, can_position is used to check for the scenarios where the
 primary plane is not covering the complete crtc. This could be due
 to positioning or a smaller fb on primary plane.
 With Ville's patch, we would be able to allow positioning to happen.
 But I need it here, to create a smaller fb for 90/270 rotation.
 
 I agree that, until Ville's patch is there, we won't be entertaining
 any positioning requests on the primary plane and we will not be
 throwing any error also.
 
 Right...and I think failing to throw an error would be seen as a bug,
 which is why I think Ville's patch needs to go in first.  Since it's
 already been reviewed, I'm not aware of anything holding that up from
 happening.
 
 Agree, will check with Daniel on this.
 
 But for the 90/270 testcase in kms_rotation_crc to go through, we
 would need this to create a smaller fb so that we can rotate it.
 
 So is your worry here that drm_plane_helper_check_update() doesn't
 understand rotation and winds up mixing up width/height?  If so, I think
 the proper course of action is to write a patch for the helper function
 that makes it rotation-aware.
 
 No, the worry is that it rejects a smaller fb for primary plane for all the
 platforms. I mentioned 90/270 rotation, because I create a smaller fb
 (rather than the full screen fb), so that the rotated plane fits into the
 screen. If it is lets say 1920x1080, and the pipe is set at 1920x1080, after
 rotation the plane becomes 1080x1920 and the height of the plane surpasses
 that of crtc.
 For gen =9 , we can have smaller fb for primary plane which might not cover
 the entire crtc.

That sounds like a bug in the helper though if it rejects such a
framebuffer.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-04-05 Thread Jindal, Sonika 



On 4/2/2015 9:18 PM, Matt Roper wrote:

On Thu, Apr 02, 2015 at 10:08:27AM +0530, Jindal, Sonika wrote:



On 4/1/2015 11:51 PM, Matt Roper wrote:

On Mon, Mar 30, 2015 at 02:04:56PM +0530, Sonika Jindal wrote:

Signed-off-by: Sonika Jindal sonika.jin...@intel.com


It looks like this is dependent on Ville's patch

   [PATCH v2 6/9] drm/i915: Pass the primary plane position to 
.update_primary_plane()

to actually let us do something sensible with the destination rectangle
at the hardware level.  Looks like that patch has a r-b, but hasn't made
it into di-nightly yet.


Right now, can_position is used to check for the scenarios where the
primary plane is not covering the complete crtc. This could be due
to positioning or a smaller fb on primary plane.
With Ville's patch, we would be able to allow positioning to happen.
But I need it here, to create a smaller fb for 90/270 rotation.

I agree that, until Ville's patch is there, we won't be entertaining
any positioning requests on the primary plane and we will not be
throwing any error also.


Right...and I think failing to throw an error would be seen as a bug,
which is why I think Ville's patch needs to go in first.  Since it's
already been reviewed, I'm not aware of anything holding that up from
happening.


Agree, will check with Daniel on this.


But for the 90/270 testcase in kms_rotation_crc to go through, we
would need this to create a smaller fb so that we can rotate it.


So is your worry here that drm_plane_helper_check_update() doesn't
understand rotation and winds up mixing up width/height?  If so, I think
the proper course of action is to write a patch for the helper function
that makes it rotation-aware.

No, the worry is that it rejects a smaller fb for primary plane for all 
the platforms. I mentioned 90/270 rotation, because I create a smaller 
fb (rather than the full screen fb), so that the rotated plane fits into 
the screen. If it is lets say 1920x1080, and the pipe is set at 
1920x1080, after rotation the plane becomes 1080x1920 and the height of 
the plane surpasses that of crtc.
For gen =9 , we can have smaller fb for primary plane which might not 
cover the entire crtc.


Regards,
Sonika


Matt




Assuming Ville's patch lands first, this is
Reviewed-by: Matt Roper matthew.d.ro...@intel.com


Matt


---
  drivers/gpu/drm/i915/intel_display.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ceb2e61..f0bbc22 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,
struct drm_rect *dest = state-dst;
struct drm_rect *src = state-src;
const struct drm_rect *clip = state-clip;
+   bool can_position = false;
int ret;

crtc = crtc ? crtc : plane-crtc;
intel_crtc = to_intel_crtc(crtc);

+   if (INTEL_INFO(dev)-gen = 9)
+   can_position = true;
+
ret = drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
-   false, true, state-visible);
+   can_position, true,
+   state-visible);
if (ret)
return ret;

--
1.7.10.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-04-02 Thread Matt Roper 
On Thu, Apr 02, 2015 at 10:08:27AM +0530, Jindal, Sonika wrote:
 
 
 On 4/1/2015 11:51 PM, Matt Roper wrote:
 On Mon, Mar 30, 2015 at 02:04:56PM +0530, Sonika Jindal wrote:
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com
 
 It looks like this is dependent on Ville's patch
 
[PATCH v2 6/9] drm/i915: Pass the primary plane position to 
  .update_primary_plane()
 
 to actually let us do something sensible with the destination rectangle
 at the hardware level.  Looks like that patch has a r-b, but hasn't made
 it into di-nightly yet.
 
 Right now, can_position is used to check for the scenarios where the
 primary plane is not covering the complete crtc. This could be due
 to positioning or a smaller fb on primary plane.
 With Ville's patch, we would be able to allow positioning to happen.
 But I need it here, to create a smaller fb for 90/270 rotation.
 
 I agree that, until Ville's patch is there, we won't be entertaining
 any positioning requests on the primary plane and we will not be
 throwing any error also.

Right...and I think failing to throw an error would be seen as a bug,
which is why I think Ville's patch needs to go in first.  Since it's
already been reviewed, I'm not aware of anything holding that up from
happening.

 But for the 90/270 testcase in kms_rotation_crc to go through, we
 would need this to create a smaller fb so that we can rotate it.

So is your worry here that drm_plane_helper_check_update() doesn't
understand rotation and winds up mixing up width/height?  If so, I think
the proper course of action is to write a patch for the helper function
that makes it rotation-aware.


Matt

 
 Assuming Ville's patch lands first, this is
 Reviewed-by: Matt Roper matthew.d.ro...@intel.com
 
 
 Matt
 
 ---
   drivers/gpu/drm/i915/intel_display.c |7 ++-
   1 file changed, 6 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index ceb2e61..f0bbc22 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,
 struct drm_rect *dest = state-dst;
 struct drm_rect *src = state-src;
 const struct drm_rect *clip = state-clip;
 +   bool can_position = false;
 int ret;
 
 crtc = crtc ? crtc : plane-crtc;
 intel_crtc = to_intel_crtc(crtc);
 
 +   if (INTEL_INFO(dev)-gen = 9)
 +   can_position = true;
 +
 ret = drm_plane_helper_check_update(plane, crtc, fb,
 src, dest, clip,
 DRM_PLANE_HELPER_NO_SCALING,
 DRM_PLANE_HELPER_NO_SCALING,
 -   false, true, state-visible);
 +   can_position, true,
 +   state-visible);
 if (ret)
 return ret;
 
 --
 1.7.10.4
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling  Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-04-01 Thread Jindal, Sonika 



On 4/1/2015 11:51 PM, Matt Roper wrote:

On Mon, Mar 30, 2015 at 02:04:56PM +0530, Sonika Jindal wrote:

Signed-off-by: Sonika Jindal sonika.jin...@intel.com


It looks like this is dependent on Ville's patch

   [PATCH v2 6/9] drm/i915: Pass the primary plane position to 
.update_primary_plane()

to actually let us do something sensible with the destination rectangle
at the hardware level.  Looks like that patch has a r-b, but hasn't made
it into di-nightly yet.

Right now, can_position is used to check for the scenarios where the 
primary plane is not covering the complete crtc. This could be due to 
positioning or a smaller fb on primary plane.

With Ville's patch, we would be able to allow positioning to happen.
But I need it here, to create a smaller fb for 90/270 rotation.

I agree that, until Ville's patch is there, we won't be entertaining any 
positioning requests on the primary plane and we will not be throwing 
any error also.


But for the 90/270 testcase in kms_rotation_crc to go through, we would 
need this to create a smaller fb so that we can rotate it.



Assuming Ville's patch lands first, this is
Reviewed-by: Matt Roper matthew.d.ro...@intel.com


Matt


---
  drivers/gpu/drm/i915/intel_display.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ceb2e61..f0bbc22 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,
struct drm_rect *dest = state-dst;
struct drm_rect *src = state-src;
const struct drm_rect *clip = state-clip;
+   bool can_position = false;
int ret;

crtc = crtc ? crtc : plane-crtc;
intel_crtc = to_intel_crtc(crtc);

+   if (INTEL_INFO(dev)-gen = 9)
+   can_position = true;
+
ret = drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
-   false, true, state-visible);
+   can_position, true,
+   state-visible);
if (ret)
return ret;

--
1.7.10.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-04-01 Thread Matt Roper 
On Mon, Mar 30, 2015 at 02:04:56PM +0530, Sonika Jindal wrote:
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com

It looks like this is dependent on Ville's patch

  [PATCH v2 6/9] drm/i915: Pass the primary plane position to 
.update_primary_plane()

to actually let us do something sensible with the destination rectangle
at the hardware level.  Looks like that patch has a r-b, but hasn't made
it into di-nightly yet. 

Assuming Ville's patch lands first, this is
Reviewed-by: Matt Roper matthew.d.ro...@intel.com


Matt

 ---
  drivers/gpu/drm/i915/intel_display.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index ceb2e61..f0bbc22 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,
   struct drm_rect *dest = state-dst;
   struct drm_rect *src = state-src;
   const struct drm_rect *clip = state-clip;
 + bool can_position = false;
   int ret;
  
   crtc = crtc ? crtc : plane-crtc;
   intel_crtc = to_intel_crtc(crtc);
  
 + if (INTEL_INFO(dev)-gen = 9)
 + can_position = true;
 +
   ret = drm_plane_helper_check_update(plane, crtc, fb,
   src, dest, clip,
   DRM_PLANE_HELPER_NO_SCALING,
   DRM_PLANE_HELPER_NO_SCALING,
 - false, true, state-visible);
 + can_position, true,
 + state-visible);
   if (ret)
   return ret;
  
 -- 
 1.7.10.4
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling  Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

2015-03-30 Thread Sonika Jindal 
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ceb2e61..f0bbc22 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,
struct drm_rect *dest = state-dst;
struct drm_rect *src = state-src;
const struct drm_rect *clip = state-clip;
+   bool can_position = false;
int ret;
 
crtc = crtc ? crtc : plane-crtc;
intel_crtc = to_intel_crtc(crtc);
 
+   if (INTEL_INFO(dev)-gen = 9)
+   can_position = true;
+
ret = drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
-   false, true, state-visible);
+   can_position, true,
+   state-visible);
if (ret)
return ret;
 
-- 
1.7.10.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx