On Tue, 16 Aug 2022, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote: > On Thu, Aug 11, 2022 at 06:07:35PM +0300, Jani Nikula wrote: >>Move display related members under drm_i915_private display sub-struct. >> >>Prefer adding anonymous sub-structs even for single members that aren't >>our own structs. >> >>Abstract mmio base member access in register definitions in a macro. >> >>Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >>--- >> .../gpu/drm/i915/display/intel_display_core.h | 5 + >> drivers/gpu/drm/i915/display/vlv_dsi.c | 4 +- >> drivers/gpu/drm/i915/display/vlv_dsi_regs.h | 188 +++++++++--------- >> drivers/gpu/drm/i915/i915_drv.h | 3 - >> 4 files changed, 102 insertions(+), 98 deletions(-) >> >>diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h >>index 533c2e3a6685..db1ba379797e 100644 >>--- a/drivers/gpu/drm/i915/display/intel_display_core.h >>+++ b/drivers/gpu/drm/i915/display/intel_display_core.h >>@@ -251,6 +251,11 @@ struct intel_display { >> unsigned int max_cdclk_freq; >> } cdclk; >> >>+ struct { >>+ /* VLV/CHV/BXT/GLK DSI MMIO register base address */ > > this is already outdated, so maybe remove the platforms from the > comment? What did I miss? It's only used for vlv_dsi, not icl_dsi which is the newer thing. > > >>+ u32 mmio_base; >>+ } dsi; >>+ >> struct { >> /* list of fbdev register on this device */ >> struct intel_fbdev *fbdev; >>diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c >>index b9b1fed99874..9651a1f00587 100644 >>--- a/drivers/gpu/drm/i915/display/vlv_dsi.c >>+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c >>@@ -1872,9 +1872,9 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv) >> return; >> >> if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) >>- dev_priv->mipi_mmio_base = BXT_MIPI_BASE; >>+ dev_priv->display.dsi.mmio_base = BXT_MIPI_BASE; >> else >>- dev_priv->mipi_mmio_base = VLV_MIPI_BASE; >>+ dev_priv->display.dsi.mmio_base = VLV_MIPI_BASE; >> >> intel_dsi = kzalloc(sizeof(*intel_dsi), GFP_KERNEL); >> if (!intel_dsi) >>diff --git a/drivers/gpu/drm/i915/display/vlv_dsi_regs.h b/drivers/gpu/drm/i915/display/vlv_dsi_regs.h >>index 356e51515346..e065b8f2ee08 100644 >>--- a/drivers/gpu/drm/i915/display/vlv_dsi_regs.h >>+++ b/drivers/gpu/drm/i915/display/vlv_dsi_regs.h >>@@ -11,6 +11,8 @@ >> #define VLV_MIPI_BASE VLV_DISPLAY_BASE >> #define BXT_MIPI_BASE 0x60000 >> >>+#define _MIPI_MMIO_BASE(__i915) ((__i915)->display.dsi.mmio_base) >>+ >> #define _MIPI_PORT(port, a, c) (((port) == PORT_A) ? a : c) /* ports A and C only */ >> #define _MMIO_MIPI(port, a, c) _MMIO(_MIPI_PORT(port, a, c)) >> >>@@ -96,8 +98,8 @@ >> >> /* MIPI DSI Controller and D-PHY registers */ >> >>-#define _MIPIA_DEVICE_READY (dev_priv->mipi_mmio_base + 0xb000) >>-#define _MIPIC_DEVICE_READY (dev_priv->mipi_mmio_base + 0xb800) >>+#define _MIPIA_DEVICE_READY (_MIPI_MMIO_BASE(dev_priv) + 0xb000) > > ugh, and I thought we wouldn't have so many implicit params anymore. > Mind adding a "TODO: remove implicit dev_priv parameter" ? It's been on my private todo list like 10 years. :( BR, Jani. > > > Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> > > > Lucas De Marchi -- Jani Nikula, Intel Open Source Graphics Center