[Intel-gfx] [PATCH v2 0/7] Make GEN macros more similar

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Wed Nov 7 10:05:19 UTC 2018


On 06/11/2018 21:51, Lucas De Marchi wrote:
> This is the second version of the series trying to make GEN checks
> more similar. These or roughly the changes from v1:
> 
> - We don't have a single macro receiving 2 or 3 parameters. Now there
>    is GT_GEN_RANGE(), and GT_GEN(). The firs is the conversion from
>    IS_GEN() while the second is the conversion from IS_GEN<N>()
> - Bring GEN_FOREVER back to be used with above macros
> - Patch converting <, <=, ==, >, >=  checks using INTEL_GEN() to
>    use the macros above was added
> - INTEL_GEN() is removed to avoid it being used when we should prefer
>    the new macros
> 
> The idea of the names is to pave the way for checks of the display version,
> which would be named DISPLAY_GEN(), DISPLAY_GEN_RANGE().
> 
> In the commit messages we have the scripts to regenerate the patch to make
> it easier to apply after the discussions and also to be able to convert
> inflight patches.
> 
> Sorry in advance for the noise this causes in the codebase, but hopefully
> it is for the greater good.
> 
> 
> Lucas De Marchi (6):
>    Revert "drm/i915: Kill GEN_FOREVER"
>    drm/i915: replace IS_GEN<N> with GT_GEN(..., N)
>    drm/i915: rename IS_GEN9_* to GT_GEN9_*
>    drm/i915: replace gen checks using operators by GT_GEN/GT_GEN_RANGE

I have reservations about this patch, where I think forcing only one 
flavour maybe is not the best thing. Because for plain if-ladder cases 
it feels more readable to stick with the current scheme of arithmetic 
comparisons. And it is more efficient in code as well.

Range checks are on the other hand useful either when combined in the 
same conditional as some other bitmask based test, or when both ends of 
the comparison edge are bound.

>    drm/i915: merge gen checks to use range
>    drm/i915: remove INTEL_GEN macro

I have reservations about this as as well, especially considering the 
previous paragraph. But even on it's own I am not sure we want to go 
back to more verbose.

Perhaps in the new scheme of things it should be renamed to 
INTEL_GT_GEN? I know it doesn't fit nicely with the naming scheme of 
GT/DISPLAY_GEN.. so maybe:

GT_GEN -> IS_GT_GEN
GT_GEN_RANGE -> IS_GT_GEN_RANGE
INTEL_GEN -> GT_GEN (but churn!?)

This leaves GT and DISPLAY namespaces completely parallel in syntax and 
semantics.

Regards,

Tvrtko



> 
> Rodrigo Vivi (1):
>    drm/i915: Rename IS_GEN to GT_RANGE
> 
>   drivers/gpu/drm/i915/gvt/gtt.c                |   4 +-
>   drivers/gpu/drm/i915/gvt/handlers.c           |   2 +-
>   drivers/gpu/drm/i915/gvt/vgpu.c               |   4 +-
>   drivers/gpu/drm/i915/i915_cmd_parser.c        |   2 +-
>   drivers/gpu/drm/i915/i915_debugfs.c           | 145 +++++----
>   drivers/gpu/drm/i915/i915_drv.c               |  50 +--
>   drivers/gpu/drm/i915/i915_drv.h               |  67 ++--
>   drivers/gpu/drm/i915/i915_gem.c               |  30 +-
>   drivers/gpu/drm/i915/i915_gem_context.c       |   4 +-
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c    |  10 +-
>   drivers/gpu/drm/i915/i915_gem_fence_reg.c     |  16 +-
>   drivers/gpu/drm/i915/i915_gem_gtt.c           |  36 +-
>   drivers/gpu/drm/i915/i915_gem_render_state.c  |   2 +-
>   drivers/gpu/drm/i915/i915_gem_stolen.c        |  15 +-
>   drivers/gpu/drm/i915/i915_gem_tiling.c        |  12 +-
>   drivers/gpu/drm/i915/i915_gpu_error.c         |  62 ++--
>   drivers/gpu/drm/i915/i915_irq.c               | 120 +++----
>   drivers/gpu/drm/i915/i915_perf.c              |  14 +-
>   drivers/gpu/drm/i915/i915_pmu.c               |   6 +-
>   drivers/gpu/drm/i915/i915_reg.h               |   8 +-
>   drivers/gpu/drm/i915/i915_suspend.c           |  24 +-
>   drivers/gpu/drm/i915/i915_sysfs.c             |   2 +-
>   drivers/gpu/drm/i915/intel_atomic.c           |   4 +-
>   drivers/gpu/drm/i915/intel_audio.c            |   4 +-
>   drivers/gpu/drm/i915/intel_bios.c             |  12 +-
>   drivers/gpu/drm/i915/intel_cdclk.c            |  28 +-
>   drivers/gpu/drm/i915/intel_color.c            |   6 +-
>   drivers/gpu/drm/i915/intel_crt.c              |  12 +-
>   drivers/gpu/drm/i915/intel_csr.c              |   2 +-
>   drivers/gpu/drm/i915/intel_ddi.c              |  58 ++--
>   drivers/gpu/drm/i915/intel_device_info.c      |  46 +--
>   drivers/gpu/drm/i915/intel_display.c          | 308 +++++++++---------
>   drivers/gpu/drm/i915/intel_dp.c               |  82 ++---
>   drivers/gpu/drm/i915/intel_dp_mst.c           |   2 +-
>   drivers/gpu/drm/i915/intel_dpll_mgr.c         |  10 +-
>   drivers/gpu/drm/i915/intel_drv.h              |   2 +-
>   drivers/gpu/drm/i915/intel_engine_cs.c        |  44 +--
>   drivers/gpu/drm/i915/intel_fbc.c              |  52 +--
>   drivers/gpu/drm/i915/intel_fifo_underrun.c    |   8 +-
>   drivers/gpu/drm/i915/intel_guc_fw.c           |   4 +-
>   drivers/gpu/drm/i915/intel_hangcheck.c        |   6 +-
>   drivers/gpu/drm/i915/intel_hdcp.c             |   2 +-
>   drivers/gpu/drm/i915/intel_hdmi.c             |  18 +-
>   drivers/gpu/drm/i915/intel_i2c.c              |  14 +-
>   drivers/gpu/drm/i915/intel_lrc.c              |  32 +-
>   drivers/gpu/drm/i915/intel_lvds.c             |  14 +-
>   drivers/gpu/drm/i915/intel_mocs.c             |   8 +-
>   drivers/gpu/drm/i915/intel_overlay.c          |  12 +-
>   drivers/gpu/drm/i915/intel_panel.c            |  20 +-
>   drivers/gpu/drm/i915/intel_pipe_crc.c         |  12 +-
>   drivers/gpu/drm/i915/intel_pm.c               | 196 +++++------
>   drivers/gpu/drm/i915/intel_psr.c              |  26 +-
>   drivers/gpu/drm/i915/intel_ringbuffer.c       |  68 ++--
>   drivers/gpu/drm/i915/intel_ringbuffer.h       |   4 +-
>   drivers/gpu/drm/i915/intel_runtime_pm.c       |  32 +-
>   drivers/gpu/drm/i915/intel_sdvo.c             |  14 +-
>   drivers/gpu/drm/i915/intel_sprite.c           |  34 +-
>   drivers/gpu/drm/i915/intel_tv.c               |   2 +-
>   drivers/gpu/drm/i915/intel_uc.c               |   4 +-
>   drivers/gpu/drm/i915/intel_uncore.c           |  60 ++--
>   drivers/gpu/drm/i915/intel_wopcm.c            |   8 +-
>   drivers/gpu/drm/i915/intel_workarounds.c      |  20 +-
>   drivers/gpu/drm/i915/selftests/huge_pages.c   |   2 +-
>   .../drm/i915/selftests/i915_gem_coherency.c   |   4 +-
>   .../gpu/drm/i915/selftests/i915_gem_context.c |  10 +-
>   .../gpu/drm/i915/selftests/i915_gem_object.c  |  12 +-
>   drivers/gpu/drm/i915/selftests/i915_request.c |   2 +-
>   .../gpu/drm/i915/selftests/intel_hangcheck.c  |   8 +-
>   drivers/gpu/drm/i915/selftests/intel_lrc.c    |   2 +-
>   drivers/gpu/drm/i915/selftests/intel_uncore.c |   2 +-
>   .../drm/i915/selftests/intel_workarounds.c    |   2 +-
>   drivers/gpu/drm/i915/vlv_dsi.c                |  40 +--
>   72 files changed, 1003 insertions(+), 1006 deletions(-)
> 


More information about the Intel-gfx mailing list