On Tue, 16 Aug 2022, Lucas De Marchi <lucas.demar...@intel.com> 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.nik...@intel.com>
>>---
>> .../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.demar...@intel.com>
>
>
> Lucas De Marchi

-- 
Jani Nikula, Intel Open Source Graphics Center

Reply via email to