[Intel-gfx] [PATCH v3 20/21] drm/i915: Make HWS_NEEDS_PHYSICAL the exception
Rodrigo Vivi
rodrigo.vivi at gmail.com
Mon Aug 15 21:00:11 UTC 2016
On Tue, Aug 09, 2016 at 11:45:26AM -0700, Carlos Santa wrote:
> Make the .hws_needs_physical the exception by switching the flag
> on earlier platforms since they are fewer to support. Remove the flag on
> later GPUs hardware since they all use GTT hws by default.
>
> Switch the logic as well in the driver to reflect this change.
>
> Signed-off-by: Carlos Santa <carlos.santa at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 ++--
> drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
> drivers/gpu/drm/i915/i915_pci.c | 27 +++++++++++++++++----------
> drivers/gpu/drm/i915/intel_ringbuffer.c | 20 ++++++++++----------
> 4 files changed, 30 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 233feb9..8e0e0fa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -755,7 +755,7 @@ struct intel_csr {
> func(is_i915g) sep \
> func(is_i945gm) sep \
> func(is_g33) sep \
> - func(need_gfx_hws) sep \
> + func(hws_needs_physical) sep \
> func(is_g4x) sep \
> func(is_pineview) sep \
> func(is_broadwater) sep \
> @@ -2731,7 +2731,7 @@ struct drm_i915_cmd_table {
> #define HAS_EDRAM(dev) (!!(__I915__(dev)->edram_cap & EDRAM_ENABLED))
> #define HAS_WT(dev) ((IS_HASWELL(dev) || IS_BROADWELL(dev)) && \
> HAS_EDRAM(dev))
> -#define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)->need_gfx_hws)
> +#define HWS_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)->hws_needs_physical)
At first sight I was going to complain about the name, since need_hws_physical sounded the correct,
but indeed it seems that for this HWS (what ever that is) we need this physical status page because
we don't have the HSW specific registers on those platforms.
So now I believe the name you chose is indeed better.
When applying this patch I got one conflict... wiggle could solve and it seems correct but maybe a
rebased version is better.
Anyways:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->has_hw_contexts)
> #define HAS_LOGICAL_RING_CONTEXTS(dev) (INTEL_INFO(dev)->has_logical_ring_contexts)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index eecb870..ba68327 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1008,7 +1008,7 @@ static void error_record_engine_registers(struct drm_i915_error_state *error,
> ee->tail = I915_READ_TAIL(engine);
> ee->ctl = I915_READ_CTL(engine);
>
> - if (I915_NEED_GFX_HWS(dev_priv)) {
> + if (!HWS_NEEDS_PHYSICAL(dev_priv)) {
> i915_reg_t mmio;
>
> if (IS_GEN7(dev_priv)) {
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 1c2f5fa..c5f4078 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -58,6 +58,7 @@
> .gen = 2, \
> .has_overlay = 1, .overlay_needs_physical = 1, \
> .has_gmch_display = 1, \
> + .hws_needs_physical = 1, \
> .ring_mask = RENDER_RING, \
> GEN_DEFAULT_PIPEOFFSETS, \
> CURSOR_OFFSETS
> @@ -95,6 +96,7 @@ static const struct intel_device_info intel_i915g_info = {
> GEN3_FEATURES,
> .is_i915g = 1, .cursor_needs_physical = 1,
> .has_overlay = 1, .overlay_needs_physical = 1,
> + .hws_needs_physical = 1,
> };
> static const struct intel_device_info intel_i915gm_info = {
> GEN3_FEATURES,
> @@ -103,11 +105,13 @@ static const struct intel_device_info intel_i915gm_info = {
> .has_overlay = 1, .overlay_needs_physical = 1,
> .supports_tv = 1,
> .has_fbc = 1,
> + .hws_needs_physical = 1,
> };
> static const struct intel_device_info intel_i945g_info = {
> GEN3_FEATURES,
> .has_hotplug = 1, .cursor_needs_physical = 1,
> .has_overlay = 1, .overlay_needs_physical = 1,
> + .hws_needs_physical = 1,
> };
> static const struct intel_device_info intel_i945gm_info = {
> GEN3_FEATURES,
> @@ -116,6 +120,7 @@ static const struct intel_device_info intel_i945gm_info = {
> .has_overlay = 1, .overlay_needs_physical = 1,
> .supports_tv = 1,
> .has_fbc = 1,
> + .hws_needs_physical = 1,
> };
>
> #define GEN4_FEATURES \
> @@ -130,6 +135,7 @@ static const struct intel_device_info intel_i965g_info = {
> GEN4_FEATURES,
> .is_broadwater = 1,
> .has_overlay = 1,
> + .hws_needs_physical = 1,
> };
>
> static const struct intel_device_info intel_i965gm_info = {
> @@ -138,11 +144,12 @@ static const struct intel_device_info intel_i965gm_info = {
> .is_mobile = 1, .has_fbc = 1,
> .has_overlay = 1,
> .supports_tv = 1,
> + .hws_needs_physical = 1,
> };
>
> static const struct intel_device_info intel_g33_info = {
> .gen = 3, .is_g33 = 1, .num_pipes = 2,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> + .has_hotplug = 1,
> .has_overlay = 1,
> .ring_mask = RENDER_RING,
> GEN_DEFAULT_PIPEOFFSETS,
> @@ -151,7 +158,7 @@ static const struct intel_device_info intel_g33_info = {
>
> static const struct intel_device_info intel_g45_info = {
> GEN4_FEATURES,
> - .is_g4x = 1, .need_gfx_hws = 1,
> + .is_g4x = 1,
> .has_pipe_cxsr = 1,
> .ring_mask = RENDER_RING | BSD_RING,
> };
> @@ -159,7 +166,7 @@ static const struct intel_device_info intel_g45_info = {
> static const struct intel_device_info intel_gm45_info = {
> GEN4_FEATURES,
> .is_g4x = 1,
> - .is_mobile = 1, .need_gfx_hws = 1, .has_fbc = 1,
> + .is_mobile = 1, .has_fbc = 1,
> .has_pipe_cxsr = 1,
> .supports_tv = 1,
> .ring_mask = RENDER_RING | BSD_RING,
> @@ -167,7 +174,7 @@ static const struct intel_device_info intel_gm45_info = {
>
> static const struct intel_device_info intel_pineview_info = {
> .gen = 3, .is_g33 = 1, .is_pineview = 1, .is_mobile = 1, .num_pipes = 2,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> + .has_hotplug = 1,
> .has_overlay = 1,
> .ring_mask = RENDER_RING,
> GEN_DEFAULT_PIPEOFFSETS,
> @@ -176,7 +183,7 @@ static const struct intel_device_info intel_pineview_info = {
>
> #define GEN5_FEATURES \
> .gen = 5, .num_pipes = 2, \
> - .need_gfx_hws = 1, .has_hotplug = 1, \
> + .has_hotplug = 1, \
> .has_gmbus_irq = 1, \
> .ring_mask = RENDER_RING | BSD_RING, \
> GEN_DEFAULT_PIPEOFFSETS, \
> @@ -193,7 +200,7 @@ static const struct intel_device_info intel_ironlake_m_info = {
>
> #define GEN6_FEATURES \
> .gen = 6, .num_pipes = 2, \
> - .need_gfx_hws = 1, .has_hotplug = 1, \
> + .has_hotplug = 1, \
> .has_fbc = 1, \
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> .has_llc = 1, \
> @@ -215,7 +222,7 @@ static const struct intel_device_info intel_sandybridge_m_info = {
>
> #define GEN7_FEATURES \
> .gen = 7, .num_pipes = 3, \
> - .need_gfx_hws = 1, .has_hotplug = 1, \
> + .has_hotplug = 1, \
> .has_fbc = 1, \
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> .has_llc = 1, \
> @@ -254,7 +261,7 @@ static const struct intel_device_info intel_ivybridge_q_info = {
> .has_gmbus_irq = 1, \
> .has_hw_contexts = 1, \
> .has_gmch_display = 1, \
> - .need_gfx_hws = 1, .has_hotplug = 1, \
> + .has_hotplug = 1, \
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> .display_mmio_offset = VLV_DISPLAY_BASE, \
> GEN_DEFAULT_PIPEOFFSETS, \
> @@ -302,7 +309,7 @@ static const struct intel_device_info intel_broadwell_gt3_info = {
>
> static const struct intel_device_info intel_cherryview_info = {
> .gen = 8, .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> + .has_hotplug = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> .is_cherryview = 1,
> .has_psr = 1,
> @@ -337,7 +344,7 @@ static const struct intel_device_info intel_skylake_gt3_info = {
> static const struct intel_device_info intel_broxton_info = {
> .is_broxton = 1,
> .gen = 9,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> + .has_hotplug = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> .num_pipes = 3,
> .has_ddi = 1,
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index e08a1e1..f19ce8a 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -559,10 +559,10 @@ static int init_ring_common(struct intel_engine_cs *engine)
> }
> }
>
> - if (I915_NEED_GFX_HWS(dev_priv))
> - intel_ring_setup_status_page(engine);
> - else
> + if (HWS_NEEDS_PHYSICAL(dev_priv))
> ring_setup_phys_status_page(engine);
> + else
> + intel_ring_setup_status_page(engine);
>
> /* Enforce ordering by reading HEAD register back */
> I915_READ_HEAD(engine);
> @@ -2167,13 +2167,13 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine)
> }
> engine->buffer = ring;
>
> - if (I915_NEED_GFX_HWS(dev_priv)) {
> - ret = init_status_page(engine);
> + if (HWS_NEEDS_PHYSICAL(dev_priv)) {
> + WARN_ON(engine->id != RCS);
> + ret = init_phys_status_page(engine);
> if (ret)
> goto error;
> } else {
> - WARN_ON(engine->id != RCS);
> - ret = init_phys_status_page(engine);
> + ret = init_status_page(engine);
> if (ret)
> goto error;
> }
> @@ -2213,11 +2213,11 @@ void intel_engine_cleanup(struct intel_engine_cs *engine)
> if (engine->cleanup)
> engine->cleanup(engine);
>
> - if (I915_NEED_GFX_HWS(dev_priv)) {
> - cleanup_status_page(engine);
> - } else {
> + if (HWS_NEEDS_PHYSICAL(dev_priv)) {
> WARN_ON(engine->id != RCS);
> cleanup_phys_status_page(engine);
> + } else {
> + cleanup_status_page(engine);
> }
>
> intel_engine_cleanup_common(engine);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list