| Message ID | 20250225180905.1588084-3-chaitanya.kumar.borah@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/xe/display: Program double buffered LUT registers | expand |
On Tue, 25 Feb 2025, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> wrote: > From PTL, LUT registers are made double buffered. With this change, > we don't need to wait for vblank to program them. Start DSB1 for > programming them without waiting for vblank. > > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 919e236a9650..9c3fdfcd6759 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -7352,6 +7352,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, > { > struct intel_crtc_state *new_crtc_state = > intel_atomic_get_new_crtc_state(state, crtc); > + struct intel_display *display = to_intel_display(state); Please always put display local variable first. > > if (!new_crtc_state->use_dsb && !new_crtc_state->dsb_color_vblank) > return; > @@ -7408,7 +7409,8 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, > > if (new_crtc_state->dsb_color_vblank) > intel_dsb_chain(state, new_crtc_state->dsb_commit, > - new_crtc_state->dsb_color_vblank, true); > + new_crtc_state->dsb_color_vblank, > + HAS_DOUBLE_BUFFERED_LUT(display) ? false : true); HAS_DOUBLE_BUFFERED_LUT(display) ? false : true => !HAS_DOUBLE_BUFFERED_LUT(display) > > intel_dsb_finish(new_crtc_state->dsb_commit); > }
On Tue, Feb 25, 2025 at 11:39:05PM +0530, Chaitanya Kumar Borah wrote: > >From PTL, LUT registers are made double buffered. With this change, > we don't need to wait for vblank to program them. Start DSB1 for > programming them without waiting for vblank. > > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 919e236a9650..9c3fdfcd6759 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -7352,6 +7352,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, > { > struct intel_crtc_state *new_crtc_state = > intel_atomic_get_new_crtc_state(state, crtc); > + struct intel_display *display = to_intel_display(state); > > if (!new_crtc_state->use_dsb && !new_crtc_state->dsb_color_vblank) > return; > @@ -7408,7 +7409,8 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, > > if (new_crtc_state->dsb_color_vblank) > intel_dsb_chain(state, new_crtc_state->dsb_commit, > - new_crtc_state->dsb_color_vblank, true); > + new_crtc_state->dsb_color_vblank, > + HAS_DOUBLE_BUFFERED_LUT(display) ? false : true); Using chaining for this is a bit ugly. GOSUB would seem more appropriate. Here's a quick 1h effort to implement the basics for that (completely untested): https://github.com/vsyrjala/linux.git dsb_gosub > > intel_dsb_finish(new_crtc_state->dsb_commit); > } > -- > 2.25.1
Thank you, Jani, for the review. > -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Thursday, February 27, 2025 6:11 PM > To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel- > xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Cc: ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; > Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com> > Subject: Re: [PATCH 2/2] drm/i915/display: Don't wait for vblank for LUT DSB > programming > > On Tue, 25 Feb 2025, Chaitanya Kumar Borah > <chaitanya.kumar.borah@intel.com> wrote: > > From PTL, LUT registers are made double buffered. With this change, we > > don't need to wait for vblank to program them. Start DSB1 for > > programming them without waiting for vblank. > > > > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 919e236a9650..9c3fdfcd6759 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -7352,6 +7352,7 @@ static void intel_atomic_dsb_finish(struct > > intel_atomic_state *state, { > > struct intel_crtc_state *new_crtc_state = > > intel_atomic_get_new_crtc_state(state, crtc); > > + struct intel_display *display = to_intel_display(state); > > Please always put display local variable first. > Ack. What about the cases it is dependent on other local variables. Should we put it immediately after them? > > > > if (!new_crtc_state->use_dsb && !new_crtc_state->dsb_color_vblank) > > return; > > @@ -7408,7 +7409,8 @@ static void intel_atomic_dsb_finish(struct > > intel_atomic_state *state, > > > > if (new_crtc_state->dsb_color_vblank) > > intel_dsb_chain(state, new_crtc_state->dsb_commit, > > - new_crtc_state->dsb_color_vblank, true); > > + new_crtc_state->dsb_color_vblank, > > + HAS_DOUBLE_BUFFERED_LUT(display) ? false > : true); > > HAS_DOUBLE_BUFFERED_LUT(display) ? false : true > > => > > !HAS_DOUBLE_BUFFERED_LUT(display) > Ack. Regards Chaitanya > > > > > intel_dsb_finish(new_crtc_state->dsb_commit); > > } > > -- > Jani Nikula, Intel
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Friday, February 28, 2025 10:02 PM > To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Shankar, > Uma <uma.shankar@intel.com> > Subject: Re: [PATCH 2/2] drm/i915/display: Don't wait for vblank for LUT DSB > programming > > On Tue, Feb 25, 2025 at 11:39:05PM +0530, Chaitanya Kumar Borah wrote: > > >From PTL, LUT registers are made double buffered. With this change, > > we don't need to wait for vblank to program them. Start DSB1 for > > programming them without waiting for vblank. > > > > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 919e236a9650..9c3fdfcd6759 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -7352,6 +7352,7 @@ static void intel_atomic_dsb_finish(struct > > intel_atomic_state *state, { > > struct intel_crtc_state *new_crtc_state = > > intel_atomic_get_new_crtc_state(state, crtc); > > + struct intel_display *display = to_intel_display(state); > > > > if (!new_crtc_state->use_dsb && !new_crtc_state->dsb_color_vblank) > > return; > > @@ -7408,7 +7409,8 @@ static void intel_atomic_dsb_finish(struct > > intel_atomic_state *state, > > > > if (new_crtc_state->dsb_color_vblank) > > intel_dsb_chain(state, new_crtc_state->dsb_commit, > > - new_crtc_state->dsb_color_vblank, true); > > + new_crtc_state->dsb_color_vblank, > > + HAS_DOUBLE_BUFFERED_LUT(display) ? false > : true); > > Using chaining for this is a bit ugly. GOSUB would seem more appropriate. > Here's a quick 1h effort to implement the basics for that (completely > untested): > https://github.com/vsyrjala/linux.git dsb_gosub > Thank you, Ville, for sharing this, I will give it a try. Regards Chaitanya > > > > intel_dsb_finish(new_crtc_state->dsb_commit); > > } > > -- > > 2.25.1 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 919e236a9650..9c3fdfcd6759 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7352,6 +7352,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, { struct intel_crtc_state *new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc); + struct intel_display *display = to_intel_display(state); if (!new_crtc_state->use_dsb && !new_crtc_state->dsb_color_vblank) return; @@ -7408,7 +7409,8 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, if (new_crtc_state->dsb_color_vblank) intel_dsb_chain(state, new_crtc_state->dsb_commit, - new_crtc_state->dsb_color_vblank, true); + new_crtc_state->dsb_color_vblank, + HAS_DOUBLE_BUFFERED_LUT(display) ? false : true); intel_dsb_finish(new_crtc_state->dsb_commit); }
From PTL, LUT registers are made double buffered. With this change, we don't need to wait for vblank to program them. Start DSB1 for programming them without waiting for vblank. Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)