On 04/11/2016 15:32, Ville Syrjälä wrote:
On Fri, Nov 04, 2016 at 02:42:45PM +0000, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
A small selection of macros which can only accept dev_priv from
now on and a resulting trickle of fixups.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 27 ++++++++++++---------------
drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
drivers/gpu/drm/i915/i915_irq.c | 6 +++---
drivers/gpu/drm/i915/intel_crt.c | 8 ++++----
drivers/gpu/drm/i915/intel_display.c | 4 ++--
drivers/gpu/drm/i915/intel_dp.c | 2 +-
drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
drivers/gpu/drm/i915/intel_psr.c | 2 +-
8 files changed, 25 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 45a30f730216..6060e41d25e5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2901,28 +2901,25 @@ struct drm_i915_cmd_table {
#define HAS_128_BYTE_Y_TILING(dev_priv) (!IS_GEN2(dev_priv) && \
!(IS_I915G(dev_priv) || \
IS_I915GM(dev_priv)))
-#define SUPPORTS_TV(dev) (INTEL_INFO(dev)->supports_tv)
-#define I915_HAS_HOTPLUG(dev) (INTEL_INFO(dev)->has_hotplug)
-
-#define HAS_FW_BLC(dev_priv) (INTEL_GEN(dev_priv) > 2)
-#define HAS_PIPE_CXSR(dev) (INTEL_INFO(dev)->has_pipe_cxsr)
-#define HAS_FBC(dev) (INTEL_INFO(dev)->has_fbc)
+#define SUPPORTS_TV(dev_priv) ((dev_priv)->info.supports_tv)
+#define I915_HAS_HOTPLUG(dev_priv) ((dev_priv)->info.has_hotplug)
+#define HAS_FW_BLC(dev_priv) (INTEL_GEN(dev_priv) > 2)
+#define HAS_PIPE_CXSR(dev_priv) ((dev_priv)->info.has_pipe_cxsr)
+#define HAS_FBC(dev_priv) ((dev_priv)->info.has_fbc)
#define HAS_IPS(dev_priv) (IS_HSW_ULT(dev_priv) || IS_BROADWELL(dev_priv))
-
-#define HAS_DP_MST(dev) (INTEL_INFO(dev)->has_dp_mst)
-
+#define HAS_DP_MST(dev_priv) ((dev_priv)->info.has_dp_mst)
#define HAS_DDI(dev_priv) ((dev_priv)->info.has_ddi)
-#define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
-#define HAS_PSR(dev) (INTEL_INFO(dev)->has_psr)
-#define HAS_RC6(dev) (INTEL_INFO(dev)->has_rc6)
-#define HAS_RC6p(dev) (INTEL_INFO(dev)->has_rc6p)
-
-#define HAS_CSR(dev) (INTEL_INFO(dev)->has_csr)
+#define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
+#define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
+#define HAS_RC6p(dev_priv) ((dev_priv)->info.has_rc6p)
+#define HAS_CSR(dev_priv) ((dev_priv)->info.has_csr)
#define HAS_RUNTIME_PM(dev_priv) ((dev_priv)->info.has_runtime_pm)
#define HAS_64BIT_RELOC(dev_priv) ((dev_priv)->info.has_64bit_reloc)
+#define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)->info.has_fpga_dbg)
What's confusing me is this reordering of these macros. Was there a
particular reason for doing that?
Just because of its long name, so I pulled it out and separated so the
alignment is nicer in the blocks above it.
Outside that it all looks pretty reasonable. Could got a bit further
with passing around dev_priv in some cases, but I guess we can leave
that to future work.
Yes, I mention that in the cover letter.
One random idea that did pop into my head was this:
static inline const struct ... *
intel_info(struct drm_i915_private *dev_priv)
{
return &dev_priv->info;
}
#define HAS_WHATEVER(dev_priv) (intel_info(dev_priv)->whatever)
for some extra type safety. Any thoughts?
Sounds like a good idea to me. And it would be really easy to do,
localized to i915_drv.h, and then when the last INTEL_INFO(dev) gets
converted we can make it use the inline as well.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
