On Wed, 2023-02-01 at 19:09 +0200, Jani Nikula wrote: > On Wed, 01 Feb 2023, Luca Coelho <luciano.coelho@xxxxxxxxx> wrote: > > There are a few macros (e.g. DPLL()) that implicitly use dev_priv, by > > using other macros that implcitily use dev_priv. > > > > In an effort to align all definitions of struct drm_i915_private to be > > declared as i915 instead of arbitrarily using either i915 or dev_priv, > > we need to make these macros explicitly use dev_priv, so that we can > > change them later to be defined as i915. > > Lucas posted a slightly related patch [1], and I think based on the > discussion we should probably add AUX and DPLL registers that are > VLV/CHV specific, and include the MMIO offset directly without dev_priv, > and non-VLV/CHV macros that will have MMIO offset 0. This would reduce > the implicit dev_priv considerably, and avoid the need to pass i915 > pointer to those register macros altogether. Yes, I saw that. > > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi_regs.h b/drivers/gpu/drm/i915/display/vlv_dsi_regs.h > > index abbe427e462e..d00e9321064a 100644 > > --- a/drivers/gpu/drm/i915/display/vlv_dsi_regs.h > > +++ b/drivers/gpu/drm/i915/display/vlv_dsi_regs.h > > @@ -100,7 +100,7 @@ > > > > #define _MIPIA_DEVICE_READY (_MIPI_MMIO_BASE(dev_priv) + 0xb000) > > #define _MIPIC_DEVICE_READY (_MIPI_MMIO_BASE(dev_priv) + 0xb800) > > -#define MIPI_DEVICE_READY(port) _MMIO_MIPI(port, _MIPIA_DEVICE_READY, _MIPIC_DEVICE_READY) > > +#define MIPI_DEVICE_READY(dev_priv, port) _MMIO_MIPI(port, _MIPIA_DEVICE_READY, _MIPIC_DEVICE_READY) > > While this kind of passes dev_priv as parameter, the dev_priv in > _MIPIA_DEVICE_READY and _MIPIC_DEVICE_READY is still implicit. Yes, this was on purpose and my second change is to modify the the calls to the inner macros to pass the parameter as well. In any case, this already works as is, even if we pass i915 to MIPI_DEVICE_READY() here, because the dev_priv that MIPI*_DEVICE_READY use will be expanded to whatever we passed as dev_priv to the outer macro. > I think these could use a similar treatment as in [1], moving the > _MIPI_MMIO_BASE() part one level up. Yes, and this can also be done with more rules after my other patches. The problem is if we all start making changes in this area at the same time, then there will be conflict after conflict. -- Cheers, Luca.
