| Message ID | 1415226371-1880-6-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=348/348->348/348
PNV: pass/total=324/328->324/328
ILK: pass/total=330/330->330/330
IVB: pass/total=488/497->488/497
SNB: pass/total=515/525->516/525
HSW: pass/total=497/505->497/505
BDW: pass/total=430/435->429/435
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
PNV: Intel_gpu_tools, igt_drm_vma_limiter_cached, PASS(4, M23M25) -> DMESG_WARN(1, M25)PASS(3, M25)
PNV: Intel_gpu_tools, igt_gem_linear_blits_normal, CRASH(1, M23)DMESG_WARN(1, M24)PASS(8, M24M25) -> PASS(4, M25)
PNV: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, DMESG_WARN(1, M23)TIMEOUT(3, M25) -> DMESG_WARN(3, M25)TIMEOUT(1, M25)
IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-sliding, DMESG_WARN(1, M4)PASS(9, M4M34) -> TIMEOUT(1, M34)PASS(3, M34)
IVB: Intel_gpu_tools, igt_kms_pipe_crc_basic_hang-read-crc-pipe-B, PASS(4, M4M34) -> TIMEOUT(1, M34)PASS(3, M34)
SNB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-offscreen, DMESG_WARN(1, M35)PASS(6, M35) -> PASS(4, M35)
SNB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-sliding, DMESG_WARN(1, M35)PASS(9, M35M22) -> PASS(4, M35)
SNB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(3, M35)PASS(1, M35) -> TIMEOUT(1, M35)PASS(3, M35)
BDW: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(3, M30)PASS(1, M42) -> TIMEOUT(1, M30)PASS(3, M30)
On 11/06/2014 12:26 AM, Jesse Barnes wrote: > This should allow us to avoid mode sets for some panel fitter config > changes. > > v2: > - fixup pfit comment (Ander) > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/intel_display.c | 61 +++++++++++++++++++++++++++++------- > 1 file changed, 50 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3f1515d..49281d7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2835,17 +2835,8 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) > return; > > /* > - * Update pipe size and adjust fitter if needed: the reason for this is > - * that in compute_mode_changes we check the native mode (not the pfit > - * mode) to see if we can flip rather than do a full mode set. In the > - * fastboot case, we'll flip, but if we don't update the pipesrc and > - * pfit state, we'll end up with a big fb scanned out into the wrong > - * sized surface. > - * > - * To fix this properly, we need to hoist the checks up into > - * compute_mode_changes (or above), check the actual pfit state and > - * whether the platform allows pfit disable with pipe active, and only > - * then update the pipesrc and pfit state, even on the flip path. > + * See intel_pfit_changed for info on when we're allowed to > + * do this w/o a pipe shutdown. > */ > > adjusted_mode = &crtc->config.adjusted_mode; > @@ -11171,6 +11162,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc) > crtc->new_config = NULL; > } > > +/* Do we need a mode set due to pfit changes? */ > +static bool intel_pfit_changed(struct drm_device *dev, > + struct intel_crtc_config *new_config, > + struct intel_crtc_config *cur_config) > +{ > + bool ret = false; > + > + if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) { > + /* > + * On PCH platforms we can disable pfit w/o a pipe shutdown, > + * otherwise we'll need a mode set. > + */ > + if (new_config->pch_pfit.enabled && > + cur_config->pch_pfit.enabled) > + ret = false; > + else if (new_config->pch_pfit.enabled && > + !cur_config->pch_pfit.enabled) > + ret = true; > + else if (!new_config->pch_pfit.enabled && > + cur_config->pch_pfit.enabled) > + ret = false; > + else if (!new_config->pch_pfit.enabled && > + !cur_config->pch_pfit.enabled) > + ret = false; > + } else { > + bool new_enabled, old_enabled; > + > + new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE); > + old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE); > + > + /* 9xx only needs a shutdown to disable pfit */ > + if (new_enabled && old_enabled) > + ret = false; > + else if (new_enabled && !old_enabled) > + ret = false; > + else if (!new_enabled && old_enabled) > + ret = true; > + else if (!new_enabled && !old_enabled) > + ret = false; > + } Maybe I missed something, but I couldn't find anything in the documentation the supports the claim above. However, [1] and [2] read that "[p]anel fitting should be enabled or disabled before the pipe is enabled" on the documentation for the PFIT_CONTROL. [1] https://01.org/linuxgraphics/sites/default/files/documentation/g45_vol_3_register_0_0.pdf [2] https://01.org/linuxgraphics/sites/default/files/documentation/965_g35_vol_3_display_registers_updated_1.pdf Ander > + > + return ret; > +} > + > static int intel_crtc_set_config(struct drm_mode_set *set) > { > struct drm_device *dev; > @@ -11239,6 +11274,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set) > if (to_intel_crtc(set->crtc)->new_config->has_infoframe || > to_intel_crtc(set->crtc)->config.has_infoframe) > config->mode_changed = true; > + > + if (intel_pfit_changed(dev, to_intel_crtc(set->crtc)->new_config, > + &to_intel_crtc(set->crtc)->config)) > + config->mode_changed = true; > } > > /* set_mode will free it in the mode_changed case */ >
On Mon, 10 Nov 2014 18:20:56 +0200 Ander Conselvan de Oliveira <conselvan2@gmail.com> wrote: > On 11/06/2014 12:26 AM, Jesse Barnes wrote: > > This should allow us to avoid mode sets for some panel fitter config > > changes. > > > > v2: > > - fixup pfit comment (Ander) > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/intel_display.c | 61 +++++++++++++++++++++++++++++------- > > 1 file changed, 50 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 3f1515d..49281d7 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2835,17 +2835,8 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) > > return; > > > > /* > > - * Update pipe size and adjust fitter if needed: the reason for this is > > - * that in compute_mode_changes we check the native mode (not the pfit > > - * mode) to see if we can flip rather than do a full mode set. In the > > - * fastboot case, we'll flip, but if we don't update the pipesrc and > > - * pfit state, we'll end up with a big fb scanned out into the wrong > > - * sized surface. > > - * > > - * To fix this properly, we need to hoist the checks up into > > - * compute_mode_changes (or above), check the actual pfit state and > > - * whether the platform allows pfit disable with pipe active, and only > > - * then update the pipesrc and pfit state, even on the flip path. > > + * See intel_pfit_changed for info on when we're allowed to > > + * do this w/o a pipe shutdown. > > */ > > > > adjusted_mode = &crtc->config.adjusted_mode; > > @@ -11171,6 +11162,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc) > > crtc->new_config = NULL; > > } > > > > +/* Do we need a mode set due to pfit changes? */ > > +static bool intel_pfit_changed(struct drm_device *dev, > > + struct intel_crtc_config *new_config, > > + struct intel_crtc_config *cur_config) > > +{ > > + bool ret = false; > > + > > + if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) { > > + /* > > + * On PCH platforms we can disable pfit w/o a pipe shutdown, > > + * otherwise we'll need a mode set. > > + */ > > + if (new_config->pch_pfit.enabled && > > + cur_config->pch_pfit.enabled) > > + ret = false; > > + else if (new_config->pch_pfit.enabled && > > + !cur_config->pch_pfit.enabled) > > + ret = true; > > + else if (!new_config->pch_pfit.enabled && > > + cur_config->pch_pfit.enabled) > > + ret = false; > > + else if (!new_config->pch_pfit.enabled && > > + !cur_config->pch_pfit.enabled) > > + ret = false; > > + } else { > > + bool new_enabled, old_enabled; > > + > > + new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE); > > + old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE); > > + > > + /* 9xx only needs a shutdown to disable pfit */ > > + if (new_enabled && old_enabled) > > + ret = false; > > + else if (new_enabled && !old_enabled) > > + ret = false; > > + else if (!new_enabled && old_enabled) > > + ret = true; > > + else if (!new_enabled && !old_enabled) > > + ret = false; > > + } > > Maybe I missed something, but I couldn't find anything in the > documentation the supports the claim above. However, [1] and [2] read > that "[p]anel fitting should be enabled or disabled before the pipe is > enabled" on the documentation for the PFIT_CONTROL. > > > [1] > https://01.org/linuxgraphics/sites/default/files/documentation/g45_vol_3_register_0_0.pdf > [2] > https://01.org/linuxgraphics/sites/default/files/documentation/965_g35_vol_3_display_registers_updated_1.pdf Yeah the docs are extra conservative on this. But from memory, this is what 965 allowed. It would be good to do some extra testing though; 915/945 may allow both pfit enable and disable without a pipe shutdown.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3f1515d..49281d7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2835,17 +2835,8 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) return; /* - * Update pipe size and adjust fitter if needed: the reason for this is - * that in compute_mode_changes we check the native mode (not the pfit - * mode) to see if we can flip rather than do a full mode set. In the - * fastboot case, we'll flip, but if we don't update the pipesrc and - * pfit state, we'll end up with a big fb scanned out into the wrong - * sized surface. - * - * To fix this properly, we need to hoist the checks up into - * compute_mode_changes (or above), check the actual pfit state and - * whether the platform allows pfit disable with pipe active, and only - * then update the pipesrc and pfit state, even on the flip path. + * See intel_pfit_changed for info on when we're allowed to + * do this w/o a pipe shutdown. */ adjusted_mode = &crtc->config.adjusted_mode; @@ -11171,6 +11162,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc) crtc->new_config = NULL; } +/* Do we need a mode set due to pfit changes? */ +static bool intel_pfit_changed(struct drm_device *dev, + struct intel_crtc_config *new_config, + struct intel_crtc_config *cur_config) +{ + bool ret = false; + + if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) { + /* + * On PCH platforms we can disable pfit w/o a pipe shutdown, + * otherwise we'll need a mode set. + */ + if (new_config->pch_pfit.enabled && + cur_config->pch_pfit.enabled) + ret = false; + else if (new_config->pch_pfit.enabled && + !cur_config->pch_pfit.enabled) + ret = true; + else if (!new_config->pch_pfit.enabled && + cur_config->pch_pfit.enabled) + ret = false; + else if (!new_config->pch_pfit.enabled && + !cur_config->pch_pfit.enabled) + ret = false; + } else { + bool new_enabled, old_enabled; + + new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE); + old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE); + + /* 9xx only needs a shutdown to disable pfit */ + if (new_enabled && old_enabled) + ret = false; + else if (new_enabled && !old_enabled) + ret = false; + else if (!new_enabled && old_enabled) + ret = true; + else if (!new_enabled && !old_enabled) + ret = false; + } + + return ret; +} + static int intel_crtc_set_config(struct drm_mode_set *set) { struct drm_device *dev; @@ -11239,6 +11274,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set) if (to_intel_crtc(set->crtc)->new_config->has_infoframe || to_intel_crtc(set->crtc)->config.has_infoframe) config->mode_changed = true; + + if (intel_pfit_changed(dev, to_intel_crtc(set->crtc)->new_config, + &to_intel_crtc(set->crtc)->config)) + config->mode_changed = true; } /* set_mode will free it in the mode_changed case */
This should allow us to avoid mode sets for some panel fitter config changes. v2: - fixup pfit comment (Ander) Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/intel_display.c | 61 +++++++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 11 deletions(-)