On Fri, Nov 07, 2025 at 09:05:38PM -0300, Gustavo Sousa wrote:
> We will need to know the FBC id respective to the pipe in other parts of
> the driver. Let's promote the static function skl_fbc_id_for_pipe() to a
> public one named intel_fbc_id_for_pipe().
> 
> Signed-off-by: Gustavo Sousa <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c           | 5 +++++
>  drivers/gpu/drm/i915/display/intel_fbc.h           | 2 ++
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 9 ++-------
>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c 
> b/drivers/gpu/drm/i915/display/intel_fbc.c
> index a1e3083022ee..435bfd05109c 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -129,6 +129,11 @@ struct intel_fbc {
>       const char *no_fbc_reason;
>  };
>  
> +enum intel_fbc_id intel_fbc_id_for_pipe(enum pipe pipe)
> +{
> +     return pipe - PIPE_A + INTEL_FBC_A;

The existing usage of skl_fbc_id_for_pipe() was to call this function to
receive a (possibly bogus) FBC ID, and then follow up with a call to
skl_plane_has_fbc() which had checks to make sure the returned FBC ID
actually existed on the platform.  So, for example, calling
skl_fbc_id_for_pipe(PIPE_B) on something like an ICL would return
INTEL_FBC_B here, but then the subsequent call to skl_plane_has_fbc()
would realize that there is no FBC_B on that platform and bail out.
It's only relatively recently (MTL and beyond I think?) that FBC has
become usable on pipes other than A.

Now that we're promoting this function to be more general, should we
also adjust the logic so that this function either returns a *valid* FBC
ID or and error?  Otherwise it may not be apparent to people writing new
code that the result returned here can't be immediately trusted without
additional checking.


Matt

> +}
> +
>  /* plane stride in pixels */
>  static unsigned int intel_fbc_plane_stride(const struct intel_plane_state 
> *plane_state)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h 
> b/drivers/gpu/drm/i915/display/intel_fbc.h
> index 91424563206a..3d02f3fe5630 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.h
> @@ -9,6 +9,7 @@
>  #include <linux/types.h>
>  
>  enum fb_op_origin;
> +enum pipe;
>  struct intel_atomic_state;
>  struct intel_crtc;
>  struct intel_crtc_state;
> @@ -27,6 +28,7 @@ enum intel_fbc_id {
>       I915_MAX_FBCS,
>  };
>  
> +enum intel_fbc_id intel_fbc_id_for_pipe(enum pipe pipe);
>  int intel_fbc_atomic_check(struct intel_atomic_state *state);
>  int intel_fbc_min_cdclk(const struct intel_crtc_state *crtc_state);
>  bool intel_fbc_pre_update(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c 
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index bc55fafe9ce3..275ee2903219 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -439,11 +439,6 @@ static int skl_plane_max_height(const struct 
> drm_framebuffer *fb,
>       return 4096;
>  }
>  
> -static enum intel_fbc_id skl_fbc_id_for_pipe(enum pipe pipe)
> -{
> -     return pipe - PIPE_A + INTEL_FBC_A;
> -}
> -
>  static bool skl_plane_has_fbc(struct intel_display *display,
>                             enum intel_fbc_id fbc_id, enum plane_id plane_id)
>  {
> @@ -896,7 +891,7 @@ static void x3p_lpd_plane_update_pixel_normalizer(struct 
> intel_dsb *dsb,
>                                                 bool enable)
>  {
>       struct intel_display *display = to_intel_display(plane);
> -     enum intel_fbc_id fbc_id = skl_fbc_id_for_pipe(plane->pipe);
> +     enum intel_fbc_id fbc_id = intel_fbc_id_for_pipe(plane->pipe);
>       u32 val;
>  
>       /* Only HDR planes have pixel normalizer and don't matter if no FBC */
> @@ -2442,7 +2437,7 @@ void icl_link_nv12_planes(struct intel_plane_state 
> *uv_plane_state,
>  static struct intel_fbc *skl_plane_fbc(struct intel_display *display,
>                                      enum pipe pipe, enum plane_id plane_id)
>  {
> -     enum intel_fbc_id fbc_id = skl_fbc_id_for_pipe(pipe);
> +     enum intel_fbc_id fbc_id = intel_fbc_id_for_pipe(pipe);
>  
>       if (skl_plane_has_fbc(display, fbc_id, plane_id))
>               return display->fbc[fbc_id];
> 
> -- 
> 2.51.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Reply via email to