On 12/18/2025 4:06 PM, Jani Nikula wrote:
On Thu, 18 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. 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. Signed-off-by: Ankit Nautiyal <[email protected]> --- drivers/gpu/drm/i915/display/intel_gvt_api.c | 11 +++++++++++ drivers/gpu/drm/i915/display/intel_gvt_api.h | 1 + drivers/gpu/drm/i915/gvt/display.c | 6 +++--- drivers/gpu/drm/i915/gvt/display_helpers.h | 4 ++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_gvt_api.c b/drivers/gpu/drm/i915/display/intel_gvt_api.c index 8abea318fbc2..45f12f239a2d 100644 --- a/drivers/gpu/drm/i915/display/intel_gvt_api.c +++ b/drivers/gpu/drm/i915/display/intel_gvt_api.c @@ -32,3 +32,14 @@ u32 intel_display_device_mmio_base(struct intel_display *display) return DISPLAY_MMIO_BASE(display); } EXPORT_SYMBOL_GPL(intel_display_device_mmio_base); + +bool intel_display_device_pipe_valid(struct intel_display *display, enum pipe pipe) +{ + u8 pipe_mask = DISPLAY_RUNTIME_INFO(display)->pipe_mask; + + if (pipe < PIPE_A || pipe >= I915_MAX_PIPES) + return false; + + return !!(pipe_mask & BIT(pipe));Nitpick, return pipe_mask & BIT(pipe); is sufficient, the !! is superfluous.
Ok sure, will drop this.
+} +EXPORT_SYMBOL_GPL(intel_display_device_pipe_valid);EXPORT_SYMBOL_NS_GPL(..., "I915_GVT");
Will change this as suggested.
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..11855c71e05e 100644 --- a/drivers/gpu/drm/i915/gvt/display.c +++ b/drivers/gpu/drm/i915/gvt/display.c @@ -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; @@ -672,7 +672,7 @@ void intel_vgpu_emulate_vblank(struct intel_vgpu *vgpu) int 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 3af878e3d78e..a910f8b8833d 100644 --- a/drivers/gpu/drm/i915/gvt/display_helpers.h +++ b/drivers/gpu/drm/i915/gvt/display_helpers.h @@ -32,4 +32,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) = 0; (__p) < I915_MAX_PIPES; (__p)++) \ + for_each_if(intel_display_device_pipe_valid((display), (enum pipe)(__p)))I think the caller should use enum pipe instead of int pipe and casting here.
Hmm.. ok will change this in the caller.Besides this, does the rename of macro to gvt_for_each_pipe() makes sense? Or can there be any way we can retain for_each_pipe without using the #ifdefs/#undefs?
Thanks again for the comments and suggestions. Regards, Ankit
BR, Jani.+ #endif /* __DISPLAY_HELPERS_H__ */
