| Message ID | 1401992671-2548-5-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 05 Jun 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > Let them eat mincemeat pie. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > index d05a2af..081ab2f 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { > .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > .disable_power_well = 1, > .enable_ips = 1, > - .fastboot = 0, > + .fastboot = 42, The answer to the ultimate question of life, the universe, and everything should simply be "true" here. Personally, I don't think it's a bad answer. > .prefault_disable = 0, > .reset = true, > .invert_brightness = 0, > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); > > module_param_named(fastboot, i915.fastboot, bool, 0600); Side note, why do we allow the param to be changed after boot? BR, Jani. > MODULE_PARM_DESC(fastboot, > - "Try to skip unnecessary mode sets at boot time (default: false)"); > + "Try to skip unnecessary mode sets at boot time (default: true)"); > > module_param_named(prefault_disable, i915.prefault_disable, bool, 0600); > MODULE_PARM_DESC(prefault_disable, > -- > 1.8.3.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jun 05, 2014 at 11:24:31AM -0700, Jesse Barnes wrote: > Let them eat mincemeat pie. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > index d05a2af..081ab2f 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { > .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > .disable_power_well = 1, > .enable_ips = 1, > - .fastboot = 0, > + .fastboot = 42, > .prefault_disable = 0, > .reset = true, > .invert_brightness = 0, > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); > > module_param_named(fastboot, i915.fastboot, bool, 0600); > MODULE_PARM_DESC(fastboot, > - "Try to skip unnecessary mode sets at boot time (default: false)"); > + "Try to skip unnecessary mode sets at boot time (default: true)"); Nah, that wasn't the intention of this option. It was meant as a hack to experiment around with fastboot and get things going, but imo we need to really do the full modeset and short-circuit if the state matches. And there's still a bunch of things we don't track like infoframes which we either need to fix up (similar to the pfit fixup) or quirk to disallow fastboot. -Daniel > > module_param_named(prefault_disable, i915.prefault_disable, bool, 0600); > MODULE_PARM_DESC(prefault_disable, > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Jun 06, 2014 at 02:12:44PM +0300, Jani Nikula wrote: > On Thu, 05 Jun 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > Let them eat mincemeat pie. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > index d05a2af..081ab2f 100644 > > --- a/drivers/gpu/drm/i915/i915_params.c > > +++ b/drivers/gpu/drm/i915/i915_params.c > > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { > > .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > > .disable_power_well = 1, > > .enable_ips = 1, > > - .fastboot = 0, > > + .fastboot = 42, > > The answer to the ultimate question of life, the universe, and > everything should simply be "true" here. Personally, I don't think it's > a bad answer. > > > .prefault_disable = 0, > > .reset = true, > > .invert_brightness = 0, > > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); > > > > module_param_named(fastboot, i915.fastboot, bool, 0600); > > Side note, why do we allow the param to be changed after boot? Because fastboot works for every modeset, or at least should. E.g. if you go from an upscale panel resolotion to the full one we still want to just fixup the pfit, even when it's a normal modeset. So fastboot really is a bit a misnomer, since the leftover bits (after I've convinced Jesse that the fb takeover should be done unconditionally really) is just some modeset fastpaths. Aside: We still have a leak on the inherited fb, see https://bugs.freedesktop.org/show_bug.cgi?id=77511 -Daniel > > BR, > Jani. > > > MODULE_PARM_DESC(fastboot, > > - "Try to skip unnecessary mode sets at boot time (default: false)"); > > + "Try to skip unnecessary mode sets at boot time (default: true)"); > > > > module_param_named(prefault_disable, i915.prefault_disable, bool, 0600); > > MODULE_PARM_DESC(prefault_disable, > > -- > > 1.8.3.2 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 10 Jun 2014 16:07:44 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Jun 05, 2014 at 11:24:31AM -0700, Jesse Barnes wrote: > > Let them eat mincemeat pie. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > index d05a2af..081ab2f 100644 > > --- a/drivers/gpu/drm/i915/i915_params.c > > +++ b/drivers/gpu/drm/i915/i915_params.c > > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { > > .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > > .disable_power_well = 1, > > .enable_ips = 1, > > - .fastboot = 0, > > + .fastboot = 42, > > .prefault_disable = 0, > > .reset = true, > > .invert_brightness = 0, > > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); > > > > module_param_named(fastboot, i915.fastboot, bool, 0600); > > MODULE_PARM_DESC(fastboot, > > - "Try to skip unnecessary mode sets at boot time (default: false)"); > > + "Try to skip unnecessary mode sets at boot time (default: true)"); > > Nah, that wasn't the intention of this option. It was meant as a hack to > experiment around with fastboot and get things going, but imo we need to > really do the full modeset and short-circuit if the state matches. > > And there's still a bunch of things we don't track like infoframes which > we either need to fix up (similar to the pfit fixup) or quirk to disallow > fastboot. Hm that contradicts our earlier discussions w/Damien when we decided the infoframes stuff were too esoteric to matter... Also do you want the mod_parm_desc updated to be more accurate? Not sure what the request is here.
On Tue, Jun 10, 2014 at 10:31 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > On Tue, 10 Jun 2014 16:07:44 +0200 > Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Thu, Jun 05, 2014 at 11:24:31AM -0700, Jesse Barnes wrote: >> > Let them eat mincemeat pie. >> > >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> >> > --- >> > drivers/gpu/drm/i915/i915_params.c | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c >> > index d05a2af..081ab2f 100644 >> > --- a/drivers/gpu/drm/i915/i915_params.c >> > +++ b/drivers/gpu/drm/i915/i915_params.c >> > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { >> > .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), >> > .disable_power_well = 1, >> > .enable_ips = 1, >> > - .fastboot = 0, >> > + .fastboot = 42, >> > .prefault_disable = 0, >> > .reset = true, >> > .invert_brightness = 0, >> > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); >> > >> > module_param_named(fastboot, i915.fastboot, bool, 0600); >> > MODULE_PARM_DESC(fastboot, >> > - "Try to skip unnecessary mode sets at boot time (default: false)"); >> > + "Try to skip unnecessary mode sets at boot time (default: true)"); >> >> Nah, that wasn't the intention of this option. It was meant as a hack to >> experiment around with fastboot and get things going, but imo we need to >> really do the full modeset and short-circuit if the state matches. >> >> And there's still a bunch of things we don't track like infoframes which >> we either need to fix up (similar to the pfit fixup) or quirk to disallow >> fastboot. > > Hm that contradicts our earlier discussions w/Damien when we decided > the infoframes stuff were too esoteric to matter... My 2 cents is that I've seen some really bad TVs which didn't work because infoframes were missing (IIRC it relied on the VIC to detect the video mode). Stéphane
On Tue, 10 Jun 2014 11:01:06 -0700 Stéphane Marchesin <stephane.marchesin@gmail.com> wrote: > On Tue, Jun 10, 2014 at 10:31 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > On Tue, 10 Jun 2014 16:07:44 +0200 > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > >> On Thu, Jun 05, 2014 at 11:24:31AM -0700, Jesse Barnes wrote: > >> > Let them eat mincemeat pie. > >> > > >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > >> > --- > >> > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > >> > 1 file changed, 2 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > >> > index d05a2af..081ab2f 100644 > >> > --- a/drivers/gpu/drm/i915/i915_params.c > >> > +++ b/drivers/gpu/drm/i915/i915_params.c > >> > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { > >> > .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > >> > .disable_power_well = 1, > >> > .enable_ips = 1, > >> > - .fastboot = 0, > >> > + .fastboot = 42, > >> > .prefault_disable = 0, > >> > .reset = true, > >> > .invert_brightness = 0, > >> > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); > >> > > >> > module_param_named(fastboot, i915.fastboot, bool, 0600); > >> > MODULE_PARM_DESC(fastboot, > >> > - "Try to skip unnecessary mode sets at boot time (default: false)"); > >> > + "Try to skip unnecessary mode sets at boot time (default: true)"); > >> > >> Nah, that wasn't the intention of this option. It was meant as a hack to > >> experiment around with fastboot and get things going, but imo we need to > >> really do the full modeset and short-circuit if the state matches. > >> > >> And there's still a bunch of things we don't track like infoframes which > >> we either need to fix up (similar to the pfit fixup) or quirk to disallow > >> fastboot. > > > > Hm that contradicts our earlier discussions w/Damien when we decided > > the infoframes stuff were too esoteric to matter... > > My 2 cents is that I've seen some really bad TVs which didn't work > because infoframes were missing (IIRC it relied on the VIC to detect > the video mode). Yeah so we'd still leave them in place in this case, and apply them on the next mode set as well, but we wouldn't be explicitly cross checking for them, at least not yet. It's a good thing to add, I just didn't think it was a blocker based on our last discussion about this.
On Tue, Jun 10, 2014 at 11:42:37AM -0700, Jesse Barnes wrote: > On Tue, 10 Jun 2014 11:01:06 -0700 > Stéphane Marchesin <stephane.marchesin@gmail.com> wrote: > > > On Tue, Jun 10, 2014 at 10:31 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > > On Tue, 10 Jun 2014 16:07:44 +0200 > > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > >> On Thu, Jun 05, 2014 at 11:24:31AM -0700, Jesse Barnes wrote: > > >> > Let them eat mincemeat pie. > > >> > > > >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > >> > --- > > >> > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > > >> > 1 file changed, 2 insertions(+), 2 deletions(-) > > >> > > > >> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > > >> > index d05a2af..081ab2f 100644 > > >> > --- a/drivers/gpu/drm/i915/i915_params.c > > >> > +++ b/drivers/gpu/drm/i915/i915_params.c > > >> > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { > > >> > .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > > >> > .disable_power_well = 1, > > >> > .enable_ips = 1, > > >> > - .fastboot = 0, > > >> > + .fastboot = 42, > > >> > .prefault_disable = 0, > > >> > .reset = true, > > >> > .invert_brightness = 0, > > >> > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); > > >> > > > >> > module_param_named(fastboot, i915.fastboot, bool, 0600); > > >> > MODULE_PARM_DESC(fastboot, > > >> > - "Try to skip unnecessary mode sets at boot time (default: false)"); > > >> > + "Try to skip unnecessary mode sets at boot time (default: true)"); > > >> > > >> Nah, that wasn't the intention of this option. It was meant as a hack to > > >> experiment around with fastboot and get things going, but imo we need to > > >> really do the full modeset and short-circuit if the state matches. > > >> > > >> And there's still a bunch of things we don't track like infoframes which > > >> we either need to fix up (similar to the pfit fixup) or quirk to disallow > > >> fastboot. > > > > > > Hm that contradicts our earlier discussions w/Damien when we decided > > > the infoframes stuff were too esoteric to matter... I'm pretty sure I've never claimed that infoframes are too esoteric. Like Stéphane mentions they're really good at breaking shitty TVs and resulting in black screens. > > My 2 cents is that I've seen some really bad TVs which didn't work > > because infoframes were missing (IIRC it relied on the VIC to detect > > the video mode). > > Yeah so we'd still leave them in place in this case, and apply them on > the next mode set as well, but we wouldn't be explicitly cross checking > for them, at least not yet. > > It's a good thing to add, I just didn't think it was a blocker based on > our last discussion about this. Imo we should quirk hdmi and prevent fastboot exactly because infoframes are such a pain. Relying on the BIOS for the just means we'll loose a lot of testing coverage on random screens out there, which I very much want to avoid. Another piece of state we don't fix up atm is sound. Since my latest rework this is tracked in the pipe config, so at least we'll catch that. Also iirc Chris complained about the modeset state checker overhead caused by the fastboot option since a normal flip done through setCrtc now hits that unconditionally. If we'd push the fastboot logic into the overall modeset path we could restrict modeset state checks to only when we need them. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index d05a2af..081ab2f 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), .disable_power_well = 1, .enable_ips = 1, - .fastboot = 0, + .fastboot = 42, .prefault_disable = 0, .reset = true, .invert_brightness = 0, @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); module_param_named(fastboot, i915.fastboot, bool, 0600); MODULE_PARM_DESC(fastboot, - "Try to skip unnecessary mode sets at boot time (default: false)"); + "Try to skip unnecessary mode sets at boot time (default: true)"); module_param_named(prefault_disable, i915.prefault_disable, bool, 0600); MODULE_PARM_DESC(prefault_disable,
Let them eat mincemeat pie. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_params.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)