| Message ID | 20200306113927.16904-3-karthik.b.s@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Asynchronous flip implementation for i915 | expand |
Em sex, 2020-03-06 às 17:09 +0530, Karthik B S escreveu: > Enable support for async flips in I915. > Set the Async Address Update Enable bit in plane ctl > when async flip is requested. > > Signed-off-by: Karthik B S <karthik.b.s@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 4 ++++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > 2 files changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index dd47eb65b563..4ce9897f5c58 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -4756,6 +4756,9 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state, > plane_ctl |= PLANE_CTL_YUV_RANGE_CORRECTION_DISABLE; > } > > + if (crtc_state->uapi.async_flip) > + plane_ctl |= PLANE_CTL_ASYNC_FLIP; > + > plane_ctl |= skl_plane_ctl_format(fb->format->format); > plane_ctl |= skl_plane_ctl_tiling(fb->modifier); > plane_ctl |= skl_plane_ctl_rotate(rotation & DRM_MODE_ROTATE_MASK); > @@ -17738,6 +17741,7 @@ static void intel_mode_config_init(struct drm_i915_private *i915) > > mode_config->funcs = &intel_mode_funcs; > > + mode_config->async_page_flip = true; We should only enable the feature to user space after it has been fully implemented inside the Kernel. Think about the case where git-bisect decides to land at exactly this commit when someone is debugging a failure unrelated to async vblanks. Also, when features have trivial on/off switches like the line above, it's better if the patch that enables the feature only contains the line that toggles the on/off switch. This way, if a revert is needed, we can just switch it to off without removing more code. Also, it enables us to land the rest of the code while keeping the feature off for stabilization. Also, the line above is enabling the feature for every platform, which is probably not a goal of this series. > /* > * Maximum framebuffer dimensions, chosen to match > * the maximum render engine surface size on gen4+. > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 80cf02a6eec1..42037aee9b78 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6794,6 +6794,7 @@ enum { > #define PLANE_CTL_TILED_X (1 << 10) > #define PLANE_CTL_TILED_Y (4 << 10) > #define PLANE_CTL_TILED_YF (5 << 10) > +#define PLANE_CTL_ASYNC_FLIP (1 << 9) > #define PLANE_CTL_FLIP_HORIZONTAL (1 << 8) > #define PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE (1 << 4) /* TGL+ */ > #define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
> -----Original Message----- > From: Zanoni, Paulo R <paulo.r.zanoni@intel.com> > Sent: Tuesday, March 10, 2020 4:48 AM > To: B S, Karthik <karthik.b.s@intel.com>; intel-gfx@lists.freedesktop.org > Cc: ville.syrjala@linux.intel.com; Kulkarni, Vandita > <vandita.kulkarni@intel.com>; Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [RFC 2/7] drm/i915: Add support for async flips in I915 > > Em sex, 2020-03-06 às 17:09 +0530, Karthik B S escreveu: > > Enable support for async flips in I915. > > Set the Async Address Update Enable bit in plane ctl when async flip > > is requested. > > > > Signed-off-by: Karthik B S <karthik.b.s@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 4 ++++ > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > 2 files changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index dd47eb65b563..4ce9897f5c58 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -4756,6 +4756,9 @@ u32 skl_plane_ctl(const struct intel_crtc_state > *crtc_state, > > plane_ctl |= > PLANE_CTL_YUV_RANGE_CORRECTION_DISABLE; > > } > > > > + if (crtc_state->uapi.async_flip) > > + plane_ctl |= PLANE_CTL_ASYNC_FLIP; > > + > > plane_ctl |= skl_plane_ctl_format(fb->format->format); > > plane_ctl |= skl_plane_ctl_tiling(fb->modifier); > > plane_ctl |= skl_plane_ctl_rotate(rotation & > DRM_MODE_ROTATE_MASK); > > @@ -17738,6 +17741,7 @@ static void intel_mode_config_init(struct > > drm_i915_private *i915) > > > > mode_config->funcs = &intel_mode_funcs; > > > > + mode_config->async_page_flip = true; > > We should only enable the feature to user space after it has been fully > implemented inside the Kernel. Think about the case where git-bisect > decides to land at exactly this commit when someone is debugging a failure > unrelated to async vblanks. > > Also, when features have trivial on/off switches like the line above, it's > better if the patch that enables the feature only contains the line that toggles > the on/off switch. This way, if a revert is needed, we can just switch it to off > without removing more code. Also, it enables us to land the rest of the code > while keeping the feature off for stabilization. > > Also, the line above is enabling the feature for every platform, which is > probably not a goal of this series. Agreed. Will make the on/off part a separate patch and also add a gen check for it. > > > > /* > > * Maximum framebuffer dimensions, chosen to match > > * the maximum render engine surface size on gen4+. > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index 80cf02a6eec1..42037aee9b78 > > 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -6794,6 +6794,7 @@ enum { > > #define PLANE_CTL_TILED_X (1 << 10) > > #define PLANE_CTL_TILED_Y (4 << 10) > > #define PLANE_CTL_TILED_YF (5 << 10) > > +#define PLANE_CTL_ASYNC_FLIP (1 << 9) > > #define PLANE_CTL_FLIP_HORIZONTAL (1 << 8) > > #define PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE (1 << 4) /* > TGL+ */ > > #define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK > */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index dd47eb65b563..4ce9897f5c58 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -4756,6 +4756,9 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state, plane_ctl |= PLANE_CTL_YUV_RANGE_CORRECTION_DISABLE; } + if (crtc_state->uapi.async_flip) + plane_ctl |= PLANE_CTL_ASYNC_FLIP; + plane_ctl |= skl_plane_ctl_format(fb->format->format); plane_ctl |= skl_plane_ctl_tiling(fb->modifier); plane_ctl |= skl_plane_ctl_rotate(rotation & DRM_MODE_ROTATE_MASK); @@ -17738,6 +17741,7 @@ static void intel_mode_config_init(struct drm_i915_private *i915) mode_config->funcs = &intel_mode_funcs; + mode_config->async_page_flip = true; /* * Maximum framebuffer dimensions, chosen to match * the maximum render engine surface size on gen4+. diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 80cf02a6eec1..42037aee9b78 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6794,6 +6794,7 @@ enum { #define PLANE_CTL_TILED_X (1 << 10) #define PLANE_CTL_TILED_Y (4 << 10) #define PLANE_CTL_TILED_YF (5 << 10) +#define PLANE_CTL_ASYNC_FLIP (1 << 9) #define PLANE_CTL_FLIP_HORIZONTAL (1 << 8) #define PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE (1 << 4) /* TGL+ */ #define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
Enable support for async flips in I915. Set the Async Address Update Enable bit in plane ctl when async flip is requested. Signed-off-by: Karthik B S <karthik.b.s@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 4 ++++ drivers/gpu/drm/i915/i915_reg.h | 1 + 2 files changed, 5 insertions(+)