drm/i915/skl: Allow universal planes to position
Subsystems (3)
PATCH 1/2 drm/i915/skl: Allow universal planes to position
--- drivers/gpu/drm/i915/intel_display.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/intel_display.c +6 -1
@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip; +bool can_position = false;int ret; crtc = crtc ? crtc : plane->crtc; intel_crtc =to_intel_crtc (crtc); +if (INTEL_INFO (dev)->gen >=9 ) + can_position = true; + ret =drm_plane_helper_check_update (plane, crtc, fb, src, dest, clip,DRM_PLANE_HELPER_NO_SCALING ,DRM_PLANE_HELPER_NO_SCALING , - false, true, &state->visible); + can_position, true, + &state->visible);if (ret)return ret;
Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 2/2 drm/i915/skl: Support for 90/270 rotation
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)
--- 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(-)
drivers/gpu/drm/i915/i915_reg.h +2 -0
@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388
drivers/gpu/drm/i915/intel_atomic_plane.c +24 -0
@@ -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); }
drivers/gpu/drm/i915/intel_display.c +62 -26
127 lines not on this page · raw message
drivers/gpu/drm/i915/intel_drv.h +6 -0
@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);
drivers/gpu/drm/i915/intel_sprite.c +37 -15
81 lines not on this page · raw message
Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
Quoted 11 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)
Quoted 2 lines
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>
Patches are missing the Testcase: tag, please add that. Also, are all the igt committed or not yet? Pulled these two in meanwhile. -Daniel
Quoted 305 lines
--- 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(-)drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388drivers/gpu/drm/i915/intel_atomic_plane.c +24
-0@@ -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); }drivers/gpu/drm/i915/intel_display.c +62
-26@@ -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 |=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 (intel_rotation_90_or_270 (rotation)) { +/* 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 ); + } + plane_offset = y_offset <<16 | x_offset;I915_WRITE (PLANE_CTL (pipe,0 ), plane_ctl);I915_WRITE (PLANE_POS (pipe,0 ),0 ); -I915_WRITE (PLANE_OFFSET (pipe,0 ), (y <<16 ) | x); -I915_WRITE (PLANE_SIZE (pipe,0 ), - (intel_crtc->config->pipe_src_h -1 ) <<16 | - (intel_crtc->config->pipe_src_w -1 )); -I915_WRITE (PLANE_STRIDE (pipe,0 ), fb->pitches[0 ] / stride_div); +I915_WRITE (PLANE_OFFSET (pipe,0 ), plane_offset); +I915_WRITE (PLANE_SIZE (pipe,0 ), plane_size); +I915_WRITE (PLANE_STRIDE (pipe,0 ), stride);I915_WRITE (PLANE_SURF (pipe,0 ), surf_addr);POSTING_READ (PLANE_SURF (pipe,0 ));@@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, intel_primary_formats, num_formats, DRM_PLANE_TYPE_PRIMARY); -if (INTEL_INFO (dev)->gen >=4 ) { -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); -if (dev->mode_config.rotation_property) -drm_object_attach_property (&primary->base.base, - dev->mode_config.rotation_property, - state->base.rotation); - } +if (INTEL_INFO (dev)->gen >=4 ) +intel_create_rotation_property (dev, primary);drm_plane_helper_add (&primary->base, &intel_plane_helper_funcs);return &primary->base; } +void intel_create_rotation_property (struct drm_device *dev,struct intel_plane *plane) +{ +if (!dev->mode_config.rotation_property) { +unsigned long flags =BIT (DRM_ROTATE_0 ) | +BIT (DRM_ROTATE_180 ); + +if (INTEL_INFO (dev)->gen >=9 ) + flags |=BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 ); + + dev->mode_config.rotation_property = +drm_mode_create_rotation_property (dev, flags); + } +if (dev->mode_config.rotation_property) +drm_object_attach_property (&plane->base.base, + dev->mode_config.rotation_property, + plane->base.state->rotation); +} +static int intel_check_cursor_plane (struct drm_plane *plane,struct intel_plane_state *state)drivers/gpu/drm/i915/intel_drv.h +6
-0@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);drivers/gpu/drm/i915/intel_sprite.c +37
-15@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,struct drm_i915_gem_object *obj =intel_fb_obj (fb);const int pipe = intel_plane->pipe;const int plane = intel_plane->plane +1 ; -u32 plane_ctl, stride_div; +u32 plane_ctl, stride_div, stride;int pixel_size =drm_format_plane_cpp (fb->pixel_format,0 );const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;unsigned long surf_addr; +u32 tile_height, plane_offset, plane_size; +unsigned int rotation; +int x_offset, y_offset; plane_ctl =PLANE_CTL_ENABLE |PLANE_CTL_PIPE_CSC_ENABLE ;@@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,MISSING_CASE (fb->modifier[0 ]); } -if (drm_plane->state->rotation ==BIT (DRM_ROTATE_180 )) + rotation = drm_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 ; + } intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h, pixel_size, true,@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, surf_addr =intel_plane_obj_offset (intel_plane, obj); -I915_WRITE (PLANE_OFFSET (pipe, plane), (y <<16 ) | x); -I915_WRITE (PLANE_STRIDE (pipe, plane), fb->pitches[0 ] / stride_div); +if (intel_rotation_90_or_270 (rotation)) { +/* 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); + plane_size = (src_w <<16 ) | src_h; + x_offset = stride * tile_height - y - (src_h +1 ); + y_offset = x; + }else { + stride = fb->pitches[0 ] / stride_div; + plane_size = (src_h <<16 ) | src_w; + x_offset = x; + y_offset = y; + } + plane_offset = y_offset <<16 | x_offset; + +I915_WRITE (PLANE_OFFSET (pipe, plane), plane_offset); +I915_WRITE (PLANE_STRIDE (pipe, plane), stride);I915_WRITE (PLANE_POS (pipe, plane), (crtc_y <<16 ) | crtc_x); -I915_WRITE (PLANE_SIZE (pipe, plane), (crtc_h <<16 ) | crtc_w); +I915_WRITE (PLANE_SIZE (pipe, plane), plane_size);I915_WRITE (PLANE_CTL (pipe, plane), plane_ctl);I915_WRITE (PLANE_SURF (pipe, plane), surf_addr);POSTING_READ (PLANE_SURF (pipe, plane));@@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) goto out; } -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); - -if (dev->mode_config.rotation_property) -drm_object_attach_property (&intel_plane->base.base, - dev->mode_config.rotation_property, - state->base.rotation); +intel_create_rotation_property (dev, intel_plane);drm_plane_helper_add (&intel_plane->base, &intel_plane_helper_funcs);Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
Quoted 12 lines · Daniel Vetter
Quoted 11 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)
Quoted 5 lines
Quoted 2 lines
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>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).
Also I see my review comment about the 90 vs. 270 rotation mixup was ignored at least.
Quoted 316 lines
-Daniel
Quoted 305 lines
--- 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(-)drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388drivers/gpu/drm/i915/intel_atomic_plane.c +24
-0@@ -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); }drivers/gpu/drm/i915/intel_display.c +62
-26@@ -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 |=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 (intel_rotation_90_or_270 (rotation)) { +/* 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 ); + } + plane_offset = y_offset <<16 | x_offset;I915_WRITE (PLANE_CTL (pipe,0 ), plane_ctl);I915_WRITE (PLANE_POS (pipe,0 ),0 ); -I915_WRITE (PLANE_OFFSET (pipe,0 ), (y <<16 ) | x); -I915_WRITE (PLANE_SIZE (pipe,0 ), - (intel_crtc->config->pipe_src_h -1 ) <<16 | - (intel_crtc->config->pipe_src_w -1 )); -I915_WRITE (PLANE_STRIDE (pipe,0 ), fb->pitches[0 ] / stride_div); +I915_WRITE (PLANE_OFFSET (pipe,0 ), plane_offset); +I915_WRITE (PLANE_SIZE (pipe,0 ), plane_size); +I915_WRITE (PLANE_STRIDE (pipe,0 ), stride);I915_WRITE (PLANE_SURF (pipe,0 ), surf_addr);POSTING_READ (PLANE_SURF (pipe,0 ));@@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, intel_primary_formats, num_formats, DRM_PLANE_TYPE_PRIMARY); -if (INTEL_INFO (dev)->gen >=4 ) { -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); -if (dev->mode_config.rotation_property) -drm_object_attach_property (&primary->base.base, - dev->mode_config.rotation_property, - state->base.rotation); - } +if (INTEL_INFO (dev)->gen >=4 ) +intel_create_rotation_property (dev, primary);drm_plane_helper_add (&primary->base, &intel_plane_helper_funcs);return &primary->base; } +void intel_create_rotation_property (struct drm_device *dev,struct intel_plane *plane) +{ +if (!dev->mode_config.rotation_property) { +unsigned long flags =BIT (DRM_ROTATE_0 ) | +BIT (DRM_ROTATE_180 ); + +if (INTEL_INFO (dev)->gen >=9 ) + flags |=BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 ); + + dev->mode_config.rotation_property = +drm_mode_create_rotation_property (dev, flags); + } +if (dev->mode_config.rotation_property) +drm_object_attach_property (&plane->base.base, + dev->mode_config.rotation_property, + plane->base.state->rotation); +} +static int intel_check_cursor_plane (struct drm_plane *plane,struct intel_plane_state *state)drivers/gpu/drm/i915/intel_drv.h +6
-0@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);drivers/gpu/drm/i915/intel_sprite.c +37
-15@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,struct drm_i915_gem_object *obj =intel_fb_obj (fb);const int pipe = intel_plane->pipe;const int plane = intel_plane->plane +1 ; -u32 plane_ctl, stride_div; +u32 plane_ctl, stride_div, stride;int pixel_size =drm_format_plane_cpp (fb->pixel_format,0 );const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;unsigned long surf_addr; +u32 tile_height, plane_offset, plane_size; +unsigned int rotation; +int x_offset, y_offset; plane_ctl =PLANE_CTL_ENABLE |PLANE_CTL_PIPE_CSC_ENABLE ;@@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,MISSING_CASE (fb->modifier[0 ]); } -if (drm_plane->state->rotation ==BIT (DRM_ROTATE_180 )) + rotation = drm_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 ; + } intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h, pixel_size, true,@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, surf_addr =intel_plane_obj_offset (intel_plane, obj); -I915_WRITE (PLANE_OFFSET (pipe, plane), (y <<16 ) | x); -I915_WRITE (PLANE_STRIDE (pipe, plane), fb->pitches[0 ] / stride_div); +if (intel_rotation_90_or_270 (rotation)) { +/* 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); + plane_size = (src_w <<16 ) | src_h; + x_offset = stride * tile_height - y - (src_h +1 ); + y_offset = x; + }else { + stride = fb->pitches[0 ] / stride_div; + plane_size = (src_h <<16 ) | src_w; + x_offset = x; + y_offset = y; + } + plane_offset = y_offset <<16 | x_offset; + +I915_WRITE (PLANE_OFFSET (pipe, plane), plane_offset); +I915_WRITE (PLANE_STRIDE (pipe, plane), stride);I915_WRITE (PLANE_POS (pipe, plane), (crtc_y <<16 ) | crtc_x); -I915_WRITE (PLANE_SIZE (pipe, plane), (crtc_h <<16 ) | crtc_w); +I915_WRITE (PLANE_SIZE (pipe, plane), plane_size);I915_WRITE (PLANE_CTL (pipe, plane), plane_ctl);I915_WRITE (PLANE_SURF (pipe, plane), surf_addr);POSTING_READ (PLANE_SURF (pipe, plane));@@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) goto out; } -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); - -if (dev->mode_config.rotation_property) -drm_object_attach_property (&intel_plane->base.base, - dev->mode_config.rotation_property, - state->base.rotation); +intel_create_rotation_property (dev, intel_plane);drm_plane_helper_add (&intel_plane->base, &intel_plane_helper_funcs);Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfxSignature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
Quoted 16 lines · Ville Syrjälä
Quoted 15 lines · Daniel Vetter
Quoted 13 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>
Quoted 9 lines
Quoted 2 lines
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.
Quoted 3 lines
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
Quoted 303 lines
Quoted 303 lines
-Daniel
Quoted 301 lines
--- 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(-)drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388drivers/gpu/drm/i915/intel_atomic_plane.c +24
-0@@ -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); }drivers/gpu/drm/i915/intel_display.c +62
-26@@ -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 |=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 (intel_rotation_90_or_270 (rotation)) { +/* 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 ); + } + plane_offset = y_offset <<16 | x_offset;I915_WRITE (PLANE_CTL (pipe,0 ), plane_ctl);I915_WRITE (PLANE_POS (pipe,0 ),0 ); -I915_WRITE (PLANE_OFFSET (pipe,0 ), (y <<16 ) | x); -I915_WRITE (PLANE_SIZE (pipe,0 ), - (intel_crtc->config->pipe_src_h -1 ) <<16 | - (intel_crtc->config->pipe_src_w -1 )); -I915_WRITE (PLANE_STRIDE (pipe,0 ), fb->pitches[0 ] / stride_div); +I915_WRITE (PLANE_OFFSET (pipe,0 ), plane_offset); +I915_WRITE (PLANE_SIZE (pipe,0 ), plane_size); +I915_WRITE (PLANE_STRIDE (pipe,0 ), stride);I915_WRITE (PLANE_SURF (pipe,0 ), surf_addr);POSTING_READ (PLANE_SURF (pipe,0 ));@@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, intel_primary_formats, num_formats, DRM_PLANE_TYPE_PRIMARY); -if (INTEL_INFO (dev)->gen >=4 ) { -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); -if (dev->mode_config.rotation_property) -drm_object_attach_property (&primary->base.base, - dev->mode_config.rotation_property, - state->base.rotation); - } +if (INTEL_INFO (dev)->gen >=4 ) +intel_create_rotation_property (dev, primary);drm_plane_helper_add (&primary->base, &intel_plane_helper_funcs);return &primary->base; } +void intel_create_rotation_property (struct drm_device *dev,struct intel_plane *plane) +{ +if (!dev->mode_config.rotation_property) { +unsigned long flags =BIT (DRM_ROTATE_0 ) | +BIT (DRM_ROTATE_180 ); + +if (INTEL_INFO (dev)->gen >=9 ) + flags |=BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 ); + + dev->mode_config.rotation_property = +drm_mode_create_rotation_property (dev, flags); + } +if (dev->mode_config.rotation_property) +drm_object_attach_property (&plane->base.base, + dev->mode_config.rotation_property, + plane->base.state->rotation); +} +static int intel_check_cursor_plane (struct drm_plane *plane,struct intel_plane_state *state)drivers/gpu/drm/i915/intel_drv.h +6
-0@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);drivers/gpu/drm/i915/intel_sprite.c +37
-16@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,struct drm_i915_gem_object *obj =intel_fb_obj (fb);const int pipe = intel_plane->pipe;const int plane = intel_plane->plane +1 ; -u32 plane_ctl, stride_div; +u32 plane_ctl, stride_div, stride;int pixel_size =drm_format_plane_cpp (fb->pixel_format,0 );const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;unsigned long surf_addr; +u32 tile_height, plane_offset, plane_size; +unsigned int rotation; +int x_offset, y_offset; plane_ctl =PLANE_CTL_ENABLE |PLANE_CTL_PIPE_CSC_ENABLE ;@@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,MISSING_CASE (fb->modifier[0 ]); } -if (drm_plane->state->rotation ==BIT (DRM_ROTATE_180 )) + rotation = drm_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 ; + } intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h, pixel_size, true,@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, surf_addr =intel_plane_obj_offset (intel_plane, obj); -I915_WRITE (PLANE_OFFSET (pipe, plane), (y <<16 ) | x); -I915_WRITE (PLANE_STRIDE (pipe, plane), fb->pitches[0 ] / stride_div); +if (intel_rotation_90_or_270 (rotation)) { +/* 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); + plane_size = (src_w <<16 ) | src_h; + x_offset = stride * tile_height - y - (src_h +1 ); + y_offset = x; + }else { + stride = fb->pitches[0 ] / stride_div; + plane_size = (src_h <<16 ) | src_w; + x_offset = x; + y_offset = y; + } + plane_offset = y_offset <<16 | x_offset; + +I915_WRITE (PLANE_OFFSET (pipe, plane), plane_offset); +I915_WRITE (PLANE_STRIDE (pipe, plane), stride);I915_WRITE (PLANE_POS (pipe, plane), (crtc_y <<16 ) | crtc_x); -I915_WRITE (PLANE_SIZE (pipe, plane), (crtc_h <<16 ) | crtc_w); +I915_WRITE (PLANE_SIZE (pipe, plane), plane_size);I915_WRITE (PLANE_CTL (pipe, plane), plane_ctl);I915_WRITE (PLANE_SURF (pipe, plane), surf_addr);POSTING_READ (PLANE_SURF (pipe, plane));@@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) goto out; } -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); - -if (dev->mode_config.rotation_property) -drm_object_attach_property (&intel_plane->base.base, - dev->mode_config.rotation_property, - state->base.rotation); +intel_create_rotation_property (dev, intel_plane);drm_plane_helper_add (&intel_plane->base, &intel_plane_helper_funcs); --1.7.10.4
Quoted 14 lines
Quoted 5 lines
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfxQuoted 8 lines
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
Quoted 30 lines · Sonika
Quoted 16 lines · Ville Syrjälä
Quoted 15 lines · Daniel Vetter
Quoted 13 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>Quoted 9 lines
Quoted 2 lines
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 320 lines
-Sonika
Quoted 317 lines
Quoted 316 lines
-Daniel
Quoted 301 lines
--- 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(-)drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388drivers/gpu/drm/i915/intel_atomic_plane.c +24
-0@@ -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); }drivers/gpu/drm/i915/intel_display.c +62
-26@@ -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 |=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 (intel_rotation_90_or_270 (rotation)) { +/* 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 ); + } + plane_offset = y_offset <<16 | x_offset;I915_WRITE (PLANE_CTL (pipe,0 ), plane_ctl);I915_WRITE (PLANE_POS (pipe,0 ),0 ); -I915_WRITE (PLANE_OFFSET (pipe,0 ), (y <<16 ) | x); -I915_WRITE (PLANE_SIZE (pipe,0 ), - (intel_crtc->config->pipe_src_h -1 ) <<16 | - (intel_crtc->config->pipe_src_w -1 )); -I915_WRITE (PLANE_STRIDE (pipe,0 ), fb->pitches[0 ] / stride_div); +I915_WRITE (PLANE_OFFSET (pipe,0 ), plane_offset); +I915_WRITE (PLANE_SIZE (pipe,0 ), plane_size); +I915_WRITE (PLANE_STRIDE (pipe,0 ), stride);I915_WRITE (PLANE_SURF (pipe,0 ), surf_addr);POSTING_READ (PLANE_SURF (pipe,0 ));@@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, intel_primary_formats, num_formats, DRM_PLANE_TYPE_PRIMARY); -if (INTEL_INFO (dev)->gen >=4 ) { -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); -if (dev->mode_config.rotation_property) -drm_object_attach_property (&primary->base.base, - dev->mode_config.rotation_property, - state->base.rotation); - } +if (INTEL_INFO (dev)->gen >=4 ) +intel_create_rotation_property (dev, primary);drm_plane_helper_add (&primary->base, &intel_plane_helper_funcs);return &primary->base; } +void intel_create_rotation_property (struct drm_device *dev,struct intel_plane *plane) +{ +if (!dev->mode_config.rotation_property) { +unsigned long flags =BIT (DRM_ROTATE_0 ) | +BIT (DRM_ROTATE_180 ); + +if (INTEL_INFO (dev)->gen >=9 ) + flags |=BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 ); + + dev->mode_config.rotation_property = +drm_mode_create_rotation_property (dev, flags); + } +if (dev->mode_config.rotation_property) +drm_object_attach_property (&plane->base.base, + dev->mode_config.rotation_property, + plane->base.state->rotation); +} +static int intel_check_cursor_plane (struct drm_plane *plane,struct intel_plane_state *state)drivers/gpu/drm/i915/intel_drv.h +6
-0@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);drivers/gpu/drm/i915/intel_sprite.c +37
-16@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,struct drm_i915_gem_object *obj =intel_fb_obj (fb);const int pipe = intel_plane->pipe;const int plane = intel_plane->plane +1 ; -u32 plane_ctl, stride_div; +u32 plane_ctl, stride_div, stride;int pixel_size =drm_format_plane_cpp (fb->pixel_format,0 );const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;unsigned long surf_addr; +u32 tile_height, plane_offset, plane_size; +unsigned int rotation; +int x_offset, y_offset; plane_ctl =PLANE_CTL_ENABLE |PLANE_CTL_PIPE_CSC_ENABLE ;@@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,MISSING_CASE (fb->modifier[0 ]); } -if (drm_plane->state->rotation ==BIT (DRM_ROTATE_180 )) + rotation = drm_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 ; + } intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h, pixel_size, true,@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, surf_addr =intel_plane_obj_offset (intel_plane, obj); -I915_WRITE (PLANE_OFFSET (pipe, plane), (y <<16 ) | x); -I915_WRITE (PLANE_STRIDE (pipe, plane), fb->pitches[0 ] / stride_div); +if (intel_rotation_90_or_270 (rotation)) { +/* 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); + plane_size = (src_w <<16 ) | src_h; + x_offset = stride * tile_height - y - (src_h +1 ); + y_offset = x; + }else { + stride = fb->pitches[0 ] / stride_div; + plane_size = (src_h <<16 ) | src_w; + x_offset = x; + y_offset = y; + } + plane_offset = y_offset <<16 | x_offset; + +I915_WRITE (PLANE_OFFSET (pipe, plane), plane_offset); +I915_WRITE (PLANE_STRIDE (pipe, plane), stride);I915_WRITE (PLANE_POS (pipe, plane), (crtc_y <<16 ) | crtc_x); -I915_WRITE (PLANE_SIZE (pipe, plane), (crtc_h <<16 ) | crtc_w); +I915_WRITE (PLANE_SIZE (pipe, plane), plane_size);I915_WRITE (PLANE_CTL (pipe, plane), plane_ctl);I915_WRITE (PLANE_SURF (pipe, plane), surf_addr);POSTING_READ (PLANE_SURF (pipe, plane));@@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) goto out; } -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); - -if (dev->mode_config.rotation_property) -drm_object_attach_property (&intel_plane->base.base, - dev->mode_config.rotation_property, - state->base.rotation); +intel_create_rotation_property (dev, intel_plane);drm_plane_helper_add (&intel_plane->base, &intel_plane_helper_funcs); --1.7.10.4
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
Quoted 34 lines · Ville Syrjälä
Quoted 33 lines · Sonika
Quoted 16 lines · Ville Syrjälä
Quoted 15 lines · Daniel Vetter
Quoted 13 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>Quoted 9 lines
Quoted 2 lines
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.
Quoted 3 lines
Also I see my review comment about the 90 vs. 270 rotation mixup was ignored at least.
Quoted 8 lines
Quoted 4 lines
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.
Quoted 306 lines
Quoted 306 lines
-Sonika
Quoted 303 lines
Quoted 303 lines
-Daniel
Quoted 301 lines
--- 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(-)drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388drivers/gpu/drm/i915/intel_atomic_plane.c +24
-0@@ -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); }drivers/gpu/drm/i915/intel_display.c +62
-26@@ -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 |=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 (intel_rotation_90_or_270 (rotation)) { +/* 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 ); + } + plane_offset = y_offset <<16 | x_offset;I915_WRITE (PLANE_CTL (pipe,0 ), plane_ctl);I915_WRITE (PLANE_POS (pipe,0 ),0 ); -I915_WRITE (PLANE_OFFSET (pipe,0 ), (y <<16 ) | x); -I915_WRITE (PLANE_SIZE (pipe,0 ), - (intel_crtc->config->pipe_src_h -1 ) <<16 | - (intel_crtc->config->pipe_src_w -1 )); -I915_WRITE (PLANE_STRIDE (pipe,0 ), fb->pitches[0 ] / stride_div); +I915_WRITE (PLANE_OFFSET (pipe,0 ), plane_offset); +I915_WRITE (PLANE_SIZE (pipe,0 ), plane_size); +I915_WRITE (PLANE_STRIDE (pipe,0 ), stride);I915_WRITE (PLANE_SURF (pipe,0 ), surf_addr);POSTING_READ (PLANE_SURF (pipe,0 ));@@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, intel_primary_formats, num_formats, DRM_PLANE_TYPE_PRIMARY); -if (INTEL_INFO (dev)->gen >=4 ) { -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); -if (dev->mode_config.rotation_property) -drm_object_attach_property (&primary->base.base, - dev->mode_config.rotation_property, - state->base.rotation); - } +if (INTEL_INFO (dev)->gen >=4 ) +intel_create_rotation_property (dev, primary);drm_plane_helper_add (&primary->base, &intel_plane_helper_funcs);return &primary->base; } +void intel_create_rotation_property (struct drm_device *dev,struct intel_plane *plane) +{ +if (!dev->mode_config.rotation_property) { +unsigned long flags =BIT (DRM_ROTATE_0 ) | +BIT (DRM_ROTATE_180 ); + +if (INTEL_INFO (dev)->gen >=9 ) + flags |=BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 ); + + dev->mode_config.rotation_property = +drm_mode_create_rotation_property (dev, flags); + } +if (dev->mode_config.rotation_property) +drm_object_attach_property (&plane->base.base, + dev->mode_config.rotation_property, + plane->base.state->rotation); +} +static int intel_check_cursor_plane (struct drm_plane *plane,struct intel_plane_state *state)drivers/gpu/drm/i915/intel_drv.h +6
-0@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);drivers/gpu/drm/i915/intel_sprite.c +37
-16@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,struct drm_i915_gem_object *obj =intel_fb_obj (fb);const int pipe = intel_plane->pipe;const int plane = intel_plane->plane +1 ; -u32 plane_ctl, stride_div; +u32 plane_ctl, stride_div, stride;int pixel_size =drm_format_plane_cpp (fb->pixel_format,0 );const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;unsigned long surf_addr; +u32 tile_height, plane_offset, plane_size; +unsigned int rotation; +int x_offset, y_offset; plane_ctl =PLANE_CTL_ENABLE |PLANE_CTL_PIPE_CSC_ENABLE ;@@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,MISSING_CASE (fb->modifier[0 ]); } -if (drm_plane->state->rotation ==BIT (DRM_ROTATE_180 )) + rotation = drm_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 ; + } intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h, pixel_size, true,@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, surf_addr =intel_plane_obj_offset (intel_plane, obj); -I915_WRITE (PLANE_OFFSET (pipe, plane), (y <<16 ) | x); -I915_WRITE (PLANE_STRIDE (pipe, plane), fb->pitches[0 ] / stride_div); +if (intel_rotation_90_or_270 (rotation)) { +/* 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); + plane_size = (src_w <<16 ) | src_h; + x_offset = stride * tile_height - y - (src_h +1 ); + y_offset = x; + }else { + stride = fb->pitches[0 ] / stride_div; + plane_size = (src_h <<16 ) | src_w; + x_offset = x; + y_offset = y; + } + plane_offset = y_offset <<16 | x_offset; + +I915_WRITE (PLANE_OFFSET (pipe, plane), plane_offset); +I915_WRITE (PLANE_STRIDE (pipe, plane), stride);I915_WRITE (PLANE_POS (pipe, plane), (crtc_y <<16 ) | crtc_x); -I915_WRITE (PLANE_SIZE (pipe, plane), (crtc_h <<16 ) | crtc_w); +I915_WRITE (PLANE_SIZE (pipe, plane), plane_size);I915_WRITE (PLANE_CTL (pipe, plane), plane_ctl);I915_WRITE (PLANE_SURF (pipe, plane), surf_addr);POSTING_READ (PLANE_SURF (pipe, plane));@@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) goto out; } -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); - -if (dev->mode_config.rotation_property) -drm_object_attach_property (&intel_plane->base.base, - dev->mode_config.rotation_property, - state->base.rotation); +intel_create_rotation_property (dev, intel_plane);drm_plane_helper_add (&intel_plane->base, &intel_plane_helper_funcs); --1.7.10.4
Quoted 15 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfxQuoted 9 lines
Quoted 9 lines
Quoted 8 lines
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
Quoted 42 lines · Sonika
Quoted 39 lines · Ville Syrjälä
Quoted 37 lines · Sonika
Quoted 16 lines · Ville Syrjälä
Quoted 15 lines · Daniel Vetter
Quoted 13 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>Quoted 9 lines
Quoted 2 lines
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.
Quoted 3 lines
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?
Quoted 5 lines
Quoted 3 lines
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.
Quoted 322 lines
Quoted 322 lines
Quoted 321 lines
-Sonika
Quoted 317 lines
Quoted 316 lines
-Daniel
Quoted 301 lines
--- 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(-)drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388drivers/gpu/drm/i915/intel_atomic_plane.c +24
-0@@ -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); }drivers/gpu/drm/i915/intel_display.c +62
-26@@ -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 |=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 (intel_rotation_90_or_270 (rotation)) { +/* 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 ); + } + plane_offset = y_offset <<16 | x_offset;I915_WRITE (PLANE_CTL (pipe,0 ), plane_ctl);I915_WRITE (PLANE_POS (pipe,0 ),0 ); -I915_WRITE (PLANE_OFFSET (pipe,0 ), (y <<16 ) | x); -I915_WRITE (PLANE_SIZE (pipe,0 ), - (intel_crtc->config->pipe_src_h -1 ) <<16 | - (intel_crtc->config->pipe_src_w -1 )); -I915_WRITE (PLANE_STRIDE (pipe,0 ), fb->pitches[0 ] / stride_div); +I915_WRITE (PLANE_OFFSET (pipe,0 ), plane_offset); +I915_WRITE (PLANE_SIZE (pipe,0 ), plane_size); +I915_WRITE (PLANE_STRIDE (pipe,0 ), stride);I915_WRITE (PLANE_SURF (pipe,0 ), surf_addr);POSTING_READ (PLANE_SURF (pipe,0 ));@@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, intel_primary_formats, num_formats, DRM_PLANE_TYPE_PRIMARY); -if (INTEL_INFO (dev)->gen >=4 ) { -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); -if (dev->mode_config.rotation_property) -drm_object_attach_property (&primary->base.base, - dev->mode_config.rotation_property, - state->base.rotation); - } +if (INTEL_INFO (dev)->gen >=4 ) +intel_create_rotation_property (dev, primary);drm_plane_helper_add (&primary->base, &intel_plane_helper_funcs);return &primary->base; } +void intel_create_rotation_property (struct drm_device *dev,struct intel_plane *plane) +{ +if (!dev->mode_config.rotation_property) { +unsigned long flags =BIT (DRM_ROTATE_0 ) | +BIT (DRM_ROTATE_180 ); + +if (INTEL_INFO (dev)->gen >=9 ) + flags |=BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 ); + + dev->mode_config.rotation_property = +drm_mode_create_rotation_property (dev, flags); + } +if (dev->mode_config.rotation_property) +drm_object_attach_property (&plane->base.base, + dev->mode_config.rotation_property, + plane->base.state->rotation); +} +static int intel_check_cursor_plane (struct drm_plane *plane,struct intel_plane_state *state)drivers/gpu/drm/i915/intel_drv.h +6
-0@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);drivers/gpu/drm/i915/intel_sprite.c +37
-16@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,struct drm_i915_gem_object *obj =intel_fb_obj (fb);const int pipe = intel_plane->pipe;const int plane = intel_plane->plane +1 ; -u32 plane_ctl, stride_div; +u32 plane_ctl, stride_div, stride;int pixel_size =drm_format_plane_cpp (fb->pixel_format,0 );const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;unsigned long surf_addr; +u32 tile_height, plane_offset, plane_size; +unsigned int rotation; +int x_offset, y_offset; plane_ctl =PLANE_CTL_ENABLE |PLANE_CTL_PIPE_CSC_ENABLE ;@@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,MISSING_CASE (fb->modifier[0 ]); } -if (drm_plane->state->rotation ==BIT (DRM_ROTATE_180 )) + rotation = drm_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 ; + } intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h, pixel_size, true,@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, surf_addr =intel_plane_obj_offset (intel_plane, obj); -I915_WRITE (PLANE_OFFSET (pipe, plane), (y <<16 ) | x); -I915_WRITE (PLANE_STRIDE (pipe, plane), fb->pitches[0 ] / stride_div); +if (intel_rotation_90_or_270 (rotation)) { +/* 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); + plane_size = (src_w <<16 ) | src_h; + x_offset = stride * tile_height - y - (src_h +1 ); + y_offset = x; + }else { + stride = fb->pitches[0 ] / stride_div; + plane_size = (src_h <<16 ) | src_w; + x_offset = x; + y_offset = y; + } + plane_offset = y_offset <<16 | x_offset; + +I915_WRITE (PLANE_OFFSET (pipe, plane), plane_offset); +I915_WRITE (PLANE_STRIDE (pipe, plane), stride);I915_WRITE (PLANE_POS (pipe, plane), (crtc_y <<16 ) | crtc_x); -I915_WRITE (PLANE_SIZE (pipe, plane), (crtc_h <<16 ) | crtc_w); +I915_WRITE (PLANE_SIZE (pipe, plane), plane_size);I915_WRITE (PLANE_CTL (pipe, plane), plane_ctl);I915_WRITE (PLANE_SURF (pipe, plane), surf_addr);POSTING_READ (PLANE_SURF (pipe, plane));@@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) goto out; } -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); - -if (dev->mode_config.rotation_property) -drm_object_attach_property (&intel_plane->base.base, - dev->mode_config.rotation_property, - state->base.rotation); +intel_create_rotation_property (dev, intel_plane);drm_plane_helper_add (&intel_plane->base, &intel_plane_helper_funcs); --1.7.10.4
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
Quoted 46 lines · Ville Syrjälä
Quoted 45 lines · Sonika
Quoted 42 lines · Ville Syrjälä
Quoted 30 lines · Sonika
Quoted 16 lines · Ville Syrjälä
Quoted 15 lines · Daniel Vetter
Quoted 13 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>Quoted 9 lines
Quoted 2 lines
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 2 lines
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
Quoted 331 lines
Quoted 323 lines
Quoted 323 lines
Quoted 322 lines
-Sonika
Quoted 317 lines
Quoted 316 lines
-Daniel
Quoted 301 lines
--- 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(-)drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388drivers/gpu/drm/i915/intel_atomic_plane.c +24
-0@@ -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); }drivers/gpu/drm/i915/intel_display.c +62
-26@@ -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 |=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 (intel_rotation_90_or_270 (rotation)) { +/* 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 ); + } + plane_offset = y_offset <<16 | x_offset;I915_WRITE (PLANE_CTL (pipe,0 ), plane_ctl);I915_WRITE (PLANE_POS (pipe,0 ),0 ); -I915_WRITE (PLANE_OFFSET (pipe,0 ), (y <<16 ) | x); -I915_WRITE (PLANE_SIZE (pipe,0 ), - (intel_crtc->config->pipe_src_h -1 ) <<16 | - (intel_crtc->config->pipe_src_w -1 )); -I915_WRITE (PLANE_STRIDE (pipe,0 ), fb->pitches[0 ] / stride_div); +I915_WRITE (PLANE_OFFSET (pipe,0 ), plane_offset); +I915_WRITE (PLANE_SIZE (pipe,0 ), plane_size); +I915_WRITE (PLANE_STRIDE (pipe,0 ), stride);I915_WRITE (PLANE_SURF (pipe,0 ), surf_addr);POSTING_READ (PLANE_SURF (pipe,0 ));@@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, intel_primary_formats, num_formats, DRM_PLANE_TYPE_PRIMARY); -if (INTEL_INFO (dev)->gen >=4 ) { -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); -if (dev->mode_config.rotation_property) -drm_object_attach_property (&primary->base.base, - dev->mode_config.rotation_property, - state->base.rotation); - } +if (INTEL_INFO (dev)->gen >=4 ) +intel_create_rotation_property (dev, primary);drm_plane_helper_add (&primary->base, &intel_plane_helper_funcs);return &primary->base; } +void intel_create_rotation_property (struct drm_device *dev,struct intel_plane *plane) +{ +if (!dev->mode_config.rotation_property) { +unsigned long flags =BIT (DRM_ROTATE_0 ) | +BIT (DRM_ROTATE_180 ); + +if (INTEL_INFO (dev)->gen >=9 ) + flags |=BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 ); + + dev->mode_config.rotation_property = +drm_mode_create_rotation_property (dev, flags); + } +if (dev->mode_config.rotation_property) +drm_object_attach_property (&plane->base.base, + dev->mode_config.rotation_property, + plane->base.state->rotation); +} +static int intel_check_cursor_plane (struct drm_plane *plane,struct intel_plane_state *state)drivers/gpu/drm/i915/intel_drv.h +6
-0@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);drivers/gpu/drm/i915/intel_sprite.c +37
-16@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,struct drm_i915_gem_object *obj =intel_fb_obj (fb);const int pipe = intel_plane->pipe;const int plane = intel_plane->plane +1 ; -u32 plane_ctl, stride_div; +u32 plane_ctl, stride_div, stride;int pixel_size =drm_format_plane_cpp (fb->pixel_format,0 );const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;unsigned long surf_addr; +u32 tile_height, plane_offset, plane_size; +unsigned int rotation; +int x_offset, y_offset; plane_ctl =PLANE_CTL_ENABLE |PLANE_CTL_PIPE_CSC_ENABLE ;@@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,MISSING_CASE (fb->modifier[0 ]); } -if (drm_plane->state->rotation ==BIT (DRM_ROTATE_180 )) + rotation = drm_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 ; + } intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h, pixel_size, true,@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, surf_addr =intel_plane_obj_offset (intel_plane, obj); -I915_WRITE (PLANE_OFFSET (pipe, plane), (y <<16 ) | x); -I915_WRITE (PLANE_STRIDE (pipe, plane), fb->pitches[0 ] / stride_div); +if (intel_rotation_90_or_270 (rotation)) { +/* 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); + plane_size = (src_w <<16 ) | src_h; + x_offset = stride * tile_height - y - (src_h +1 ); + y_offset = x; + }else { + stride = fb->pitches[0 ] / stride_div; + plane_size = (src_h <<16 ) | src_w; + x_offset = x; + y_offset = y; + } + plane_offset = y_offset <<16 | x_offset; + +I915_WRITE (PLANE_OFFSET (pipe, plane), plane_offset); +I915_WRITE (PLANE_STRIDE (pipe, plane), stride);I915_WRITE (PLANE_POS (pipe, plane), (crtc_y <<16 ) | crtc_x); -I915_WRITE (PLANE_SIZE (pipe, plane), (crtc_h <<16 ) | crtc_w); +I915_WRITE (PLANE_SIZE (pipe, plane), plane_size);I915_WRITE (PLANE_CTL (pipe, plane), plane_ctl);I915_WRITE (PLANE_SURF (pipe, plane), surf_addr);POSTING_READ (PLANE_SURF (pipe, plane));@@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) goto out; } -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); - -if (dev->mode_config.rotation_property) -drm_object_attach_property (&intel_plane->base.base, - dev->mode_config.rotation_property, - state->base.rotation); +intel_create_rotation_property (dev, intel_plane);drm_plane_helper_add (&intel_plane->base, &intel_plane_helper_funcs); --1.7.10.4
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfxSignature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 9 lines
Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
Quoted 54 lines · Matt Roper
Quoted 46 lines · Ville Syrjälä
Quoted 45 lines · Sonika
Quoted 42 lines · Ville Syrjälä
Quoted 30 lines · Sonika
Quoted 16 lines · Ville Syrjälä
Quoted 15 lines · Daniel Vetter
Quoted 13 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>Quoted 9 lines
Quoted 2 lines
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 2 lines
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.
Quoted 6 lines
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
Quoted 7 lines
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.
Quoted 334 lines
Matt
Quoted 331 lines
Quoted 308 lines
Quoted 308 lines
Quoted 308 lines
-Sonika
Quoted 303 lines
Quoted 303 lines
-Daniel
Quoted 301 lines
--- 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(-)drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388drivers/gpu/drm/i915/intel_atomic_plane.c +24
-0@@ -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); }drivers/gpu/drm/i915/intel_display.c +62
-26@@ -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 |=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 (intel_rotation_90_or_270 (rotation)) { +/* 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 ); + } + plane_offset = y_offset <<16 | x_offset;I915_WRITE (PLANE_CTL (pipe,0 ), plane_ctl);I915_WRITE (PLANE_POS (pipe,0 ),0 ); -I915_WRITE (PLANE_OFFSET (pipe,0 ), (y <<16 ) | x); -I915_WRITE (PLANE_SIZE (pipe,0 ), - (intel_crtc->config->pipe_src_h -1 ) <<16 | - (intel_crtc->config->pipe_src_w -1 )); -I915_WRITE (PLANE_STRIDE (pipe,0 ), fb->pitches[0 ] / stride_div); +I915_WRITE (PLANE_OFFSET (pipe,0 ), plane_offset); +I915_WRITE (PLANE_SIZE (pipe,0 ), plane_size); +I915_WRITE (PLANE_STRIDE (pipe,0 ), stride);I915_WRITE (PLANE_SURF (pipe,0 ), surf_addr);POSTING_READ (PLANE_SURF (pipe,0 ));@@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, intel_primary_formats, num_formats, DRM_PLANE_TYPE_PRIMARY); -if (INTEL_INFO (dev)->gen >=4 ) { -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); -if (dev->mode_config.rotation_property) -drm_object_attach_property (&primary->base.base, - dev->mode_config.rotation_property, - state->base.rotation); - } +if (INTEL_INFO (dev)->gen >=4 ) +intel_create_rotation_property (dev, primary);drm_plane_helper_add (&primary->base, &intel_plane_helper_funcs);return &primary->base; } +void intel_create_rotation_property (struct drm_device *dev,struct intel_plane *plane) +{ +if (!dev->mode_config.rotation_property) { +unsigned long flags =BIT (DRM_ROTATE_0 ) | +BIT (DRM_ROTATE_180 ); + +if (INTEL_INFO (dev)->gen >=9 ) + flags |=BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 ); + + dev->mode_config.rotation_property = +drm_mode_create_rotation_property (dev, flags); + } +if (dev->mode_config.rotation_property) +drm_object_attach_property (&plane->base.base, + dev->mode_config.rotation_property, + plane->base.state->rotation); +} +static int intel_check_cursor_plane (struct drm_plane *plane,struct intel_plane_state *state)drivers/gpu/drm/i915/intel_drv.h +6
-0@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);drivers/gpu/drm/i915/intel_sprite.c +37
-16@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,struct drm_i915_gem_object *obj =intel_fb_obj (fb);const int pipe = intel_plane->pipe;const int plane = intel_plane->plane +1 ; -u32 plane_ctl, stride_div; +u32 plane_ctl, stride_div, stride;int pixel_size =drm_format_plane_cpp (fb->pixel_format,0 );const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;unsigned long surf_addr; +u32 tile_height, plane_offset, plane_size; +unsigned int rotation; +int x_offset, y_offset; plane_ctl =PLANE_CTL_ENABLE |PLANE_CTL_PIPE_CSC_ENABLE ;@@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,MISSING_CASE (fb->modifier[0 ]); } -if (drm_plane->state->rotation ==BIT (DRM_ROTATE_180 )) + rotation = drm_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 ; + } intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h, pixel_size, true,@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, surf_addr =intel_plane_obj_offset (intel_plane, obj); -I915_WRITE (PLANE_OFFSET (pipe, plane), (y <<16 ) | x); -I915_WRITE (PLANE_STRIDE (pipe, plane), fb->pitches[0 ] / stride_div); +if (intel_rotation_90_or_270 (rotation)) { +/* 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); + plane_size = (src_w <<16 ) | src_h; + x_offset = stride * tile_height - y - (src_h +1 ); + y_offset = x; + }else { + stride = fb->pitches[0 ] / stride_div; + plane_size = (src_h <<16 ) | src_w; + x_offset = x; + y_offset = y; + } + plane_offset = y_offset <<16 | x_offset; + +I915_WRITE (PLANE_OFFSET (pipe, plane), plane_offset); +I915_WRITE (PLANE_STRIDE (pipe, plane), stride);I915_WRITE (PLANE_POS (pipe, plane), (crtc_y <<16 ) | crtc_x); -I915_WRITE (PLANE_SIZE (pipe, plane), (crtc_h <<16 ) | crtc_w); +I915_WRITE (PLANE_SIZE (pipe, plane), plane_size);I915_WRITE (PLANE_CTL (pipe, plane), plane_ctl);I915_WRITE (PLANE_SURF (pipe, plane), surf_addr);POSTING_READ (PLANE_SURF (pipe, plane));@@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) goto out; } -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); - -if (dev->mode_config.rotation_property) -drm_object_attach_property (&intel_plane->base.base, - dev->mode_config.rotation_property, - state->base.rotation); +intel_create_rotation_property (dev, intel_plane);drm_plane_helper_add (&intel_plane->base, &intel_plane_helper_funcs); --1.7.10.4
Quoted 15 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfxQuoted 10 lines
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
Quoted 63 lines · Sonika
Quoted 54 lines · Matt Roper
Quoted 46 lines · Ville Syrjälä
Quoted 45 lines · Sonika
Quoted 42 lines · Ville Syrjälä
Quoted 30 lines · Sonika
Quoted 16 lines · Ville Syrjälä
Quoted 15 lines · Daniel Vetter
Quoted 13 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>Quoted 9 lines
Quoted 2 lines
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 2 lines
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.Quoted 6 lines
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.
Quoted 8 lines
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_90and usePLANE_CTL_ROTATE_270for 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
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 1/2 drm/i915: Swapping 90 and 270 to be compliant with Xrandr
Since DRM_ROTATE is counter clockwise (which is compliant with Xrandr), and HW rotation is clockwise, swapping 90/270 to work as expected from userspace.
--- drivers/gpu/drm/i915/intel_display.c | 8 ++++++-- drivers/gpu/drm/i915/intel_sprite.c | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/intel_display.c +6 -2
@@ -3039,8 +3039,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, plane = crtc->primary; rotation = plane->state->rotation;switch (rotation) { +/* +* DRM_ROTATE_ is counter clockwise to stay compatible with Xrandr +* while i915 HW rotation is clockwise, thats why this swapping. +*/ case BIT (DRM_ROTATE_90 ): - plane_ctl |=PLANE_CTL_ROTATE_90 ; + plane_ctl |=PLANE_CTL_ROTATE_270 ;break ;case BIT (DRM_ROTATE_180 ):
@@ -3048,7 +3052,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,break ;case BIT (DRM_ROTATE_270 ): - plane_ctl |=PLANE_CTL_ROTATE_270 ; + plane_ctl |=PLANE_CTL_ROTATE_90 ;break ; }
drivers/gpu/drm/i915/intel_sprite.c +6 -2
@@ -259,8 +259,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, rotation = drm_plane->state->rotation;switch (rotation) { +/* +* DRM_ROTATE_ is counter clockwise to stay compatible with Xrandr +* while i915 HW rotation is clockwise, thats why this swapping. +*/ case BIT (DRM_ROTATE_90 ): - plane_ctl |=PLANE_CTL_ROTATE_90 ; + plane_ctl |=PLANE_CTL_ROTATE_270 ;break ;case BIT (DRM_ROTATE_180 ):
@@ -268,7 +272,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,break ;case BIT (DRM_ROTATE_270 ): - plane_ctl |=PLANE_CTL_ROTATE_270 ; + plane_ctl |=PLANE_CTL_ROTATE_90 ;break ; }
Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 2/2 Documentation/drm: Update rotation property with 90/270 and description
--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Documentation/DocBook/drm.tmpl +5 -2
@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td> <td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td> <td valign="top" >Plane</td> - <td valign="top" >TBD</td> + <td valign="top" >To set plane HW rotation. This rotation property does + the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td> </tr> <tr> <td rowspan="17" valign="top" >SDVO-TV</td>
Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] Documentation/drm: Update rotation property with 90/270 and description
Quoted 11 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/DocBook/drm.tmpl +0
-0@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td>
Documentation/DocBook/drm.tmpl void intel_crt_init(struct drm_device *dev) · line 2861
Quoted 10 lines
Documentation/DocBook/drm.tmpl +5
-2<td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td> <td valign="top" >Plane</td> - <td valign="top" >TBD</td> + <td valign="top" >To set plane HW rotation. This rotation property does + the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td>
Since this touches shared code can you please resend this patch with dri-devel added to cc? BKM is to add a Cc: dri-devel ... line to the sob section of the patch, then git send-email will automatically pick it up. -Daniel
Quoted 10 lines
Documentation/DocBook/drm.tmpl +0
-0</tr> <tr> <td rowspan="17" valign="top" >SDVO-TV</td>Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] Documentation/drm: Update rotation property with 90/270 and description
Quoted 19 lines · Daniel Vetter
Quoted 18 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/DocBook/drm.tmpl +2
-2@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td> <td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td> <td valign="top" >Plane</td> - <td valign="top" >TBD</td>
Quoted 9 lines
Documentation/DocBook/drm.tmpl void intel_crt_init(struct drm_device *dev) · line 2861
Quoted 3 lines
Documentation/DocBook/drm.tmpl +3
-0+ <td valign="top" >To set plane HW rotation. This rotation property does + the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td>Since this touches shared code can you please resend this patch with dri-devel added to cc? BKM is to add a Cc: dri-devel ... line to the sob section of the patch, then git send-email will automatically pick it up. -Daniel
But I am changing the description for only the rotation property for i915.
Quoted 11 lines
Quoted 10 lines
Documentation/DocBook/drm.tmpl +0
-1</tr> <tr> <td rowspan="17" valign="top" >SDVO-TV</td> --1.7.10.4
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] Documentation/drm: Update rotation property with 90/270 and description
Quoted 31 lines · Sonika
Quoted 19 lines · Daniel Vetter
Quoted 18 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/DocBook/drm.tmpl +2
-2@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td> <td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td> <td valign="top" >Plane</td> - <td valign="top" >TBD</td>Quoted 9 lines
Documentation/DocBook/drm.tmpl void intel_crt_init(struct drm_device *dev) · line 2861
Quoted 3 lines
Documentation/DocBook/drm.tmpl +3
-0+ <td valign="top" >To set plane HW rotation. This rotation property does + the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td>Since this touches shared code can you please resend this patch with dri-devel added to cc? BKM is to add a Cc: dri-devel ... line to the sob section of the patch, then git send-email will automatically pick it up. -Daniel
Quoted 1 line
But I am changing the description for only the rotation property for i915.
Property names are supposed to be somewhat standardized across all drivers, and omapdrm already supports rotation. This is way we have this shared table, so that other driver authors know what's going on. Please resend.
-Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 1/1 Documentation/drm: Update rotation property with 90/270 and description
--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Documentation/DocBook/drm.tmpl +5 -2
@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td> <td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td> <td valign="top" >Plane</td> - <td valign="top" >TBD</td> + <td valign="top" >To set plane HW rotation. This rotation property does + the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td> </tr> <tr> <td rowspan="17" valign="top" >SDVO-TV</td>
Signature · 6 lines
1.7.10.4 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] Documentation/drm: Update rotation property with 90/270 and description
Quoted 12 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/DocBook/drm.tmpl +0
-0@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td>
Documentation/DocBook/drm.tmpl void intel_crt_init(struct drm_device *dev) · line 2861
Quoted 10 lines
Documentation/DocBook/drm.tmpl +5
-2<td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td> <td valign="top" >Plane</td> - <td valign="top" >TBD</td> + <td valign="top" >To set plane HW rotation. This rotation property does + the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td>
I would suggest moving the thing to the generci props section since we have omap and i915 both supporting it.
As for the description, we should also document the reflect flags.
I might write it as something like this:
"rotate-0,rotate-90,rotate-180,rotate-270 rotate the image by the specified amount in degrees in a counter clockwise direction. reflect-x,reflect-y reflect the image along the specified axis, prior to rotation."
Quoted 10 lines
Documentation/DocBook/drm.tmpl +0
-0</tr> <tr> <td rowspan="17" valign="top" >SDVO-TV</td>Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] Documentation/drm: Update rotation property with 90/270 and description
Quoted 18 lines · Ville Syrjälä
Quoted 17 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/DocBook/drm.tmpl +2
-1@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td> <td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td>
Quoted 8 lines
Documentation/DocBook/drm.tmpl void intel_crt_init(struct drm_device *dev) · line 2861
Quoted 5 lines
Documentation/DocBook/drm.tmpl +3
-1<td valign="top" >Plane</td> - <td valign="top" >TBD</td> + <td valign="top" >To set plane HW rotation. This rotation property does + the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td>I would suggest moving the thing to the generci props section since we have omap and i915 both supporting it.
You mean in DRM properties section?
Right now, OMAP section also has rotation property. I will remove it from OMAP section as well if you think drm is the better place.
Quoted 20 lines
As for the description, we should also document the reflect flags.
I might write it as something like this:
"rotate-0,rotate-90,rotate-180,rotate-270 rotate the image by the specified amount in degrees in a counter clockwise direction. reflect-x,reflect-y reflect the image along the specified axis, prior to rotation."Quoted 10 lines
Documentation/DocBook/drm.tmpl +0
-1</tr> <tr> <td rowspan="17" valign="top" >SDVO-TV</td> --1.7.10.4
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Documentation/drm: Update rotation property with 90/270 and description
Quoted 31 lines · Sonika
Quoted 20 lines · Ville Syrjälä
Quoted 19 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/DocBook/drm.tmpl +2
-1@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td> <td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td> <td valign="top" >Plane</td>Quoted 8 lines
Documentation/DocBook/drm.tmpl void intel_crt_init(struct drm_device *dev) · line 2861
Quoted 5 lines
Documentation/DocBook/drm.tmpl +1
-1- <td valign="top" >TBD</td> + <td valign="top" >To set plane HW rotation. This rotationproperty does
Documentation/DocBook/drm.tmpl +2
-0+ the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td>I would suggest moving the thing to the generci props section since we have omap and i915 both supporting it.
Quoted 7 lines
You mean in DRM properties section?
Right now, OMAP section also has rotation property. I will remove it from OMAP section as well if you think drm is the better place.Quoted 3 lines
As for the description, we should also document the reflect flags.
Also, i915 doesn't support reflect flags. Only rotation is supported there. For the "generic" part, you meant just moving to the generic group in i915 property section?
Quoted 12 lines
Quoted 12 lines
I might write it as something like this:
"rotate-0,rotate-90,rotate-180,rotate-270 rotate the image by the specified amount in degrees in a counter clockwise direction. reflect-x,reflect-y reflect the image along the specified axis, prior to rotation."Quoted 6 lines
</tr> <tr> <td rowspan="17" valign="top" >SDVO-TV</td>--
1.7.10.4
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH] Documentation/drm: Update rotation property with 90/270 and description
In reply to Jindal, SonikaQuoted 31 lines · Sonika
Quoted 20 lines · Ville Syrjälä
Quoted 19 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/DocBook/drm.tmpl +2
-1@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td> <td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td> <td valign="top" >Plane</td>Quoted 8 lines
Documentation/DocBook/drm.tmpl void intel_crt_init(struct drm_device *dev) · line 2861
Quoted 5 lines
Documentation/DocBook/drm.tmpl +1
-1- <td valign="top" >TBD</td> + <td valign="top" >To set plane HW rotation. This rotationproperty does
Documentation/DocBook/drm.tmpl +2
-0+ the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td>I would suggest moving the thing to the generci props section since we have omap and i915 both supporting it.
Quoted 7 lines
You mean in DRM properties section?
Right now, OMAP section also has rotation property. I will remove it from OMAP section as well if you think drm is the better place.Quoted 3 lines
As for the description, we should also document the reflect flags.
i915 doesn't support reflect flags. It only create rotation property with rotation flags.
For "generic" section, you mean moving to generic group of properties in i915 only right?
Quoted 12 lines
Quoted 12 lines
I might write it as something like this:
"rotate-0,rotate-90,rotate-180,rotate-270 rotate the image by the specified amount in degrees in a counter clockwise direction. reflect-x,reflect-y reflect the image along the specified axis, prior to rotation."Quoted 6 lines
</tr> <tr> <td rowspan="17" valign="top" >SDVO-TV</td>--
1.7.10.4
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH] Documentation/drm: Update rotation property with 90/270 and description
Quoted 34 lines · Sonika
Quoted 31 lines · Sonika
Quoted 28 lines · Ville Syrjälä
Quoted 22 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>--- Documentation/DocBook/drm.tmpl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)Documentation/DocBook/drm.tmpl +3
-2@@ -2853,9 +2853,12 @@ void intel_crt_init(struct drm_device *dev) <td rowspan="1" valign="top" >Plane</td> <td valign="top" >“rotation”</td> <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> + <td valign="top" >{ 0, "rotate-0" }, { 1, "rotate-90" }, + { 2, "rotate-180" }, { 3, "rotate-270" }</td> <td valign="top" >Plane</td> - <td valign="top" >TBD</td> + <td valign="top" >To set plane HW rotation. This rotationproperty does
Documentation/DocBook/drm.tmpl void intel_crt_init(struct drm_device *dev) · line 2861
Quoted 2 lines
Documentation/DocBook/drm.tmpl +2
-0+ the plane rotation in counter clockwise direction which is + inline with the way XRandr works.</td>I would suggest moving the thing to the generci props section since we have omap and i915 both supporting it.
Quoted 8 lines
Quoted 7 lines
You mean in DRM properties section?
Right now, OMAP section also has rotation property. I will remove it from OMAP section as well if you think drm is the better place.Quoted 3 lines
As for the description, we should also document the reflect flags.
i915 doesn't support reflect flags.
It will as soon as I (or someone else) find a bit of time to implement it.
Quoted 5 lines
It only create rotation property
with rotation flags.For "generic" section, you mean moving to generic group of properties in i915 only right?
I meant the entire thing. I don't think we should care too much about which actual bits each driver supports since that can even depend on which plane is used in some specific platform. So we should just document all the possible bits, and if you want you could add a note stating that a specific driver/platform/plane may not support all of them.
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
I might write it as something like this:
"rotate-0,rotate-90,rotate-180,rotate-270 rotate the image by the specified amount in degrees in a counter clockwise direction. reflect-x,reflect-y reflect the image along the specified axis, prior to rotation."Quoted 10 lines
</tr> <tr> <td rowspan="17" valign="top" >SDVO-TV</td>--
1.7.10.4_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 1/1 Documentation/drm: Update rotation property
Moving rotation property to "Drm" and removing from i915 and omap. Also, adding description to the property
--- Documentation/DocBook/drm.tmpl | 41 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 23 deletions(-)
Documentation/DocBook/drm.tmpl +18 -23
@@ -2585,7 +2585,22 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >Description/Restrictions</td> </tr> <tr> - <td rowspan="36" valign="top" >DRM</td> + <td rowspan="37" valign="top" >DRM</td> + <td valign="top" >Generic</td> + <td valign="top" >“rotation”</td> + <td valign="top" >BITMASK</td> + <td valign="top" >{ 0, "rotate-0" }, + { 1, "rotate-90" }, + { 2, "rotate-180" }, + { 3, "rotate-270" }, + { 4, "reflect-x" }, + { 5, "reflect-y" }</td> + <td valign="top" >CRTC, Plane</td> + <td valign="top" >rotate-(degrees) rotates the image by the specified amount in degrees + in counter clockwise direction. reflect-x and reflect-y reflects the + image along the specified axis prior to rotation</td> + </tr> + <tr> <td rowspan="5" valign="top" >Connector</td> <td valign="top" >“EDID”</td> <td valign="top" >BLOB | IMMUTABLE</td>
@@ -2846,7 +2861,7 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >TBD</td> </tr> <tr> - <td rowspan="21" valign="top" >i915</td> + <td rowspan="20" valign="top" >i915</td> <td rowspan="2" valign="top" >Generic</td> <td valign="top" >"Broadcast RGB"</td> <td valign="top" >ENUM</td>
@@ -2862,14 +2877,6 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >TBD</td> </tr> <tr> - <td rowspan="1" valign="top" >Plane</td> - <td valign="top" >“rotation”</td> - <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> - <td valign="top" >Plane</td> - <td valign="top" >TBD</td> - </tr> - <tr> <td rowspan="17" valign="top" >SDVO-TV</td> <td valign="top" >“mode”</td> <td valign="top" >ENUM</td>
@@ -3377,19 +3384,7 @@ void intel_crt_init(struct drm_device *dev) </tr> <tr> <td rowspan="2" valign="top" >omap</td> - <td rowspan="2" valign="top" >Generic</td> - <td valign="top" >“rotation”</td> - <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, - { 1, "rotate-90" }, - { 2, "rotate-180" }, - { 3, "rotate-270" }, - { 4, "reflect-x" }, - { 5, "reflect-y" }</td> - <td valign="top" >CRTC, Plane</td> - <td valign="top" >TBD</td> - </tr> - <tr> + <td valign="top" >Generic</td> <td valign="top" >“zorder”</td> <td valign="top" >RANGE</td> <td valign="top" >Min=0, Max=3</td>
Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter
AppliedRe: [Intel-gfx] [PATCH] Documentation/drm: Update rotation property
Quoted 6 lines · Sonika Jindal
Moving rotation property to "Drm" and removing from i915 and omap. Also, adding description to the property
Cc DRI Development <[email protected]>Suggested-by Ville Syrjälä <[email protected]>Signed-off-by Sonika Jindal <[email protected]>
Applied to drm-misc, thanks.
-Daniel
Quoted 84 lines
--- Documentation/DocBook/drm.tmpl | 41 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 23 deletions(-)Documentation/DocBook/drm.tmpl +18
-23@@ -2585,7 +2585,22 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >Description/Restrictions</td> </tr> <tr> - <td rowspan="36" valign="top" >DRM</td> + <td rowspan="37" valign="top" >DRM</td> + <td valign="top" >Generic</td> + <td valign="top" >“rotation”</td> + <td valign="top" >BITMASK</td> + <td valign="top" >{ 0, "rotate-0" }, + { 1, "rotate-90" }, + { 2, "rotate-180" }, + { 3, "rotate-270" }, + { 4, "reflect-x" }, + { 5, "reflect-y" }</td> + <td valign="top" >CRTC, Plane</td> + <td valign="top" >rotate-(degrees) rotates the image by the specified amount in degrees + in counter clockwise direction. reflect-x and reflect-y reflects the + image along the specified axis prior to rotation</td> + </tr> + <tr> <td rowspan="5" valign="top" >Connector</td> <td valign="top" >“EDID”</td> <td valign="top" >BLOB | IMMUTABLE</td>@@ -2846,7 +2861,7 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >TBD</td> </tr> <tr> - <td rowspan="21" valign="top" >i915</td> + <td rowspan="20" valign="top" >i915</td> <td rowspan="2" valign="top" >Generic</td> <td valign="top" >"Broadcast RGB"</td> <td valign="top" >ENUM</td>@@ -2862,14 +2877,6 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >TBD</td> </tr> <tr> - <td rowspan="1" valign="top" >Plane</td> - <td valign="top" >“rotation”</td> - <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td> - <td valign="top" >Plane</td> - <td valign="top" >TBD</td> - </tr> - <tr> <td rowspan="17" valign="top" >SDVO-TV</td> <td valign="top" >“mode”</td> <td valign="top" >ENUM</td>@@ -3377,19 +3384,7 @@ void intel_crt_init(struct drm_device *dev) </tr> <tr> <td rowspan="2" valign="top" >omap</td> - <td rowspan="2" valign="top" >Generic</td> - <td valign="top" >“rotation”</td> - <td valign="top" >BITMASK</td> - <td valign="top" >{ 0, "rotate-0" }, - { 1, "rotate-90" }, - { 2, "rotate-180" }, - { 3, "rotate-270" }, - { 4, "reflect-x" }, - { 5, "reflect-y" }</td> - <td valign="top" >CRTC, Plane</td> - <td valign="top" >TBD</td> - </tr> - <tr> + <td valign="top" >Generic</td> <td valign="top" >“zorder”</td> <td valign="top" >RANGE</td> <td valign="top" >Min=0, Max=3</td>Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/i915: Swapping 90 and 270 to be compliant with Xrandr
In reply to Sonika JindalQuoted 7 lines · Sonika Jindal
Since
DRM_ROTATEis counter clockwise (which is compliant with Xrandr), and HW rotation is clockwise, swapping 90/270 to work as expected from userspace.Suggested-by Ville Syrjälä <[email protected]>Cc Ville Syrjälä <[email protected]>Signed-off-by Sonika Jindal <[email protected]>
Looks like this needs a rebase, but other than that this is
Quoted 61 lines
--- drivers/gpu/drm/i915/intel_display.c | 8 ++++++-- drivers/gpu/drm/i915/intel_sprite.c | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-)drivers/gpu/drm/i915/intel_display.c +6
-2@@ -3039,8 +3039,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, plane = crtc->primary; rotation = plane->state->rotation;switch (rotation) { +/* +* DRM_ROTATE_ is counter clockwise to stay compatible with Xrandr +* while i915 HW rotation is clockwise, thats why this swapping. +*/ case BIT (DRM_ROTATE_90 ): - plane_ctl |=PLANE_CTL_ROTATE_90 ; + plane_ctl |=PLANE_CTL_ROTATE_270 ;break ;case BIT (DRM_ROTATE_180 ):@@ -3048,7 +3052,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,break ;case BIT (DRM_ROTATE_270 ): - plane_ctl |=PLANE_CTL_ROTATE_270 ; + plane_ctl |=PLANE_CTL_ROTATE_90 ;break ; }drivers/gpu/drm/i915/intel_sprite.c +6
-2@@ -259,8 +259,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, rotation = drm_plane->state->rotation;switch (rotation) { +/* +* DRM_ROTATE_ is counter clockwise to stay compatible with Xrandr +* while i915 HW rotation is clockwise, thats why this swapping. +*/ case BIT (DRM_ROTATE_90 ): - plane_ctl |=PLANE_CTL_ROTATE_90 ; + plane_ctl |=PLANE_CTL_ROTATE_270 ;break ;case BIT (DRM_ROTATE_180 ):@@ -268,7 +272,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,break ;case BIT (DRM_ROTATE_270 ): - plane_ctl |=PLANE_CTL_ROTATE_270 ; + plane_ctl |=PLANE_CTL_ROTATE_90 ;break ; }Signature · 1 line
1.7.10.4
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 1/1 drm/i915: Swapping 90 and 270 to be compliant with Xrandr
Since DRM_ROTATE is counter clockwise (which is compliant with Xrandr), and HW rotation is clockwise, swapping 90/270 to work as expected from userspace.
v2: Rebased
--- drivers/gpu/drm/i915/intel_display.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_display.c +6 -2
@@ -3010,12 +3010,16 @@ u32 skl_plane_ctl_rotation(unsigned int rotation)switch (rotation) {case BIT (DRM_ROTATE_0 ):break ; +/* +* DRM_ROTATE_ is counter clockwise to stay compatible with Xrandr +* while i915 HW rotation is clockwise, thats why this swapping. +*/ case BIT (DRM_ROTATE_90 ): -return PLANE_CTL_ROTATE_90 ; +return PLANE_CTL_ROTATE_270 ;case BIT (DRM_ROTATE_180 ):return PLANE_CTL_ROTATE_180 ;case BIT (DRM_ROTATE_270 ): -return PLANE_CTL_ROTATE_270 ; +return PLANE_CTL_ROTATE_90 ;default :MISSING_CASE (rotation); }
Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH] drm/i915: Swapping 90 and 270 to be compliant with Xrandr
Quoted 10 lines · Sonika Jindal
Since
DRM_ROTATEis counter clockwise (which is compliant with Xrandr), and HW rotation is clockwise, swapping 90/270 to work as expected from userspace.v2: Rebased
Suggested-by Ville Syrjälä <[email protected]>Cc Ville Syrjälä <[email protected]>Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Ville Syrjälä <[email protected]>
I've fixed the summary to make it clear this is skl-only and applied the patch to dinq, thanks.
-Daniel
Quoted 34 lines
--- drivers/gpu/drm/i915/intel_display.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)drivers/gpu/drm/i915/intel_display.c +6
-2@@ -3010,12 +3010,16 @@ u32 skl_plane_ctl_rotation(unsigned int rotation)switch (rotation) {case BIT (DRM_ROTATE_0 ):break ; +/* +* DRM_ROTATE_ is counter clockwise to stay compatible with Xrandr +* while i915 HW rotation is clockwise, thats why this swapping. +*/ case BIT (DRM_ROTATE_90 ): -return PLANE_CTL_ROTATE_90 ; +return PLANE_CTL_ROTATE_270 ;case BIT (DRM_ROTATE_180 ):return PLANE_CTL_ROTATE_180 ;case BIT (DRM_ROTATE_270 ): -return PLANE_CTL_ROTATE_270 ; +return PLANE_CTL_ROTATE_90 ;default :MISSING_CASE (rotation); }Signature · 6 lines
1.7.10.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
In reply to Jindal, SonikaQuoted 10 lines · Sonika
Quoted 8 lines
Quoted 4 lines
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.
Signature · 5 lines
Damien _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
In reply to Daniel VetterQuoted 12 lines · Daniel Vetter
Quoted 11 lines · 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 forpixel_format, some cleanup (review comments)
v4: Adding rightpixel_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: Movingpixel_formatcheck tointel_atomic_plane_check(Matt)
Quoted 6 lines
Quoted 2 lines
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>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
Quoted 299 lines
Quoted 299 lines
--- 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(-)drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -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_A 0x70188#define _PLANE_STRIDE_2_A 0x70288#define _PLANE_STRIDE_3_A 0x70388drivers/gpu/drm/i915/intel_atomic_plane.c +24
-0@@ -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); }drivers/gpu/drm/i915/intel_display.c +62
-26@@ -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 |=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 (intel_rotation_90_or_270 (rotation)) { +/* 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 ); + } + plane_offset = y_offset <<16 | x_offset;I915_WRITE (PLANE_CTL (pipe,0 ), plane_ctl);I915_WRITE (PLANE_POS (pipe,0 ),0 ); -I915_WRITE (PLANE_OFFSET (pipe,0 ), (y <<16 ) | x); -I915_WRITE (PLANE_SIZE (pipe,0 ), - (intel_crtc->config->pipe_src_h -1 ) <<16 | - (intel_crtc->config->pipe_src_w -1 )); -I915_WRITE (PLANE_STRIDE (pipe,0 ), fb->pitches[0 ] / stride_div); +I915_WRITE (PLANE_OFFSET (pipe,0 ), plane_offset); +I915_WRITE (PLANE_SIZE (pipe,0 ), plane_size); +I915_WRITE (PLANE_STRIDE (pipe,0 ), stride);I915_WRITE (PLANE_SURF (pipe,0 ), surf_addr);POSTING_READ (PLANE_SURF (pipe,0 ));@@ -12406,23 +12433,32 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, intel_primary_formats, num_formats, DRM_PLANE_TYPE_PRIMARY); -if (INTEL_INFO (dev)->gen >=4 ) { -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); -if (dev->mode_config.rotation_property) -drm_object_attach_property (&primary->base.base, - dev->mode_config.rotation_property, - state->base.rotation); - } +if (INTEL_INFO (dev)->gen >=4 ) +intel_create_rotation_property (dev, primary);drm_plane_helper_add (&primary->base, &intel_plane_helper_funcs);return &primary->base; } +void intel_create_rotation_property (struct drm_device *dev,struct intel_plane *plane) +{ +if (!dev->mode_config.rotation_property) { +unsigned long flags =BIT (DRM_ROTATE_0 ) | +BIT (DRM_ROTATE_180 ); + +if (INTEL_INFO (dev)->gen >=9 ) + flags |=BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 ); + + dev->mode_config.rotation_property = +drm_mode_create_rotation_property (dev, flags); + } +if (dev->mode_config.rotation_property) +drm_object_attach_property (&plane->base.base, + dev->mode_config.rotation_property, + plane->base.state->rotation); +} +static int intel_check_cursor_plane (struct drm_plane *plane,struct intel_plane_state *state)drivers/gpu/drm/i915/intel_drv.h +6
-0@@ -995,6 +995,12 @@ intel_rotation_90_or_270(unsigned int rotation)return rotation & (BIT (DRM_ROTATE_90 ) |BIT (DRM_ROTATE_270 )); } +unsigned int +intel_tile_height (struct drm_device *dev,uint32_t bits_per_pixel, +uint64_t fb_modifier); +void intel_create_rotation_property (struct drm_device *dev, +struct intel_plane *plane); +bool intel_wm_need_update (struct drm_plane *plane,struct drm_plane_state *state);drivers/gpu/drm/i915/intel_sprite.c +37
-15@@ -190,10 +190,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,struct drm_i915_gem_object *obj =intel_fb_obj (fb);const int pipe = intel_plane->pipe;const int plane = intel_plane->plane +1 ; -u32 plane_ctl, stride_div; +u32 plane_ctl, stride_div, stride;int pixel_size =drm_format_plane_cpp (fb->pixel_format,0 );const struct drm_intel_sprite_colorkey *key = &intel_plane->ckey;unsigned long surf_addr; +u32 tile_height, plane_offset, plane_size; +unsigned int rotation; +int x_offset, y_offset; plane_ctl =PLANE_CTL_ENABLE |PLANE_CTL_PIPE_CSC_ENABLE ;@@ -254,8 +257,20 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,MISSING_CASE (fb->modifier[0 ]); } -if (drm_plane->state->rotation ==BIT (DRM_ROTATE_180 )) + rotation = drm_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 ; + } intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h, pixel_size, true,@@ -283,10 +298,26 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, surf_addr =intel_plane_obj_offset (intel_plane, obj); -I915_WRITE (PLANE_OFFSET (pipe, plane), (y <<16 ) | x); -I915_WRITE (PLANE_STRIDE (pipe, plane), fb->pitches[0 ] / stride_div); +if (intel_rotation_90_or_270 (rotation)) { +/* 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); + plane_size = (src_w <<16 ) | src_h; + x_offset = stride * tile_height - y - (src_h +1 ); + y_offset = x; + }else { + stride = fb->pitches[0 ] / stride_div; + plane_size = (src_h <<16 ) | src_w; + x_offset = x; + y_offset = y; + } + plane_offset = y_offset <<16 | x_offset; + +I915_WRITE (PLANE_OFFSET (pipe, plane), plane_offset); +I915_WRITE (PLANE_STRIDE (pipe, plane), stride);I915_WRITE (PLANE_POS (pipe, plane), (crtc_y <<16 ) | crtc_x); -I915_WRITE (PLANE_SIZE (pipe, plane), (crtc_h <<16 ) | crtc_w); +I915_WRITE (PLANE_SIZE (pipe, plane), plane_size);I915_WRITE (PLANE_CTL (pipe, plane), plane_ctl);I915_WRITE (PLANE_SURF (pipe, plane), surf_addr);POSTING_READ (PLANE_SURF (pipe, plane));@@ -1310,16 +1341,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) goto out; } -if (!dev->mode_config.rotation_property) - dev->mode_config.rotation_property = -drm_mode_create_rotation_property (dev, -BIT (DRM_ROTATE_0 ) | -BIT (DRM_ROTATE_180 )); - -if (dev->mode_config.rotation_property) -drm_object_attach_property (&intel_plane->base.base, - dev->mode_config.rotation_property, - state->base.rotation); +intel_create_rotation_property (dev, intel_plane);drm_plane_helper_add (&intel_plane->base, &intel_plane_helper_funcs);
Quoted 8 lines
Quoted 7 lines
--
1.7.10.4_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 2/2] drm/i915/skl: Support for 90/270 rotation
In reply to Sonika JindalTask 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 Test drm-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 [email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 24 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/intel_display.c +4
-0@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip; +bool can_position = false;int ret; crtc = crtc ? crtc : plane->crtc; intel_crtc =to_intel_crtc (crtc); +if (INTEL_INFO (dev)->gen >=9 ) + can_position = true; +
drivers/gpu/drm/i915/intel_display.c intel_check_primary_plane(struct drm_plane *plane, · line 12170
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +2
-1ret =drm_plane_helper_check_update (plane, crtc, fb, src, dest, clip,DRM_PLANE_HELPER_NO_SCALING ,DRM_PLANE_HELPER_NO_SCALING , - false, true, &state->visible); + can_position, true, + &state->visible);if (ret)return ret;
I have discovered today that, while this allows SetCrtc and SetPlane ioctls to work with frame buffers which do not cover the plane, page flips are not that lucky and fail roughly with:
[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC viewport 1920x1080+0+0.
I have posted a quick IGT exerciser for this as "kms_rotation_crc: Excercise page flips with 90 degree rotation". May not be that great but shows the failure.
I am not that hot on meddling with this code, nor do I feel competent to even try on my own at least. :/ Maybe just because the atomic and plane related rewrites have been going on for so long, and have multiple people involved, it all sounds pretty scary and fragile.
But I think some sort of plan on how to fix this could be in order?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 41 lines · Tvrtko Ursulin
Quoted 24 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/intel_display.c +4
-0@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip; +bool can_position = false;int ret; crtc = crtc ? crtc : plane->crtc; intel_crtc =to_intel_crtc (crtc); +if (INTEL_INFO (dev)->gen >=9 ) + can_position = true; +drivers/gpu/drm/i915/intel_display.c intel_check_primary_plane(struct drm_plane *plane, · line 12170
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +2
-1ret =drm_plane_helper_check_update (plane, crtc, fb, src, dest, clip,DRM_PLANE_HELPER_NO_SCALING ,DRM_PLANE_HELPER_NO_SCALING , - false, true, &state->visible); + can_position, true, + &state->visible);if (ret)return ret;I have discovered today that, while this allows SetCrtc and SetPlane ioctls to work with frame buffers which do not cover the plane, page flips are not that lucky and fail roughly with:
Quoted 2 lines
[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC viewport 1920x1080+0+0.
Maybe I'm misunderstanding your explanation, but a framebuffer is always required to fill/cover the plane scanning out of it. What this patch is supposed to be allowing is for the primary plane to not cover the entire CRTC (since that's something that only became possible for Intel hardware on the gen9+ platforms). I.e., the primary plane is now allowed to positioned and resized to cover a subset of the CRTC area, just like "sprite" planes have always been able to.
If you've got a 1080x1080 framebuffer, then it's legal to have a 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT. However it is not legal to size the primary plane as 1920x1080 and use this same 1080x1080 framebuffer with any of our interfaces (setplane, setcrtc, pageflip, or atomic).
Are you using ioctls/libdrm directly or are you using igt_kms helpers? IIRC, the IGT helpers will try to be extra helpful and automatically size the plane to match the framebuffer (unless you override that behavior), so that might be what's causing the confusion here.
Matt
Quoted 16 lines
I have posted a quick IGT exerciser for this as "
kms_rotation_crc:
Excercise page flips with 90 degree rotation". May not be that great but shows the failure.I am not that hot on meddling with this code, nor do I feel competent to even try on my own at least. :/ Maybe just because the atomic and plane related rewrites have been going on for so long, and have multiple people involved, it all sounds pretty scary and fragile.
But I think some sort of plan on how to fix this could be in order?
Regards,
Tvrtko
Signature · 9 lines
Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 59 lines · Matt Roper
Quoted 41 lines · Tvrtko Ursulin
Quoted 24 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/intel_display.c +4
-0@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip; +bool can_position = false;int ret; crtc = crtc ? crtc : plane->crtc; intel_crtc =to_intel_crtc (crtc); +if (INTEL_INFO (dev)->gen >=9 ) + can_position = true; +drivers/gpu/drm/i915/intel_display.c intel_check_primary_plane(struct drm_plane *plane, · line 12170
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +2
-1ret =drm_plane_helper_check_update (plane, crtc, fb, src, dest, clip,DRM_PLANE_HELPER_NO_SCALING ,DRM_PLANE_HELPER_NO_SCALING , - false, true, &state->visible); + can_position, true, + &state->visible);if (ret)return ret;I have discovered today that, while this allows SetCrtc and SetPlane ioctls to work with frame buffers which do not cover the plane, page flips are not that lucky and fail roughly with:
Quoted 2 lines
[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC viewport 1920x1080+0+0.
Maybe I'm misunderstanding your explanation, but a framebuffer is always required to fill/cover the plane scanning out of it. What this patch is supposed to be allowing is for the primary plane to not cover the entire CRTC (since that's something that only became possible for Intel hardware on the gen9+ platforms). I.e., the primary plane is now allowed to positioned and resized to cover a subset of the CRTC area, just like "sprite" planes have always been able to.
If you've got a 1080x1080 framebuffer, then it's legal to have a 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT. However it is not legal to size the primary plane as 1920x1080 and use this same 1080x1080 framebuffer with any of our interfaces (setplane, setcrtc, pageflip, or atomic).
Quoted 4 lines
Are you using ioctls/libdrm directly or are you using
igt_kmshelpers? IIRC, the IGT helpers will try to be extra helpful and automatically size the plane to match the framebuffer (unless you override that behavior), so that might be what's causing the confusion here.
The problem is clear as day in drm_mode_page_flip_ioctl():
ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb); if (ret)
goto out;
The fix should be easy; just extract the current src coordinates from the plane state and check those against the new fb size. And then hope that the plane state is really up to date.
And I'm sure rotated cases will go boom in some other ways. Probably we should just switch over to using the full plane update for mmio flips to fix it.
Quoted 27 lines
Matt
Quoted 16 lines
I have posted a quick IGT exerciser for this as "
kms_rotation_crc:
Excercise page flips with 90 degree rotation". May not be that great but shows the failure.I am not that hot on meddling with this code, nor do I feel competent to even try on my own at least. :/ Maybe just because the atomic and plane related rewrites have been going on for so long, and have multiple people involved, it all sounds pretty scary and fragile.
But I think some sort of plan on how to fix this could be in order?
Regards,
Tvrtko
Signature · 5 lines
Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 65 lines · Ville Syrjälä
Quoted 59 lines · Matt Roper
Quoted 41 lines · Tvrtko Ursulin
Quoted 24 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/intel_display.c +4
-0@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip; +bool can_position = false;int ret; crtc = crtc ? crtc : plane->crtc; intel_crtc =to_intel_crtc (crtc); +if (INTEL_INFO (dev)->gen >=9 ) + can_position = true; +drivers/gpu/drm/i915/intel_display.c intel_check_primary_plane(struct drm_plane *plane, · line 12170
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +2
-1ret =drm_plane_helper_check_update (plane, crtc, fb, src, dest, clip,DRM_PLANE_HELPER_NO_SCALING ,DRM_PLANE_HELPER_NO_SCALING , - false, true, &state->visible); + can_position, true, + &state->visible);if (ret)return ret;I have discovered today that, while this allows SetCrtc and SetPlane ioctls to work with frame buffers which do not cover the plane, page flips are not that lucky and fail roughly with:
Quoted 2 lines
[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC viewport 1920x1080+0+0.
Maybe I'm misunderstanding your explanation, but a framebuffer is always required to fill/cover the plane scanning out of it. What this patch is supposed to be allowing is for the primary plane to not cover the entire CRTC (since that's something that only became possible for Intel hardware on the gen9+ platforms). I.e., the primary plane is now allowed to positioned and resized to cover a subset of the CRTC area, just like "sprite" planes have always been able to.
If you've got a 1080x1080 framebuffer, then it's legal to have a 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT. However it is not legal to size the primary plane as 1920x1080 and use this same 1080x1080 framebuffer with any of our interfaces (setplane, setcrtc, pageflip, or atomic).
Quoted 4 lines
Are you using ioctls/libdrm directly or are you using
igt_kmshelpers? IIRC, the IGT helpers will try to be extra helpful and automatically size the plane to match the framebuffer (unless you override that behavior), so that might be what's causing the confusion here.
Quoted 8 lines
The problem is clear as day in
drm_mode_page_flip_ioctl():
ret =drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb); if (ret)goto out;The fix should be easy; just extract the current src coordinates from the plane state and check those against the new fb size. And then hope that the plane state is really up to date.
Yep, that's the conclusion we came to once Tvrtko explained what he was seeing on IRC. I'm not sure whether non-atomic drivers have enough state setup by the default helpers to work properly. Worst case we'll just assume that a non-atomic driver won't support primary plane windowing (since none have in the past) and fall back to looking at the mode for legacy non-atomic drivers.
Quoted 4 lines
And I'm sure rotated cases will go boom in some other ways. Probably we should just switch over to using the full plane update for mmio flips to fix it.
Yeah; the core looks at a drm_plane->invert_dimensions field that's only set by omap. That should probably be updated to look at the state's rotation on atomic-capable drivers.
Matt
Quoted 32 lines
Quoted 28 lines
Matt
Quoted 16 lines
I have posted a quick IGT exerciser for this as "
kms_rotation_crc:
Excercise page flips with 90 degree rotation". May not be that great but shows the failure.I am not that hot on meddling with this code, nor do I feel competent to even try on my own at least. :/ Maybe just because the atomic and plane related rewrites have been going on for so long, and have multiple people involved, it all sounds pretty scary and fragile.
But I think some sort of plan on how to fix this could be in order?
Regards,
Tvrtko
Signature · 5 lines
Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795Signature · 2 lines
Ville Syrjälä Intel OTC
Signature · 9 lines
Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 83 lines · Matt Roper
Quoted 65 lines · Ville Syrjälä
Quoted 59 lines · Matt Roper
Quoted 41 lines · Tvrtko Ursulin
Quoted 24 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/intel_display.c +4
-0@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip; +bool can_position = false;int ret; crtc = crtc ? crtc : plane->crtc; intel_crtc =to_intel_crtc (crtc); +if (INTEL_INFO (dev)->gen >=9 ) + can_position = true; +drivers/gpu/drm/i915/intel_display.c intel_check_primary_plane(struct drm_plane *plane, · line 12170
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +2
-1ret =drm_plane_helper_check_update (plane, crtc, fb, src, dest, clip,DRM_PLANE_HELPER_NO_SCALING ,DRM_PLANE_HELPER_NO_SCALING , - false, true, &state->visible); + can_position, true, + &state->visible);if (ret)return ret;I have discovered today that, while this allows SetCrtc and SetPlane ioctls to work with frame buffers which do not cover the plane, page flips are not that lucky and fail roughly with:
Quoted 2 lines
[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC viewport 1920x1080+0+0.
Maybe I'm misunderstanding your explanation, but a framebuffer is always required to fill/cover the plane scanning out of it. What this patch is supposed to be allowing is for the primary plane to not cover the entire CRTC (since that's something that only became possible for Intel hardware on the gen9+ platforms). I.e., the primary plane is now allowed to positioned and resized to cover a subset of the CRTC area, just like "sprite" planes have always been able to.
If you've got a 1080x1080 framebuffer, then it's legal to have a 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT. However it is not legal to size the primary plane as 1920x1080 and use this same 1080x1080 framebuffer with any of our interfaces (setplane, setcrtc, pageflip, or atomic).
Quoted 4 lines
Are you using ioctls/libdrm directly or are you using
igt_kmshelpers? IIRC, the IGT helpers will try to be extra helpful and automatically size the plane to match the framebuffer (unless you override that behavior), so that might be what's causing the confusion here.Quoted 8 lines
The problem is clear as day in
drm_mode_page_flip_ioctl():
ret =drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb); if (ret)goto out;The fix should be easy; just extract the current src coordinates from the plane state and check those against the new fb size. And then hope that the plane state is really up to date.
Yep, that's the conclusion we came to once Tvrtko explained what he was seeing on IRC. I'm not sure whether non-atomic drivers have enough state setup by the default helpers to work properly. Worst case we'll just assume that a non-atomic driver won't support primary plane windowing (since none have in the past) and fall back to looking at the mode for legacy non-atomic drivers.
Quoted 1 line
Quoted 7 lines
Quoted 3 lines
And I'm sure rotated cases will go boom in some other ways. Probably we should just switch over to using the full plane update for mmio flips to fix it.
Yeah; the core looks at a
drm_plane->invert_dimensionsfield that's only set by omap. That should probably be updated to look at the state's rotation on atomic-capable drivers.
We can just look at the src coordinates. Those always match the fb orientation.
Quoted 43 lines
Matt
Quoted 32 lines
Quoted 28 lines
Matt
Quoted 16 lines
I have posted a quick IGT exerciser for this as "
kms_rotation_crc:
Excercise page flips with 90 degree rotation". May not be that great but shows the failure.I am not that hot on meddling with this code, nor do I feel competent to even try on my own at least. :/ Maybe just because the atomic and plane related rewrites have been going on for so long, and have multiple people involved, it all sounds pretty scary and fragile.
But I think some sort of plan on how to fix this could be in order?
Regards,
Tvrtko
Signature · 5 lines
Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795Signature · 2 lines
Ville Syrjälä Intel OTCSignature · 5 lines
Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 88 lines · Ville Syrjälä
Quoted 87 lines · Matt Roper
Quoted 65 lines · Ville Syrjälä
Quoted 59 lines · Matt Roper
Quoted 41 lines · Tvrtko Ursulin
Quoted 24 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/intel_display.c +4
-0@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip; +bool can_position = false;int ret; crtc = crtc ? crtc : plane->crtc; intel_crtc =to_intel_crtc (crtc); +if (INTEL_INFO (dev)->gen >=9 ) + can_position = true; +drivers/gpu/drm/i915/intel_display.c intel_check_primary_plane(struct drm_plane *plane, · line 12170
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +2
-1ret =drm_plane_helper_check_update (plane, crtc, fb, src, dest, clip,DRM_PLANE_HELPER_NO_SCALING ,DRM_PLANE_HELPER_NO_SCALING , - false, true, &state->visible); + can_position, true, + &state->visible);if (ret)return ret;I have discovered today that, while this allows SetCrtc and SetPlane ioctls to work with frame buffers which do not cover the plane, page flips are not that lucky and fail roughly with:
Quoted 2 lines
[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC viewport 1920x1080+0+0.
Maybe I'm misunderstanding your explanation, but a framebuffer is always required to fill/cover the plane scanning out of it. What this patch is supposed to be allowing is for the primary plane to not cover the entire CRTC (since that's something that only became possible for Intel hardware on the gen9+ platforms). I.e., the primary plane is now allowed to positioned and resized to cover a subset of the CRTC area, just like "sprite" planes have always been able to.
If you've got a 1080x1080 framebuffer, then it's legal to have a 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT. However it is not legal to size the primary plane as 1920x1080 and use this same 1080x1080 framebuffer with any of our interfaces (setplane, setcrtc, pageflip, or atomic).
Quoted 4 lines
Are you using ioctls/libdrm directly or are you using
igt_kmshelpers? IIRC, the IGT helpers will try to be extra helpful and automatically size the plane to match the framebuffer (unless you override that behavior), so that might be what's causing the confusion here.Quoted 8 lines
The problem is clear as day in
drm_mode_page_flip_ioctl():
ret =drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb); if (ret)goto out;The fix should be easy; just extract the current src coordinates from the plane state and check those against the new fb size. And then hope that the plane state is really up to date.
Yep, that's the conclusion we came to once Tvrtko explained what he was seeing on IRC. I'm not sure whether non-atomic drivers have enough state setup by the default helpers to work properly. Worst case we'll just assume that a non-atomic driver won't support primary plane windowing (since none have in the past) and fall back to looking at the mode for legacy non-atomic drivers.
Quoted 4 lines
And I'm sure rotated cases will go boom in some other ways. Probably we should just switch over to using the full plane update for mmio flips to fix it.
Quoted 6 lines
Quoted 3 lines
Yeah; the core looks at a
drm_plane->invert_dimensionsfield that's only set by omap. That should probably be updated to look at the state's rotation on atomic-capable drivers.We can just look at the src coordinates. Those always match the fb orientation.
Can we just not bother with legacy pageflips on rotated planes? setplane works and once you rotate it kinda gets nasty anyway.
The problem I see is that with legacy pageflip we also need to hack up something that doesn't look at plane->state for legacy and all that for a grand total of about 2 drivers, both getting converted to atomic. -Daniel
Quoted 48 lines
Quoted 44 lines
Matt
Quoted 32 lines
Quoted 28 lines
Matt
Quoted 16 lines
I have posted a quick IGT exerciser for this as "
kms_rotation_crc:
Excercise page flips with 90 degree rotation". May not be that great but shows the failure.I am not that hot on meddling with this code, nor do I feel competent to even try on my own at least. :/ Maybe just because the atomic and plane related rewrites have been going on for so long, and have multiple people involved, it all sounds pretty scary and fragile.
But I think some sort of plan on how to fix this could be in order?
Regards,
Tvrtko
Signature · 5 lines
Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795Signature · 2 lines
Ville Syrjälä Intel OTCSignature · 5 lines
Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795Signature · 2 lines
Ville Syrjälä Intel OTC
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 93 lines · Daniel Vetter
Quoted 92 lines · Ville Syrjälä
Quoted 90 lines · Matt Roper
Quoted 65 lines · Ville Syrjälä
Quoted 59 lines · Matt Roper
Quoted 41 lines · Tvrtko Ursulin
Quoted 24 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/intel_display.c +4
-0@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip; +bool can_position = false;int ret; crtc = crtc ? crtc : plane->crtc; intel_crtc =to_intel_crtc (crtc); +if (INTEL_INFO (dev)->gen >=9 ) + can_position = true; +drivers/gpu/drm/i915/intel_display.c intel_check_primary_plane(struct drm_plane *plane, · line 12170
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +2
-1ret =drm_plane_helper_check_update (plane, crtc, fb, src, dest, clip,DRM_PLANE_HELPER_NO_SCALING ,DRM_PLANE_HELPER_NO_SCALING , - false, true, &state->visible); + can_position, true, + &state->visible);if (ret)return ret;I have discovered today that, while this allows SetCrtc and SetPlane ioctls to work with frame buffers which do not cover the plane, page flips are not that lucky and fail roughly with:
Quoted 2 lines
[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC viewport 1920x1080+0+0.
Maybe I'm misunderstanding your explanation, but a framebuffer is always required to fill/cover the plane scanning out of it. What this patch is supposed to be allowing is for the primary plane to not cover the entire CRTC (since that's something that only became possible for Intel hardware on the gen9+ platforms). I.e., the primary plane is now allowed to positioned and resized to cover a subset of the CRTC area, just like "sprite" planes have always been able to.
If you've got a 1080x1080 framebuffer, then it's legal to have a 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT. However it is not legal to size the primary plane as 1920x1080 and use this same 1080x1080 framebuffer with any of our interfaces (setplane, setcrtc, pageflip, or atomic).
Quoted 4 lines
Are you using ioctls/libdrm directly or are you using
igt_kmshelpers? IIRC, the IGT helpers will try to be extra helpful and automatically size the plane to match the framebuffer (unless you override that behavior), so that might be what's causing the confusion here.Quoted 8 lines
The problem is clear as day in
drm_mode_page_flip_ioctl():
ret =drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb); if (ret)goto out;The fix should be easy; just extract the current src coordinates from the plane state and check those against the new fb size. And then hope that the plane state is really up to date.
Yep, that's the conclusion we came to once Tvrtko explained what he was seeing on IRC. I'm not sure whether non-atomic drivers have enough state setup by the default helpers to work properly. Worst case we'll just assume that a non-atomic driver won't support primary plane windowing (since none have in the past) and fall back to looking at the mode for legacy non-atomic drivers.
Quoted 4 lines
And I'm sure rotated cases will go boom in some other ways. Probably we should just switch over to using the full plane update for mmio flips to fix it.
Yeah; the core looks at a
drm_plane->invert_dimensionsfield that's only set by omap. That should probably be updated to look at the state's rotation on atomic-capable drivers.
Quoted 5 lines
Quoted 2 lines
We can just look at the src coordinates. Those always match the fb orientation.
Can we just not bother with legacy pageflips on rotated planes? setplane works and once you rotate it kinda gets nasty anyway.
I don't know - thought it is simple enough to make it work so why not? Just " [PATCH] drm/i915: Consider plane rotation when calculating stride in skl_do_mmio_flip" I posted, plus Matt's "[PATCH] drm: Check fb against plane size rather than CRTC mode for pageflip" to allow smaller than mode planes.
Quoted 3 lines
The problem I see is that with legacy pageflip we also need to hack up something that doesn't look at
plane->statefor legacy and all that for a grand total of about 2 drivers, both getting converted to atomic.
I'll leave the legacy/atomic/etc considerations to the experts. :)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 115 lines · Tvrtko Ursulin
Quoted 102 lines · Daniel Vetter
Quoted 101 lines · Ville Syrjälä
Quoted 99 lines · Matt Roper
Quoted 76 lines · Ville Syrjälä
Quoted 65 lines · Matt Roper
Quoted 43 lines · Tvrtko Ursulin
Quoted 26 lines · Sonika Jindal
Signed-off-by Sonika Jindal <[email protected]>Reviewed-by Matt Roper <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/intel_display.c +3
-0@@ -12150,16 +12150,21 @@ intel_check_primary_plane(structdrm_plane *plane, struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip; +bool can_position = false;int ret; crtc = crtc ? crtc : plane->crtc; intel_crtc =to_intel_crtc (crtc); +if (INTEL_INFO (dev)->gen >=9 ) + can_position = true;drivers/gpu/drm/i915/intel_display.c intel_check_primary_plane(struct · line 12170
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +3
-1+ ret =drm_plane_helper_check_update (plane, crtc, fb, src, dest, clip,DRM_PLANE_HELPER_NO_SCALING ,DRM_PLANE_HELPER_NO_SCALING , - false, true, &state->visible); + can_position, true, + &state->visible);if (ret)return ret;I have discovered today that, while this allows SetCrtc and SetPlane ioctls to work with frame buffers which do not cover the plane, page flips are not that lucky and fail roughly with:
Quoted 2 lines
[drm:drm_crtc_check_viewport] Invalid fb size 1080x1080 for CRTC viewport 1920x1080+0+0.
Maybe I'm misunderstanding your explanation, but a framebuffer is always
required to fill/cover the plane scanning out of it. What this patch is
supposed to be allowing is for the primary plane to not cover the entire
CRTC (since that's something that only became possible for Intel hardware on the gen9+ platforms). I.e., the primary plane is now allowed to positioned and resized to cover a subset of the CRTC area, just like "sprite" planes have always been able to.If you've got a 1080x1080 framebuffer, then it's legal to have a 1080x1080 primary plane while running in 1920x1080 mode on SKL/BXT. However it is not legal to size the primary plane as 1920x1080 and use
this same 1080x1080 framebuffer with any of our interfaces (setplane, setcrtc, pageflip, or atomic).Quoted 5 lines
Are you using ioctls/libdrm directly or are you using
igt_kmshelpers?
IIRC, the IGT helpers will try to be extra helpful and automatically size the plane to match the framebuffer (unless you override that behavior), so that might be what's causing the confusion here.The problem is clear as day in
drm_mode_page_flip_ioctl():
ret =drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb);
if (ret)Quoted 5 lines
goto out;The fix should be easy; just extract the current src coordinates from the plane state and check those against the new fb size. And then hope that the plane state is really up to date.
Yep, that's the conclusion we came to once Tvrtko explained what he was seeing on IRC. I'm not sure whether non-atomic drivers have enough state setup by the default helpers to work properly. Worst case we'll just assume that a non-atomic driver won't support primary plane windowing (since none have in the past) and fall back to looking at the mode for legacy non-atomic drivers.
Quoted 4 lines
And I'm sure rotated cases will go boom in some other ways. Probably we should just switch over to using the full plane update for mmio flips to fix it.
Yeah; the core looks at a
drm_plane->invert_dimensionsfield that's only
set by omap. That should probably be updated to look at the state's rotation on atomic-capable drivers.Quoted 5 lines
Quoted 2 lines
We can just look at the src coordinates. Those always match the fb orientation.
Can we just not bother with legacy pageflips on rotated planes? setplane works and once you rotate it kinda gets nasty anyway.
I don't know - thought it is simple enough to make it work so why not? Just " [PATCH] drm/i915: Consider plane rotation when calculating stride in
skl_do_mmio_flip" I posted, plus Matt's "[PATCH] drm: Check fb against plane size rather than CRTC mode for pageflip" to allow smaller than mode planes.
Quoted 5 lines
Quoted 3 lines
The problem I see is that with legacy pageflip we also need to hack up something that doesn't look at
plane->statefor legacy and all that for a grand total of about 2 drivers, both getting converted to atomic.I'll leave the legacy/atomic/etc considerations to the experts. :)
Are we sure any efforts to support rotation in legacy page flips is not worth it?
So far there were three patches for this: Plane programming fix (very simple) and an IGT test case (simple as well) from me, and a sub-crtc size plane support from Matt.
It kind of remained hanging a bit so I think it would be good to make a definitive decision.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx