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
