[Intel-gfx] [intel-gfx][PATCH 2/2] drm/i915: Add a new ring buffer on Sandybridge
Chris Wilson
chris at chris-wilson.co.uk
Thu Sep 2 09:16:35 CEST 2010
Comments inline.
On Thu, 2 Sep 2010 21:46:54 +0800, "Xiang, Haihao" <haihao.xiang at intel.com> wrote:
> This ring buffer is used for video decoding/encoding on Sandybridge.
>
> Signed-off-by: Xiang, Haihao <haihao.xiang at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 6 ++-
> drivers/gpu/drm/i915/i915_irq.c | 15 +++--
> drivers/gpu/drm/i915/i915_reg.h | 22 +++++-
> drivers/gpu/drm/i915/intel_ringbuffer.c | 121 +++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
> 6 files changed, 158 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 047cd7c..22d098b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1203,7 +1203,7 @@ extern void intel_overlay_print_error_state(struct seq_file *m, struct intel_ove
> (dev)->pci_device == 0x2A42 || \
> (dev)->pci_device == 0x2E42)
>
> -#define HAS_BSD(dev) (IS_IRONLAKE(dev) || IS_G4X(dev))
> +#define HAS_BSD(dev) (IS_IRONLAKE(dev) || IS_G4X(dev) || IS_GEN6(dev))
Convert HAS_BSD(dev) to INTEL_INFO(dev)->has_bsd_ring and update
capabilities accordingly.
> #define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)->need_gfx_hws)
>
> /* With the 945 and later, Y tiling got adjusted so that it was 32 128-byte
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8ccb55a..d2c825a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4504,7 +4504,11 @@ i915_gem_init_ringbuffer(struct drm_device *dev)
> goto cleanup_pipe_control;
>
> if (HAS_BSD(dev)) {
> - dev_priv->bsd_ring = bsd_ring;
> + if (IS_GEN6(dev))
> + dev_priv->bsd_ring = gen6_bsd_ring;
> + else
> + dev_priv->bsd_ring = bsd_ring;
> +
Lets stop exporting these struct initialisers before we end up with one
per generation per ring. Introduce intel_init_render_ring_buffer() and
intel_init_bsd_ring_buffer() and similarly tidy up the HWS page
initialisation.
> ret = intel_init_ring_buffer(dev, &dev_priv->bsd_ring);
> if (ret)
> goto cleanup_render_ring;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 16861b8..82e708c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -312,6 +312,10 @@ irqreturn_t ironlake_irq_handler(struct drm_device *dev)
> u32 de_iir, gt_iir, de_ier, pch_iir;
> struct drm_i915_master_private *master_priv;
> struct intel_ring_buffer *render_ring = &dev_priv->render_ring;
> + u32 bsd_usr_interrupt = GT_BSD_USER_INTERRUPT;
> +
> + if (IS_GEN6(dev))
> + bsd_usr_interrupt = GT_GEN6_BSD_USER_INTERRUPT;
>
> /* disable master interrupt before clearing iir */
> de_ier = I915_READ(DEIER);
> @@ -342,10 +346,9 @@ irqreturn_t ironlake_irq_handler(struct drm_device *dev)
> dev_priv->hangcheck_count = 0;
> mod_timer(&dev_priv->hangcheck_timer, jiffies + DRM_I915_HANGCHECK_PERIOD);
> }
> - if (gt_iir & GT_BSD_USER_INTERRUPT)
> + if (gt_iir & bsd_usr_interrupt)
> DRM_WAKEUP(&dev_priv->bsd_ring.irq_queue);
>
> -
> if (de_iir & DE_GSE)
> ironlake_opregion_gse_intr(dev);
>
> @@ -1381,17 +1384,19 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
> I915_WRITE(DEIER, dev_priv->de_irq_enable_reg);
> (void) I915_READ(DEIER);
>
> - /* Gen6 only needs render pipe_control now */
> if (IS_GEN6(dev))
> - render_mask = GT_PIPE_NOTIFY;
> + render_mask = GT_PIPE_NOTIFY | GT_GEN6_BSD_USER_INTERRUPT;
>
> dev_priv->gt_irq_mask_reg = ~render_mask;
> dev_priv->gt_irq_enable_reg = render_mask;
>
> I915_WRITE(GTIIR, I915_READ(GTIIR));
> I915_WRITE(GTIMR, dev_priv->gt_irq_mask_reg);
> - if (IS_GEN6(dev))
> + if (IS_GEN6(dev)) {
> I915_WRITE(GEN6_RENDER_IMR, ~GEN6_RENDER_PIPE_CONTROL_NOTIFY_INTERRUPT);
> + I915_WRITE(GEN6_BSD_IMR, ~GEN6_BSD_IMR_USER_INTERRUPT);
> + }
> +
> I915_WRITE(GTIER, dev_priv->gt_irq_enable_reg);
> (void) I915_READ(GTIER);
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 67e3ec1..9d57ecc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -192,11 +192,11 @@
> #define MI_STORE_DWORD_INDEX MI_INSTR(0x21, 1)
> #define MI_STORE_DWORD_INDEX_SHIFT 2
> #define MI_LOAD_REGISTER_IMM MI_INSTR(0x22, 1)
> +#define MI_FLUSH_DW MI_INSTR(0x26, 2) /* for GEN6 */
Random new abbreviation. Use MI_FLUSH_DWORD for consistency.
> #define MI_BATCH_BUFFER MI_INSTR(0x30, 1)
> #define MI_BATCH_NON_SECURE (1)
> #define MI_BATCH_NON_SECURE_I965 (1<<8)
> #define MI_BATCH_BUFFER_START MI_INSTR(0x31, 0)
> -
> /*
> * 3D instructions used by the kernel
> */
> @@ -476,6 +476,24 @@
> #define BSD_HWS_PGA 0x04080
>
> /*
> + * video command stream instruction and interrupt control register defines
> + * for GEN6
> + */
> +#define GEN6_BSD_RING_TAIL 0x12030
> +#define GEN6_BSD_RING_HEAD 0x12034
> +#define GEN6_BSD_RING_START 0x12038
> +#define GEN6_BSD_RING_CTL 0x1203c
> +#define GEN6_BSD_RING_ACTHD 0x12074
> +#define GEN6_BSD_HWS_PGA 0x14080
> +
> +#define GEN6_BSD_SLEEP_PSMI_CONTROL 0x12050
> +#define GEN6_BSD_SLEEP_PSMI_CONTROL_IDLE_INDICATOR (1 << 3)
> +
> +#define GEN6_BSD_IMR 0x120a8
> +#define GEN6_BSD_IMR_USER_INTERRUPT (1 << 12)
> +#define GEN6_BSD_RNCID 0x12198
> +
> +/*
> * Framebuffer compression (915+ only)
> */
>
> @@ -2507,7 +2525,7 @@
> #define GT_SYNC_STATUS (1 << 2)
> #define GT_USER_INTERRUPT (1 << 0)
> #define GT_BSD_USER_INTERRUPT (1 << 5)
> -
> +#define GT_GEN6_BSD_USER_INTERRUPT (1 << 12)
>
> #define GTISR 0x44010
> #define GTIMR 0x44014
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index dda2751..4c0f8c4 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -32,6 +32,7 @@
> #include "i915_drv.h"
> #include "i915_drm.h"
> #include "i915_trace.h"
> +#include "intel_drv.h"
>
> static u32 i915_gem_get_seqno(struct drm_device *dev)
> {
> @@ -874,3 +875,123 @@ struct intel_ring_buffer bsd_ring = {
> .status_page = {NULL, 0, NULL},
> .map = {0,}
> };
> +
> +static void gen6_bsd_setup_status_page(struct drm_device *dev,
> + struct intel_ring_buffer *ring)
> +{
> + drm_i915_private_t *dev_priv = dev->dev_private;
> + I915_WRITE(GEN6_BSD_HWS_PGA, ring->status_page.gfx_addr);
> + I915_READ(GEN6_BSD_HWS_PGA);
> +}
> +
> +static inline unsigned int gen6_bsd_ring_get_head(struct drm_device *dev,
> + struct intel_ring_buffer *ring)
> +{
> + drm_i915_private_t *dev_priv = dev->dev_private;
> + return I915_READ(GEN6_BSD_RING_HEAD) & HEAD_ADDR;
> +}
> +
> +static inline unsigned int gen6_bsd_ring_get_tail(struct drm_device *dev,
> + struct intel_ring_buffer *ring)
> +{
> + drm_i915_private_t *dev_priv = dev->dev_private;
> + return I915_READ(GEN6_BSD_RING_TAIL) & TAIL_ADDR;
> +}
> +
> +static inline void gen6_bsd_ring_set_tail(struct drm_device *dev,
> + u32 value)
> +{
> + drm_i915_private_t *dev_priv = dev->dev_private;
> +
> + /* Every tail move must follow the sequence below */
> + I915_WRITE(GEN6_BSD_SLEEP_PSMI_CONTROL, 0x00010001);
Magic number. (1 << 16) | (1 << 0) would be better just to distinguish
between the mask and enable bits (guess). Actually naming the bits for
that register would be preferable.
> + I915_WRITE(GEN6_BSD_RNCID, 0x00000000);
> +
> + if (wait_for((I915_READ(GEN6_BSD_SLEEP_PSMI_CONTROL) &
> + GEN6_BSD_SLEEP_PSMI_CONTROL_IDLE_INDICATOR) == 0,
> + 50, 0))
> + DRM_ERROR("timed out waiting for IDLE Indicator\n");
> +
> + I915_WRITE(GEN6_BSD_RING_TAIL, value);
> + I915_WRITE(GEN6_BSD_SLEEP_PSMI_CONTROL, 0x00010000);
Do you need to clear the IDLE_INDICATOR here as well?
> +}
> +
> +static inline unsigned int gen6_bsd_ring_get_active_head(struct drm_device *dev,
> + struct intel_ring_buffer *ring)
> +{
> + drm_i915_private_t *dev_priv = dev->dev_private;
> + return I915_READ(GEN6_BSD_RING_ACTHD);
> +}
> +
> +static inline void gen6_bsd_ring_advance_ring(struct drm_device *dev,
> + struct intel_ring_buffer *ring)
> +{
> + gen6_bsd_ring_set_tail(dev, ring->tail);
> +}
> +
> +static void gen6_bsd_ring_flush(struct drm_device *dev,
> + struct intel_ring_buffer *ring,
> + u32 invalidate_domains,
> + u32 flush_domains)
> +{
> + intel_ring_begin(dev, ring, 4);
> + intel_ring_emit(dev, ring, MI_FLUSH_DW);
> + intel_ring_emit(dev, ring, 0);
> + intel_ring_emit(dev, ring, 0);
> + intel_ring_emit(dev, ring, 0);
> + intel_ring_advance(dev, ring);
> +}
> +
> +static int
> +gen6_bsd_ring_dispatch_gem_execbuffer(struct drm_device *dev,
> + struct intel_ring_buffer *ring,
> + struct drm_i915_gem_execbuffer2 *exec,
> + struct drm_clip_rect *cliprects,
> + uint64_t exec_offset)
> +{
> + uint32_t exec_start;
> + exec_start = (uint32_t) exec_offset + exec->batch_start_offset;
> + intel_ring_begin(dev, ring, 2);
> + intel_ring_emit(dev, ring, MI_BATCH_BUFFER_START | MI_BATCH_NON_SECURE_I965); /* bit0-7 is the length on GEN6+ */
> + intel_ring_emit(dev, ring, exec_start);
> + intel_ring_advance(dev, ring);
> + return 0;
> +}
> +
> +/* ring buffer for Video Codec for Gen6+ */
> +struct intel_ring_buffer gen6_bsd_ring = {
> + .name = "gen6 bsd ring",
> + .regs = {
> + .ctl = GEN6_BSD_RING_CTL,
> + .head = GEN6_BSD_RING_HEAD,
> + .tail = GEN6_BSD_RING_TAIL,
> + .start = GEN6_BSD_RING_START
> + },
> + .ring_flag = I915_EXEC_BSD,
> + .size = 32 * PAGE_SIZE,
> + .alignment = PAGE_SIZE,
> + .virtual_start = NULL,
> + .dev = NULL,
> + .gem_object = NULL,
> + .head = 0,
> + .tail = 0,
> + .space = 0,
> + .user_irq_refcount = 0,
> + .irq_gem_seqno = 0,
> + .waiting_gem_seqno = 0,
> + .setup_status_page = gen6_bsd_setup_status_page,
> + .init = init_bsd_ring,
> + .get_head = gen6_bsd_ring_get_head,
> + .get_tail = gen6_bsd_ring_get_tail,
> + .set_tail = gen6_bsd_ring_set_tail,
> + .get_active_head = gen6_bsd_ring_get_active_head,
> + .advance_ring = gen6_bsd_ring_advance_ring,
> + .flush = gen6_bsd_ring_flush,
> + .add_request = bsd_ring_add_request,
> + .get_gem_seqno = bsd_ring_get_gem_seqno,
> + .user_irq_get = bsd_ring_get_user_irq,
> + .user_irq_put = bsd_ring_put_user_irq,
> + .dispatch_gem_execbuffer = gen6_bsd_ring_dispatch_gem_execbuffer,
> + .status_page = {NULL, 0, NULL},
> + .map = {0,}
> +};
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 8245261..9d38c33 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -129,5 +129,6 @@ u32 intel_ring_get_seqno(struct drm_device *dev,
>
> extern struct intel_ring_buffer render_ring;
> extern struct intel_ring_buffer bsd_ring;
> +extern struct intel_ring_buffer gen6_bsd_ring;
>
> #endif /* _INTEL_RINGBUFFER_H_ */
> --
> 1.7.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
Chris Wilson, Intel Open Source Technology Centre
More information about the Intel-gfx
mailing list