Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-08-11 Thread Jindal, Sonika


Hi Daniel,
Are you taking this patch back in drm-intel?

-Sonika

On 8/7/2014 5:41 PM, Daniel Vetter wrote:

On Thu, Aug 07, 2014 at 01:45:31PM +0200, Daniel Vetter wrote:

On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:

On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:

+   /* FBC does not work on some platforms for rotated planes */
+   if (INTEL_INFO(dev)-gen = 4  !IS_G4X(dev)) {
+   if (dev_priv-fbc.plane == intel_crtc-plane 
+   intel_plane-rotation != BIT(DRM_ROTATE_0))
+   intel_disable_fbc(dev);
+   /* If rotation was set earlier and new rotation is 0,
+   we might have disabled fbc earlier. So update it now */
+   else if (intel_plane-rotation == 
BIT(DRM_ROTATE_0)
+old_val != BIT(DRM_ROTATE_0)) {
+   mutex_lock(dev-struct_mutex);
+   intel_update_fbc(dev);
+   mutex_unlock(dev-struct_mutex);
+   }
+   }


Indentation is screwed up here. Also if we convert some of the checks into
early bails we could de-indent this by one level.

Also Chris mentioned that on some platforms this won't work and it's more
future-proof to just do a full modeset until we have the proper
infrastructure.


Apparently this review here was never addressed, as Chris just pointed out
on irc. I've dropped the patch again.

I think we need:
- The same sequence as with the sprite set_property function, i.e. we need
   to call the update_plane function (not the raw low-level one, the
   high-level with all the checks).
- The fbc check is wrong and will miss updates when the crtc is off. We
   need to move this into the general list of checks in intel_update_fbc.
- Since this seems to be buggy I want added testcases to combine fbc
   correctness with screen rotation. Probably best to reuse the existing
   fbc testcase and add a bunch or rotated tests.


Ok, the check in update_fbc is there, I've been blind. Sorry about all the
confusion. So just amounts of calling the higher level function and we can
forgo the fbc testing.
-Daniel


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


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-08-11 Thread Daniel Vetter
On Mon, Aug 11, 2014 at 04:37:00PM +0530, Jindal, Sonika wrote:
 
 Hi Daniel,
 Are you taking this patch back in drm-intel?

We should still call the primary_plane-update hook directly instead of
open-coding it.
-Daniel

 
 -Sonika
 
 On 8/7/2014 5:41 PM, Daniel Vetter wrote:
 On Thu, Aug 07, 2014 at 01:45:31PM +0200, Daniel Vetter wrote:
 On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:
 On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:
 + /* FBC does not work on some platforms for rotated planes */
 + if (INTEL_INFO(dev)-gen = 4  !IS_G4X(dev)) {
 + if (dev_priv-fbc.plane == intel_crtc-plane 
 + intel_plane-rotation != BIT(DRM_ROTATE_0))
 + intel_disable_fbc(dev);
 + /* If rotation was set earlier and new rotation is 0,
 + we might have disabled fbc earlier. So update it now */
 + else if (intel_plane-rotation == 
 BIT(DRM_ROTATE_0)
 +  old_val != BIT(DRM_ROTATE_0)) {
 + mutex_lock(dev-struct_mutex);
 + intel_update_fbc(dev);
 + mutex_unlock(dev-struct_mutex);
 + }
 + }
 
 Indentation is screwed up here. Also if we convert some of the checks into
 early bails we could de-indent this by one level.
 
 Also Chris mentioned that on some platforms this won't work and it's more
 future-proof to just do a full modeset until we have the proper
 infrastructure.
 
 Apparently this review here was never addressed, as Chris just pointed out
 on irc. I've dropped the patch again.
 
 I think we need:
 - The same sequence as with the sprite set_property function, i.e. we need
to call the update_plane function (not the raw low-level one, the
high-level with all the checks).
 - The fbc check is wrong and will miss updates when the crtc is off. We
need to move this into the general list of checks in intel_update_fbc.
 - Since this seems to be buggy I want added testcases to combine fbc
correctness with screen rotation. Probably best to reuse the existing
fbc testcase and add a bunch or rotated tests.
 
 Ok, the check in update_fbc is there, I've been blind. Sorry about all the
 confusion. So just amounts of calling the higher level function and we can
 forgo the fbc testing.
 -Daniel
 

-- 
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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-08-11 Thread Jindal, Sonika



On 8/11/2014 5:12 PM, Daniel Vetter wrote:

On Mon, Aug 11, 2014 at 04:37:00PM +0530, Jindal, Sonika wrote:


Hi Daniel,
Are you taking this patch back in drm-intel?


We should still call the primary_plane-update hook directly instead of
open-coding it.
-Daniel

But we are already doing dev_priv-display.update_primary_plane. Can you 
please elaborate? Last time we had a discussion on this same point, and 
we thought for some platforms more work might be needed but for current 
platforms this looks ok. Following was the discussion:


 Also Chris mentioned that on some platforms this won't work and it's
 more future-proof to just do a full modeset until we have the proper
 infrastructure.
 
 Can you please elaborate on this? What needs to be done?

Apparently nothing, it turned out that on the platforms currently 
supported everything is fine with your patch. Damien promised to follow 
up with the details on internal mailing lists.

-Daniel




-Sonika

On 8/7/2014 5:41 PM, Daniel Vetter wrote:

On Thu, Aug 07, 2014 at 01:45:31PM +0200, Daniel Vetter wrote:

On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:

On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:

+   /* FBC does not work on some platforms for rotated planes */
+   if (INTEL_INFO(dev)-gen = 4  !IS_G4X(dev)) {
+   if (dev_priv-fbc.plane == intel_crtc-plane 
+   intel_plane-rotation != BIT(DRM_ROTATE_0))
+   intel_disable_fbc(dev);
+   /* If rotation was set earlier and new rotation is 0,
+   we might have disabled fbc earlier. So update it now */
+   else if (intel_plane-rotation == 
BIT(DRM_ROTATE_0)
+old_val != BIT(DRM_ROTATE_0)) {
+   mutex_lock(dev-struct_mutex);
+   intel_update_fbc(dev);
+   mutex_unlock(dev-struct_mutex);
+   }
+   }


Indentation is screwed up here. Also if we convert some of the checks into
early bails we could de-indent this by one level.

Also Chris mentioned that on some platforms this won't work and it's more
future-proof to just do a full modeset until we have the proper
infrastructure.


Apparently this review here was never addressed, as Chris just pointed out
on irc. I've dropped the patch again.

I think we need:
- The same sequence as with the sprite set_property function, i.e. we need
   to call the update_plane function (not the raw low-level one, the
   high-level with all the checks).
- The fbc check is wrong and will miss updates when the crtc is off. We
   need to move this into the general list of checks in intel_update_fbc.
- Since this seems to be buggy I want added testcases to combine fbc
   correctness with screen rotation. Probably best to reuse the existing
   fbc testcase and add a bunch or rotated tests.


Ok, the check in update_fbc is there, I've been blind. Sorry about all the
confusion. So just amounts of calling the higher level function and we can
forgo the fbc testing.
-Daniel




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


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-08-11 Thread Daniel Vetter
On Mon, Aug 11, 2014 at 05:37:20PM +0530, Jindal, Sonika wrote:
 
 
 On 8/11/2014 5:12 PM, Daniel Vetter wrote:
 On Mon, Aug 11, 2014 at 04:37:00PM +0530, Jindal, Sonika wrote:
 
 Hi Daniel,
 Are you taking this patch back in drm-intel?
 
 We should still call the primary_plane-update hook directly instead of
 open-coding it.
 -Daniel
 
 But we are already doing dev_priv-display.update_primary_plane. Can you
 please elaborate? Last time we had a discussion on this same point, and we
 thought for some platforms more work might be needed but for current
 platforms this looks ok. Following was the discussion:

That's a different cook used internally in the driver (mostly for
historical reasons nowadays). I'm talking about the
drm_plane-fops-update_plane hook, which is implemented for primary
planes by intel_primary_plane_setplane.

The idea is to have the exact same code flow as for sprite planes, since
once we have atomic modesets that will be what we need. In the sprite
set_prop function you pretty directly call the update_plane hook, and I
think we should do the same here. Actually we might as well directly reuse
the intel_plane_restore function, it seems to exactly do what we want.

  Also Chris mentioned that on some platforms this won't work and it's
  more future-proof to just do a full modeset until we have the proper
  infrastructure.
  
  Can you please elaborate on this? What needs to be done?
 
 Apparently nothing, it turned out that on the platforms currently supported
 everything is fine with your patch. Damien promised to follow up with the
 details on internal mailing lists.

Damien follow up internally. Since we're now allowed to talk about skl
I'll paste this here:

On SKL, for 90/270, we'll also need to update the watermarks and remap
the fb.

We already duplicate some of the logic: the FBC update, wait for
pending flips, ...

So maybe we should try to find a way to take the same path for all
updates. I'm not too sure how practical this is though.

Damien also said that he doesn't want to still the series on this, but
since we now have universal plane support for the primary plane I think it
makes a lot of sense.
-Daniel


 -Daniel
 
 
 
 -Sonika
 
 On 8/7/2014 5:41 PM, Daniel Vetter wrote:
 On Thu, Aug 07, 2014 at 01:45:31PM +0200, Daniel Vetter wrote:
 On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:
 On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:
 +   /* FBC does not work on some platforms for rotated 
 planes */
 +   if (INTEL_INFO(dev)-gen = 4  !IS_G4X(dev)) {
 +   if (dev_priv-fbc.plane == 
 intel_crtc-plane 
 +   intel_plane-rotation != 
 BIT(DRM_ROTATE_0))
 +   intel_disable_fbc(dev);
 +   /* If rotation was set earlier and new rotation 
 is 0,
 +   we might have disabled fbc earlier. So update 
 it now */
 +   else if (intel_plane-rotation == 
 BIT(DRM_ROTATE_0)
 +old_val != 
 BIT(DRM_ROTATE_0)) {
 +   mutex_lock(dev-struct_mutex);
 +   intel_update_fbc(dev);
 +   
 mutex_unlock(dev-struct_mutex);
 +   }
 +   }
 
 Indentation is screwed up here. Also if we convert some of the checks 
 into
 early bails we could de-indent this by one level.
 
 Also Chris mentioned that on some platforms this won't work and it's more
 future-proof to just do a full modeset until we have the proper
 infrastructure.
 
 Apparently this review here was never addressed, as Chris just pointed out
 on irc. I've dropped the patch again.
 
 I think we need:
 - The same sequence as with the sprite set_property function, i.e. we need
to call the update_plane function (not the raw low-level one, the
high-level with all the checks).
 - The fbc check is wrong and will miss updates when the crtc is off. We
need to move this into the general list of checks in intel_update_fbc.
 - Since this seems to be buggy I want added testcases to combine fbc
correctness with screen rotation. Probably best to reuse the existing
fbc testcase and add a bunch or rotated tests.
 
 Ok, the check in update_fbc is there, I've been blind. Sorry about all the
 confusion. So just amounts of calling the higher level function and we can
 forgo the fbc testing.
 -Daniel
 
 

-- 
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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-08-11 Thread Jindal, Sonika



On 8/11/2014 5:53 PM, Daniel Vetter wrote:

On Mon, Aug 11, 2014 at 05:37:20PM +0530, Jindal, Sonika wrote:



On 8/11/2014 5:12 PM, Daniel Vetter wrote:

On Mon, Aug 11, 2014 at 04:37:00PM +0530, Jindal, Sonika wrote:


Hi Daniel,
Are you taking this patch back in drm-intel?


We should still call the primary_plane-update hook directly instead of
open-coding it.
-Daniel


But we are already doing dev_priv-display.update_primary_plane. Can you
please elaborate? Last time we had a discussion on this same point, and we
thought for some platforms more work might be needed but for current
platforms this looks ok. Following was the discussion:


That's a different cook used internally in the driver (mostly for
historical reasons nowadays). I'm talking about the
drm_plane-fops-update_plane hook, which is implemented for primary
planes by intel_primary_plane_setplane.


Ok, let me add that and post a new patch for this.

-Sonika

The idea is to have the exact same code flow as for sprite planes, since
once we have atomic modesets that will be what we need. In the sprite
set_prop function you pretty directly call the update_plane hook, and I
think we should do the same here. Actually we might as well directly reuse
the intel_plane_restore function, it seems to exactly do what we want.


 Also Chris mentioned that on some platforms this won't work and it's

more future-proof to just do a full modeset until we have the proper
infrastructure.


Can you please elaborate on this? What needs to be done?


Apparently nothing, it turned out that on the platforms currently supported
everything is fine with your patch. Damien promised to follow up with the
details on internal mailing lists.


Damien follow up internally. Since we're now allowed to talk about skl
I'll paste this here:

On SKL, for 90/270, we'll also need to update the watermarks and remap
the fb.

We already duplicate some of the logic: the FBC update, wait for
pending flips, ...

So maybe we should try to find a way to take the same path for all
updates. I'm not too sure how practical this is though.

Damien also said that he doesn't want to still the series on this, but
since we now have universal plane support for the primary plane I think it
makes a lot of sense.
-Daniel



-Daniel




-Sonika

On 8/7/2014 5:41 PM, Daniel Vetter wrote:

On Thu, Aug 07, 2014 at 01:45:31PM +0200, Daniel Vetter wrote:

On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:

On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:

+   /* FBC does not work on some platforms for rotated planes */
+   if (INTEL_INFO(dev)-gen = 4  !IS_G4X(dev)) {
+   if (dev_priv-fbc.plane == intel_crtc-plane 
+   intel_plane-rotation != BIT(DRM_ROTATE_0))
+   intel_disable_fbc(dev);
+   /* If rotation was set earlier and new rotation is 0,
+   we might have disabled fbc earlier. So update it now */
+   else if (intel_plane-rotation == 
BIT(DRM_ROTATE_0)
+old_val != BIT(DRM_ROTATE_0)) {
+   mutex_lock(dev-struct_mutex);
+   intel_update_fbc(dev);
+   mutex_unlock(dev-struct_mutex);
+   }
+   }


Indentation is screwed up here. Also if we convert some of the checks into
early bails we could de-indent this by one level.

Also Chris mentioned that on some platforms this won't work and it's more
future-proof to just do a full modeset until we have the proper
infrastructure.


Apparently this review here was never addressed, as Chris just pointed out
on irc. I've dropped the patch again.

I think we need:
- The same sequence as with the sprite set_property function, i.e. we need
   to call the update_plane function (not the raw low-level one, the
   high-level with all the checks).
- The fbc check is wrong and will miss updates when the crtc is off. We
   need to move this into the general list of checks in intel_update_fbc.
- Since this seems to be buggy I want added testcases to combine fbc
   correctness with screen rotation. Probably best to reuse the existing
   fbc testcase and add a bunch or rotated tests.


Ok, the check in update_fbc is there, I've been blind. Sorry about all the
confusion. So just amounts of calling the higher level function and we can
forgo the fbc testing.
-Daniel






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


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-08-07 Thread Daniel Vetter
On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:
 On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:
  +   /* FBC does not work on some platforms for rotated planes */
  +   if (INTEL_INFO(dev)-gen = 4  !IS_G4X(dev)) {
  +   if (dev_priv-fbc.plane == intel_crtc-plane 
  +   intel_plane-rotation != BIT(DRM_ROTATE_0))
  +   intel_disable_fbc(dev);
  +   /* If rotation was set earlier and new rotation is 0,
  +   we might have disabled fbc earlier. So update it now */
  +   else if (intel_plane-rotation == 
  BIT(DRM_ROTATE_0)
  +old_val != BIT(DRM_ROTATE_0)) {
  +   mutex_lock(dev-struct_mutex);
  +   intel_update_fbc(dev);
  +   mutex_unlock(dev-struct_mutex);
  +   }
  +   }
 
 Indentation is screwed up here. Also if we convert some of the checks into
 early bails we could de-indent this by one level.
 
 Also Chris mentioned that on some platforms this won't work and it's more
 future-proof to just do a full modeset until we have the proper
 infrastructure.

Apparently this review here was never addressed, as Chris just pointed out
on irc. I've dropped the patch again.

I think we need:
- The same sequence as with the sprite set_property function, i.e. we need
  to call the update_plane function (not the raw low-level one, the
  high-level with all the checks).
- The fbc check is wrong and will miss updates when the crtc is off. We
  need to move this into the general list of checks in intel_update_fbc.
- Since this seems to be buggy I want added testcases to combine fbc
  correctness with screen rotation. Probably best to reuse the existing
  fbc testcase and add a bunch or rotated tests.

Thanks, Daniel
-- 
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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-08-07 Thread Daniel Vetter
On Thu, Aug 07, 2014 at 01:45:31PM +0200, Daniel Vetter wrote:
 On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:
  On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:
   + /* FBC does not work on some platforms for rotated planes */
   + if (INTEL_INFO(dev)-gen = 4  !IS_G4X(dev)) {
   + if (dev_priv-fbc.plane == intel_crtc-plane 
   + intel_plane-rotation != BIT(DRM_ROTATE_0))
   + intel_disable_fbc(dev);
   + /* If rotation was set earlier and new rotation is 0,
   + we might have disabled fbc earlier. So update it now */
   + else if (intel_plane-rotation == 
   BIT(DRM_ROTATE_0)
   +  old_val != BIT(DRM_ROTATE_0)) {
   + mutex_lock(dev-struct_mutex);
   + intel_update_fbc(dev);
   + mutex_unlock(dev-struct_mutex);
   + }
   + }
  
  Indentation is screwed up here. Also if we convert some of the checks into
  early bails we could de-indent this by one level.
  
  Also Chris mentioned that on some platforms this won't work and it's more
  future-proof to just do a full modeset until we have the proper
  infrastructure.
 
 Apparently this review here was never addressed, as Chris just pointed out
 on irc. I've dropped the patch again.
 
 I think we need:
 - The same sequence as with the sprite set_property function, i.e. we need
   to call the update_plane function (not the raw low-level one, the
   high-level with all the checks).
 - The fbc check is wrong and will miss updates when the crtc is off. We
   need to move this into the general list of checks in intel_update_fbc.
 - Since this seems to be buggy I want added testcases to combine fbc
   correctness with screen rotation. Probably best to reuse the existing
   fbc testcase and add a bunch or rotated tests.

Ok, the check in update_fbc is there, I've been blind. Sorry about all the
confusion. So just amounts of calling the higher level function and we can
forgo the fbc testing.
-Daniel
-- 
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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-07-15 Thread Daniel Vetter
On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:
 From: Sonika Jindal sonika.jin...@intel.com
 
 Primary planes support 180 degree rotation. Expose the feature
 through rotation drm property.
 
 v2: Calculating linear/tiled offsets based on pipe source width and
 height. Added 180 degree rotation support in ironlake_update_plane.
 
 v3: Checking if CRTC is active before issueing update_plane. Added
 wait for vblank to make sure we dont overtake page flips. Disabling
 FBC since it does not work with rotated planes.
 
 v4: Updated rotation checks for pending flips, fbc disable. Creating
 rotation property only for Gen4 onwards. Property resetting as part
 of lastclose.
 
 v5: Resetting property in i915_driver_lastclose properly for planes
 and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
 width in i9xx_update_plane and ironlake_update_plane. Removed tab
 based indentation and unnecessary braces in intel_crtc_set_property
 and intel_update_fbc. FBC and flip related checks should be done only
 for valid crtcs.
 
 v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
 and positioning the disable code in intel_update_fbc.
 
 v7: In case rotation property on inactive crtc is updated, we return
 successfully printing debug log as crtc is inactive and only property change
 is preserved.
 
 v8: update_plane is changed to update_primary_plane, crtc-fb is changed to
 crtc-primary-fb  and return value of update_primary_plane is ignored.
 
 v9: added rotation property to primary plane instead of crtc. Removing reset
 of rotation property from lastclose. rotation_property is moved to
 drm_mode_config, so drm layer will take care of resetting. Adding updation of
 fbc when rotation is set to 0. Allowing rotation only if value is
 different than old one.
 
 Testcase: igt/kms_rotation_crc
 
 Signed-off-by: Uma Shankar uma.shan...@intel.com
 Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com

Some stuff I've noticed below.
-Daniel

 ---
  drivers/gpu/drm/i915/i915_reg.h  |1 +
  drivers/gpu/drm/i915/intel_display.c |  104 
 --
  drivers/gpu/drm/i915/intel_pm.c  |6 ++
  3 files changed, 107 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 74283d5..f39e2e7 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -4107,6 +4107,7 @@ enum punit_power_well {
  #define   DISPPLANE_NO_LINE_DOUBLE   0
  #define   DISPPLANE_STEREO_POLARITY_FIRST0
  #define   DISPPLANE_STEREO_POLARITY_SECOND   (118)
 +#define   DISPPLANE_ROTATE_180 (115)
  #define   DISPPLANE_TRICKLE_FEED_DISABLE (114) /* Ironlake */
  #define   DISPPLANE_TILED(110)
  #define _DSPAADDR0x70184
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 023c770..e881dcf 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2408,11 +2408,15 @@ static void i9xx_update_primary_plane(struct drm_crtc 
 *crtc,
   unsigned long linear_offset;
   u32 dspcntr;
   u32 reg;
 + int pixel_size;
 +
 + pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
  
   reg = DSPCNTR(plane);
   dspcntr = I915_READ(reg);
   /* Mask out pixel format bits in case we change it */
   dspcntr = ~DISPPLANE_PIXFORMAT_MASK;
 + dspcntr = ~DISPPLANE_ROTATE_180;
   switch (fb-pixel_format) {
   case DRM_FORMAT_C8:
   dspcntr |= DISPPLANE_8BPP;
 @@ -2454,8 +2458,6 @@ static void i9xx_update_primary_plane(struct drm_crtc 
 *crtc,
   if (IS_G4X(dev))
   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
  
 - I915_WRITE(reg, dspcntr);
 -
   linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8);
  
   if (INTEL_INFO(dev)-gen = 4) {
 @@ -2467,6 +2469,17 @@ static void i9xx_update_primary_plane(struct drm_crtc 
 *crtc,
   } else {
   intel_crtc-dspaddr_offset = linear_offset;
   }
 + if (to_intel_plane(crtc-primary)-rotation == BIT(DRM_ROTATE_180)) {
 + dspcntr |= DISPPLANE_ROTATE_180;
 +
 + x += (intel_crtc-config.pipe_src_w - 1);
 + y += (intel_crtc-config.pipe_src_h - 1);
 + linear_offset += (intel_crtc-config.pipe_src_h - 1) *
 + fb-pitches[0] +
 + (intel_crtc-config.pipe_src_w - 1) * pixel_size;

We already have helper functions to compute the linear offset properly for
tiling, I think we should put the rotation adjustments in there to avoid
dpulicated code of this. And a comment about how this works would be nice.

 + }
 +
 + I915_WRITE(reg, dspcntr);
  
   DRM_DEBUG_KMS(Writing base %08lX %08lX %d %d %d\n,
 i915_gem_obj_ggtt_offset(obj), linear_offset, x, 

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-07-15 Thread Jindal, Sonika



On 7/15/2014 2:41 PM, Daniel Vetter wrote:

On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:

From: Sonika Jindal sonika.jin...@intel.com

Primary planes support 180 degree rotation. Expose the feature
through rotation drm property.

v2: Calculating linear/tiled offsets based on pipe source width and
height. Added 180 degree rotation support in ironlake_update_plane.

v3: Checking if CRTC is active before issueing update_plane. Added
wait for vblank to make sure we dont overtake page flips. Disabling
FBC since it does not work with rotated planes.

v4: Updated rotation checks for pending flips, fbc disable. Creating
rotation property only for Gen4 onwards. Property resetting as part
of lastclose.

v5: Resetting property in i915_driver_lastclose properly for planes
and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
width in i9xx_update_plane and ironlake_update_plane. Removed tab
based indentation and unnecessary braces in intel_crtc_set_property
and intel_update_fbc. FBC and flip related checks should be done only
for valid crtcs.

v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
and positioning the disable code in intel_update_fbc.

v7: In case rotation property on inactive crtc is updated, we return
successfully printing debug log as crtc is inactive and only property change
is preserved.

v8: update_plane is changed to update_primary_plane, crtc-fb is changed to
crtc-primary-fb  and return value of update_primary_plane is ignored.

v9: added rotation property to primary plane instead of crtc. Removing reset
of rotation property from lastclose. rotation_property is moved to
drm_mode_config, so drm layer will take care of resetting. Adding updation of
fbc when rotation is set to 0. Allowing rotation only if value is
different than old one.

Testcase: igt/kms_rotation_crc

Signed-off-by: Uma Shankar uma.shan...@intel.com
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
Signed-off-by: Sonika Jindal sonika.jin...@intel.com


Some stuff I've noticed below.
-Daniel


---
  drivers/gpu/drm/i915/i915_reg.h  |1 +
  drivers/gpu/drm/i915/intel_display.c |  104 --
  drivers/gpu/drm/i915/intel_pm.c  |6 ++
  3 files changed, 107 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 74283d5..f39e2e7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4107,6 +4107,7 @@ enum punit_power_well {
  #define   DISPPLANE_NO_LINE_DOUBLE0
  #define   DISPPLANE_STEREO_POLARITY_FIRST 0
  #define   DISPPLANE_STEREO_POLARITY_SECOND(118)
+#define   DISPPLANE_ROTATE_180 (115)
  #define   DISPPLANE_TRICKLE_FEED_DISABLE  (114) /* Ironlake */
  #define   DISPPLANE_TILED (110)
  #define _DSPAADDR 0x70184
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 023c770..e881dcf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2408,11 +2408,15 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
unsigned long linear_offset;
u32 dspcntr;
u32 reg;
+   int pixel_size;
+
+   pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);

reg = DSPCNTR(plane);
dspcntr = I915_READ(reg);
/* Mask out pixel format bits in case we change it */
dspcntr = ~DISPPLANE_PIXFORMAT_MASK;
+   dspcntr = ~DISPPLANE_ROTATE_180;
switch (fb-pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2454,8 +2458,6 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;

-   I915_WRITE(reg, dspcntr);
-
linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8);

if (INTEL_INFO(dev)-gen = 4) {
@@ -2467,6 +2469,17 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
} else {
intel_crtc-dspaddr_offset = linear_offset;
}
+   if (to_intel_plane(crtc-primary)-rotation == BIT(DRM_ROTATE_180)) {
+   dspcntr |= DISPPLANE_ROTATE_180;
+
+   x += (intel_crtc-config.pipe_src_w - 1);
+   y += (intel_crtc-config.pipe_src_h - 1);
+   linear_offset += (intel_crtc-config.pipe_src_h - 1) *
+   fb-pitches[0] +
+   (intel_crtc-config.pipe_src_w - 1) * pixel_size;


We already have helper functions to compute the linear offset properly for
tiling, I think we should put the rotation adjustments in there to avoid
dpulicated code of this. And a comment about how this works would be nice.


Ok, I will add the comments.

+   }
+
+   I915_WRITE(reg, dspcntr);

DRM_DEBUG_KMS(Writing base %08lX %08lX %d %d %d\n,
  

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support

2014-07-15 Thread Daniel Vetter
On Tue, Jul 15, 2014 at 03:08:37PM +0530, Jindal, Sonika wrote:
 On 7/15/2014 2:41 PM, Daniel Vetter wrote:
 On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:
 From: Sonika Jindal sonika.jin...@intel.com
 Ok, I will add
 Also Chris mentioned that on some platforms this won't work and it's more
 future-proof to just do a full modeset until we have the proper
 infrastructure.
 
 Can you please elaborate on this? What needs to be done?

Apparently nothing, it turned out that on the platforms currently
supported everything is fine with your patch. Damien promised to follow up
with the details on internal mailing lists.
-Daniel
-- 
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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx