<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 4, 2014 at 4:26 AM, Damien Lespiau <span dir="ltr"><<a href="mailto:damien.lespiau@intel.com" target="_blank">damien.lespiau@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Skylake makes primary planes the same as sprite planes and call the<br>
result "universal planes".<br>
<br>
This commit emulates a primary plane with plane 0, taking the<br>
opportunity to redefine primary and sprite registers to be identical now<br>
that the underlying hardware is. It also makes sense as plenty of fields<br>
have changed.<br>
<br>
v2: Rebase on top of the vma code.<br>
<br>
v3: Follow upstream evolution:<br>
- Drop return values.<br>
- Remove pipe checks since redudant and BUG instead.<br>
- Remove tiling checks and BUG instead.<br>
- Drop commented out DISP_MODIFY usage.<br>
<br>
v4: s/plane/primary_plane/<br>
<br>
v5: Misc fixes:<br>
- Fix the fields we need to clear up<br>
- Disable trickle feed<br>
- Correctly use PLANE_OFFSET for the panning<br>
<br>
v6: (Jesse)<br>
Use pipe src size when programming plane size. This makes cloned configs<br>
work correctly w/o the use of a panel fitter.<br>
<br>
v7: Rebase on top of Ville's rmw elimination series<br>
<br>
Signed-off-by: Damien Lespiau <<a href="mailto:damien.lespiau@intel.com">damien.lespiau@intel.com</a>> (v1,5,6,7)<br>
Signed-off-by: Daniel Vetter <<a href="mailto:daniel.vetter@ffwll.ch">daniel.vetter@ffwll.ch</a>> (v2,3)<br>
---<br>
drivers/gpu/drm/i915/i915_reg.h | 110 ++++++++++++++++++++++++++++++++++-<br>
drivers/gpu/drm/i915/intel_display.c | 88 +++++++++++++++++++++++++++-<br>
2 files changed, 195 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h<br>
index 15c0eaa..087085c 100644<br>
--- a/drivers/gpu/drm/i915/i915_reg.h<br>
+++ b/drivers/gpu/drm/i915/i915_reg.h<br>
@@ -26,8 +26,8 @@<br>
#define _I915_REG_H_<br>
<br>
#define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))<br>
+#define _PLANE(plane, a, b) _PIPE(plane, a, b)<br>
#define _TRANSCODER(tran, a, b) ((a) + (tran)*((b)-(a)))<br>
-<br>
#define _PORT(port, a, b) ((a) + (port)*((b)-(a)))<br>
#define _PIPE3(pipe, a, b, c) ((pipe) == PIPE_A ? (a) : \<br>
(pipe) == PIPE_B ? (b) : (c))<br>
@@ -4495,6 +4495,114 @@ enum punit_power_well {<br>
#define SPCONSTALPHA(pipe, plane) _PIPE(pipe * 2 + plane, _SPACONSTALPHA, _SPBCONSTALPHA)<br>
#define SPGAMC(pipe, plane) _PIPE(pipe * 2 + plane, _SPAGAMC, _SPBGAMC)<br>
<br>
+/* Skylake plane registers */<br>
+<br>
+#define _PLANE_CTL_1_A 0x70180<br>
+#define _PLANE_CTL_2_A 0x70280<br>
+#define _PLANE_CTL_3_A 0x70380</blockquote><div>#first-bickesheding ;)</div><div> Since the definitions os 4_A, {1,2,3,4}_B and {1,2,3,4}_C are all in the same spec with same bits below I'd prefer to define them here.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+#define PLANE_CTL_ENABLE (1 << 31)<br>
+#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)<br>
+#define PLANE_CTL_FORMAT_MASK (0xf << 24)<br>
+#define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)<br>
+#define PLANE_CTL_FORMAT_NV12 ( 1 << 24)<br>
+#define PLANE_CTL_FORMAT_XRGB_2101010 ( 2 << 24)<br>
+#define PLANE_CTL_FORMAT_XRGB_8888 ( 4 << 24)<br>
+#define PLANE_CTL_FORMAT_XRGB_16161616F ( 6 << 24)<br>
+#define PLANE_CTL_FORMAT_AYUV ( 8 << 24)<br></blockquote><div><br></div><div>1010 missed or useless to add here?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+#define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)<br>
+#define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)<br>
+#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)<br>
+#define PLANE_CTL_KEY_ENABLE (1 << 22)<br></blockquote><div><br></div><div>Key is [22:21] and 01, i.e 22=0 also is a kind of Key enabled.</div><div>Or just the source matters?</div><div>In this case second bikesheding would be change for SOURCE_KEY_ENABLE</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+#define PLANE_CTL_ORDER_BGRX (0 << 20)<br>
+#define PLANE_CTL_ORDER_RGBX (1 << 20)<br>
+#define PLANE_CTL_YUV422_ORDER_MASK (0x3 << 16)<br>
+#define PLANE_CTL_YUV422_YUYV ( 0 << 16)<br>
+#define PLANE_CTL_YUV422_UYVY ( 1 << 16)<br>
+#define PLANE_CTL_YUV422_YVYU ( 2 << 16)<br>
+#define PLANE_CTL_YUV422_VYUY ( 3 << 16)<br>
+#define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15) </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+#define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)<br></blockquote><div> </div><div>On Spec there is a restrictions: "Do not program this field to 1b." So I'd prefer to declare FEED_ENABLE (0 << 14). Just to avoid have to always ~ it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)<br>
+#define PLANE_CTL_TILED_MASK (0x7 << 10)<br>
+#define PLANE_CTL_TILED_LINEAR ( 0 << 10)<br>
+#define PLANE_CTL_TILED_X ( 1 << 10)<br>
+#define PLANE_CTL_TILED_Y ( 4 << 10)<br>
+#define PLANE_CTL_TILED_YF ( 5 << 10)<br>
+#define PLANE_CTL_ALPHA_MASK (0x3 << 4)<br>
+#define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)<br>
+#define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)<br>
+#define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)<br>
+#define _PLANE_STRIDE_1_A 0x70188<br>
+#define _PLANE_STRIDE_2_A 0x70288<br>
+#define _PLANE_STRIDE_3_A 0x70388<br>
+#define _PLANE_POS_1_A 0x7018c<br>
+#define _PLANE_POS_2_A 0x7028c<br>
+#define _PLANE_POS_3_A 0x7038c<br>
+#define _PLANE_SIZE_1_A 0x70190<br>
+#define _PLANE_SIZE_2_A 0x70290<br>
+#define _PLANE_SIZE_3_A 0x70390<br>
+#define _PLANE_SURF_1_A 0x7019c<br>
+#define _PLANE_SURF_2_A 0x7029c<br>
+#define _PLANE_SURF_3_A 0x7039c<br>
+#define _PLANE_OFFSET_1_A 0x701a4<br>
+#define _PLANE_OFFSET_2_A 0x702a4<br>
+#define _PLANE_OFFSET_3_A 0x703a4<br>
+<br>
+#define _PLANE_CTL_1_B 0x71180<br>
+#define _PLANE_CTL_2_B 0x71280<br>
+#define _PLANE_CTL_3_B 0x71380<br>
+#define _PLANE_CTL_1(pipe) _PIPE(pipe, _PLANE_CTL_1_A, _PLANE_CTL_1_B)<br>
+#define _PLANE_CTL_2(pipe) _PIPE(pipe, _PLANE_CTL_2_A, _PLANE_CTL_2_B)<br>
+#define _PLANE_CTL_3(pipe) _PIPE(pipe, _PLANE_CTL_3_A, _PLANE_CTL_3_B)<br>
+#define PLANE_CTL(pipe, plane) \<br>
+ _PLANE(plane, _PLANE_CTL_1(pipe), _PLANE_CTL_2(pipe))<br>
+<br>
+#define _PLANE_STRIDE_1_B 0x71188<br>
+#define _PLANE_STRIDE_2_B 0x71288<br>
+#define _PLANE_STRIDE_3_B 0x71388<br>
+#define _PLANE_STRIDE_1(pipe) \<br>
+ _PIPE(pipe, _PLANE_STRIDE_1_A, _PLANE_STRIDE_1_B)<br>
+#define _PLANE_STRIDE_2(pipe) \<br>
+ _PIPE(pipe, _PLANE_STRIDE_2_A, _PLANE_STRIDE_2_B)<br>
+#define _PLANE_STRIDE_3(pipe) \<br>
+ _PIPE(pipe, _PLANE_STRIDE_3_A, _PLANE_STRIDE_3_B)<br>
+#define PLANE_STRIDE(pipe, plane) \<br>
+ _PLANE(plane, _PLANE_STRIDE_1(pipe), _PLANE_STRIDE_2(pipe))<br>
+<br>
+#define _PLANE_POS_1_B 0x7118c<br>
+#define _PLANE_POS_2_B 0x7128c<br>
+#define _PLANE_POS_3_B 0x7138c<br>
+#define _PLANE_POS_1(pipe) _PIPE(pipe, _PLANE_POS_1_A, _PLANE_POS_1_B)<br>
+#define _PLANE_POS_2(pipe) _PIPE(pipe, _PLANE_POS_2_A, _PLANE_POS_2_B)<br>
+#define _PLANE_POS_3(pipe) _PIPE(pipe, _PLANE_POS_3_A, _PLANE_POS_3_B)<br>
+#define PLANE_POS(pipe, plane) \<br>
+ _PLANE(plane, _PLANE_POS_1(pipe), _PLANE_POS_2(pipe))<br>
+<br>
+#define _PLANE_SIZE_1_B 0x71190<br>
+#define _PLANE_SIZE_2_B 0x71290<br>
+#define _PLANE_SIZE_3_B 0x71390<br>
+#define _PLANE_SIZE_1(pipe) _PIPE(pipe, _PLANE_SIZE_1_A, _PLANE_SIZE_1_B)<br>
+#define _PLANE_SIZE_2(pipe) _PIPE(pipe, _PLANE_SIZE_2_A, _PLANE_SIZE_2_B)<br>
+#define _PLANE_SIZE_3(pipe) _PIPE(pipe, _PLANE_SIZE_3_A, _PLANE_SIZE_3_B)<br>
+#define PLANE_SIZE(pipe, plane) \<br>
+ _PLANE(plane, _PLANE_SIZE_1(pipe), _PLANE_SIZE_2(pipe))<br>
+<br>
+#define _PLANE_SURF_1_B 0x7119c<br>
+#define _PLANE_SURF_2_B 0x7129c<br>
+#define _PLANE_SURF_3_B 0x7139c<br>
+#define _PLANE_SURF_1(pipe) _PIPE(pipe, _PLANE_SURF_1_A, _PLANE_SURF_1_B)<br>
+#define _PLANE_SURF_2(pipe) _PIPE(pipe, _PLANE_SURF_2_A, _PLANE_SURF_2_B)<br>
+#define _PLANE_SURF_3(pipe) _PIPE(pipe, _PLANE_SURF_3_A, _PLANE_SURF_3_B)<br>
+#define PLANE_SURF(pipe, plane) \<br>
+ _PLANE(plane, _PLANE_SURF_1(pipe), _PLANE_SURF_2(pipe))<br>
+<br>
+#define _PLANE_OFFSET_1_B 0x711a4<br>
+#define _PLANE_OFFSET_2_B 0x712a4<br>
+#define _PLANE_OFFSET_1(pipe) _PIPE(pipe, _PLANE_OFFSET_1_A, _PLANE_OFFSET_1_B)<br>
+#define _PLANE_OFFSET_2(pipe) _PIPE(pipe, _PLANE_OFFSET_2_A, _PLANE_OFFSET_2_B)<br>
+#define PLANE_OFFSET(pipe, plane) \<br>
+ _PLANE(plane, _PLANE_OFFSET_1(pipe), _PLANE_OFFSET_2(pipe))<br>
+<br>
/* VBIOS regs */<br>
#define VGACNTRL 0x71400<br>
# define VGA_DISP_DISABLE (1 << 31)<br>
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c<br>
index 02236f9..f98d1c4 100644<br>
--- a/drivers/gpu/drm/i915/intel_display.c<br>
+++ b/drivers/gpu/drm/i915/intel_display.c<br>
@@ -2640,6 +2640,86 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,<br>
POSTING_READ(reg);<br>
}<br>
<br>
+static void skylake_update_primary_plane(struct drm_crtc *crtc,<br>
+ struct drm_framebuffer *fb,<br>
+ int x, int y)<br>
+{<br>
+ struct drm_device *dev = crtc->dev;<br>
+ struct drm_i915_private *dev_priv = dev->dev_private;<br>
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);<br>
+ struct intel_framebuffer *intel_fb;<br>
+ struct drm_i915_gem_object *obj;<br>
+ int pipe = intel_crtc->pipe;<br>
+ u32 plane_ctl, stride;<br>
+<br>
+ if (!intel_crtc->primary_enabled) {<br>
+ I915_WRITE(PLANE_CTL(pipe, 0), 0);<br>
+ I915_WRITE(PLANE_SURF(pipe, 0), 0);<br>
+ POSTING_READ(PLANE_CTL(pipe, 0));<br>
+ return;<br>
+ }<br>
+<br>
+ plane_ctl = PLANE_CTL_ENABLE |<br>
+ PLANE_CTL_PIPE_GAMMA_ENABLE |<br>
+ PLANE_CTL_PIPE_CSC_ENABLE;<br>
+<br>
+ switch (fb->pixel_format) {<br>
+ case DRM_FORMAT_RGB565:<br>
+ plane_ctl |= PLANE_CTL_FORMAT_RGB_565;<br>
+ break;<br>
+ case DRM_FORMAT_XRGB8888:<br>
+ plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;<br>
+ break;<br>
+ case DRM_FORMAT_XBGR8888:<br>
+ plane_ctl |= PLANE_CTL_ORDER_RGBX;<br>
+ plane_ctl |= PLANE_CTL_FORMAT_XRGB_8888;<br>
+ break;<br>
+ case DRM_FORMAT_XRGB2101010:<br>
+ plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;<br>
+ break;<br>
+ case DRM_FORMAT_XBGR2101010:<br>
+ plane_ctl |= PLANE_CTL_ORDER_RGBX;<br>
+ plane_ctl |= PLANE_CTL_FORMAT_XRGB_2101010;<br>
+ break;<br>
+ default:<br>
+ BUG();<br>
+ }<br>
+<br>
+ intel_fb = to_intel_framebuffer(fb);<br>
+ obj = intel_fb->obj;<br>
+ switch (obj->tiling_mode) {<br>
+ case I915_TILING_NONE:<br>
+ stride = fb->pitches[0] >> 6;<br></blockquote><div>A comment here would be good to explain this:</div><div>"For Linear memory this field specifies the stride in chunks of 64 bytes" </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ break;<br>
+ case I915_TILING_X:<br>
+ plane_ctl |= PLANE_CTL_TILED_X;<br>
+ stride = fb->pitches[0] >> 9;</blockquote><div>and something like or better than the following here:</div><div>"Fox X-tiled this field specifies the stride in number of tiles" 1/512 bytes.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ break;<br>
+ default:<br></blockquote><div> </div><div>is I915_TILING_Y impossible? </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ BUG();<br>
+ }<br>
+<br>
+ plane_ctl &= ~PLANE_CTL_TRICKLE_FEED_DISABLE;<br>
+ plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;<br>
+<br>
+ I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);<br>
+<br>
+ DRM_DEBUG_KMS("Writing base %08lX %d,%d,%d,%d pitch=%d\n",<br>
+ i915_gem_obj_ggtt_offset(obj),<br>
+ x, y, fb->width, fb->height,<br>
+ fb->pitches[0]);<br>
+<br>
+ I915_WRITE(PLANE_POS(pipe, 0), 0);<br>
+ I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);<br>
+ I915_WRITE(PLANE_SIZE(pipe, 0),<br>
+ (intel_crtc->config.pipe_src_h - 1) << 16 |<br>
+ (intel_crtc->config.pipe_src_w - 1));<br>
+ I915_WRITE(PLANE_STRIDE(pipe, 0), stride);<br>
+ I915_WRITE(PLANE_SURF(pipe, 0), i915_gem_obj_ggtt_offset(obj));<br>
+<br>
+ POSTING_READ(PLANE_SURF(pipe, 0));<br>
+}<br>
+<br>
/* Assume fb object is pinned & idle & fenced and just update base pointers */<br>
static int<br>
intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,<br>
@@ -12481,8 +12561,12 @@ static void intel_init_display(struct drm_device *dev)<br>
dev_priv->display.crtc_enable = haswell_crtc_enable;<br>
dev_priv->display.crtc_disable = haswell_crtc_disable;<br>
dev_priv->display.off = ironlake_crtc_off;<br>
- dev_priv->display.update_primary_plane =<br>
- ironlake_update_primary_plane;<br>
+ if (INTEL_INFO(dev)->gen >= 9)<br>
+ dev_priv->display.update_primary_plane =<br>
+ skylake_update_primary_plane;<br>
+ else<br>
+ dev_priv->display.update_primary_plane =<br>
+ ironlake_update_primary_plane;<br>
} else if (HAS_PCH_SPLIT(dev)) {<br>
dev_priv->display.get_pipe_config = ironlake_get_pipe_config;<br>
dev_priv->display.get_plane_config = ironlake_get_plane_config;<br>
<span class=""><font color="#888888">--<br>
1.8.3.1<br>
<br>
_______________________________________________<br>
Intel-gfx mailing list<br>
<a href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/intel-gfx" target="_blank">http://lists.freedesktop.org/mailman/listinfo/intel-gfx</a><br>
</font></span></blockquote></div><br><br>Regardless the bikeshedings, and if all answers to the questions above result in no change to the code, consider this</div><div class="gmail_extra">Reviewed-by: Rodrigo Vivi <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>></div><div class="gmail_extra"><br clear="all"><div><br></div>-- <br><div>Rodrigo Vivi</div><div>Blog: <a href="http://blog.vivi.eng.br" target="_blank">http://blog.vivi.eng.br</a></div><div> </div>
</div></div>