From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH V3 2/6] drm/i915:Initialize the second BSD ring on BDW GT3 machine Date: Wed, 16 Apr 2014 19:23:04 +0300 Message-ID: <1397665384.4215.43.camel@intelbox> References: <1397616077-1125-1-git-send-email-yakui.zhao@intel.com> <1397616077-1125-3-git-send-email-yakui.zhao@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1050964455==" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 3920D6EAB9 for ; Wed, 16 Apr 2014 09:23:08 -0700 (PDT) In-Reply-To: <1397616077-1125-3-git-send-email-yakui.zhao@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Zhao Yakui Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1050964455== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-W5uVbui0zYSixOX5MMur" --=-W5uVbui0zYSixOX5MMur Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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. >=20 > Signed-off-by: Zhao Yakui A couple of nitpicks below, with or without those: Reviewed-by: Imre Deak > --- > 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(-) >=20 > 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 =3D { > static const struct intel_device_info intel_broadwell_gt3d_info =3D { > .gen =3D 8, .num_pipes =3D 3, > .need_gfx_hws =3D 1, .has_hotplug =3D 1, > - .ring_mask =3D RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, > + .ring_mask =3D RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RI= NG, > .has_llc =3D 1, > .has_ddi =3D 1, > .has_fbc =3D 1, > @@ -292,7 +292,7 @@ static const struct intel_device_info intel_broadwell= _gt3d_info =3D { > static const struct intel_device_info intel_broadwell_gt3m_info =3D { > .gen =3D 8, .is_mobile =3D 1, .num_pipes =3D 3, > .need_gfx_hws =3D 1, .has_hotplug =3D 1, > - .ring_mask =3D RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, > + .ring_mask =3D RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RI= NG, > .has_llc =3D 1, > .has_ddi =3D 1, > .has_fbc =3D 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< #define BLT_RING (1< #define VEBOX_RING (1< +#define BSD2_RING (1< #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_RI= NG) > #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; > } > =20 > + if (HAS_BSD2(dev)) { > + ret =3D intel_init_bsd2_ring_buffer(dev); > + if (ret) > + goto cleanup_vebox_ring; > + } > =20 > ret =3D i915_gem_set_seqno(dev, ((u32)~0 - 0x1000)); > if (ret) > - goto cleanup_vebox_ring; > + goto cleanup_ring; maybe cleanup_bsd2_ring? > =20 > return 0; > =20 > +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/i9= 15/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_devi= ce *dev) > ring->semaphore_register[VCS] =3D MI_SEMAPHORE_SYNC_RV; > ring->semaphore_register[BCS] =3D MI_SEMAPHORE_SYNC_RB; > ring->semaphore_register[VECS] =3D MI_SEMAPHORE_SYNC_RVE; > + ring->semaphore_register[VCS2] =3D 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] =3D GEN6_NOSYNC; > ring->signal_mbox[VCS] =3D GEN6_VRSYNC; > ring->signal_mbox[BCS] =3D GEN6_BRSYNC; > ring->signal_mbox[VECS] =3D GEN6_VERSYNC; > + ring->signal_mbox[VCS2] =3D GEN6_NOSYNC; > } else if (IS_GEN5(dev)) { > ring->add_request =3D pc_render_add_request; > ring->flush =3D gen4_render_ring_flush; > @@ -2100,10 +2102,12 @@ int intel_init_bsd_ring_buffer(struct drm_device = *dev) > ring->semaphore_register[VCS] =3D MI_SEMAPHORE_SYNC_INVALID; > ring->semaphore_register[BCS] =3D MI_SEMAPHORE_SYNC_VB; > ring->semaphore_register[VECS] =3D MI_SEMAPHORE_SYNC_VVE; > + ring->semaphore_register[VCS2] =3D MI_SEMAPHORE_SYNC_INVALID; > ring->signal_mbox[RCS] =3D GEN6_RVSYNC; > ring->signal_mbox[VCS] =3D GEN6_NOSYNC; > ring->signal_mbox[BCS] =3D GEN6_BVSYNC; > ring->signal_mbox[VECS] =3D GEN6_VEVSYNC; > + ring->signal_mbox[VCS2] =3D GEN6_NOSYNC; > } else { > ring->mmio_base =3D BSD_RING_BASE; > ring->flush =3D bsd_ring_flush; > @@ -2126,6 +2130,52 @@ int intel_init_bsd_ring_buffer(struct drm_device *= dev) > return intel_init_ring_buffer(dev, ring); > } > =20 > +/** > + * 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 =3D dev->dev_private; > + struct intel_ring_buffer *ring =3D &dev_priv->ring[VCS2]; > + > + if ((INTEL_INFO(dev)->gen !=3D 8) ) { > + DRM_ERROR("No dual-BSD ring on non-BDW machine\n"); > + return -EINVAL; > + } > + > + ring->name =3D "second bsd ring"; "bsd2 ring" would be more concise > + ring->id =3D VCS2; > + > + ring->write_tail =3D ring_write_tail; > + ring->mmio_base =3D GEN8_BSD2_RING_BASE; > + ring->flush =3D gen6_bsd_ring_flush; > + ring->add_request =3D gen6_add_request; > + ring->get_seqno =3D gen6_ring_get_seqno; > + ring->set_seqno =3D ring_set_seqno; > + ring->irq_enable_mask =3D > + GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT; > + ring->irq_get =3D gen8_ring_get_irq; > + ring->irq_put =3D gen8_ring_put_irq; > + ring->dispatch_execbuffer =3D > + gen8_ring_dispatch_execbuffer; > + ring->sync_to =3D gen6_ring_sync; > + ring->semaphore_register[RCS] =3D MI_SEMAPHORE_SYNC_INVALID; > + ring->semaphore_register[VCS] =3D MI_SEMAPHORE_SYNC_INVALID; > + ring->semaphore_register[BCS] =3D MI_SEMAPHORE_SYNC_INVALID; > + ring->semaphore_register[VECS] =3D MI_SEMAPHORE_SYNC_INVALID; > + ring->semaphore_register[VCS2] =3D MI_SEMAPHORE_SYNC_INVALID; > + ring->signal_mbox[RCS] =3D GEN6_NOSYNC; > + ring->signal_mbox[VCS] =3D GEN6_NOSYNC; > + ring->signal_mbox[BCS] =3D GEN6_NOSYNC; > + ring->signal_mbox[VECS] =3D GEN6_NOSYNC; > + ring->signal_mbox[VCS2] =3D GEN6_NOSYNC; > + > + ring->init =3D 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 =3D dev->dev_private; > @@ -2157,10 +2207,12 @@ int intel_init_blt_ring_buffer(struct drm_device = *dev) > ring->semaphore_register[VCS] =3D MI_SEMAPHORE_SYNC_BV; > ring->semaphore_register[BCS] =3D MI_SEMAPHORE_SYNC_INVALID; > ring->semaphore_register[VECS] =3D MI_SEMAPHORE_SYNC_BVE; > + ring->semaphore_register[VCS2] =3D MI_SEMAPHORE_SYNC_INVALID; > ring->signal_mbox[RCS] =3D GEN6_RBSYNC; > ring->signal_mbox[VCS] =3D GEN6_VBSYNC; > ring->signal_mbox[BCS] =3D GEN6_NOSYNC; > ring->signal_mbox[VECS] =3D GEN6_VEBSYNC; > + ring->signal_mbox[VCS2] =3D GEN6_NOSYNC; > ring->init =3D init_ring_common; > =20 > return intel_init_ring_buffer(dev, ring); > @@ -2198,10 +2250,12 @@ int intel_init_vebox_ring_buffer(struct drm_devic= e *dev) > ring->semaphore_register[VCS] =3D MI_SEMAPHORE_SYNC_VEV; > ring->semaphore_register[BCS] =3D MI_SEMAPHORE_SYNC_VEB; > ring->semaphore_register[VECS] =3D MI_SEMAPHORE_SYNC_INVALID; > + ring->semaphore_register[VCS2] =3D MI_SEMAPHORE_SYNC_INVALID; > ring->signal_mbox[RCS] =3D GEN6_RVESYNC; > ring->signal_mbox[VCS] =3D GEN6_VVESYNC; > ring->signal_mbox[BCS] =3D GEN6_BVESYNC; > ring->signal_mbox[VECS] =3D GEN6_NOSYNC; > + ring->signal_mbox[VCS2] =3D GEN6_NOSYNC; > ring->init =3D init_ring_common; > =20 > return intel_init_ring_buffer(dev, ring); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i9= 15/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_rin= g_buffer *ring); > =20 > 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); > =20 --=-W5uVbui0zYSixOX5MMur Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQEcBAABAgAGBQJTTq5oAAoJEORIIAnNuWDF3ZwIAJhAF/yO6sL5ZxIZgznj9kc4 hCvhcp9352f5GOpUfbq+HwoJ7RoxSRMDeOKT3+Ukh0nIheqlfm+aWV4VcQusGn65 3xKx+r6bEyYvvnqxJIvsXjMNZSUV0As3FncIJmZ/lU4U/zBOBIIQ3Gy2UCwj6xNc CllSOdfefj4li0EbI4WNfLJsFroTPVljaYk4l+mjzqSjEP/EhTPaAbg7Y6HzuxTh zfXQPy1lE8//4f/yh0yiLHuilpHsgEw2CJ8XB+FXl5zY4hApXlB1QZXvcwYKQpuo 8u7rcCBbaNGkoygpPaPTlZX82G7kFgkiwlycxdc2g6qf8neQ+ejLgnZP46d4fRQ= =P8p+ -----END PGP SIGNATURE----- --=-W5uVbui0zYSixOX5MMur-- --===============1050964455== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1050964455==--