| Message ID | 1465855396-23851-1-git-send-email-radhakrishna.sripada@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I believe we should use whatever BSpec recommends. If that is not the best behavior and block things out than the spec needs to be updated or a workaround documented there. Art? thoughts? On Mon, Jun 13, 2016 at 3:03 PM, Radhakrishna Sripada <radhakrishna.sripada@intel.com> wrote: > The bspec suggests giving cursor planes a fixed allocation of 8 > blocks when running in a multi-CRTC configuration. However we > have found that this small allocation can only accommodate level > 0 watermarks on many platforms, which in turn prevents the > system from entering deeper sleep states. Let's use a slightly > higher allocation of 16 blocks for the cursor to increase our > chances of enabling lower power states. > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 658a756..a949dac 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2933,7 +2933,8 @@ static unsigned int skl_cursor_allocation(int num_active) > if (num_active == 1) > return 32; > > - return 8; > + /* higher than bspec recommendation (8) */ > + return 16; > } > > static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry, u32 reg) > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Patchwork [mailto:patchwork@emeril.freedesktop.org] > Sent: Monday, June 13, 2016 10:54 PM > To: Sripada, Radhakrishna > Cc: intel-gfx@lists.freedesktop.org > Subject: ✗ Ro.CI.BAT: failure for drm/i915/skl: Increase cursor ddb blocks in > multi-pipe config > > == Series Details == > > Series: drm/i915/skl: Increase cursor ddb blocks in multi-pipe config > URL : https://patchwork.freedesktop.org/series/8656/ > State : failure > > == Summary == > > Series 8656v1 drm/i915/skl: Increase cursor ddb blocks in multi-pipe config > http://patchwork.freedesktop.org/api/1.0/series/8656/revisions/1/mbox > > Test kms_pipe_crc_basic: > Subgroup nonblocking-crc-pipe-a-frame-sequence: > pass -> FAIL (ro-bdw-i7-5557U) The code change is specific to SKL. This is probably a false positive. > > fi-bdw-i7-5557u total:213 pass:201 dwarn:0 dfail:0 fail:0 skip:12 > fi-skl-i7-6700k total:213 pass:188 dwarn:0 dfail:0 fail:0 skip:25 > fi-snb-i7-2600 total:213 pass:174 dwarn:0 dfail:0 fail:0 skip:39 > ro-bdw-i5-5250u total:213 pass:197 dwarn:2 dfail:0 fail:0 skip:14 > ro-bdw-i7-5557U total:213 pass:197 dwarn:0 dfail:0 fail:1 skip:15 > ro-bdw-i7-5600u total:213 pass:185 dwarn:0 dfail:0 fail:0 skip:28 > ro-bsw-n3050 total:213 pass:172 dwarn:0 dfail:0 fail:2 skip:39 > ro-byt-n2820 total:213 pass:173 dwarn:0 dfail:0 fail:3 skip:37 > ro-hsw-i3-4010u total:213 pass:190 dwarn:0 dfail:0 fail:0 skip:23 > ro-hsw-i7-4770r total:213 pass:190 dwarn:0 dfail:0 fail:0 skip:23 > ro-ilk-i7-620lm total:213 pass:150 dwarn:0 dfail:0 fail:1 skip:62 > ro-ilk1-i5-650 total:208 pass:150 dwarn:0 dfail:0 fail:1 skip:57 > ro-ivb-i7-3770 total:213 pass:181 dwarn:0 dfail:0 fail:0 skip:32 > ro-ivb2-i7-3770 total:213 pass:185 dwarn:0 dfail:0 fail:0 skip:28 > ro-skl3-i5-6260u total:213 pass:201 dwarn:1 dfail:0 fail:0 skip:11 > ro-snb-i7-2620M total:213 pass:174 dwarn:0 dfail:0 fail:1 skip:38 > fi-hsw-i7-4770k failed to connect after reboot > > Results at /archive/results/CI_IGT_test/RO_Patchwork_1179/ > > b7936d4 drm-intel-nightly: 2016y-06m-13d-16h-41m-03s UTC integration > manifest e4d9faf drm/i915/skl: Increase cursor ddb blocks in multi-pipe config -- Radhakrishna Sripada
The bspec says "These are basic methods that can be used for single and multi-pipe modes. For optimal power usage, the display driver can choose to use more advanced allocation techniques as desired." So we leave it up to the driver to optimize as it sees fit. -----Original Message----- From: Rodrigo Vivi [mailto:rodrigo.vivi@gmail.com] Sent: Thursday, June 16, 2016 4:20 PM To: Sripada, Radhakrishna Cc: intel-gfx; drm-intel-fixes@lists.freedesktop.org; Runyan, Arthur J Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb blocks in multi-pipe config I believe we should use whatever BSpec recommends. If that is not the best behavior and block things out than the spec needs to be updated or a workaround documented there. Art? thoughts? On Mon, Jun 13, 2016 at 3:03 PM, Radhakrishna Sripada <radhakrishna.sripada@intel.com> wrote: > The bspec suggests giving cursor planes a fixed allocation of 8 blocks > when running in a multi-CRTC configuration. However we have found > that this small allocation can only accommodate level > 0 watermarks on many platforms, which in turn prevents the system from > entering deeper sleep states. Let's use a slightly higher allocation > of 16 blocks for the cursor to increase our chances of enabling lower > power states. > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c index 658a756..a949dac 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2933,7 +2933,8 @@ static unsigned int skl_cursor_allocation(int num_active) > if (num_active == 1) > return 32; > > - return 8; > + /* higher than bspec recommendation (8) */ > + return 16; > } > > static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry, > u32 reg) > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br
Thanks Art. I believe the commit message should be updated to reflect this is flexible. Probably coping and pasting this part of spec: "More allocation might be required to support deeper low power states." So I went now to the spec to review the code and besides the line above I also notice for this specific case BSpec actually recommend 8 * num_active. "For each enabled cursor CursorBufAlloc = 8" "BlocksAvailable = TotalBlocksAvailable - (8 * NumPipes)." What I believe in this code it should be return 8 * num_active instead of a fixed number of 8 or 16. Right? Thanks, Rodrigo. On Thu, Jun 23, 2016 at 12:20 PM, Runyan, Arthur J <arthur.j.runyan@intel.com> wrote: > The bspec says "These are basic methods that can be used for single and multi-pipe modes. For optimal power usage, the display driver can choose to use more advanced allocation techniques as desired." > So we leave it up to the driver to optimize as it sees fit. > > -----Original Message----- > From: Rodrigo Vivi [mailto:rodrigo.vivi@gmail.com] > Sent: Thursday, June 16, 2016 4:20 PM > To: Sripada, Radhakrishna > Cc: intel-gfx; drm-intel-fixes@lists.freedesktop.org; Runyan, Arthur J > Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb blocks in multi-pipe config > > I believe we should use whatever BSpec recommends. > > If that is not the best behavior and block things out than the spec needs to be updated or a workaround documented there. > > Art? thoughts? > > On Mon, Jun 13, 2016 at 3:03 PM, Radhakrishna Sripada <radhakrishna.sripada@intel.com> wrote: >> The bspec suggests giving cursor planes a fixed allocation of 8 blocks >> when running in a multi-CRTC configuration. However we have found >> that this small allocation can only accommodate level >> 0 watermarks on many platforms, which in turn prevents the system from >> entering deeper sleep states. Let's use a slightly higher allocation >> of 16 blocks for the cursor to increase our chances of enabling lower >> power states. >> >> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> >> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> >> --- >> drivers/gpu/drm/i915/intel_pm.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c index 658a756..a949dac 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -2933,7 +2933,8 @@ static unsigned int skl_cursor_allocation(int num_active) >> if (num_active == 1) >> return 32; >> >> - return 8; >> + /* higher than bspec recommendation (8) */ >> + return 16; >> } >> >> static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry, >> u32 reg) >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br
That part is trying to just allocate 8 to each cursor. The buffer used up will be 8*numpipes, but that's because its assuming you can end up enabling a cursor on each pipe. I think its good to go up to 16. The kind of latencies we get on skl mean that a 64x64 32bpp cursor with 8 blocks will be restricting package C states even at 1920x1080 60hz. The 8 number was based on what hardware did for allocation on past projects. -----Original Message----- From: Rodrigo Vivi [mailto:rodrigo.vivi@gmail.com] Sent: Thursday, June 23, 2016 12:50 PM To: Runyan, Arthur J Cc: Sripada, Radhakrishna; intel-gfx; drm-intel-fixes@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb blocks in multi-pipe config Thanks Art. I believe the commit message should be updated to reflect this is flexible. Probably coping and pasting this part of spec: "More allocation might be required to support deeper low power states." So I went now to the spec to review the code and besides the line above I also notice for this specific case BSpec actually recommend 8 * num_active. "For each enabled cursor CursorBufAlloc = 8" "BlocksAvailable = TotalBlocksAvailable - (8 * NumPipes)." What I believe in this code it should be return 8 * num_active instead of a fixed number of 8 or 16. Right? Thanks, Rodrigo. On Thu, Jun 23, 2016 at 12:20 PM, Runyan, Arthur J <arthur.j.runyan@intel.com> wrote: > The bspec says "These are basic methods that can be used for single and multi-pipe modes. For optimal power usage, the display driver can choose to use more advanced allocation techniques as desired." > So we leave it up to the driver to optimize as it sees fit. > > -----Original Message----- > From: Rodrigo Vivi [mailto:rodrigo.vivi@gmail.com] > Sent: Thursday, June 16, 2016 4:20 PM > To: Sripada, Radhakrishna > Cc: intel-gfx; drm-intel-fixes@lists.freedesktop.org; Runyan, Arthur J > Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb > blocks in multi-pipe config > > I believe we should use whatever BSpec recommends. > > If that is not the best behavior and block things out than the spec needs to be updated or a workaround documented there. > > Art? thoughts? > > On Mon, Jun 13, 2016 at 3:03 PM, Radhakrishna Sripada <radhakrishna.sripada@intel.com> wrote: >> The bspec suggests giving cursor planes a fixed allocation of 8 >> blocks when running in a multi-CRTC configuration. However we have >> found that this small allocation can only accommodate level >> 0 watermarks on many platforms, which in turn prevents the system >> from entering deeper sleep states. Let's use a slightly higher >> allocation of 16 blocks for the cursor to increase our chances of >> enabling lower power states. >> >> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> >> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> >> --- >> drivers/gpu/drm/i915/intel_pm.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c index 658a756..a949dac 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -2933,7 +2933,8 @@ static unsigned int skl_cursor_allocation(int num_active) >> if (num_active == 1) >> return 32; >> >> - return 8; >> + /* higher than bspec recommendation (8) */ >> + return 16; >> } >> >> static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry, >> u32 reg) >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br -- Rodrigo Vivi Blog: http://blog.vivi.eng.br
Thanks for the input Rodrigo, Arthur. I will update commit message and repost the patch. Thanks, Radhakrishna Sripada -----Original Message----- From: Runyan, Arthur J Sent: Thursday, June 23, 2016 1:04 PM To: Rodrigo Vivi <rodrigo.vivi@gmail.com> Cc: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-gfx <intel-gfx@lists.freedesktop.org>; drm-intel-fixes@lists.freedesktop.org Subject: RE: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb blocks in multi-pipe config That part is trying to just allocate 8 to each cursor. The buffer used up will be 8*numpipes, but that's because its assuming you can end up enabling a cursor on each pipe. I think its good to go up to 16. The kind of latencies we get on skl mean that a 64x64 32bpp cursor with 8 blocks will be restricting package C states even at 1920x1080 60hz. The 8 number was based on what hardware did for allocation on past projects. -----Original Message----- From: Rodrigo Vivi [mailto:rodrigo.vivi@gmail.com] Sent: Thursday, June 23, 2016 12:50 PM To: Runyan, Arthur J Cc: Sripada, Radhakrishna; intel-gfx; drm-intel-fixes@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb blocks in multi-pipe config Thanks Art. I believe the commit message should be updated to reflect this is flexible. Probably coping and pasting this part of spec: "More allocation might be required to support deeper low power states." So I went now to the spec to review the code and besides the line above I also notice for this specific case BSpec actually recommend 8 * num_active. "For each enabled cursor CursorBufAlloc = 8" "BlocksAvailable = TotalBlocksAvailable - (8 * NumPipes)." What I believe in this code it should be return 8 * num_active instead of a fixed number of 8 or 16. Right? Thanks, Rodrigo. On Thu, Jun 23, 2016 at 12:20 PM, Runyan, Arthur J <arthur.j.runyan@intel.com> wrote: > The bspec says "These are basic methods that can be used for single and multi-pipe modes. For optimal power usage, the display driver can choose to use more advanced allocation techniques as desired." > So we leave it up to the driver to optimize as it sees fit. > > -----Original Message----- > From: Rodrigo Vivi [mailto:rodrigo.vivi@gmail.com] > Sent: Thursday, June 16, 2016 4:20 PM > To: Sripada, Radhakrishna > Cc: intel-gfx; drm-intel-fixes@lists.freedesktop.org; Runyan, Arthur J > Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Increase cursor ddb > blocks in multi-pipe config > > I believe we should use whatever BSpec recommends. > > If that is not the best behavior and block things out than the spec needs to be updated or a workaround documented there. > > Art? thoughts? > > On Mon, Jun 13, 2016 at 3:03 PM, Radhakrishna Sripada <radhakrishna.sripada@intel.com> wrote: >> The bspec suggests giving cursor planes a fixed allocation of 8 >> blocks when running in a multi-CRTC configuration. However we have >> found that this small allocation can only accommodate level >> 0 watermarks on many platforms, which in turn prevents the system >> from entering deeper sleep states. Let's use a slightly higher >> allocation of 16 blocks for the cursor to increase our chances of >> enabling lower power states. >> >> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> >> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> >> --- >> drivers/gpu/drm/i915/intel_pm.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c index 658a756..a949dac 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -2933,7 +2933,8 @@ static unsigned int skl_cursor_allocation(int num_active) >> if (num_active == 1) >> return 32; >> >> - return 8; >> + /* higher than bspec recommendation (8) */ >> + return 16; >> } >> >> static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry, >> u32 reg) >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br -- Rodrigo Vivi Blog: http://blog.vivi.eng.br
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 658a756..a949dac 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2933,7 +2933,8 @@ static unsigned int skl_cursor_allocation(int num_active) if (num_active == 1) return 32; - return 8; + /* higher than bspec recommendation (8) */ + return 16; } static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry, u32 reg)