On Fri, 19 Dec 2025, Ankit Nautiyal <[email protected]> wrote: > Add a new API to check if a given pipe is valid using > DISPLAY_RUNTIME_INFO() for GVT. > > Update GVT to use this API instead of accessing > `DISPLAY_RUNTIME_INFO->pipe_mask` directly in the `for_each_pipe` macro. > > Since `for_each_pipe` is defined in i915/display/intel_display.h, which > also contains other macros used by gvt/display.c, we cannot drop the > intel_display.h header yet. This causes a build error because > `for_each_pipe` is included from both i915/display/intel_display.h and > gvt/display_helpers.h. > > To resolve this, rename the GVT macro to `gvt_for_each_pipe` and make it > call the new API. This avoids exposing display internals and prepares for > display modularization.
I'm fine with the separate name for this, to underline the difference. However, I think in the long run gvt should include fewer things from display/. This may mean refactoring display headers a bit to limit the exposure. Like, there's current intel_display_types.h and intel_display.h includes, and we shouldn't have them, but it's beyond this series. Reviewed-by: Jani Nikula <[email protected]> > > v2: > - Expose API to check if pipe is valid rather than the runtime info > pipe mask. (Jani) > - Rename the macro to `gvt_for_each_pipe` to resolve build error. > v3: > - Use EXPORT_SYMBOL_NS_GPL(..., "I915_GVT"); (Jani) > - Use enum pipe at call sites instead of casting in the macro. (Jani) > > Signed-off-by: Ankit Nautiyal <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_gvt_api.c | 9 +++++++++ > drivers/gpu/drm/i915/display/intel_gvt_api.h | 1 + > drivers/gpu/drm/i915/gvt/display.c | 10 +++++----- > drivers/gpu/drm/i915/gvt/display_helpers.h | 4 ++++ > 4 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_gvt_api.c > b/drivers/gpu/drm/i915/display/intel_gvt_api.c > index b1bfe4843135..a69e249395ae 100644 > --- a/drivers/gpu/drm/i915/display/intel_gvt_api.c > +++ b/drivers/gpu/drm/i915/display/intel_gvt_api.c > @@ -32,3 +32,12 @@ u32 intel_display_device_mmio_base(struct intel_display > *display) > return DISPLAY_MMIO_BASE(display); > } > EXPORT_SYMBOL_NS_GPL(intel_display_device_mmio_base, "I915_GVT"); > + > +bool intel_display_device_pipe_valid(struct intel_display *display, enum > pipe pipe) > +{ > + if (pipe < PIPE_A || pipe >= I915_MAX_PIPES) > + return false; > + > + return DISPLAY_RUNTIME_INFO(display)->pipe_mask & BIT(pipe); > +} > +EXPORT_SYMBOL_NS_GPL(intel_display_device_pipe_valid, "I915_GVT"); > diff --git a/drivers/gpu/drm/i915/display/intel_gvt_api.h > b/drivers/gpu/drm/i915/display/intel_gvt_api.h > index e9a1122a988d..a53687f7d934 100644 > --- a/drivers/gpu/drm/i915/display/intel_gvt_api.h > +++ b/drivers/gpu/drm/i915/display/intel_gvt_api.h > @@ -16,5 +16,6 @@ u32 intel_display_device_pipe_offset(struct intel_display > *display, enum pipe pi > u32 intel_display_device_trans_offset(struct intel_display *display, enum > transcoder trans); > u32 intel_display_device_cursor_offset(struct intel_display *display, enum > pipe pipe); > u32 intel_display_device_mmio_base(struct intel_display *display); > +bool intel_display_device_pipe_valid(struct intel_display *display, enum > pipe pipe); > > #endif /* __INTEL_GVT_API_H__ */ > diff --git a/drivers/gpu/drm/i915/gvt/display.c > b/drivers/gpu/drm/i915/gvt/display.c > index 9d6b22b2e4d0..1d0c581a8ccc 100644 > --- a/drivers/gpu/drm/i915/gvt/display.c > +++ b/drivers/gpu/drm/i915/gvt/display.c > @@ -188,7 +188,7 @@ static void emulate_monitor_status_change(struct > intel_vgpu *vgpu) > { > struct drm_i915_private *dev_priv = vgpu->gvt->gt->i915; > struct intel_display *display = dev_priv->display; > - int pipe; > + enum pipe pipe; > > if (IS_BROXTON(dev_priv)) { > enum transcoder trans; > @@ -200,7 +200,7 @@ static void emulate_monitor_status_change(struct > intel_vgpu *vgpu) > GEN8_DE_PORT_HOTPLUG(HPD_PORT_B) | > GEN8_DE_PORT_HOTPLUG(HPD_PORT_C)); > > - for_each_pipe(display, pipe) { > + gvt_for_each_pipe(display, pipe) { > vgpu_vreg_t(vgpu, TRANSCONF(display, pipe)) &= > ~(TRANSCONF_ENABLE | TRANSCONF_STATE_ENABLE); > vgpu_vreg_t(vgpu, DSPCNTR(display, pipe)) &= > ~DISP_ENABLE; > @@ -516,7 +516,7 @@ static void emulate_monitor_status_change(struct > intel_vgpu *vgpu) > vgpu_vreg_t(vgpu, PCH_ADPA) &= ~ADPA_CRT_HOTPLUG_MONITOR_MASK; > > /* Disable Primary/Sprite/Cursor plane */ > - for_each_pipe(display, pipe) { > + gvt_for_each_pipe(display, pipe) { > vgpu_vreg_t(vgpu, DSPCNTR(display, pipe)) &= ~DISP_ENABLE; > vgpu_vreg_t(vgpu, SPRCTL(pipe)) &= ~SPRITE_ENABLE; > vgpu_vreg_t(vgpu, CURCNTR(display, pipe)) &= ~MCURSOR_MODE_MASK; > @@ -669,10 +669,10 @@ void intel_vgpu_emulate_vblank(struct intel_vgpu *vgpu) > { > struct drm_i915_private *i915 = vgpu->gvt->gt->i915; > struct intel_display *display = i915->display; > - int pipe; > + enum pipe pipe; > > mutex_lock(&vgpu->vgpu_lock); > - for_each_pipe(display, pipe) > + gvt_for_each_pipe(display, pipe) > emulate_vblank_on_pipe(vgpu, pipe); > mutex_unlock(&vgpu->vgpu_lock); > } > diff --git a/drivers/gpu/drm/i915/gvt/display_helpers.h > b/drivers/gpu/drm/i915/gvt/display_helpers.h > index 46c5192a79a7..7c6e15aa280a 100644 > --- a/drivers/gpu/drm/i915/gvt/display_helpers.h > +++ b/drivers/gpu/drm/i915/gvt/display_helpers.h > @@ -42,4 +42,8 @@ > #define INTEL_DISPLAY_DEVICE_CURSOR_OFFSET(display, pipe) \ > intel_display_device_cursor_offset((display), (pipe)) > > +#define gvt_for_each_pipe(display, __p) \ > + for ((__p) = PIPE_A; (__p) < I915_MAX_PIPES; (__p)++) \ > + for_each_if(intel_display_device_pipe_valid((display), (__p))) > + > #endif /* __DISPLAY_HELPERS_H__ */ -- Jani Nikula, Intel
