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

[Lucas De Marchi]

On Wed, Aug 17, 2022 at 10:00:01AM +0300, Jani Nikula wrote:
>On Tue, 16 Aug 2022, Lucas De Marchi <lucas.demarchi at 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 at 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.

you missed that I misread this part. It's the icl_dsi that is used
later.

>
>>
>>
>>>+		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. :(

and we are finally removing that from the todo list soon. right? :)

Lucas De Marchi

>
>BR,
>Jani.
>
>
>>
>>
>> Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>
>>
>> Lucas De Marchi
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center