Re: [Intel-gfx] [PATCH 24/39] drm/i915: move mipi_mmio_base to display.dsi

[Jani Nikula <jani.nikula@intel.com>]

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

-- 
Jani Nikula, Intel Open Source Graphics Center