[PATCH V3 0/6] drm/i915: Add the support of dual BSD rings on BDW GT3

[PATCH V3 0/6] drm/i915: Add the support of dual BSD rings on BDW GT3

[Zhao Yakui]

This is the patch set that tries to add the support of dual BSD rings on BDW
GT3. Based on hardware spec, the BDW GT3 has two independent BSD rings, which
can be used to process the video commands. To be simpler, it is transparent 
to user-space driver/middleware. In such case the kernel driver will decide
which ring is to dispatch the BSD video command.

As every BSD ring is powerful, it is enough to dispatch the BSD video command
based on the drm fd. In such case the different BSD ring is used for video playing
back and encoding. 

V1->V2: Follow Daniel's comment to do the following update:
   a. consider the stolen check for BDW in kernel/early-quirks.c in patch 01
   b. update the comment in Patch 04
   c. use the simple ping-pong mechanism to add the support of dual BSD rings.
The further optimization will be considered in another patch set.

V2->V3: Follow Daniel's comment to use the struct_mutext instead of
atomic_t during determining which ring can be used to dispatch Video command.


Zhao Yakui (6):
  drm/i915: Split the BDW device definition to prepare for dual BSD
    rings on BDW GT3
  drm/i915:Initialize the second BSD ring on BDW GT3 machine
  drm/i915:Handle the irq interrupt for the second BSD ring
  drm/i915:Add the VCS2 switch in Intel_ring_setup_status_page for Gen7
    to remove the switch check warning
  drm/i915: Update the restrict check to filter out wrong Ring ID
    passed by user-space
  drm/i915: Use the coarse ping-pong mechanism based on drm fd to
    dispatch the BSD command on BDW GT3

 drivers/gpu/drm/i915/i915_dma.c            |    3 ++
 drivers/gpu/drm/i915/i915_drv.c            |   26 +++++++++++-
 drivers/gpu/drm/i915/i915_drv.h            |    5 +++
 drivers/gpu/drm/i915/i915_gem.c            |    9 ++++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   42 +++++++++++++++++++-
 drivers/gpu/drm/i915/i915_gpu_error.c      |    1 +
 drivers/gpu/drm/i915/i915_irq.c            |    5 ++-
 drivers/gpu/drm/i915/i915_reg.h            |    1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c    |   59 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    5 ++-
 include/drm/i915_pciids.h                  |   22 ++++++++---
 11 files changed, 166 insertions(+), 12 deletions(-)

-- 
1.7.10.1

[PATCH V3 1/6] drm/i915: Split the BDW device definition to prepare for dual BSD rings on BDW GT3

[Zhao Yakui]

Based on the hardware spec, the BDW GT3 has the different configuration
with the BDW GT1/GT2. So split the BDW device info definition.
This is to do the preparation for adding the Dual BSD rings on BDW GT3 machine.

V1->V2: Follow Daniel's comment to pay attention to the stolen check for BDW
in kernel/early-quirks.c

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |   26 ++++++++++++++++++++++++--
 include/drm/i915_pciids.h       |   22 +++++++++++++++++-----
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5d8250f..17fbbe5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -279,6 +279,26 @@ static const struct intel_device_info intel_broadwell_m_info = {
 	GEN_DEFAULT_PIPEOFFSETS,
 };
 
+static const struct intel_device_info intel_broadwell_gt3d_info = {
+	.gen = 8, .num_pipes = 3,
+	.need_gfx_hws = 1, .has_hotplug = 1,
+	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+	.has_llc = 1,
+	.has_ddi = 1,
+	.has_fbc = 1,
+	GEN_DEFAULT_PIPEOFFSETS,
+};
+
+static const struct intel_device_info intel_broadwell_gt3m_info = {
+	.gen = 8, .is_mobile = 1, .num_pipes = 3,
+	.need_gfx_hws = 1, .has_hotplug = 1,
+	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+	.has_llc = 1,
+	.has_ddi = 1,
+	.has_fbc = 1,
+	GEN_DEFAULT_PIPEOFFSETS,
+};
+
 /*
  * Make sure any device matches here are from most specific to most
  * general.  For example, since the Quanta match is based on the subsystem
@@ -311,8 +331,10 @@ static const struct intel_device_info intel_broadwell_m_info = {
 	INTEL_HSW_M_IDS(&intel_haswell_m_info), \
 	INTEL_VLV_M_IDS(&intel_valleyview_m_info),	\
 	INTEL_VLV_D_IDS(&intel_valleyview_d_info),	\
-	INTEL_BDW_M_IDS(&intel_broadwell_m_info),	\
-	INTEL_BDW_D_IDS(&intel_broadwell_d_info)
+	INTEL_BDW_GT12M_IDS(&intel_broadwell_m_info),	\
+	INTEL_BDW_GT12D_IDS(&intel_broadwell_d_info),	\
+	INTEL_BDW_GT3M_IDS(&intel_broadwell_gt3m_info),	\
+	INTEL_BDW_GT3D_IDS(&intel_broadwell_gt3d_info)
 
 static const struct pci_device_id pciidlist[] = {		/* aka */
 	INTEL_PCI_IDS,
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 940ece4..24f3cad 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -223,14 +223,26 @@
 	_INTEL_BDW_D(gt, 0x160A, info), /* Server */ \
 	_INTEL_BDW_D(gt, 0x160D, info) /* Workstation */
 
-#define INTEL_BDW_M_IDS(info) \
+#define INTEL_BDW_GT12M_IDS(info) \
 	_INTEL_BDW_M_IDS(1, info), \
-	_INTEL_BDW_M_IDS(2, info), \
-	_INTEL_BDW_M_IDS(3, info)
+	_INTEL_BDW_M_IDS(2, info)
 
-#define INTEL_BDW_D_IDS(info) \
+#define INTEL_BDW_GT12D_IDS(info) \
 	_INTEL_BDW_D_IDS(1, info), \
-	_INTEL_BDW_D_IDS(2, info), \
+	_INTEL_BDW_D_IDS(2, info)
+
+#define INTEL_BDW_GT3M_IDS(info) \
+	_INTEL_BDW_M_IDS(3, info)
+
+#define INTEL_BDW_GT3D_IDS(info) \
 	_INTEL_BDW_D_IDS(3, info)
 
+#define INTEL_BDW_M_IDS(info) \
+	INTEL_BDW_GT12M_IDS(info), \
+	INTEL_BDW_GT3M_IDS(info)
+
+#define INTEL_BDW_D_IDS(info) \
+	INTEL_BDW_GT12D_IDS(info), \
+	INTEL_BDW_GT3D_IDS(info)
+
 #endif /* _I915_PCIIDS_H */
-- 
1.7.10.1

Re: [PATCH V3 1/6] drm/i915: Split the BDW device definition to prepare for dual BSD rings on BDW GT3

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 3547 bytes --]

On Wed, 2014-04-16 at 10:41 +0800, Zhao Yakui wrote:
> Based on the hardware spec, the BDW GT3 has the different configuration
> with the BDW GT1/GT2. So split the BDW device info definition.
> This is to do the preparation for adding the Dual BSD rings on BDW GT3 machine.
> 
> V1->V2: Follow Daniel's comment to pay attention to the stolen check for BDW
> in kernel/early-quirks.c
> 
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c |   26 ++++++++++++++++++++++++--
>  include/drm/i915_pciids.h       |   22 +++++++++++++++++-----
>  2 files changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 5d8250f..17fbbe5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -279,6 +279,26 @@ static const struct intel_device_info intel_broadwell_m_info = {
>  	GEN_DEFAULT_PIPEOFFSETS,
>  };
>  
> +static const struct intel_device_info intel_broadwell_gt3d_info = {
> +	.gen = 8, .num_pipes = 3,
> +	.need_gfx_hws = 1, .has_hotplug = 1,
> +	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> +	.has_llc = 1,
> +	.has_ddi = 1,
> +	.has_fbc = 1,
> +	GEN_DEFAULT_PIPEOFFSETS,
> +};
> +
> +static const struct intel_device_info intel_broadwell_gt3m_info = {
> +	.gen = 8, .is_mobile = 1, .num_pipes = 3,
> +	.need_gfx_hws = 1, .has_hotplug = 1,
> +	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> +	.has_llc = 1,
> +	.has_ddi = 1,
> +	.has_fbc = 1,
> +	GEN_DEFAULT_PIPEOFFSETS,
> +};
> +
>  /*
>   * Make sure any device matches here are from most specific to most
>   * general.  For example, since the Quanta match is based on the subsystem
> @@ -311,8 +331,10 @@ static const struct intel_device_info intel_broadwell_m_info = {
>  	INTEL_HSW_M_IDS(&intel_haswell_m_info), \
>  	INTEL_VLV_M_IDS(&intel_valleyview_m_info),	\
>  	INTEL_VLV_D_IDS(&intel_valleyview_d_info),	\
> -	INTEL_BDW_M_IDS(&intel_broadwell_m_info),	\
> -	INTEL_BDW_D_IDS(&intel_broadwell_d_info)
> +	INTEL_BDW_GT12M_IDS(&intel_broadwell_m_info),	\
> +	INTEL_BDW_GT12D_IDS(&intel_broadwell_d_info),	\
> +	INTEL_BDW_GT3M_IDS(&intel_broadwell_gt3m_info),	\
> +	INTEL_BDW_GT3D_IDS(&intel_broadwell_gt3d_info)
>  
>  static const struct pci_device_id pciidlist[] = {		/* aka */
>  	INTEL_PCI_IDS,
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 940ece4..24f3cad 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -223,14 +223,26 @@
>  	_INTEL_BDW_D(gt, 0x160A, info), /* Server */ \
>  	_INTEL_BDW_D(gt, 0x160D, info) /* Workstation */
>  
> -#define INTEL_BDW_M_IDS(info) \
> +#define INTEL_BDW_GT12M_IDS(info) \
>  	_INTEL_BDW_M_IDS(1, info), \
> -	_INTEL_BDW_M_IDS(2, info), \
> -	_INTEL_BDW_M_IDS(3, info)
> +	_INTEL_BDW_M_IDS(2, info)
>  
> -#define INTEL_BDW_D_IDS(info) \
> +#define INTEL_BDW_GT12D_IDS(info) \
>  	_INTEL_BDW_D_IDS(1, info), \
> -	_INTEL_BDW_D_IDS(2, info), \
> +	_INTEL_BDW_D_IDS(2, info)
> +
> +#define INTEL_BDW_GT3M_IDS(info) \
> +	_INTEL_BDW_M_IDS(3, info)
> +
> +#define INTEL_BDW_GT3D_IDS(info) \
>  	_INTEL_BDW_D_IDS(3, info)
>  
> +#define INTEL_BDW_M_IDS(info) \
> +	INTEL_BDW_GT12M_IDS(info), \
> +	INTEL_BDW_GT3M_IDS(info)
> +
> +#define INTEL_BDW_D_IDS(info) \
> +	INTEL_BDW_GT12D_IDS(info), \
> +	INTEL_BDW_GT3D_IDS(info)
> +
>  #endif /* _I915_PCIIDS_H */


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH V3 2/6] drm/i915:Initialize the second BSD ring on BDW GT3 machine

[Zhao Yakui]

Based on the hardware spec, the BDW GT3 machine has two independent
BSD ring that can be used to dispatch the video commands.
So just initialize it.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |    4 +--
 drivers/gpu/drm/i915/i915_drv.h         |    2 ++
 drivers/gpu/drm/i915/i915_gem.c         |    9 +++++-
 drivers/gpu/drm/i915/i915_gpu_error.c   |    1 +
 drivers/gpu/drm/i915/i915_reg.h         |    1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |   54 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h |    4 ++-
 7 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 17fbbe5..2a7842b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -282,7 +282,7 @@ static const struct intel_device_info intel_broadwell_m_info = {
 static const struct intel_device_info intel_broadwell_gt3d_info = {
 	.gen = 8, .num_pipes = 3,
 	.need_gfx_hws = 1, .has_hotplug = 1,
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 	.has_llc = 1,
 	.has_ddi = 1,
 	.has_fbc = 1,
@@ -292,7 +292,7 @@ static const struct intel_device_info intel_broadwell_gt3d_info = {
 static const struct intel_device_info intel_broadwell_gt3m_info = {
 	.gen = 8, .is_mobile = 1, .num_pipes = 3,
 	.need_gfx_hws = 1, .has_hotplug = 1,
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
+	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 	.has_llc = 1,
 	.has_ddi = 1,
 	.has_fbc = 1,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 92c3095..74aef6a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1833,7 +1833,9 @@ struct drm_i915_cmd_table {
 #define BSD_RING		(1<<VCS)
 #define BLT_RING		(1<<BCS)
 #define VEBOX_RING		(1<<VECS)
+#define BSD2_RING		(1<<VCS2)
 #define HAS_BSD(dev)            (INTEL_INFO(dev)->ring_mask & BSD_RING)
+#define HAS_BSD2(dev)		(INTEL_INFO(dev)->ring_mask & BSD2_RING)
 #define HAS_BLT(dev)            (INTEL_INFO(dev)->ring_mask & BLT_RING)
 #define HAS_VEBOX(dev)            (INTEL_INFO(dev)->ring_mask & VEBOX_RING)
 #define HAS_LLC(dev)            (INTEL_INFO(dev)->has_llc)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 85c9cf0..b4dcf2a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4374,13 +4374,20 @@ static int i915_gem_init_rings(struct drm_device *dev)
 			goto cleanup_blt_ring;
 	}
 
+	if (HAS_BSD2(dev)) {
+		ret = intel_init_bsd2_ring_buffer(dev);
+		if (ret)
+			goto cleanup_vebox_ring;
+	}
 
 	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
 	if (ret)
-		goto cleanup_vebox_ring;
+		goto cleanup_ring;
 
 	return 0;
 
+cleanup_ring:
+	intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
 cleanup_vebox_ring:
 	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
 cleanup_blt_ring:
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 4865ade..3cab7f9 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -42,6 +42,7 @@ static const char *ring_str(int ring)
 	case VCS: return "bsd";
 	case BCS: return "blt";
 	case VECS: return "vebox";
+	case VCS2: return "second bsd";
 	default: return "";
 	}
 }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8f84555..0b88508 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -760,6 +760,7 @@ enum punit_power_well {
 #define RENDER_RING_BASE	0x02000
 #define BSD_RING_BASE		0x04000
 #define GEN6_BSD_RING_BASE	0x12000
+#define GEN8_BSD2_RING_BASE	0x1c000
 #define VEBOX_RING_BASE		0x1a000
 #define BLT_RING_BASE		0x22000
 #define RING_TAIL(base)		((base)+0x30)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index eb3dd26..8b9b89080 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1924,10 +1924,12 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
 		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
 		ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_RVE;
+		ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
 		ring->signal_mbox[RCS] = GEN6_NOSYNC;
 		ring->signal_mbox[VCS] = GEN6_VRSYNC;
 		ring->signal_mbox[BCS] = GEN6_BRSYNC;
 		ring->signal_mbox[VECS] = GEN6_VERSYNC;
+		ring->signal_mbox[VCS2] = GEN6_NOSYNC;
 	} else if (IS_GEN5(dev)) {
 		ring->add_request = pc_render_add_request;
 		ring->flush = gen4_render_ring_flush;
@@ -2100,10 +2102,12 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
 		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
 		ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_VVE;
+		ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
 		ring->signal_mbox[RCS] = GEN6_RVSYNC;
 		ring->signal_mbox[VCS] = GEN6_NOSYNC;
 		ring->signal_mbox[BCS] = GEN6_BVSYNC;
 		ring->signal_mbox[VECS] = GEN6_VEVSYNC;
+		ring->signal_mbox[VCS2] = GEN6_NOSYNC;
 	} else {
 		ring->mmio_base = BSD_RING_BASE;
 		ring->flush = bsd_ring_flush;
@@ -2126,6 +2130,52 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 	return intel_init_ring_buffer(dev, ring);
 }
 
+/**
+ * Initialize the second BSD ring for Broadwell GT3.
+ * It is noted that this only exists on Broadwell GT3.
+ */
+int intel_init_bsd2_ring_buffer(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_ring_buffer *ring = &dev_priv->ring[VCS2];
+
+	if ((INTEL_INFO(dev)->gen != 8) ) {
+		DRM_ERROR("No dual-BSD ring on non-BDW machine\n");
+		return -EINVAL;
+	}
+
+	ring->name = "second bsd ring";
+	ring->id = VCS2;
+
+	ring->write_tail = ring_write_tail;
+	ring->mmio_base = GEN8_BSD2_RING_BASE;
+	ring->flush = gen6_bsd_ring_flush;
+	ring->add_request = gen6_add_request;
+	ring->get_seqno = gen6_ring_get_seqno;
+	ring->set_seqno = ring_set_seqno;
+	ring->irq_enable_mask =
+			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
+	ring->irq_get = gen8_ring_get_irq;
+	ring->irq_put = gen8_ring_put_irq;
+	ring->dispatch_execbuffer =
+			gen8_ring_dispatch_execbuffer;
+	ring->sync_to = gen6_ring_sync;
+	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->signal_mbox[RCS] = GEN6_NOSYNC;
+	ring->signal_mbox[VCS] = GEN6_NOSYNC;
+	ring->signal_mbox[BCS] = GEN6_NOSYNC;
+	ring->signal_mbox[VECS] = GEN6_NOSYNC;
+	ring->signal_mbox[VCS2] = GEN6_NOSYNC;
+
+	ring->init = init_ring_common;
+
+	return intel_init_ring_buffer(dev, ring);
+}
+
 int intel_init_blt_ring_buffer(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2157,10 +2207,12 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
 	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
 	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_BVE;
+	ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
 	ring->signal_mbox[RCS] = GEN6_RBSYNC;
 	ring->signal_mbox[VCS] = GEN6_VBSYNC;
 	ring->signal_mbox[BCS] = GEN6_NOSYNC;
 	ring->signal_mbox[VECS] = GEN6_VEBSYNC;
+	ring->signal_mbox[VCS2] = GEN6_NOSYNC;
 	ring->init = init_ring_common;
 
 	return intel_init_ring_buffer(dev, ring);
@@ -2198,10 +2250,12 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_VEV;
 	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VEB;
 	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
 	ring->signal_mbox[RCS] = GEN6_RVESYNC;
 	ring->signal_mbox[VCS] = GEN6_VVESYNC;
 	ring->signal_mbox[BCS] = GEN6_BVESYNC;
 	ring->signal_mbox[VECS] = GEN6_NOSYNC;
+	ring->signal_mbox[VCS2] = GEN6_NOSYNC;
 	ring->init = init_ring_common;
 
 	return intel_init_ring_buffer(dev, ring);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 413cdc7..8ca4285 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -61,8 +61,9 @@ struct  intel_ring_buffer {
 		VCS,
 		BCS,
 		VECS,
+		VCS2,
 	} id;
-#define I915_NUM_RINGS 4
+#define I915_NUM_RINGS 5
 	u32		mmio_base;
 	void		__iomem *virtual_start;
 	struct		drm_device *dev;
@@ -286,6 +287,7 @@ int intel_ring_invalidate_all_caches(struct intel_ring_buffer *ring);
 
 int intel_init_render_ring_buffer(struct drm_device *dev);
 int intel_init_bsd_ring_buffer(struct drm_device *dev);
+int intel_init_bsd2_ring_buffer(struct drm_device *dev);
 int intel_init_blt_ring_buffer(struct drm_device *dev);
 int intel_init_vebox_ring_buffer(struct drm_device *dev);
 
-- 
1.7.10.1

Re: [PATCH V3 2/6] drm/i915:Initialize the second BSD ring on BDW GT3 machine

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 10447 bytes --]

On Wed, 2014-04-16 at 10:41 +0800, Zhao Yakui wrote:
> Based on the hardware spec, the BDW GT3 machine has two independent
> BSD ring that can be used to dispatch the video commands.
> So just initialize it.
> 
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>

A couple of nitpicks below, with or without those:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c         |    4 +--
>  drivers/gpu/drm/i915/i915_drv.h         |    2 ++
>  drivers/gpu/drm/i915/i915_gem.c         |    9 +++++-
>  drivers/gpu/drm/i915/i915_gpu_error.c   |    1 +
>  drivers/gpu/drm/i915/i915_reg.h         |    1 +
>  drivers/gpu/drm/i915/intel_ringbuffer.c |   54 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.h |    4 ++-
>  7 files changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 17fbbe5..2a7842b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -282,7 +282,7 @@ static const struct intel_device_info intel_broadwell_m_info = {
>  static const struct intel_device_info intel_broadwell_gt3d_info = {
>  	.gen = 8, .num_pipes = 3,
>  	.need_gfx_hws = 1, .has_hotplug = 1,
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> +	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>  	.has_llc = 1,
>  	.has_ddi = 1,
>  	.has_fbc = 1,
> @@ -292,7 +292,7 @@ static const struct intel_device_info intel_broadwell_gt3d_info = {
>  static const struct intel_device_info intel_broadwell_gt3m_info = {
>  	.gen = 8, .is_mobile = 1, .num_pipes = 3,
>  	.need_gfx_hws = 1, .has_hotplug = 1,
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> +	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>  	.has_llc = 1,
>  	.has_ddi = 1,
>  	.has_fbc = 1,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 92c3095..74aef6a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1833,7 +1833,9 @@ struct drm_i915_cmd_table {
>  #define BSD_RING		(1<<VCS)
>  #define BLT_RING		(1<<BCS)
>  #define VEBOX_RING		(1<<VECS)
> +#define BSD2_RING		(1<<VCS2)
>  #define HAS_BSD(dev)            (INTEL_INFO(dev)->ring_mask & BSD_RING)
> +#define HAS_BSD2(dev)		(INTEL_INFO(dev)->ring_mask & BSD2_RING)
>  #define HAS_BLT(dev)            (INTEL_INFO(dev)->ring_mask & BLT_RING)
>  #define HAS_VEBOX(dev)            (INTEL_INFO(dev)->ring_mask & VEBOX_RING)
>  #define HAS_LLC(dev)            (INTEL_INFO(dev)->has_llc)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 85c9cf0..b4dcf2a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4374,13 +4374,20 @@ static int i915_gem_init_rings(struct drm_device *dev)
>  			goto cleanup_blt_ring;
>  	}
>  
> +	if (HAS_BSD2(dev)) {
> +		ret = intel_init_bsd2_ring_buffer(dev);
> +		if (ret)
> +			goto cleanup_vebox_ring;
> +	}
>  
>  	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
>  	if (ret)
> -		goto cleanup_vebox_ring;
> +		goto cleanup_ring;

maybe cleanup_bsd2_ring?

>  
>  	return 0;
>  
> +cleanup_ring:
> +	intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
>  cleanup_vebox_ring:
>  	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
>  cleanup_blt_ring:
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 4865ade..3cab7f9 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -42,6 +42,7 @@ static const char *ring_str(int ring)
>  	case VCS: return "bsd";
>  	case BCS: return "blt";
>  	case VECS: return "vebox";
> +	case VCS2: return "second bsd";

"bsd2" would be more concise

>  	default: return "";
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8f84555..0b88508 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -760,6 +760,7 @@ enum punit_power_well {
>  #define RENDER_RING_BASE	0x02000
>  #define BSD_RING_BASE		0x04000
>  #define GEN6_BSD_RING_BASE	0x12000
> +#define GEN8_BSD2_RING_BASE	0x1c000
>  #define VEBOX_RING_BASE		0x1a000
>  #define BLT_RING_BASE		0x22000
>  #define RING_TAIL(base)		((base)+0x30)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index eb3dd26..8b9b89080 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1924,10 +1924,12 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>  		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
>  		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
>  		ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_RVE;
> +		ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;

It's unclear why we initialize the semaphore stuff differently for VCS
and VCS2. Semaphores are disabled for GEN8 atm and they will be reworked
in the future, but maybe a comment to revisit this code would good here
and in intel_init_bsd2_ring_buffer().

>  		ring->signal_mbox[RCS] = GEN6_NOSYNC;
>  		ring->signal_mbox[VCS] = GEN6_VRSYNC;
>  		ring->signal_mbox[BCS] = GEN6_BRSYNC;
>  		ring->signal_mbox[VECS] = GEN6_VERSYNC;
> +		ring->signal_mbox[VCS2] = GEN6_NOSYNC;
>  	} else if (IS_GEN5(dev)) {
>  		ring->add_request = pc_render_add_request;
>  		ring->flush = gen4_render_ring_flush;
> @@ -2100,10 +2102,12 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>  		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
>  		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
>  		ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_VVE;
> +		ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
>  		ring->signal_mbox[RCS] = GEN6_RVSYNC;
>  		ring->signal_mbox[VCS] = GEN6_NOSYNC;
>  		ring->signal_mbox[BCS] = GEN6_BVSYNC;
>  		ring->signal_mbox[VECS] = GEN6_VEVSYNC;
> +		ring->signal_mbox[VCS2] = GEN6_NOSYNC;
>  	} else {
>  		ring->mmio_base = BSD_RING_BASE;
>  		ring->flush = bsd_ring_flush;
> @@ -2126,6 +2130,52 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>  	return intel_init_ring_buffer(dev, ring);
>  }
>  
> +/**
> + * Initialize the second BSD ring for Broadwell GT3.
> + * It is noted that this only exists on Broadwell GT3.
> + */
> +int intel_init_bsd2_ring_buffer(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_ring_buffer *ring = &dev_priv->ring[VCS2];
> +
> +	if ((INTEL_INFO(dev)->gen != 8) ) {
> +		DRM_ERROR("No dual-BSD ring on non-BDW machine\n");
> +		return -EINVAL;
> +	}
> +
> +	ring->name = "second bsd ring";

"bsd2 ring" would be more concise

> +	ring->id = VCS2;
> +
> +	ring->write_tail = ring_write_tail;
> +	ring->mmio_base = GEN8_BSD2_RING_BASE;
> +	ring->flush = gen6_bsd_ring_flush;
> +	ring->add_request = gen6_add_request;
> +	ring->get_seqno = gen6_ring_get_seqno;
> +	ring->set_seqno = ring_set_seqno;
> +	ring->irq_enable_mask =
> +			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
> +	ring->irq_get = gen8_ring_get_irq;
> +	ring->irq_put = gen8_ring_put_irq;
> +	ring->dispatch_execbuffer =
> +			gen8_ring_dispatch_execbuffer;
> +	ring->sync_to = gen6_ring_sync;
> +	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
> +	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
> +	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
> +	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_INVALID;
> +	ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> +	ring->signal_mbox[RCS] = GEN6_NOSYNC;
> +	ring->signal_mbox[VCS] = GEN6_NOSYNC;
> +	ring->signal_mbox[BCS] = GEN6_NOSYNC;
> +	ring->signal_mbox[VECS] = GEN6_NOSYNC;
> +	ring->signal_mbox[VCS2] = GEN6_NOSYNC;
> +
> +	ring->init = init_ring_common;
> +
> +	return intel_init_ring_buffer(dev, ring);
> +}
> +
>  int intel_init_blt_ring_buffer(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2157,10 +2207,12 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
>  	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
>  	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
>  	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_BVE;
> +	ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
>  	ring->signal_mbox[RCS] = GEN6_RBSYNC;
>  	ring->signal_mbox[VCS] = GEN6_VBSYNC;
>  	ring->signal_mbox[BCS] = GEN6_NOSYNC;
>  	ring->signal_mbox[VECS] = GEN6_VEBSYNC;
> +	ring->signal_mbox[VCS2] = GEN6_NOSYNC;
>  	ring->init = init_ring_common;
>  
>  	return intel_init_ring_buffer(dev, ring);
> @@ -2198,10 +2250,12 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
>  	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_VEV;
>  	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VEB;
>  	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_INVALID;
> +	ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
>  	ring->signal_mbox[RCS] = GEN6_RVESYNC;
>  	ring->signal_mbox[VCS] = GEN6_VVESYNC;
>  	ring->signal_mbox[BCS] = GEN6_BVESYNC;
>  	ring->signal_mbox[VECS] = GEN6_NOSYNC;
> +	ring->signal_mbox[VCS2] = GEN6_NOSYNC;
>  	ring->init = init_ring_common;
>  
>  	return intel_init_ring_buffer(dev, ring);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 413cdc7..8ca4285 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -61,8 +61,9 @@ struct  intel_ring_buffer {
>  		VCS,
>  		BCS,
>  		VECS,
> +		VCS2,
>  	} id;
> -#define I915_NUM_RINGS 4
> +#define I915_NUM_RINGS 5
>  	u32		mmio_base;
>  	void		__iomem *virtual_start;
>  	struct		drm_device *dev;
> @@ -286,6 +287,7 @@ int intel_ring_invalidate_all_caches(struct intel_ring_buffer *ring);
>  
>  int intel_init_render_ring_buffer(struct drm_device *dev);
>  int intel_init_bsd_ring_buffer(struct drm_device *dev);
> +int intel_init_bsd2_ring_buffer(struct drm_device *dev);
>  int intel_init_blt_ring_buffer(struct drm_device *dev);
>  int intel_init_vebox_ring_buffer(struct drm_device *dev);
>  


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH V3 2/6] drm/i915:Initialize the second BSD ring on BDW GT3 machine

[Zhao Yakui]

On Wed, 2014-04-16 at 10:23 -0600, Deak, Imre wrote:
> On Wed, 2014-04-16 at 10:41 +0800, Zhao Yakui wrote:
> > Based on the hardware spec, the BDW GT3 machine has two independent
> > BSD ring that can be used to dispatch the video commands.
> > So just initialize it.
> > 
> > Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> 
> A couple of nitpicks below, with or without those:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Hi, Imre

    Thanks for your review and the comments.

    I will update the patch based on your comment. 
   
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c         |    4 +--
> >  drivers/gpu/drm/i915/i915_drv.h         |    2 ++
> >  drivers/gpu/drm/i915/i915_gem.c         |    9 +++++-
> >  drivers/gpu/drm/i915/i915_gpu_error.c   |    1 +
> >  drivers/gpu/drm/i915/i915_reg.h         |    1 +
> >  drivers/gpu/drm/i915/intel_ringbuffer.c |   54 +++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_ringbuffer.h |    4 ++-
> >  7 files changed, 71 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 17fbbe5..2a7842b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -282,7 +282,7 @@ static const struct intel_device_info intel_broadwell_m_info = {
> >  static const struct intel_device_info intel_broadwell_gt3d_info = {
> >  	.gen = 8, .num_pipes = 3,
> >  	.need_gfx_hws = 1, .has_hotplug = 1,
> > -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> > +	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >  	.has_llc = 1,
> >  	.has_ddi = 1,
> >  	.has_fbc = 1,
> > @@ -292,7 +292,7 @@ static const struct intel_device_info intel_broadwell_gt3d_info = {
> >  static const struct intel_device_info intel_broadwell_gt3m_info = {
> >  	.gen = 8, .is_mobile = 1, .num_pipes = 3,
> >  	.need_gfx_hws = 1, .has_hotplug = 1,
> > -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> > +	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >  	.has_llc = 1,
> >  	.has_ddi = 1,
> >  	.has_fbc = 1,
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 92c3095..74aef6a 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1833,7 +1833,9 @@ struct drm_i915_cmd_table {
> >  #define BSD_RING		(1<<VCS)
> >  #define BLT_RING		(1<<BCS)
> >  #define VEBOX_RING		(1<<VECS)
> > +#define BSD2_RING		(1<<VCS2)
> >  #define HAS_BSD(dev)            (INTEL_INFO(dev)->ring_mask & BSD_RING)
> > +#define HAS_BSD2(dev)		(INTEL_INFO(dev)->ring_mask & BSD2_RING)
> >  #define HAS_BLT(dev)            (INTEL_INFO(dev)->ring_mask & BLT_RING)
> >  #define HAS_VEBOX(dev)            (INTEL_INFO(dev)->ring_mask & VEBOX_RING)
> >  #define HAS_LLC(dev)            (INTEL_INFO(dev)->has_llc)
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 85c9cf0..b4dcf2a 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4374,13 +4374,20 @@ static int i915_gem_init_rings(struct drm_device *dev)
> >  			goto cleanup_blt_ring;
> >  	}
> >  
> > +	if (HAS_BSD2(dev)) {
> > +		ret = intel_init_bsd2_ring_buffer(dev);
> > +		if (ret)
> > +			goto cleanup_vebox_ring;
> > +	}
> >  
> >  	ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
> >  	if (ret)
> > -		goto cleanup_vebox_ring;
> > +		goto cleanup_ring;
> 
> maybe cleanup_bsd2_ring?  
> 
> >  
> >  	return 0;
> >  
> > +cleanup_ring:
> > +	intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
> >  cleanup_vebox_ring:
> >  	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
> >  cleanup_blt_ring:
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 4865ade..3cab7f9 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -42,6 +42,7 @@ static const char *ring_str(int ring)
> >  	case VCS: return "bsd";
> >  	case BCS: return "blt";
> >  	case VECS: return "vebox";
> > +	case VCS2: return "second bsd";
> 
> "bsd2" would be more concise
> 

OK. I will update it.

> >  	default: return "";
> >  	}
> >  }
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 8f84555..0b88508 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -760,6 +760,7 @@ enum punit_power_well {
> >  #define RENDER_RING_BASE	0x02000
> >  #define BSD_RING_BASE		0x04000
> >  #define GEN6_BSD_RING_BASE	0x12000
> > +#define GEN8_BSD2_RING_BASE	0x1c000
> >  #define VEBOX_RING_BASE		0x1a000
> >  #define BLT_RING_BASE		0x22000
> >  #define RING_TAIL(base)		((base)+0x30)
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index eb3dd26..8b9b89080 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1924,10 +1924,12 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
> >  		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
> >  		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
> >  		ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_RVE;
> > +		ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> 
> It's unclear why we initialize the semaphore stuff differently for VCS
> and VCS2. Semaphores are disabled for GEN8 atm and they will be reworked
> in the future, but maybe a comment to revisit this code would good here
> and in intel_init_bsd2_ring_buffer().

The current code for the semaphore initialization is only applied for
pre-gen8 platform. The VCS2 only exists on the Broadwell GT3 and there
is no VCS2 on the other platform. So I initialize the semaphore stuff
differently for VCS and VCS2. 

OK. I will add more comment to clarify it.

> 
> >  		ring->signal_mbox[RCS] = GEN6_NOSYNC;
> >  		ring->signal_mbox[VCS] = GEN6_VRSYNC;
> >  		ring->signal_mbox[BCS] = GEN6_BRSYNC;
> >  		ring->signal_mbox[VECS] = GEN6_VERSYNC;
> > +		ring->signal_mbox[VCS2] = GEN6_NOSYNC;
> >  	} else if (IS_GEN5(dev)) {
> >  		ring->add_request = pc_render_add_request;
> >  		ring->flush = gen4_render_ring_flush;
> > @@ -2100,10 +2102,12 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
> >  		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
> >  		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
> >  		ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_VVE;
> > +		ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> >  		ring->signal_mbox[RCS] = GEN6_RVSYNC;
> >  		ring->signal_mbox[VCS] = GEN6_NOSYNC;
> >  		ring->signal_mbox[BCS] = GEN6_BVSYNC;
> >  		ring->signal_mbox[VECS] = GEN6_VEVSYNC;
> > +		ring->signal_mbox[VCS2] = GEN6_NOSYNC;
> >  	} else {
> >  		ring->mmio_base = BSD_RING_BASE;
> >  		ring->flush = bsd_ring_flush;
> > @@ -2126,6 +2130,52 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
> >  	return intel_init_ring_buffer(dev, ring);
> >  }
> >  
> > +/**
> > + * Initialize the second BSD ring for Broadwell GT3.
> > + * It is noted that this only exists on Broadwell GT3.
> > + */
> > +int intel_init_bsd2_ring_buffer(struct drm_device *dev)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct intel_ring_buffer *ring = &dev_priv->ring[VCS2];
> > +
> > +	if ((INTEL_INFO(dev)->gen != 8) ) {
> > +		DRM_ERROR("No dual-BSD ring on non-BDW machine\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ring->name = "second bsd ring";
> 
> "bsd2 ring" would be more concise
> 
> > +	ring->id = VCS2;
> > +
> > +	ring->write_tail = ring_write_tail;
> > +	ring->mmio_base = GEN8_BSD2_RING_BASE;
> > +	ring->flush = gen6_bsd_ring_flush;
> > +	ring->add_request = gen6_add_request;
> > +	ring->get_seqno = gen6_ring_get_seqno;
> > +	ring->set_seqno = ring_set_seqno;
> > +	ring->irq_enable_mask =
> > +			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
> > +	ring->irq_get = gen8_ring_get_irq;
> > +	ring->irq_put = gen8_ring_put_irq;
> > +	ring->dispatch_execbuffer =
> > +			gen8_ring_dispatch_execbuffer;
> > +	ring->sync_to = gen6_ring_sync;
> > +	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
> > +	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
> > +	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
> > +	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_INVALID;
> > +	ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> > +	ring->signal_mbox[RCS] = GEN6_NOSYNC;
> > +	ring->signal_mbox[VCS] = GEN6_NOSYNC;
> > +	ring->signal_mbox[BCS] = GEN6_NOSYNC;
> > +	ring->signal_mbox[VECS] = GEN6_NOSYNC;
> > +	ring->signal_mbox[VCS2] = GEN6_NOSYNC;
> > +
> > +	ring->init = init_ring_common;
> > +
> > +	return intel_init_ring_buffer(dev, ring);
> > +}
> > +
> >  int intel_init_blt_ring_buffer(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -2157,10 +2207,12 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
> >  	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
> >  	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
> >  	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_BVE;
> > +	ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> >  	ring->signal_mbox[RCS] = GEN6_RBSYNC;
> >  	ring->signal_mbox[VCS] = GEN6_VBSYNC;
> >  	ring->signal_mbox[BCS] = GEN6_NOSYNC;
> >  	ring->signal_mbox[VECS] = GEN6_VEBSYNC;
> > +	ring->signal_mbox[VCS2] = GEN6_NOSYNC;
> >  	ring->init = init_ring_common;
> >  
> >  	return intel_init_ring_buffer(dev, ring);
> > @@ -2198,10 +2250,12 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
> >  	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_VEV;
> >  	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VEB;
> >  	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_INVALID;
> > +	ring->semaphore_register[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> >  	ring->signal_mbox[RCS] = GEN6_RVESYNC;
> >  	ring->signal_mbox[VCS] = GEN6_VVESYNC;
> >  	ring->signal_mbox[BCS] = GEN6_BVESYNC;
> >  	ring->signal_mbox[VECS] = GEN6_NOSYNC;
> > +	ring->signal_mbox[VCS2] = GEN6_NOSYNC;
> >  	ring->init = init_ring_common;
> >  
> >  	return intel_init_ring_buffer(dev, ring);
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index 413cdc7..8ca4285 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -61,8 +61,9 @@ struct  intel_ring_buffer {
> >  		VCS,
> >  		BCS,
> >  		VECS,
> > +		VCS2,
> >  	} id;
> > -#define I915_NUM_RINGS 4
> > +#define I915_NUM_RINGS 5
> >  	u32		mmio_base;
> >  	void		__iomem *virtual_start;
> >  	struct		drm_device *dev;
> > @@ -286,6 +287,7 @@ int intel_ring_invalidate_all_caches(struct intel_ring_buffer *ring);
> >  
> >  int intel_init_render_ring_buffer(struct drm_device *dev);
> >  int intel_init_bsd_ring_buffer(struct drm_device *dev);
> > +int intel_init_bsd2_ring_buffer(struct drm_device *dev);
> >  int intel_init_blt_ring_buffer(struct drm_device *dev);
> >  int intel_init_vebox_ring_buffer(struct drm_device *dev);
> >  
> 

[PATCH V3 3/6] drm/i915:Handle the irq interrupt for the second BSD ring

[Zhao Yakui]

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..63bd5de 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1347,13 +1347,16 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
 			DRM_ERROR("The master control interrupt lied (GT0)!\n");
 	}
 
-	if (master_ctl & GEN8_GT_VCS1_IRQ) {
+	if (master_ctl & (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) {
 		tmp = I915_READ(GEN8_GT_IIR(1));
 		if (tmp) {
 			ret = IRQ_HANDLED;
 			vcs = tmp >> GEN8_VCS1_IRQ_SHIFT;
 			if (vcs & GT_RENDER_USER_INTERRUPT)
 				notify_ring(dev, &dev_priv->ring[VCS]);
+			vcs = tmp >> GEN8_VCS2_IRQ_SHIFT;
+			if (vcs & GT_RENDER_USER_INTERRUPT)
+				notify_ring(dev, &dev_priv->ring[VCS2]);
 			I915_WRITE(GEN8_GT_IIR(1), tmp);
 		} else
 			DRM_ERROR("The master control interrupt lied (GT1)!\n");
-- 
1.7.10.1

Re: [PATCH V3 3/6] drm/i915:Handle the irq interrupt for the second BSD ring

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 1201 bytes --]

On Wed, 2014-04-16 at 10:41 +0800, Zhao Yakui wrote:
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 7a4d3ae..63bd5de 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1347,13 +1347,16 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
>  			DRM_ERROR("The master control interrupt lied (GT0)!\n");
>  	}
>  
> -	if (master_ctl & GEN8_GT_VCS1_IRQ) {
> +	if (master_ctl & (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) {
>  		tmp = I915_READ(GEN8_GT_IIR(1));
>  		if (tmp) {
>  			ret = IRQ_HANDLED;
>  			vcs = tmp >> GEN8_VCS1_IRQ_SHIFT;
>  			if (vcs & GT_RENDER_USER_INTERRUPT)
>  				notify_ring(dev, &dev_priv->ring[VCS]);
> +			vcs = tmp >> GEN8_VCS2_IRQ_SHIFT;
> +			if (vcs & GT_RENDER_USER_INTERRUPT)
> +				notify_ring(dev, &dev_priv->ring[VCS2]);
>  			I915_WRITE(GEN8_GT_IIR(1), tmp);
>  		} else
>  			DRM_ERROR("The master control interrupt lied (GT1)!\n");


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH V3 4/6] drm/i915:Add the VCS2 switch in Intel_ring_setup_status_page for Gen7 to remove the switch check warning

[Zhao Yakui]

The Gen7 doesn't have the second BSD ring. But it will complain the switch check
warning message during compilation. So just add it to remove the
switch check warning.

V1->V2: Follow Daniel's comment to update the comment

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8b9b89080..2c89525 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -988,6 +988,11 @@ void intel_ring_setup_status_page(struct intel_ring_buffer *ring)
 		case BCS:
 			mmio = BLT_HWS_PGA_GEN7;
 			break;
+		/*
+		 * VCS2 actually doesn't exist on Gen7. Only shut up
+		 * gcc switch check warning
+		 */
+		case VCS2:
 		case VCS:
 			mmio = BSD_HWS_PGA_GEN7;
 			break;
-- 
1.7.10.1

Re: [PATCH V3 4/6] drm/i915:Add the VCS2 switch in Intel_ring_setup_status_page for Gen7 to remove the switch check warning

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 1130 bytes --]

On Wed, 2014-04-16 at 10:41 +0800, Zhao Yakui wrote:
> The Gen7 doesn't have the second BSD ring. But it will complain the switch check
> warning message during compilation. So just add it to remove the
> switch check warning.
> 
> V1->V2: Follow Daniel's comment to update the comment
> 
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 8b9b89080..2c89525 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -988,6 +988,11 @@ void intel_ring_setup_status_page(struct intel_ring_buffer *ring)
>  		case BCS:
>  			mmio = BLT_HWS_PGA_GEN7;
>  			break;
> +		/*
> +		 * VCS2 actually doesn't exist on Gen7. Only shut up
> +		 * gcc switch check warning
> +		 */
> +		case VCS2:

A WARN would've been better here, but in any case:
Reviewed-by: Imre Deak <imre.deak@intel.com>

>  		case VCS:
>  			mmio = BSD_HWS_PGA_GEN7;
>  			break;


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH V3 5/6] drm/i915: Update the restrict check to filter out wrong Ring ID passed by user-space

[Zhao Yakui]

One extra ring is added in the kernel driver but it is transparent to the
user-space application/middleware. In such case the number of the rings
in kernel driver is bigger than that exported to the user-space. So
it needs to filter out the wrong Ring ID passed by user-space.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3491402..341ec68 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1035,7 +1035,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (args->flags & I915_EXEC_IS_PINNED)
 		flags |= I915_DISPATCH_PINNED;
 
-	if ((args->flags & I915_EXEC_RING_MASK) > I915_NUM_RINGS) {
+	if ((args->flags & I915_EXEC_RING_MASK) > LAST_USER_RING) {
 		DRM_DEBUG("execbuf with unknown ring: %d\n",
 			  (int)(args->flags & I915_EXEC_RING_MASK));
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 8ca4285..59f4cdd 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -64,6 +64,7 @@ struct  intel_ring_buffer {
 		VCS2,
 	} id;
 #define I915_NUM_RINGS 5
+#define LAST_USER_RING (VECS + 1)
 	u32		mmio_base;
 	void		__iomem *virtual_start;
 	struct		drm_device *dev;
-- 
1.7.10.1

Re: [PATCH V3 5/6] drm/i915: Update the restrict check to filter out wrong Ring ID passed by user-space

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 1879 bytes --]

On Wed, 2014-04-16 at 10:41 +0800, Zhao Yakui wrote:
> One extra ring is added in the kernel driver but it is transparent to the
> user-space application/middleware. In such case the number of the rings
> in kernel driver is bigger than that exported to the user-space. So
> it needs to filter out the wrong Ring ID passed by user-space.
> 
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.h    |    1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 3491402..341ec68 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1035,7 +1035,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  	if (args->flags & I915_EXEC_IS_PINNED)
>  		flags |= I915_DISPATCH_PINNED;
>  
> -	if ((args->flags & I915_EXEC_RING_MASK) > I915_NUM_RINGS) {
> +	if ((args->flags & I915_EXEC_RING_MASK) > LAST_USER_RING) {
>  		DRM_DEBUG("execbuf with unknown ring: %d\n",
>  			  (int)(args->flags & I915_EXEC_RING_MASK));
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 8ca4285..59f4cdd 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -64,6 +64,7 @@ struct  intel_ring_buffer {
>  		VCS2,
>  	} id;
>  #define I915_NUM_RINGS 5
> +#define LAST_USER_RING (VECS + 1)

Defining this in the enum is cleaner IMO. Also this patch should ideally
be applied before 2/6 for bisectability. In any case:

Reviewed-by: Imre Deak <imre.deak@intel.com> 

>  	u32		mmio_base;
>  	void		__iomem *virtual_start;
>  	struct		drm_device *dev;


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH V3 6/6] drm/i915: Use the coarse ping-pong mechanism based on drm fd to dispatch the BSD command on BDW GT3

[Zhao Yakui]

The BDW GT3 has two independent BSD rings, which can be used to process the
video commands. To be simpler, it is transparent to user-space driver/middle.
Instead the kernel driver will decide which ring is to dispatch the BSD video
command.

As every BSD ring is powerful, it is enough to dispatch the BSD video command
based on the drm fd. In such case it can play back video stream while encoding
another video stream. The coarse ping-pong mechanism is used to determine
which BSD ring is used to dispatch the BSD video command.

V1->V2: Follow Daniel's comment and use the simple ping-pong mechanism.
This is only to add the support of dual BSD rings on BDW GT3 machine.
The further optimization will be considered in another patch set.

V2->V3: Follow Daniel's comment to use the struct_mutext instead of
atomic_t during determining which ring can be used to dispatch Video command.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c            |    3 +++
 drivers/gpu/drm/i915/i915_drv.h            |    3 +++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   40 +++++++++++++++++++++++++++-
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0b38f88..f7558f5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1572,6 +1572,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	spin_lock_init(&dev_priv->backlight_lock);
 	spin_lock_init(&dev_priv->uncore.lock);
 	spin_lock_init(&dev_priv->mm.object_stat_lock);
+	dev_priv->ring_index = 0;
 	mutex_init(&dev_priv->dpio_lock);
 	mutex_init(&dev_priv->modeset_restore_lock);
 
@@ -1929,6 +1930,8 @@ void i915_driver_postclose(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 
+	if (file_priv && file_priv->bsd_ring)
+		file_priv->bsd_ring = NULL;
 	kfree(file_priv);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 74aef6a..032f992 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1472,6 +1472,8 @@ struct drm_i915_private {
 	struct i915_dri1_state dri1;
 	/* Old ums support infrastructure, same warning applies. */
 	struct i915_ums_state ums;
+	/* the indicator for dispatch video commands on two BSD rings */
+	int ring_index;
 };
 
 static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
@@ -1679,6 +1681,7 @@ struct drm_i915_file_private {
 
 	struct i915_hw_context *private_default_ctx;
 	atomic_t rps_wait_boost;
+	struct  intel_ring_buffer *bsd_ring;
 };
 
 /*
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 341ec68..1dc6f03 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -999,6 +999,37 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
 	return 0;
 }
 
+/**
+ * Find one BSD ring to dispatch the corresponding BSD command.
+ * The Ring ID is returned.
+ */
+static int gen8_dispatch_bsd_ring(struct drm_device *dev,
+				  struct drm_file *file)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_file_private *file_priv = file->driver_priv;
+
+	/* Check whether the file_priv is using one ring */
+	if (file_priv->bsd_ring)
+		return file_priv->bsd_ring->id;
+	else {
+		/* If no, use the ping-pong mechanism to select one ring */
+		int ring_id;
+
+		mutex_lock(&dev->struct_mutex);
+		if (dev_priv->ring_index == 0) {
+			ring_id = VCS;
+			dev_priv->ring_index = 1;
+		} else {
+			ring_id = VCS2;
+			dev_priv->ring_index = 0;
+		}
+		file_priv->bsd_ring = &dev_priv->ring[ring_id];
+		mutex_unlock(&dev->struct_mutex);
+		return ring_id;
+	}
+}
+
 static int
 i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		       struct drm_file *file,
@@ -1043,7 +1074,14 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT)
 		ring = &dev_priv->ring[RCS];
-	else
+	else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
+		if (HAS_BSD2(dev)) {
+			int ring_id;
+			ring_id = gen8_dispatch_bsd_ring(dev, file);
+			ring = &dev_priv->ring[ring_id];
+		} else
+			ring = &dev_priv->ring[VCS];
+	} else
 		ring = &dev_priv->ring[(args->flags & I915_EXEC_RING_MASK) - 1];
 
 	if (!intel_ring_initialized(ring)) {
-- 
1.7.10.1

Re: [PATCH V3 6/6] drm/i915: Use the coarse ping-pong mechanism based on drm fd to dispatch the BSD command on BDW GT3

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 4937 bytes --]

On Wed, 2014-04-16 at 10:41 +0800, Zhao Yakui wrote:
> The BDW GT3 has two independent BSD rings, which can be used to process the
> video commands. To be simpler, it is transparent to user-space driver/middle.
> Instead the kernel driver will decide which ring is to dispatch the BSD video
> command.
> 
> As every BSD ring is powerful, it is enough to dispatch the BSD video command
> based on the drm fd. In such case it can play back video stream while encoding
> another video stream. The coarse ping-pong mechanism is used to determine
> which BSD ring is used to dispatch the BSD video command.
> 
> V1->V2: Follow Daniel's comment and use the simple ping-pong mechanism.
> This is only to add the support of dual BSD rings on BDW GT3 machine.
> The further optimization will be considered in another patch set.
> 
> V2->V3: Follow Daniel's comment to use the struct_mutext instead of
> atomic_t during determining which ring can be used to dispatch Video command.
> 
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_dma.c            |    3 +++
>  drivers/gpu/drm/i915/i915_drv.h            |    3 +++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   40 +++++++++++++++++++++++++++-
>  3 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 0b38f88..f7558f5 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1572,6 +1572,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	spin_lock_init(&dev_priv->backlight_lock);
>  	spin_lock_init(&dev_priv->uncore.lock);
>  	spin_lock_init(&dev_priv->mm.object_stat_lock);
> +	dev_priv->ring_index = 0;
>  	mutex_init(&dev_priv->dpio_lock);
>  	mutex_init(&dev_priv->modeset_restore_lock);
>  
> @@ -1929,6 +1930,8 @@ void i915_driver_postclose(struct drm_device *dev, struct drm_file *file)
>  {
>  	struct drm_i915_file_private *file_priv = file->driver_priv;
>  
> +	if (file_priv && file_priv->bsd_ring)
> +		file_priv->bsd_ring = NULL;
>  	kfree(file_priv);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 74aef6a..032f992 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1472,6 +1472,8 @@ struct drm_i915_private {
>  	struct i915_dri1_state dri1;
>  	/* Old ums support infrastructure, same warning applies. */
>  	struct i915_ums_state ums;
> +	/* the indicator for dispatch video commands on two BSD rings */
> +	int ring_index;
>  };
>  
>  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> @@ -1679,6 +1681,7 @@ struct drm_i915_file_private {
>  
>  	struct i915_hw_context *private_default_ctx;
>  	atomic_t rps_wait_boost;
> +	struct  intel_ring_buffer *bsd_ring;
>  };
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 341ec68..1dc6f03 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -999,6 +999,37 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
>  	return 0;
>  }
>  
> +/**
> + * Find one BSD ring to dispatch the corresponding BSD command.
> + * The Ring ID is returned.
> + */
> +static int gen8_dispatch_bsd_ring(struct drm_device *dev,
> +				  struct drm_file *file)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_file_private *file_priv = file->driver_priv;
> +
> +	/* Check whether the file_priv is using one ring */
> +	if (file_priv->bsd_ring)
> +		return file_priv->bsd_ring->id;
> +	else {
> +		/* If no, use the ping-pong mechanism to select one ring */
> +		int ring_id;
> +
> +		mutex_lock(&dev->struct_mutex);
> +		if (dev_priv->ring_index == 0) {
> +			ring_id = VCS;
> +			dev_priv->ring_index = 1;
> +		} else {
> +			ring_id = VCS2;
> +			dev_priv->ring_index = 0;
> +		}
> +		file_priv->bsd_ring = &dev_priv->ring[ring_id];
> +		mutex_unlock(&dev->struct_mutex);
> +		return ring_id;
> +	}
> +}
> +
>  static int
>  i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  		       struct drm_file *file,
> @@ -1043,7 +1074,14 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  
>  	if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT)
>  		ring = &dev_priv->ring[RCS];
> -	else
> +	else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
> +		if (HAS_BSD2(dev)) {
> +			int ring_id;
> +			ring_id = gen8_dispatch_bsd_ring(dev, file);
> +			ring = &dev_priv->ring[ring_id];
> +		} else
> +			ring = &dev_priv->ring[VCS];
> +	} else
>  		ring = &dev_priv->ring[(args->flags & I915_EXEC_RING_MASK) - 1];
>  
>  	if (!intel_ring_initialized(ring)) {


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH V3 0/6] drm/i915: Add the support of dual BSD rings on BDW GT3

[Zhao Yakui]

This is the patch set that tries to add the support of dual BSD rings on BDW
GT3. Based on hardware spec, the BDW GT3 has two independent BSD rings, which
can be used to process the video commands. To be simpler, it is transparent 
to user-space driver/middleware. In such case the kernel driver will decide
which ring is to dispatch the BSD video command.

As every BSD ring is powerful, it is enough to dispatch the BSD video command
based on the drm fd. In such case the different BSD ring is used for video playing
back and encoding. 

V1->V2: Follow Daniel's comment to do the following update:
   a. consider the stolen check for BDW in kernel/early-quirks.c in patch 01
   b. update the comment in Patch 04
   c. use the simple ping-pong mechanism to add the support of dual BSD rings.
The further optimization will be considered in another patch set.

V2->V3: Follow Daniel's comment to use the struct_mutext instead of
atomic_t during determining which ring can be used to dispatch Video command.


Zhao Yakui (6):
  drm/i915: Split the BDW device definition to prepare for dual BSD
    rings on BDW GT3
  drm/i915:Initialize the second BSD ring on BDW GT3 machine
  drm/i915:Handle the irq interrupt for the second BSD ring
  drm/i915:Add the VCS2 switch in Intel_ring_setup_status_page for Gen7
    to remove the switch check warning
  drm/i915: Update the restrict check to filter out wrong Ring ID
    passed by user-space
  drm/i915: Use the coarse ping-pong mechanism based on drm fd to
    dispatch the BSD command on BDW GT3

 drivers/gpu/drm/i915/i915_dma.c            |    3 ++
 drivers/gpu/drm/i915/i915_drv.c            |   26 +++++++++++-
 drivers/gpu/drm/i915/i915_drv.h            |    5 +++
 drivers/gpu/drm/i915/i915_gem.c            |    9 ++++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   42 +++++++++++++++++++-
 drivers/gpu/drm/i915/i915_gpu_error.c      |    1 +
 drivers/gpu/drm/i915/i915_irq.c            |    5 ++-
 drivers/gpu/drm/i915/i915_reg.h            |    1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c    |   59 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    5 ++-
 include/drm/i915_pciids.h                  |   22 ++++++++---
 11 files changed, 166 insertions(+), 12 deletions(-)

-- 
1.7.10.1

[PATCH V3 5/6] drm/i915: Update the restrict check to filter out wrong Ring ID passed by user-space

[Zhao Yakui]

One extra ring is added in the kernel driver but it is transparent to the
user-space application/middleware. In such case the number of the rings
in kernel driver is bigger than that exported to the user-space. So
it needs to filter out the wrong Ring ID passed by user-space.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3491402..341ec68 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1035,7 +1035,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (args->flags & I915_EXEC_IS_PINNED)
 		flags |= I915_DISPATCH_PINNED;
 
-	if ((args->flags & I915_EXEC_RING_MASK) > I915_NUM_RINGS) {
+	if ((args->flags & I915_EXEC_RING_MASK) > LAST_USER_RING) {
 		DRM_DEBUG("execbuf with unknown ring: %d\n",
 			  (int)(args->flags & I915_EXEC_RING_MASK));
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 8ca4285..59f4cdd 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -64,6 +64,7 @@ struct  intel_ring_buffer {
 		VCS2,
 	} id;
 #define I915_NUM_RINGS 5
+#define LAST_USER_RING (VECS + 1)
 	u32		mmio_base;
 	void		__iomem *virtual_start;
 	struct		drm_device *dev;
-- 
1.7.10.1