Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-14 Thread Jindal, Sonika 



On 4/14/2015 5:09 AM, Matt Roper wrote:

On Mon, Apr 13, 2015 at 01:49:22PM +0300, Ville Syrjälä wrote:

On Mon, Apr 13, 2015 at 03:53:22PM +0530, Jindal, Sonika wrote:



On 4/13/2015 3:40 PM, Ville Syrjälä wrote:

On Mon, Apr 13, 2015 at 09:36:02AM +0530, Jindal, Sonika wrote:



On 4/10/2015 8:14 PM, Ville Syrjälä wrote:

On Fri, Apr 10, 2015 at 04:17:17PM +0200, Daniel Vetter wrote:

On Fri, Apr 10, 2015 at 02:37:29PM +0530, Sonika Jindal wrote:

v2: Moving creation of property in a function, checking for 90/270
rotation simultaneously (Chris)
Letting primary plane to be positioned
v3: Adding if/else for 90/270 and rest params programming, adding check for
pixel_format, some cleanup (review comments)
v4: Adding right pixel_formats, using src_* params instead of crtc_* for offset
and size programming (Ville)
v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
v6: Rebased on -nightly (Tvrtko's series merged)
v7: Moving pixel_format check to intel_atomic_plane_check (Matt)

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
Reviewed-by: Matt Roper matthew.d.ro...@intel.com


Patches are missing the Testcase: tag, please add that. Also, are all the
igt committed or not yet? Pulled these two in meanwhile.


Things are going somewhat broken because you didn't apply my plane
state stuff. Hmm. Actually it sort of looks that it might work by luck
as long as the primary plane doesn't get clipped since this is bashing
the user state directly into the hardware registers and not the derived
state (ie. clipped coordinates).


I was hoping your changes get merged, but not sure why they are held up.


Also I see my review comment about the 90 vs. 270 rotation mixup was
ignored at least.


I never really got the to understand the need of reversing 90 and 270 :)
The last discussion was not concluded, AFAIR.
Things look correct to me and work fine with the testcase also.
Is there something completely different which I am unable to get?


BSpec gives me the impression the hw rotation is cw, whereas the drm one
is ccw.


Yes, HW rotation is cw as per bspec. In drm, where do we consider it as
anti-cw? I assume it is cw because all the macros work as expected, ie cw.


The ccw rule was inherited from XRandR. I'm not sure what macros you
mean, but drm_rect_rotate() will certainly give you wrong results if
you assume cw.


It seems like this information needs to be added to the property section
of the DRM DocBook; continuing to follow XRandR probably makes sense if
that's what's agreed on, but there's no indication of that design
philosophy in the actual DRM documentation today, so we're in danger of
having different driver authors use conflicting interpretations.

Ok, I will create a DocBook patch with description for 90/270 rotation 
(Anyways 90/270 rotation is not added in the rotation property in 
documentation). Will there be any other place for this or i915's 
rotation property?


Also, I will send a patch to flip 90/270 in i915. I am just worried, 
that it will look confusing to check for DRM_ROTATE_90 and use 
PLANE_CTL_ROTATE_270 for programming. I will add a comment though.

Sounds good?

-Sonika

90/270 rotation is already supported by existing drivers (omap for
several years now and atmel-hlcdc just recently).  I think it's too late
to try to redefine the meaning of rotation property values that are
already in active use, so we probably need to check whether omap is
using a cw or ccw scheme and follow (and document!) that.


Matt







-Sonika


-Daniel


---
drivers/gpu/drm/i915/i915_reg.h   |2 +
drivers/gpu/drm/i915/intel_atomic_plane.c |   24 
drivers/gpu/drm/i915/intel_display.c  |   88 
-
drivers/gpu/drm/i915/intel_drv.h  |6 ++
drivers/gpu/drm/i915/intel_sprite.c   |   52 -
5 files changed, 131 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..564bbd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
#define   PLANE_CTL_ALPHA_HW_PREMULTIPLY(  3  4)
#define   PLANE_CTL_ROTATE_MASK 0x3
#define   PLANE_CTL_ROTATE_00x0
+#define   PLANE_CTL_ROTATE_90  0x1
#define   PLANE_CTL_ROTATE_180  0x2
+#define   PLANE_CTL_ROTATE_270 0x3
#define _PLANE_STRIDE_1_A   0x70188
#define _PLANE_STRIDE_2_A   0x70288
#define _PLANE_STRIDE_3_A   0x70388
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 976b891..a27ee8c 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -162,6 +162,30 @@ static int intel_plane_atomic_check(struct drm_plane 
*plane,
(1

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-14 Thread Daniel Vetter 
On Tue, Apr 14, 2015 at 05:49:40PM +0530, Jindal, Sonika wrote:
 
 
 On 4/14/2015 5:09 AM, Matt Roper wrote:
 On Mon, Apr 13, 2015 at 01:49:22PM +0300, Ville Syrjälä wrote:
 On Mon, Apr 13, 2015 at 03:53:22PM +0530, Jindal, Sonika wrote:
 
 
 On 4/13/2015 3:40 PM, Ville Syrjälä wrote:
 On Mon, Apr 13, 2015 at 09:36:02AM +0530, Jindal, Sonika wrote:
 
 
 On 4/10/2015 8:14 PM, Ville Syrjälä wrote:
 On Fri, Apr 10, 2015 at 04:17:17PM +0200, Daniel Vetter wrote:
 On Fri, Apr 10, 2015 at 02:37:29PM +0530, Sonika Jindal wrote:
 v2: Moving creation of property in a function, checking for 90/270
 rotation simultaneously (Chris)
 Letting primary plane to be positioned
 v3: Adding if/else for 90/270 and rest params programming, adding 
 check for
 pixel_format, some cleanup (review comments)
 v4: Adding right pixel_formats, using src_* params instead of crtc_* 
 for offset
 and size programming (Ville)
 v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
 v6: Rebased on -nightly (Tvrtko's series merged)
 v7: Moving pixel_format check to intel_atomic_plane_check (Matt)
 
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com
 Reviewed-by: Matt Roper matthew.d.ro...@intel.com
 
 Patches are missing the Testcase: tag, please add that. Also, are all 
 the
 igt committed or not yet? Pulled these two in meanwhile.
 
 Things are going somewhat broken because you didn't apply my plane
 state stuff. Hmm. Actually it sort of looks that it might work by luck
 as long as the primary plane doesn't get clipped since this is bashing
 the user state directly into the hardware registers and not the derived
 state (ie. clipped coordinates).
 
 I was hoping your changes get merged, but not sure why they are held up.
 
 Also I see my review comment about the 90 vs. 270 rotation mixup was
 ignored at least.
 
 I never really got the to understand the need of reversing 90 and 270 :)
 The last discussion was not concluded, AFAIR.
 Things look correct to me and work fine with the testcase also.
 Is there something completely different which I am unable to get?
 
 BSpec gives me the impression the hw rotation is cw, whereas the drm one
 is ccw.
 
 Yes, HW rotation is cw as per bspec. In drm, where do we consider it as
 anti-cw? I assume it is cw because all the macros work as expected, ie cw.
 
 The ccw rule was inherited from XRandR. I'm not sure what macros you
 mean, but drm_rect_rotate() will certainly give you wrong results if
 you assume cw.
 
 It seems like this information needs to be added to the property section
 of the DRM DocBook; continuing to follow XRandR probably makes sense if
 that's what's agreed on, but there's no indication of that design
 philosophy in the actual DRM documentation today, so we're in danger of
 having different driver authors use conflicting interpretations.
 
 Ok, I will create a DocBook patch with description for 90/270 rotation
 (Anyways 90/270 rotation is not added in the rotation property in
 documentation). Will there be any other place for this or i915's rotation
 property?
 
 Also, I will send a patch to flip 90/270 in i915. I am just worried, that it
 will look confusing to check for DRM_ROTATE_90 and use PLANE_CTL_ROTATE_270
 for programming. I will add a comment though.

Just add a comment stating that drm is ccw (to stay compatible with
Xrandr) and i915 hw is cw. That should explain it all.

There's other places where the linux kernel and intel hw people disagree
on definitions, it just happens.
-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 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-13 Thread Matt Roper 
On Mon, Apr 13, 2015 at 01:49:22PM +0300, Ville Syrjälä wrote:
 On Mon, Apr 13, 2015 at 03:53:22PM +0530, Jindal, Sonika wrote:
  
  
  On 4/13/2015 3:40 PM, Ville Syrjälä wrote:
   On Mon, Apr 13, 2015 at 09:36:02AM +0530, Jindal, Sonika wrote:
  
  
   On 4/10/2015 8:14 PM, Ville Syrjälä wrote:
   On Fri, Apr 10, 2015 at 04:17:17PM +0200, Daniel Vetter wrote:
   On Fri, Apr 10, 2015 at 02:37:29PM +0530, Sonika Jindal wrote:
   v2: Moving creation of property in a function, checking for 90/270
   rotation simultaneously (Chris)
   Letting primary plane to be positioned
   v3: Adding if/else for 90/270 and rest params programming, adding 
   check for
   pixel_format, some cleanup (review comments)
   v4: Adding right pixel_formats, using src_* params instead of crtc_* 
   for offset
   and size programming (Ville)
   v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
   v6: Rebased on -nightly (Tvrtko's series merged)
   v7: Moving pixel_format check to intel_atomic_plane_check (Matt)
  
   Signed-off-by: Sonika Jindal sonika.jin...@intel.com
   Reviewed-by: Matt Roper matthew.d.ro...@intel.com
  
   Patches are missing the Testcase: tag, please add that. Also, are all 
   the
   igt committed or not yet? Pulled these two in meanwhile.
  
   Things are going somewhat broken because you didn't apply my plane
   state stuff. Hmm. Actually it sort of looks that it might work by luck
   as long as the primary plane doesn't get clipped since this is bashing
   the user state directly into the hardware registers and not the derived
   state (ie. clipped coordinates).
  
   I was hoping your changes get merged, but not sure why they are held up.
  
   Also I see my review comment about the 90 vs. 270 rotation mixup was
   ignored at least.
  
   I never really got the to understand the need of reversing 90 and 270 :)
   The last discussion was not concluded, AFAIR.
   Things look correct to me and work fine with the testcase also.
   Is there something completely different which I am unable to get?
  
   BSpec gives me the impression the hw rotation is cw, whereas the drm one
   is ccw.
  
  Yes, HW rotation is cw as per bspec. In drm, where do we consider it as 
  anti-cw? I assume it is cw because all the macros work as expected, ie cw.
 
 The ccw rule was inherited from XRandR. I'm not sure what macros you
 mean, but drm_rect_rotate() will certainly give you wrong results if
 you assume cw.

It seems like this information needs to be added to the property section
of the DRM DocBook; continuing to follow XRandR probably makes sense if
that's what's agreed on, but there's no indication of that design
philosophy in the actual DRM documentation today, so we're in danger of
having different driver authors use conflicting interpretations.

90/270 rotation is already supported by existing drivers (omap for
several years now and atmel-hlcdc just recently).  I think it's too late
to try to redefine the meaning of rotation property values that are
already in active use, so we probably need to check whether omap is
using a cw or ccw scheme and follow (and document!) that.


Matt

 
  
  
   -Sonika
  
   -Daniel
  
   ---
  drivers/gpu/drm/i915/i915_reg.h   |2 +
  drivers/gpu/drm/i915/intel_atomic_plane.c |   24 
  drivers/gpu/drm/i915/intel_display.c  |   88 
   -
  drivers/gpu/drm/i915/intel_drv.h  |6 ++
  drivers/gpu/drm/i915/intel_sprite.c   |   52 -
  5 files changed, 131 insertions(+), 41 deletions(-)
  
   diff --git a/drivers/gpu/drm/i915/i915_reg.h 
   b/drivers/gpu/drm/i915/i915_reg.h
   index b522eb6..564bbd5 100644
   --- a/drivers/gpu/drm/i915/i915_reg.h
   +++ b/drivers/gpu/drm/i915/i915_reg.h
   @@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
  #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY   (  3  4)
  #define   PLANE_CTL_ROTATE_MASK0x3
  #define   PLANE_CTL_ROTATE_0   0x0
   +#define   PLANE_CTL_ROTATE_900x1
  #define   PLANE_CTL_ROTATE_180 0x2
   +#define   PLANE_CTL_ROTATE_270   0x3
  #define _PLANE_STRIDE_1_A  0x70188
  #define _PLANE_STRIDE_2_A  0x70288
  #define _PLANE_STRIDE_3_A  0x70388
   diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
   b/drivers/gpu/drm/i915/intel_atomic_plane.c
   index 976b891..a27ee8c 100644
   --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
   +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
   @@ -162,6 +162,30 @@ static int intel_plane_atomic_check(struct 
   drm_plane *plane,
 (1  drm_plane_index(plane));
 }
  
   + if (state-fb  intel_rotation_90_or_270(state-rotation)) {
   + if (!(state-fb-modifier[0] == I915_FORMAT_MOD_Y_TILED 
   ||
   + state-fb-modifier[0] == 
   I915_FORMAT_MOD_Yf_TILED)) {
   +

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-13 Thread shuang . he 
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 6170
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV  270/270  270/270
ILK  303/303  303/303
SNB -21  304/304  283/304
IVB  337/337  337/337
BYT -1  287/287  286/287
HSW  361/361  361/361
BDW  309/309  309/309
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
 SNB  igt@kms_cursor_crc@cursor-size-change  NSPT(2)PASS(1)  NSPT(2)
 SNB  igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip  NSPT(2)PASS(1)  
NSPT(2)
 SNB  igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip  NSPT(2)PASS(1)  
NSPT(2)
 SNB  igt@kms_rotation_crc@primary-rotation  NSPT(2)PASS(1)  NSPT(2)
 SNB  igt@kms_rotation_crc@sprite-rotation  NSPT(2)PASS(3)  NSPT(2)
 SNB  igt@pm_rpm@cursor  NSPT(2)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@cursor-dpms  NSPT(2)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp  NSPT(2)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@dpms-non-lpsp  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@drm-resources-equal  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@fences  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@fences-dpms  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@gem-execbuf  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@gem-mmap-cpu  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@gem-mmap-gtt  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@gem-pread  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@i2c  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@modeset-non-lpsp  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@modeset-non-lpsp-stress-no-wait  NSPT(1)PASS(1)  
NSPT(2)
 SNB  igt@pm_rpm@pci-d3-state  NSPT(1)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@rte  NSPT(1)PASS(1)  NSPT(2)
*BYT  igt@gem_exec_bad_domains@conflicting-write-domain  PASS(2)  
FAIL(1)PASS(1)
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-13 Thread Ville Syrjälä 
On Mon, Apr 13, 2015 at 09:36:02AM +0530, Jindal, Sonika wrote:
 
 
 On 4/10/2015 8:14 PM, Ville Syrjälä wrote:
  On Fri, Apr 10, 2015 at 04:17:17PM +0200, Daniel Vetter wrote:
  On Fri, Apr 10, 2015 at 02:37:29PM +0530, Sonika Jindal wrote:
  v2: Moving creation of property in a function, checking for 90/270
  rotation simultaneously (Chris)
  Letting primary plane to be positioned
  v3: Adding if/else for 90/270 and rest params programming, adding check 
  for
  pixel_format, some cleanup (review comments)
  v4: Adding right pixel_formats, using src_* params instead of crtc_* for 
  offset
  and size programming (Ville)
  v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
  v6: Rebased on -nightly (Tvrtko's series merged)
  v7: Moving pixel_format check to intel_atomic_plane_check (Matt)
 
  Signed-off-by: Sonika Jindal sonika.jin...@intel.com
  Reviewed-by: Matt Roper matthew.d.ro...@intel.com
 
  Patches are missing the Testcase: tag, please add that. Also, are all the
  igt committed or not yet? Pulled these two in meanwhile.
 
  Things are going somewhat broken because you didn't apply my plane
  state stuff. Hmm. Actually it sort of looks that it might work by luck
  as long as the primary plane doesn't get clipped since this is bashing
  the user state directly into the hardware registers and not the derived
  state (ie. clipped coordinates).
 
 I was hoping your changes get merged, but not sure why they are held up.
 
  Also I see my review comment about the 90 vs. 270 rotation mixup was
  ignored at least.
 
 I never really got the to understand the need of reversing 90 and 270 :) 
 The last discussion was not concluded, AFAIR.
 Things look correct to me and work fine with the testcase also.
 Is there something completely different which I am unable to get?

BSpec gives me the impression the hw rotation is cw, whereas the drm one
is ccw.

 
 -Sonika
 
  -Daniel
 
  ---
drivers/gpu/drm/i915/i915_reg.h   |2 +
drivers/gpu/drm/i915/intel_atomic_plane.c |   24 
drivers/gpu/drm/i915/intel_display.c  |   88 
  -
drivers/gpu/drm/i915/intel_drv.h  |6 ++
drivers/gpu/drm/i915/intel_sprite.c   |   52 -
5 files changed, 131 insertions(+), 41 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index b522eb6..564bbd5 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
#define   PLANE_CTL_ALPHA_HW_PREMULTIPLY(  3  4)
#define   PLANE_CTL_ROTATE_MASK 0x3
#define   PLANE_CTL_ROTATE_00x0
  +#define   PLANE_CTL_ROTATE_900x1
#define   PLANE_CTL_ROTATE_180  0x2
  +#define   PLANE_CTL_ROTATE_270   0x3
#define _PLANE_STRIDE_1_A   0x70188
#define _PLANE_STRIDE_2_A   0x70288
#define _PLANE_STRIDE_3_A   0x70388
  diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
  b/drivers/gpu/drm/i915/intel_atomic_plane.c
  index 976b891..a27ee8c 100644
  --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
  +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
  @@ -162,6 +162,30 @@ static int intel_plane_atomic_check(struct drm_plane 
  *plane,
(1  drm_plane_index(plane));
}
 
  + if (state-fb  intel_rotation_90_or_270(state-rotation)) {
  + if (!(state-fb-modifier[0] == I915_FORMAT_MOD_Y_TILED ||
  + state-fb-modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
  + DRM_DEBUG_KMS(Y/Yf tiling required for 90/270!\n);
  + return -EINVAL;
  + }
  +
  + /*
  +  * 90/270 is not allowed with RGB64 16:16:16:16,
  +  * RGB 16-bit 5:6:5, and Indexed 8-bit.
  +  * TBD: Add RGB64 case once its added in supported format list.
  +  */
  + switch (state-fb-pixel_format) {
  + case DRM_FORMAT_C8:
  + case DRM_FORMAT_RGB565:
  + DRM_DEBUG_KMS(Unsupported pixel format %s for 
  90/270!\n,
  + 
  drm_get_format_name(state-fb-pixel_format));
  + return -EINVAL;
  +
  + default:
  + break;
  + }
  + }
  +
return intel_plane-check_plane(plane, intel_state);
}
 
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index f0bbc22..4de544c 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -2311,13 +2311,6 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view 
  *view, struct drm_framebuffer *fb,
info-pitch = fb-pitches[0];
info-fb_modifier = fb-modifier[0];
 
  - if (!(info-fb_modifier == I915_FORMAT_MOD_Y_TILED ||
  -   info-fb_modifier ==

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-13 Thread Ville Syrjälä 
On Mon, Apr 13, 2015 at 03:53:22PM +0530, Jindal, Sonika wrote:
 
 
 On 4/13/2015 3:40 PM, Ville Syrjälä wrote:
  On Mon, Apr 13, 2015 at 09:36:02AM +0530, Jindal, Sonika wrote:
 
 
  On 4/10/2015 8:14 PM, Ville Syrjälä wrote:
  On Fri, Apr 10, 2015 at 04:17:17PM +0200, Daniel Vetter wrote:
  On Fri, Apr 10, 2015 at 02:37:29PM +0530, Sonika Jindal wrote:
  v2: Moving creation of property in a function, checking for 90/270
  rotation simultaneously (Chris)
  Letting primary plane to be positioned
  v3: Adding if/else for 90/270 and rest params programming, adding check 
  for
  pixel_format, some cleanup (review comments)
  v4: Adding right pixel_formats, using src_* params instead of crtc_* 
  for offset
  and size programming (Ville)
  v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
  v6: Rebased on -nightly (Tvrtko's series merged)
  v7: Moving pixel_format check to intel_atomic_plane_check (Matt)
 
  Signed-off-by: Sonika Jindal sonika.jin...@intel.com
  Reviewed-by: Matt Roper matthew.d.ro...@intel.com
 
  Patches are missing the Testcase: tag, please add that. Also, are all the
  igt committed or not yet? Pulled these two in meanwhile.
 
  Things are going somewhat broken because you didn't apply my plane
  state stuff. Hmm. Actually it sort of looks that it might work by luck
  as long as the primary plane doesn't get clipped since this is bashing
  the user state directly into the hardware registers and not the derived
  state (ie. clipped coordinates).
 
  I was hoping your changes get merged, but not sure why they are held up.
 
  Also I see my review comment about the 90 vs. 270 rotation mixup was
  ignored at least.
 
  I never really got the to understand the need of reversing 90 and 270 :)
  The last discussion was not concluded, AFAIR.
  Things look correct to me and work fine with the testcase also.
  Is there something completely different which I am unable to get?
 
  BSpec gives me the impression the hw rotation is cw, whereas the drm one
  is ccw.
 
 Yes, HW rotation is cw as per bspec. In drm, where do we consider it as 
 anti-cw? I assume it is cw because all the macros work as expected, ie cw.

The ccw rule was inherited from XRandR. I'm not sure what macros you
mean, but drm_rect_rotate() will certainly give you wrong results if
you assume cw.

 
 
  -Sonika
 
  -Daniel
 
  ---
 drivers/gpu/drm/i915/i915_reg.h   |2 +
 drivers/gpu/drm/i915/intel_atomic_plane.c |   24 
 drivers/gpu/drm/i915/intel_display.c  |   88 
  -
 drivers/gpu/drm/i915/intel_drv.h  |6 ++
 drivers/gpu/drm/i915/intel_sprite.c   |   52 -
 5 files changed, 131 insertions(+), 41 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index b522eb6..564bbd5 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
 #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY (  3  4)
 #define   PLANE_CTL_ROTATE_MASK  0x3
 #define   PLANE_CTL_ROTATE_0 0x0
  +#define   PLANE_CTL_ROTATE_90  0x1
 #define   PLANE_CTL_ROTATE_180   0x2
  +#define   PLANE_CTL_ROTATE_270 0x3
 #define _PLANE_STRIDE_1_A0x70188
 #define _PLANE_STRIDE_2_A0x70288
 #define _PLANE_STRIDE_3_A0x70388
  diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
  b/drivers/gpu/drm/i915/intel_atomic_plane.c
  index 976b891..a27ee8c 100644
  --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
  +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
  @@ -162,6 +162,30 @@ static int intel_plane_atomic_check(struct 
  drm_plane *plane,
  (1  drm_plane_index(plane));
  }
 
  +   if (state-fb  intel_rotation_90_or_270(state-rotation)) {
  +   if (!(state-fb-modifier[0] == I915_FORMAT_MOD_Y_TILED 
  ||
  +   state-fb-modifier[0] == 
  I915_FORMAT_MOD_Yf_TILED)) {
  +   DRM_DEBUG_KMS(Y/Yf tiling required for 
  90/270!\n);
  +   return -EINVAL;
  +   }
  +
  +   /*
  +* 90/270 is not allowed with RGB64 16:16:16:16,
  +* RGB 16-bit 5:6:5, and Indexed 8-bit.
  +* TBD: Add RGB64 case once its added in supported 
  format list.
  +*/
  +   switch (state-fb-pixel_format) {
  +   case DRM_FORMAT_C8:
  +   case DRM_FORMAT_RGB565:
  +   DRM_DEBUG_KMS(Unsupported pixel format %s for 
  90/270!\n,
  +   
  drm_get_format_name(state-fb-pixel_format));
  +   return -EINVAL;
  +
  +   default:
  +   break;
  +   }
  +   }
  +

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-13 Thread Damien Lespiau 
On Mon, Apr 13, 2015 at 03:53:22PM +0530, Jindal, Sonika wrote:
 I never really got the to understand the need of reversing 90 and 270 :)
 The last discussion was not concluded, AFAIR.
 Things look correct to me and work fine with the testcase also.
 Is there something completely different which I am unable to get?
 
 BSpec gives me the impression the hw rotation is cw, whereas the drm one
 is ccw.
 
 Yes, HW rotation is cw as per bspec. In drm, where do we consider it as
 anti-cw? I assume it is cw because all the macros work as expected, ie cw.

The DRM is quite a direct implementation of the X semantics:

http://cgit.freedesktop.org/xorg/proto/randrproto/tree/randrproto.txt

In Randr, the coordinate system is rotated in a counter-clockwise direction
relative to the normal orientation

This matches the usual orientation of the trigonometric circle.

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-13 Thread Jindal, Sonika 



On 4/13/2015 3:40 PM, Ville Syrjälä wrote:

On Mon, Apr 13, 2015 at 09:36:02AM +0530, Jindal, Sonika wrote:



On 4/10/2015 8:14 PM, Ville Syrjälä wrote:

On Fri, Apr 10, 2015 at 04:17:17PM +0200, Daniel Vetter wrote:

On Fri, Apr 10, 2015 at 02:37:29PM +0530, Sonika Jindal wrote:

v2: Moving creation of property in a function, checking for 90/270
rotation simultaneously (Chris)
Letting primary plane to be positioned
v3: Adding if/else for 90/270 and rest params programming, adding check for
pixel_format, some cleanup (review comments)
v4: Adding right pixel_formats, using src_* params instead of crtc_* for offset
and size programming (Ville)
v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
v6: Rebased on -nightly (Tvrtko's series merged)
v7: Moving pixel_format check to intel_atomic_plane_check (Matt)

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
Reviewed-by: Matt Roper matthew.d.ro...@intel.com


Patches are missing the Testcase: tag, please add that. Also, are all the
igt committed or not yet? Pulled these two in meanwhile.


Things are going somewhat broken because you didn't apply my plane
state stuff. Hmm. Actually it sort of looks that it might work by luck
as long as the primary plane doesn't get clipped since this is bashing
the user state directly into the hardware registers and not the derived
state (ie. clipped coordinates).


I was hoping your changes get merged, but not sure why they are held up.


Also I see my review comment about the 90 vs. 270 rotation mixup was
ignored at least.


I never really got the to understand the need of reversing 90 and 270 :)
The last discussion was not concluded, AFAIR.
Things look correct to me and work fine with the testcase also.
Is there something completely different which I am unable to get?


BSpec gives me the impression the hw rotation is cw, whereas the drm one
is ccw.

Yes, HW rotation is cw as per bspec. In drm, where do we consider it as 
anti-cw? I assume it is cw because all the macros work as expected, ie cw.




-Sonika


-Daniel


---
   drivers/gpu/drm/i915/i915_reg.h   |2 +
   drivers/gpu/drm/i915/intel_atomic_plane.c |   24 
   drivers/gpu/drm/i915/intel_display.c  |   88 
-
   drivers/gpu/drm/i915/intel_drv.h  |6 ++
   drivers/gpu/drm/i915/intel_sprite.c   |   52 -
   5 files changed, 131 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..564bbd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
   #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY (  3  4)
   #define   PLANE_CTL_ROTATE_MASK  0x3
   #define   PLANE_CTL_ROTATE_0 0x0
+#define   PLANE_CTL_ROTATE_90  0x1
   #define   PLANE_CTL_ROTATE_180   0x2
+#define   PLANE_CTL_ROTATE_270 0x3
   #define _PLANE_STRIDE_1_A0x70188
   #define _PLANE_STRIDE_2_A0x70288
   #define _PLANE_STRIDE_3_A0x70388
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 976b891..a27ee8c 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -162,6 +162,30 @@ static int intel_plane_atomic_check(struct drm_plane 
*plane,
(1  drm_plane_index(plane));
}

+   if (state-fb  intel_rotation_90_or_270(state-rotation)) {
+   if (!(state-fb-modifier[0] == I915_FORMAT_MOD_Y_TILED ||
+   state-fb-modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
+   DRM_DEBUG_KMS(Y/Yf tiling required for 90/270!\n);
+   return -EINVAL;
+   }
+
+   /*
+* 90/270 is not allowed with RGB64 16:16:16:16,
+* RGB 16-bit 5:6:5, and Indexed 8-bit.
+* TBD: Add RGB64 case once its added in supported format list.
+*/
+   switch (state-fb-pixel_format) {
+   case DRM_FORMAT_C8:
+   case DRM_FORMAT_RGB565:
+   DRM_DEBUG_KMS(Unsupported pixel format %s for 
90/270!\n,
+   
drm_get_format_name(state-fb-pixel_format));
+   return -EINVAL;
+
+   default:
+   break;
+   }
+   }
+
return intel_plane-check_plane(plane, intel_state);
   }

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f0bbc22..4de544c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2311,13 +2311,6 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, 
struct drm_framebuffer *fb,
info-pitch = fb-pitches[0];
info-fb_modifier =

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-12 Thread Jindal, Sonika 



On 4/10/2015 7:47 PM, Daniel Vetter wrote:

On Fri, Apr 10, 2015 at 02:37:29PM +0530, Sonika Jindal wrote:

v2: Moving creation of property in a function, checking for 90/270
rotation simultaneously (Chris)
Letting primary plane to be positioned
v3: Adding if/else for 90/270 and rest params programming, adding check for
pixel_format, some cleanup (review comments)
v4: Adding right pixel_formats, using src_* params instead of crtc_* for offset
and size programming (Ville)
v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
v6: Rebased on -nightly (Tvrtko's series merged)
v7: Moving pixel_format check to intel_atomic_plane_check (Matt)

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
Reviewed-by: Matt Roper matthew.d.ro...@intel.com


Patches are missing the Testcase: tag, please add that. Also, are all the
igt committed or not yet? Pulled these two in meanwhile.
-Daniel
The kms_rotation_crc is updated for 90/270 tests and is posted on the 
mailing list. Not sure if an r-b is a must for tests too?

-Sonika



---
  drivers/gpu/drm/i915/i915_reg.h   |2 +
  drivers/gpu/drm/i915/intel_atomic_plane.c |   24 
  drivers/gpu/drm/i915/intel_display.c  |   88 -
  drivers/gpu/drm/i915/intel_drv.h  |6 ++
  drivers/gpu/drm/i915/intel_sprite.c   |   52 -
  5 files changed, 131 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..564bbd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
  #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY  (  3  4)
  #define   PLANE_CTL_ROTATE_MASK   0x3
  #define   PLANE_CTL_ROTATE_0  0x0
+#define   PLANE_CTL_ROTATE_90  0x1
  #define   PLANE_CTL_ROTATE_1800x2
+#define   PLANE_CTL_ROTATE_270 0x3
  #define _PLANE_STRIDE_1_A 0x70188
  #define _PLANE_STRIDE_2_A 0x70288
  #define _PLANE_STRIDE_3_A 0x70388
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 976b891..a27ee8c 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -162,6 +162,30 @@ static int intel_plane_atomic_check(struct drm_plane 
*plane,
(1  drm_plane_index(plane));
}

+   if (state-fb  intel_rotation_90_or_270(state-rotation)) {
+   if (!(state-fb-modifier[0] == I915_FORMAT_MOD_Y_TILED ||
+   state-fb-modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
+   DRM_DEBUG_KMS(Y/Yf tiling required for 90/270!\n);
+   return -EINVAL;
+   }
+
+   /*
+* 90/270 is not allowed with RGB64 16:16:16:16,
+* RGB 16-bit 5:6:5, and Indexed 8-bit.
+* TBD: Add RGB64 case once its added in supported format list.
+*/
+   switch (state-fb-pixel_format) {
+   case DRM_FORMAT_C8:
+   case DRM_FORMAT_RGB565:
+   DRM_DEBUG_KMS(Unsupported pixel format %s for 
90/270!\n,
+   
drm_get_format_name(state-fb-pixel_format));
+   return -EINVAL;
+
+   default:
+   break;
+   }
+   }
+
return intel_plane-check_plane(plane, intel_state);
  }

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f0bbc22..4de544c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2311,13 +2311,6 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, 
struct drm_framebuffer *fb,
info-pitch = fb-pitches[0];
info-fb_modifier = fb-modifier[0];

-   if (!(info-fb_modifier == I915_FORMAT_MOD_Y_TILED ||
- info-fb_modifier == I915_FORMAT_MOD_Yf_TILED)) {
-   DRM_DEBUG_KMS(
- Y or Yf tiling is needed for 90/270 
rotation!\n);
-   return -EINVAL;
-   }
-
return 0;
  }

@@ -2919,8 +2912,12 @@ static void skylake_update_primary_plane(struct drm_crtc 
*crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_i915_gem_object *obj;
int pipe = intel_crtc-pipe;
-   u32 plane_ctl, stride_div;
+   u32 plane_ctl, stride_div, stride;
+   u32 tile_height, plane_offset, plane_size;
+   unsigned int rotation;
+   int x_offset, y_offset;
unsigned long surf_addr;
+   struct drm_plane *plane;

if (!intel_crtc-primary_enabled) {
I915_WRITE(PLANE_CTL(pipe, 0), 0);
@@ -2981,21 +2978,51 @@ static void skylake_update_primary_plane(struct 
drm_crtc *crtc,
}

plane_ctl |=

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-12 Thread Jindal, Sonika 



On 4/10/2015 8:14 PM, Ville Syrjälä wrote:

On Fri, Apr 10, 2015 at 04:17:17PM +0200, Daniel Vetter wrote:

On Fri, Apr 10, 2015 at 02:37:29PM +0530, Sonika Jindal wrote:

v2: Moving creation of property in a function, checking for 90/270
rotation simultaneously (Chris)
Letting primary plane to be positioned
v3: Adding if/else for 90/270 and rest params programming, adding check for
pixel_format, some cleanup (review comments)
v4: Adding right pixel_formats, using src_* params instead of crtc_* for offset
and size programming (Ville)
v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
v6: Rebased on -nightly (Tvrtko's series merged)
v7: Moving pixel_format check to intel_atomic_plane_check (Matt)

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
Reviewed-by: Matt Roper matthew.d.ro...@intel.com


Patches are missing the Testcase: tag, please add that. Also, are all the
igt committed or not yet? Pulled these two in meanwhile.


Things are going somewhat broken because you didn't apply my plane
state stuff. Hmm. Actually it sort of looks that it might work by luck
as long as the primary plane doesn't get clipped since this is bashing
the user state directly into the hardware registers and not the derived
state (ie. clipped coordinates).


I was hoping your changes get merged, but not sure why they are held up.


Also I see my review comment about the 90 vs. 270 rotation mixup was
ignored at least.

I never really got the to understand the need of reversing 90 and 270 :) 
The last discussion was not concluded, AFAIR.

Things look correct to me and work fine with the testcase also.
Is there something completely different which I am unable to get?

-Sonika


-Daniel


---
  drivers/gpu/drm/i915/i915_reg.h   |2 +
  drivers/gpu/drm/i915/intel_atomic_plane.c |   24 
  drivers/gpu/drm/i915/intel_display.c  |   88 -
  drivers/gpu/drm/i915/intel_drv.h  |6 ++
  drivers/gpu/drm/i915/intel_sprite.c   |   52 -
  5 files changed, 131 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..564bbd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
  #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY  (  3  4)
  #define   PLANE_CTL_ROTATE_MASK   0x3
  #define   PLANE_CTL_ROTATE_0  0x0
+#define   PLANE_CTL_ROTATE_90  0x1
  #define   PLANE_CTL_ROTATE_1800x2
+#define   PLANE_CTL_ROTATE_270 0x3
  #define _PLANE_STRIDE_1_A 0x70188
  #define _PLANE_STRIDE_2_A 0x70288
  #define _PLANE_STRIDE_3_A 0x70388
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 976b891..a27ee8c 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -162,6 +162,30 @@ static int intel_plane_atomic_check(struct drm_plane 
*plane,
(1  drm_plane_index(plane));
}

+   if (state-fb  intel_rotation_90_or_270(state-rotation)) {
+   if (!(state-fb-modifier[0] == I915_FORMAT_MOD_Y_TILED ||
+   state-fb-modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
+   DRM_DEBUG_KMS(Y/Yf tiling required for 90/270!\n);
+   return -EINVAL;
+   }
+
+   /*
+* 90/270 is not allowed with RGB64 16:16:16:16,
+* RGB 16-bit 5:6:5, and Indexed 8-bit.
+* TBD: Add RGB64 case once its added in supported format list.
+*/
+   switch (state-fb-pixel_format) {
+   case DRM_FORMAT_C8:
+   case DRM_FORMAT_RGB565:
+   DRM_DEBUG_KMS(Unsupported pixel format %s for 
90/270!\n,
+   
drm_get_format_name(state-fb-pixel_format));
+   return -EINVAL;
+
+   default:
+   break;
+   }
+   }
+
return intel_plane-check_plane(plane, intel_state);
  }

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f0bbc22..4de544c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2311,13 +2311,6 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, 
struct drm_framebuffer *fb,
info-pitch = fb-pitches[0];
info-fb_modifier = fb-modifier[0];

-   if (!(info-fb_modifier == I915_FORMAT_MOD_Y_TILED ||
- info-fb_modifier == I915_FORMAT_MOD_Yf_TILED)) {
-   DRM_DEBUG_KMS(
- Y or Yf tiling is needed for 90/270 
rotation!\n);
-   return -EINVAL;
-   }
-
return 0;
  }

@@ -2919,8 +2912,12 @@ static

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-07 Thread Jindal, Sonika 



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

On Thu, Apr 02, 2015 at 08:59:33AM -0700, Matt Roper wrote:

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

I am not sure how it will help. drm_plane_check_pixel_format should
be used to check the pixel format of the fb which we should be doing
in some -check functions (I don't think we do that right now?)
against what is supported by the plane.
But to check for the formats which are allowed for 90/270, we would
need this kind of explicit check.


Right, I guess there are two aspects here.  First, we need to properly
test for acceptable pixel formats for the plane in general; at the
moment the DRM core setplane() tests this, but if we use the atomic
ioctl it never gets checked (which is a bug).  So as you say, we need a
test in a _check() function to verify this.  We probably also need to
add an i-g-t test for it too.


The core atomic ioctl does check for valid plane pixel formats for you.
And there shouldn't be any other entrypoint (except internal ones, but
that's ok). Is there a bug left?

Oh, yes the check is there. Will abandon that check then and just move 
the 90/270 pixel format check to intel_plane_atomic_check.


Regards,
Sonika

Definitely agree that testcases would be nice.
-Daniel


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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-07 Thread Daniel Vetter 
On Thu, Apr 02, 2015 at 08:59:33AM -0700, Matt Roper wrote:
 On Thu, Apr 02, 2015 at 10:24:02AM +0530, Jindal, Sonika wrote:
  I am not sure how it will help. drm_plane_check_pixel_format should
  be used to check the pixel format of the fb which we should be doing
  in some -check functions (I don't think we do that right now?)
  against what is supported by the plane.
  But to check for the formats which are allowed for 90/270, we would
  need this kind of explicit check.
 
 Right, I guess there are two aspects here.  First, we need to properly
 test for acceptable pixel formats for the plane in general; at the
 moment the DRM core setplane() tests this, but if we use the atomic
 ioctl it never gets checked (which is a bug).  So as you say, we need a
 test in a _check() function to verify this.  We probably also need to
 add an i-g-t test for it too.

The core atomic ioctl does check for valid plane pixel formats for you.
And there shouldn't be any other entrypoint (except internal ones, but
that's ok). Is there a bug left?

Definitely agree that testcases would be nice.
-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 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-06 Thread Jindal, Sonika 



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

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



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

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

v2: Moving creation of property in a function, checking for 90/270
rotation simultaneously (Chris)
Letting primary plane to be positioned
v3: Adding if/else for 90/270 and rest params programming, adding check for
pixel_format, some cleanup (review comments)
v4: Adding right pixel_formats, using src_* params instead of crtc_* for offset
and size programming (Ville)
v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
v6: Rebased on -nightly (Tvrtko's series merged)

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
  drivers/gpu/drm/i915/i915_reg.h  |2 +
  drivers/gpu/drm/i915/intel_display.c |  103 +++---
  drivers/gpu/drm/i915/intel_drv.h |6 ++
  drivers/gpu/drm/i915/intel_sprite.c  |   52 -
  4 files changed, 129 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..564bbd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
  #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY  (  3  4)
  #define   PLANE_CTL_ROTATE_MASK   0x3
  #define   PLANE_CTL_ROTATE_0  0x0
+#define   PLANE_CTL_ROTATE_90  0x1
  #define   PLANE_CTL_ROTATE_1800x2
+#define   PLANE_CTL_ROTATE_270 0x3
  #define _PLANE_STRIDE_1_A 0x70188
  #define _PLANE_STRIDE_2_A 0x70288
  #define _PLANE_STRIDE_3_A 0x70388
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f0bbc22..86ee0f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2318,6 +2318,28 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, 
struct drm_framebuffer *fb,
return -EINVAL;
}

+   switch (fb-pixel_format) {
+   case DRM_FORMAT_XRGB:
+   case DRM_FORMAT_ARGB:
+   case DRM_FORMAT_XBGR:
+   case DRM_FORMAT_ABGR:
+   case DRM_FORMAT_XRGB2101010:
+   case DRM_FORMAT_ARGB2101010:
+   case DRM_FORMAT_XBGR2101010:
+   case DRM_FORMAT_ABGR2101010:
+   case DRM_FORMAT_YUYV:
+   case DRM_FORMAT_YVYU:
+   case DRM_FORMAT_UYVY:
+   case DRM_FORMAT_VYUY:
+   case DRM_FORMAT_NV12:
+   break;
+
+   default:
+   DRM_DEBUG_KMS(Unsupported pixel format:%d for 90/270 
rotation!\n,
+ fb-pixel_format);
+   return -EINVAL;
+   }


Shouldn't we be matching against the list of formats the plane supports
(which may vary by platform, or by specific plane) rather than this
generic list?  We already specified what formats the plane can handle at
plane init time, so it seems like what you'd really want is just a call
to

 drm_plane_check_pixel_format(plane_state-plane, fb-pixel_format)

then follow that up with explicit checks to exclude any formats that we
can handle in 0/180, but not in 90/270.


I am not sure how it will help. drm_plane_check_pixel_format should
be used to check the pixel format of the fb which we should be doing
in some -check functions (I don't think we do that right now?)
against what is supported by the plane.
But to check for the formats which are allowed for 90/270, we would
need this kind of explicit check.


Right, I guess there are two aspects here.  First, we need to properly
test for acceptable pixel formats for the plane in general; at the
moment the DRM core setplane() tests this, but if we use the atomic
ioctl it never gets checked (which is a bug).  So as you say, we need a
test in a _check() function to verify this.  We probably also need to
add an i-g-t test for it too.


OK, I'l add that check in intel_atomic_plane_check()


Once we know that the pixel format is valid in general, it makes sense
to have a simpler test to reject some subset of those formats iff we
notice we're doing 90/270 rotation.  Maybe it's not really a big deal,
but it seems like that's a little easier to understand and verify than
having two completely separate lists, especially when future platforms
may support different formats, or even different planes of the same
platform have varying pixel format capabilities.

I will send out a patch with the changes and add negative test case in 
kms_rotation_crc.


Regards,
Sonika



Matt


I'd also move this check to intel_plane_atomic_check(), since the
'check' code path is where I'd usually go looking for these types of
checks; the function you've got it in at the moment gets called from the
'prepare' step which works as well, but seems a bit less obvious.


Yes, I agree, but this is on top of Tvrtko's patch for

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-02 Thread Tvrtko Ursulin 


Hi,

On 04/02/2015 05:54 AM, Jindal, Sonika wrote:

diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f0bbc22..86ee0f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2318,6 +2318,28 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view
*view, struct drm_framebuffer *fb,
  return -EINVAL;
  }

+switch (fb-pixel_format) {
+case DRM_FORMAT_XRGB:
+case DRM_FORMAT_ARGB:
+case DRM_FORMAT_XBGR:
+case DRM_FORMAT_ABGR:
+case DRM_FORMAT_XRGB2101010:
+case DRM_FORMAT_ARGB2101010:
+case DRM_FORMAT_XBGR2101010:
+case DRM_FORMAT_ABGR2101010:
+case DRM_FORMAT_YUYV:
+case DRM_FORMAT_YVYU:
+case DRM_FORMAT_UYVY:
+case DRM_FORMAT_VYUY:
+case DRM_FORMAT_NV12:
+break;
+
+default:
+DRM_DEBUG_KMS(Unsupported pixel format:%d for 90/270
rotation!\n,
+  fb-pixel_format);
+return -EINVAL;
+}


Shouldn't we be matching against the list of formats the plane supports
(which may vary by platform, or by specific plane) rather than this
generic list?  We already specified what formats the plane can handle at
plane init time, so it seems like what you'd really want is just a call
to

 drm_plane_check_pixel_format(plane_state-plane, fb-pixel_format)

then follow that up with explicit checks to exclude any formats that we
can handle in 0/180, but not in 90/270.


I am not sure how it will help. drm_plane_check_pixel_format should be
used to check the pixel format of the fb which we should be doing in
some -check functions (I don't think we do that right now?) against what
is supported by the plane.
But to check for the formats which are allowed for 90/270, we would need
this kind of explicit check.


I'd also move this check to intel_plane_atomic_check(), since the
'check' code path is where I'd usually go looking for these types of
checks; the function you've got it in at the moment gets called from the
'prepare' step which works as well, but seems a bit less obvious.


Yes, I agree, but this is on top of Tvrtko's patch for secondary buffer
mapping where based upon tiling and pixel format we are allowing the
rotated gtt.

Tvrtko,
Can these be moved to the intel_plane_atomic_check()


Good point, I think it can and should. I suppose it was just an 
oversight during endless rebasing, that I put the Y tiling check in 
there. So you can move that part as well while you are doing it.


Also highlights the fact we have no negative testing in kms_rotation_crc 
for this. I mean, trying to rotate by 90/270 linear or X tiled, or wrong 
pixel format.


Regards,

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-02 Thread Matt Roper 
On Thu, Apr 02, 2015 at 10:24:02AM +0530, Jindal, Sonika wrote:
 
 
 On 4/1/2015 11:52 PM, Matt Roper wrote:
 On Mon, Mar 30, 2015 at 02:04:57PM +0530, Sonika Jindal wrote:
 v2: Moving creation of property in a function, checking for 90/270
 rotation simultaneously (Chris)
 Letting primary plane to be positioned
 v3: Adding if/else for 90/270 and rest params programming, adding check for
 pixel_format, some cleanup (review comments)
 v4: Adding right pixel_formats, using src_* params instead of crtc_* for 
 offset
 and size programming (Ville)
 v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
 v6: Rebased on -nightly (Tvrtko's series merged)
 
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com
 ---
   drivers/gpu/drm/i915/i915_reg.h  |2 +
   drivers/gpu/drm/i915/intel_display.c |  103 
  +++---
   drivers/gpu/drm/i915/intel_drv.h |6 ++
   drivers/gpu/drm/i915/intel_sprite.c  |   52 -
   4 files changed, 129 insertions(+), 34 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h 
 b/drivers/gpu/drm/i915/i915_reg.h
 index b522eb6..564bbd5 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
   #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY  (  3  4)
   #define   PLANE_CTL_ROTATE_MASK   0x3
   #define   PLANE_CTL_ROTATE_0  0x0
 +#define   PLANE_CTL_ROTATE_90  0x1
   #define   PLANE_CTL_ROTATE_1800x2
 +#define   PLANE_CTL_ROTATE_270 0x3
   #define _PLANE_STRIDE_1_A 0x70188
   #define _PLANE_STRIDE_2_A 0x70288
   #define _PLANE_STRIDE_3_A 0x70388
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index f0bbc22..86ee0f0 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2318,6 +2318,28 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, 
 struct drm_framebuffer *fb,
 return -EINVAL;
 }
 
 +   switch (fb-pixel_format) {
 +   case DRM_FORMAT_XRGB:
 +   case DRM_FORMAT_ARGB:
 +   case DRM_FORMAT_XBGR:
 +   case DRM_FORMAT_ABGR:
 +   case DRM_FORMAT_XRGB2101010:
 +   case DRM_FORMAT_ARGB2101010:
 +   case DRM_FORMAT_XBGR2101010:
 +   case DRM_FORMAT_ABGR2101010:
 +   case DRM_FORMAT_YUYV:
 +   case DRM_FORMAT_YVYU:
 +   case DRM_FORMAT_UYVY:
 +   case DRM_FORMAT_VYUY:
 +   case DRM_FORMAT_NV12:
 +   break;
 +
 +   default:
 +   DRM_DEBUG_KMS(Unsupported pixel format:%d for 90/270 
 rotation!\n,
 + fb-pixel_format);
 +   return -EINVAL;
 +   }
 
 Shouldn't we be matching against the list of formats the plane supports
 (which may vary by platform, or by specific plane) rather than this
 generic list?  We already specified what formats the plane can handle at
 plane init time, so it seems like what you'd really want is just a call
 to
 
  drm_plane_check_pixel_format(plane_state-plane, fb-pixel_format)
 
 then follow that up with explicit checks to exclude any formats that we
 can handle in 0/180, but not in 90/270.
 
 I am not sure how it will help. drm_plane_check_pixel_format should
 be used to check the pixel format of the fb which we should be doing
 in some -check functions (I don't think we do that right now?)
 against what is supported by the plane.
 But to check for the formats which are allowed for 90/270, we would
 need this kind of explicit check.

Right, I guess there are two aspects here.  First, we need to properly
test for acceptable pixel formats for the plane in general; at the
moment the DRM core setplane() tests this, but if we use the atomic
ioctl it never gets checked (which is a bug).  So as you say, we need a
test in a _check() function to verify this.  We probably also need to
add an i-g-t test for it too.

Once we know that the pixel format is valid in general, it makes sense
to have a simpler test to reject some subset of those formats iff we
notice we're doing 90/270 rotation.  Maybe it's not really a big deal,
but it seems like that's a little easier to understand and verify than
having two completely separate lists, especially when future platforms
may support different formats, or even different planes of the same
platform have varying pixel format capabilities.


Matt

 I'd also move this check to intel_plane_atomic_check(), since the
 'check' code path is where I'd usually go looking for these types of
 checks; the function you've got it in at the moment gets called from the
 'prepare' step which works as well, but seems a bit less obvious.
 
 Yes, I agree, but this is on top of Tvrtko's patch for secondary
 buffer mapping where based upon tiling and pixel format we are
 allowing the rotated gtt.
 
 Tvrtko,
 Can these be moved to the intel_plane_atomic_check()
 
 +
 return 0;
   }
 
 @@ -2919,8 +2941,12 @@ static

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-01 Thread Jindal, Sonika 



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

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

v2: Moving creation of property in a function, checking for 90/270
rotation simultaneously (Chris)
Letting primary plane to be positioned
v3: Adding if/else for 90/270 and rest params programming, adding check for
pixel_format, some cleanup (review comments)
v4: Adding right pixel_formats, using src_* params instead of crtc_* for offset
and size programming (Ville)
v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
v6: Rebased on -nightly (Tvrtko's series merged)

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
  drivers/gpu/drm/i915/i915_reg.h  |2 +
  drivers/gpu/drm/i915/intel_display.c |  103 +++---
  drivers/gpu/drm/i915/intel_drv.h |6 ++
  drivers/gpu/drm/i915/intel_sprite.c  |   52 -
  4 files changed, 129 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..564bbd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
  #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY  (  3  4)
  #define   PLANE_CTL_ROTATE_MASK   0x3
  #define   PLANE_CTL_ROTATE_0  0x0
+#define   PLANE_CTL_ROTATE_90  0x1
  #define   PLANE_CTL_ROTATE_1800x2
+#define   PLANE_CTL_ROTATE_270 0x3
  #define _PLANE_STRIDE_1_A 0x70188
  #define _PLANE_STRIDE_2_A 0x70288
  #define _PLANE_STRIDE_3_A 0x70388
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f0bbc22..86ee0f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2318,6 +2318,28 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, 
struct drm_framebuffer *fb,
return -EINVAL;
}

+   switch (fb-pixel_format) {
+   case DRM_FORMAT_XRGB:
+   case DRM_FORMAT_ARGB:
+   case DRM_FORMAT_XBGR:
+   case DRM_FORMAT_ABGR:
+   case DRM_FORMAT_XRGB2101010:
+   case DRM_FORMAT_ARGB2101010:
+   case DRM_FORMAT_XBGR2101010:
+   case DRM_FORMAT_ABGR2101010:
+   case DRM_FORMAT_YUYV:
+   case DRM_FORMAT_YVYU:
+   case DRM_FORMAT_UYVY:
+   case DRM_FORMAT_VYUY:
+   case DRM_FORMAT_NV12:
+   break;
+
+   default:
+   DRM_DEBUG_KMS(Unsupported pixel format:%d for 90/270 
rotation!\n,
+ fb-pixel_format);
+   return -EINVAL;
+   }


Shouldn't we be matching against the list of formats the plane supports
(which may vary by platform, or by specific plane) rather than this
generic list?  We already specified what formats the plane can handle at
plane init time, so it seems like what you'd really want is just a call
to

 drm_plane_check_pixel_format(plane_state-plane, fb-pixel_format)

then follow that up with explicit checks to exclude any formats that we
can handle in 0/180, but not in 90/270.

I am not sure how it will help. drm_plane_check_pixel_format should be 
used to check the pixel format of the fb which we should be doing in 
some -check functions (I don't think we do that right now?) against what 
is supported by the plane.
But to check for the formats which are allowed for 90/270, we would need 
this kind of explicit check.



I'd also move this check to intel_plane_atomic_check(), since the
'check' code path is where I'd usually go looking for these types of
checks; the function you've got it in at the moment gets called from the
'prepare' step which works as well, but seems a bit less obvious.

Yes, I agree, but this is on top of Tvrtko's patch for secondary buffer 
mapping where based upon tiling and pixel format we are allowing the 
rotated gtt.


Tvrtko,
Can these be moved to the intel_plane_atomic_check()


+
return 0;
  }

@@ -2919,8 +2941,12 @@ static void skylake_update_primary_plane(struct drm_crtc 
*crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_i915_gem_object *obj;
int pipe = intel_crtc-pipe;
-   u32 plane_ctl, stride_div;
+   u32 plane_ctl, stride_div, stride;
+   u32 tile_height, plane_offset, plane_size;
+   unsigned int rotation;
+   int x_offset, y_offset;
unsigned long surf_addr;
+   struct drm_plane *plane;

if (!intel_crtc-primary_enabled) {
I915_WRITE(PLANE_CTL(pipe, 0), 0);
@@ -2981,21 +3007,51 @@ static void skylake_update_primary_plane(struct 
drm_crtc *crtc,
}

plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
-   if (crtc-primary-state-rotation == BIT(DRM_ROTATE_180))
+
+   plane = crtc-primary;
+   rotation = plane-state-rotation;
+   switch (rotation) {
+   case BIT(DRM_ROTATE_90):
+

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-04-01 Thread Matt Roper 
On Mon, Mar 30, 2015 at 02:04:57PM +0530, Sonika Jindal wrote:
 v2: Moving creation of property in a function, checking for 90/270
 rotation simultaneously (Chris)
 Letting primary plane to be positioned
 v3: Adding if/else for 90/270 and rest params programming, adding check for
 pixel_format, some cleanup (review comments)
 v4: Adding right pixel_formats, using src_* params instead of crtc_* for 
 offset
 and size programming (Ville)
 v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
 v6: Rebased on -nightly (Tvrtko's series merged)
 
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com
 ---
  drivers/gpu/drm/i915/i915_reg.h  |2 +
  drivers/gpu/drm/i915/intel_display.c |  103 
 +++---
  drivers/gpu/drm/i915/intel_drv.h |6 ++
  drivers/gpu/drm/i915/intel_sprite.c  |   52 -
  4 files changed, 129 insertions(+), 34 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index b522eb6..564bbd5 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
  #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY (  3  4)
  #define   PLANE_CTL_ROTATE_MASK  0x3
  #define   PLANE_CTL_ROTATE_0 0x0
 +#define   PLANE_CTL_ROTATE_900x1
  #define   PLANE_CTL_ROTATE_180   0x2
 +#define   PLANE_CTL_ROTATE_270   0x3
  #define _PLANE_STRIDE_1_A0x70188
  #define _PLANE_STRIDE_2_A0x70288
  #define _PLANE_STRIDE_3_A0x70388
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index f0bbc22..86ee0f0 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2318,6 +2318,28 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, 
 struct drm_framebuffer *fb,
   return -EINVAL;
   }
  
 + switch (fb-pixel_format) {
 + case DRM_FORMAT_XRGB:
 + case DRM_FORMAT_ARGB:
 + case DRM_FORMAT_XBGR:
 + case DRM_FORMAT_ABGR:
 + case DRM_FORMAT_XRGB2101010:
 + case DRM_FORMAT_ARGB2101010:
 + case DRM_FORMAT_XBGR2101010:
 + case DRM_FORMAT_ABGR2101010:
 + case DRM_FORMAT_YUYV:
 + case DRM_FORMAT_YVYU:
 + case DRM_FORMAT_UYVY:
 + case DRM_FORMAT_VYUY:
 + case DRM_FORMAT_NV12:
 + break;
 +
 + default:
 + DRM_DEBUG_KMS(Unsupported pixel format:%d for 90/270 
 rotation!\n,
 +   fb-pixel_format);
 + return -EINVAL;
 + }

Shouldn't we be matching against the list of formats the plane supports
(which may vary by platform, or by specific plane) rather than this
generic list?  We already specified what formats the plane can handle at
plane init time, so it seems like what you'd really want is just a call
to

drm_plane_check_pixel_format(plane_state-plane, fb-pixel_format)

then follow that up with explicit checks to exclude any formats that we
can handle in 0/180, but not in 90/270.

I'd also move this check to intel_plane_atomic_check(), since the
'check' code path is where I'd usually go looking for these types of
checks; the function you've got it in at the moment gets called from the
'prepare' step which works as well, but seems a bit less obvious.

 +
   return 0;
  }
  
 @@ -2919,8 +2941,12 @@ static void skylake_update_primary_plane(struct 
 drm_crtc *crtc,
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
   struct drm_i915_gem_object *obj;
   int pipe = intel_crtc-pipe;
 - u32 plane_ctl, stride_div;
 + u32 plane_ctl, stride_div, stride;
 + u32 tile_height, plane_offset, plane_size;
 + unsigned int rotation;
 + int x_offset, y_offset;
   unsigned long surf_addr;
 + struct drm_plane *plane;
  
   if (!intel_crtc-primary_enabled) {
   I915_WRITE(PLANE_CTL(pipe, 0), 0);
 @@ -2981,21 +3007,51 @@ static void skylake_update_primary_plane(struct 
 drm_crtc *crtc,
   }
  
   plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
 - if (crtc-primary-state-rotation == BIT(DRM_ROTATE_180))
 +
 + plane = crtc-primary;
 + rotation = plane-state-rotation;
 + switch (rotation) {
 + case BIT(DRM_ROTATE_90):
 + plane_ctl |= PLANE_CTL_ROTATE_90;
 + break;
 +
 + case BIT(DRM_ROTATE_180):
   plane_ctl |= PLANE_CTL_ROTATE_180;
 + break;
 +
 + case BIT(DRM_ROTATE_270):
 + plane_ctl |= PLANE_CTL_ROTATE_270;
 + break;
 + }
  
   obj = intel_fb_obj(fb);
   stride_div = intel_fb_stride_alignment(dev, fb-modifier[0],
  fb-pixel_format);

Minor nit; can we move this down inside the 'else' branch to make it
apparent how this serves a parallel role to 'tile_height' from the 'if'
branch.


 -

[Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-03-30 Thread Sonika Jindal 
v2: Moving creation of property in a function, checking for 90/270
rotation simultaneously (Chris)
Letting primary plane to be positioned
v3: Adding if/else for 90/270 and rest params programming, adding check for
pixel_format, some cleanup (review comments)
v4: Adding right pixel_formats, using src_* params instead of crtc_* for offset
and size programming (Ville)
v5: Rebased on -nightly and Tvrtko's series for gtt remapping.
v6: Rebased on -nightly (Tvrtko's series merged)

Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
 drivers/gpu/drm/i915/i915_reg.h  |2 +
 drivers/gpu/drm/i915/intel_display.c |  103 +++---
 drivers/gpu/drm/i915/intel_drv.h |6 ++
 drivers/gpu/drm/i915/intel_sprite.c  |   52 -
 4 files changed, 129 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..564bbd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4854,7 +4854,9 @@ enum skl_disp_power_wells {
 #define   PLANE_CTL_ALPHA_HW_PREMULTIPLY   (  3  4)
 #define   PLANE_CTL_ROTATE_MASK0x3
 #define   PLANE_CTL_ROTATE_0   0x0
+#define   PLANE_CTL_ROTATE_90  0x1
 #define   PLANE_CTL_ROTATE_180 0x2
+#define   PLANE_CTL_ROTATE_270 0x3
 #define _PLANE_STRIDE_1_A  0x70188
 #define _PLANE_STRIDE_2_A  0x70288
 #define _PLANE_STRIDE_3_A  0x70388
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f0bbc22..86ee0f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2318,6 +2318,28 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, 
struct drm_framebuffer *fb,
return -EINVAL;
}
 
+   switch (fb-pixel_format) {
+   case DRM_FORMAT_XRGB:
+   case DRM_FORMAT_ARGB:
+   case DRM_FORMAT_XBGR:
+   case DRM_FORMAT_ABGR:
+   case DRM_FORMAT_XRGB2101010:
+   case DRM_FORMAT_ARGB2101010:
+   case DRM_FORMAT_XBGR2101010:
+   case DRM_FORMAT_ABGR2101010:
+   case DRM_FORMAT_YUYV:
+   case DRM_FORMAT_YVYU:
+   case DRM_FORMAT_UYVY:
+   case DRM_FORMAT_VYUY:
+   case DRM_FORMAT_NV12:
+   break;
+
+   default:
+   DRM_DEBUG_KMS(Unsupported pixel format:%d for 90/270 
rotation!\n,
+ fb-pixel_format);
+   return -EINVAL;
+   }
+
return 0;
 }
 
@@ -2919,8 +2941,12 @@ static void skylake_update_primary_plane(struct drm_crtc 
*crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_i915_gem_object *obj;
int pipe = intel_crtc-pipe;
-   u32 plane_ctl, stride_div;
+   u32 plane_ctl, stride_div, stride;
+   u32 tile_height, plane_offset, plane_size;
+   unsigned int rotation;
+   int x_offset, y_offset;
unsigned long surf_addr;
+   struct drm_plane *plane;
 
if (!intel_crtc-primary_enabled) {
I915_WRITE(PLANE_CTL(pipe, 0), 0);
@@ -2981,21 +3007,51 @@ static void skylake_update_primary_plane(struct 
drm_crtc *crtc,
}
 
plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
-   if (crtc-primary-state-rotation == BIT(DRM_ROTATE_180))
+
+   plane = crtc-primary;
+   rotation = plane-state-rotation;
+   switch (rotation) {
+   case BIT(DRM_ROTATE_90):
+   plane_ctl |= PLANE_CTL_ROTATE_90;
+   break;
+
+   case BIT(DRM_ROTATE_180):
plane_ctl |= PLANE_CTL_ROTATE_180;
+   break;
+
+   case BIT(DRM_ROTATE_270):
+   plane_ctl |= PLANE_CTL_ROTATE_270;
+   break;
+   }
 
obj = intel_fb_obj(fb);
stride_div = intel_fb_stride_alignment(dev, fb-modifier[0],
   fb-pixel_format);
-   surf_addr = intel_plane_obj_offset(to_intel_plane(crtc-primary), obj);
+   surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj);
+
+   if (rotation  (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {
+   /* stride = Surface height in tiles */
+   tile_height = intel_tile_height(dev, fb-bits_per_pixel,
+   fb-modifier[0]);
+   stride = DIV_ROUND_UP(fb-height, tile_height);
+   x_offset = stride * tile_height - y - (plane-state-src_h  
16);
+   y_offset = x;
+   plane_size = ((plane-state-src_w  16) - 1)  16 |
+   ((plane-state-src_h  16) - 1);
+   } else {
+   stride = fb-pitches[0] / stride_div;
+   x_offset = x;
+   y_offset = y;
+   plane_size = ((plane-state-src_h  16) - 1)  16 |
+   ((plane-state-src_w  16) - 1);
+   }
+

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation

2015-03-30 Thread shuang . he 
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 6091
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV -4  270/270  266/270
ILK  303/303  303/303
SNB -1  304/304  303/304
IVB  337/337  337/337
BYT  287/287  287/287
HSW  361/361  361/361
BDW  309/309  309/309
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
 PNV  igt@gem_userptr_blits@coherency-sync  CRASH(2)PASS(2)  CRASH(2)
*PNV  igt@gem_fence_thrash@bo-write-verify-threaded-none  PASS(2)  
FAIL(1)PASS(1)
*PNV  igt@gem_fence_thrash@bo-write-verify-x  PASS(2)  FAIL(1)PASS(1)
*PNV  igt@gem_fence_thrash@bo-write-verify-y  PASS(2)  FAIL(1)PASS(1)
*SNB  igt@kms_rotation_crc@sprite-rotation  PASS(3)  FAIL(1)PASS(1)
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx