| Message ID | 20240419140925.157924-1-vinod.govindapillai@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] drm/xe: avoid the async_flip update in the initial plane config | expand |
On Fri, 2024-04-19 at 17:09 +0300, Vinod Govindapillai wrote: > Async flip call is not needed. The updated fb mapping is updated > as part of the fixup_initial_plane_config() call. Otherwise we > end up updating the PLAN_SURF register twice with the same info. async_flip is writing PLANE_CTL as well. Is it ok to leave that out? BR, Jouni Högander > > v2: avoid async_flip instead of removing fixup call (Ville) > > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > --- > drivers/gpu/drm/xe/display/xe_plane_initial.c | 10 ---------- > 1 file changed, 10 deletions(-) > > diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c > b/drivers/gpu/drm/xe/display/xe_plane_initial.c > index 9693c56d386b..b5f8381b593d 100644 > --- a/drivers/gpu/drm/xe/display/xe_plane_initial.c > +++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c > @@ -189,8 +189,6 @@ intel_find_initial_plane_obj(struct intel_crtc > *crtc, > to_intel_plane(crtc->base.primary); > struct intel_plane_state *plane_state = > to_intel_plane_state(plane->base.state); > - struct intel_crtc_state *crtc_state = > - to_intel_crtc_state(crtc->base.state); > struct drm_framebuffer *fb; > struct i915_vma *vma; > > @@ -236,14 +234,6 @@ intel_find_initial_plane_obj(struct intel_crtc > *crtc, > atomic_or(plane->frontbuffer_bit, &to_intel_frontbuffer(fb)- > >bits); > > plane_config->vma = vma; > - > - /* > - * Flip to the newly created mapping ASAP, so we can re-use > the > - * first part of GGTT for WOPCM, prevent flickering, and > prevent > - * the lookup of sysmem scratch pages. > - */ > - plane->check_plane(crtc_state, plane_state); > - plane->async_flip(plane, crtc_state, plane_state, true); > return; > > nofb:
On Fri, 2024-06-14 at 07:11 +0000, Hogander, Jouni wrote: > On Fri, 2024-04-19 at 17:09 +0300, Vinod Govindapillai wrote: > > Async flip call is not needed. The updated fb mapping is updated > > as part of the fixup_initial_plane_config() call. Otherwise we > > end up updating the PLAN_SURF register twice with the same info. > > async_flip is writing PLANE_CTL as well. Is it ok to leave that out? Thanks for the review Jouni! Yes.. that is not needed for the initial plane config. The i915 counterpart don't have this either! I had confirmed this with Maarten and Ville. According to Maarten, most likely this part was copied from i915 which got dropped when refactoring. BR Vinod > > BR, > > Jouni Högander > > > > > v2: avoid async_flip instead of removing fixup call (Ville) > > > > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > > --- > > drivers/gpu/drm/xe/display/xe_plane_initial.c | 10 ---------- > > 1 file changed, 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c > > b/drivers/gpu/drm/xe/display/xe_plane_initial.c > > index 9693c56d386b..b5f8381b593d 100644 > > --- a/drivers/gpu/drm/xe/display/xe_plane_initial.c > > +++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c > > @@ -189,8 +189,6 @@ intel_find_initial_plane_obj(struct intel_crtc > > *crtc, > > to_intel_plane(crtc->base.primary); > > struct intel_plane_state *plane_state = > > to_intel_plane_state(plane->base.state); > > - struct intel_crtc_state *crtc_state = > > - to_intel_crtc_state(crtc->base.state); > > struct drm_framebuffer *fb; > > struct i915_vma *vma; > > > > @@ -236,14 +234,6 @@ intel_find_initial_plane_obj(struct intel_crtc > > *crtc, > > atomic_or(plane->frontbuffer_bit, &to_intel_frontbuffer(fb)- > > > bits); > > > > plane_config->vma = vma; > > - > > - /* > > - * Flip to the newly created mapping ASAP, so we can re-use > > the > > - * first part of GGTT for WOPCM, prevent flickering, and > > prevent > > - * the lookup of sysmem scratch pages. > > - */ > > - plane->check_plane(crtc_state, plane_state); > > - plane->async_flip(plane, crtc_state, plane_state, true); > > return; > > > > nofb: >
On Fri, 2024-06-14 at 08:23 +0000, Govindapillai, Vinod wrote: > On Fri, 2024-06-14 at 07:11 +0000, Hogander, Jouni wrote: > > On Fri, 2024-04-19 at 17:09 +0300, Vinod Govindapillai wrote: > > > Async flip call is not needed. The updated fb mapping is updated > > > as part of the fixup_initial_plane_config() call. Otherwise we > > > end up updating the PLAN_SURF register twice with the same info. > > > > async_flip is writing PLANE_CTL as well. Is it ok to leave that > > out? > > Thanks for the review Jouni! > > Yes.. that is not needed for the initial plane config. The i915 > counterpart don't have this either! > I had confirmed this with Maarten and Ville. According to Maarten, > most likely this part was copied > from i915 which got dropped when refactoring. Reviewed-by: Jouni Högander <jouni.hogander@intel.com> > > BR > Vinod > > > > > BR, > > > > Jouni Högander > > > > > > > > v2: avoid async_flip instead of removing fixup call (Ville) > > > > > > Signed-off-by: Vinod Govindapillai > > > <vinod.govindapillai@intel.com> > > > --- > > > drivers/gpu/drm/xe/display/xe_plane_initial.c | 10 ---------- > > > 1 file changed, 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c > > > b/drivers/gpu/drm/xe/display/xe_plane_initial.c > > > index 9693c56d386b..b5f8381b593d 100644 > > > --- a/drivers/gpu/drm/xe/display/xe_plane_initial.c > > > +++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c > > > @@ -189,8 +189,6 @@ intel_find_initial_plane_obj(struct > > > intel_crtc > > > *crtc, > > > to_intel_plane(crtc->base.primary); > > > struct intel_plane_state *plane_state = > > > to_intel_plane_state(plane->base.state); > > > - struct intel_crtc_state *crtc_state = > > > - to_intel_crtc_state(crtc->base.state); > > > struct drm_framebuffer *fb; > > > struct i915_vma *vma; > > > > > > @@ -236,14 +234,6 @@ intel_find_initial_plane_obj(struct > > > intel_crtc > > > *crtc, > > > atomic_or(plane->frontbuffer_bit, > > > &to_intel_frontbuffer(fb)- > > > > bits); > > > > > > plane_config->vma = vma; > > > - > > > - /* > > > - * Flip to the newly created mapping ASAP, so we can re- > > > use > > > the > > > - * first part of GGTT for WOPCM, prevent flickering, and > > > prevent > > > - * the lookup of sysmem scratch pages. > > > - */ > > > - plane->check_plane(crtc_state, plane_state); > > > - plane->async_flip(plane, crtc_state, plane_state, true); > > > return; > > > > > > nofb: > > >
diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c b/drivers/gpu/drm/xe/display/xe_plane_initial.c index 9693c56d386b..b5f8381b593d 100644 --- a/drivers/gpu/drm/xe/display/xe_plane_initial.c +++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c @@ -189,8 +189,6 @@ intel_find_initial_plane_obj(struct intel_crtc *crtc, to_intel_plane(crtc->base.primary); struct intel_plane_state *plane_state = to_intel_plane_state(plane->base.state); - struct intel_crtc_state *crtc_state = - to_intel_crtc_state(crtc->base.state); struct drm_framebuffer *fb; struct i915_vma *vma; @@ -236,14 +234,6 @@ intel_find_initial_plane_obj(struct intel_crtc *crtc, atomic_or(plane->frontbuffer_bit, &to_intel_frontbuffer(fb)->bits); plane_config->vma = vma; - - /* - * Flip to the newly created mapping ASAP, so we can re-use the - * first part of GGTT for WOPCM, prevent flickering, and prevent - * the lookup of sysmem scratch pages. - */ - plane->check_plane(crtc_state, plane_state); - plane->async_flip(plane, crtc_state, plane_state, true); return; nofb:
Async flip call is not needed. The updated fb mapping is updated as part of the fixup_initial_plane_config() call. Otherwise we end up updating the PLAN_SURF register twice with the same info. v2: avoid async_flip instead of removing fixup call (Ville) Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> --- drivers/gpu/drm/xe/display/xe_plane_initial.c | 10 ---------- 1 file changed, 10 deletions(-)