* Re: [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set
@ 2013-03-27 2:09 Jesse Barnes
2013-03-27 12:49 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Jesse Barnes @ 2013-03-27 2:09 UTC (permalink / raw)
To: daniel; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 2149 bytes --]
If for example the BIOS fb is too small for the dual pipe config we detect, we may have valid timings and such, but no fb. The pfit case also hits this path (though currently only fastboots if you hack your mode clock to match).
--
Jesse Barnes, Intel Open Source Technology Center
-------- Original message --------
From: Daniel Vetter <daniel@ffwll.ch>
Date: 03/26/2013 5:03 PM (GMT-08:00)
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 8/9] drm/i915: treat no fb -> fb as simple\r flip instead of full mode set
On Tue, Mar 26, 2013 at 04:33:11PM -0700, Jesse Barnes wrote:
> In case we don't get an fb from the BIOS, we may still be able to re-use
> existing state and flip a new buffer.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
This hack here smells extremely fishy. Where do we come up with no fb, but
a real mode and want to actually fastboot?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0f3c036..f24da1a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8109,10 +8109,8 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
> /* We should be able to check here if the fb has the same properties
> * and then just flip_or_move it */
> if (set->crtc->fb != set->fb) {
> - /* If we have no fb then treat it as a full mode set */
> if (set->crtc->fb == NULL) {
> - DRM_DEBUG_KMS("crtc has no fb, full mode set\n");
> - config->mode_changed = true;
> + config->fb_changed = true;
> } else if (set->fb == NULL) {
> config->mode_changed = true;
> } else if (set->fb->depth != set->crtc->fb->depth) {
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[-- Attachment #1.2: Type: text/html, Size: 2672 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set
2013-03-27 2:09 [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set Jesse Barnes
@ 2013-03-27 12:49 ` Daniel Vetter
2013-03-27 13:40 ` Chris Wilson
2013-03-27 15:51 ` Jesse Barnes
0 siblings, 2 replies; 6+ messages in thread
From: Daniel Vetter @ 2013-03-27 12:49 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Wed, Mar 27, 2013 at 3:09 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> If for example the BIOS fb is too small for the dual pipe config we detect,
> we may have valid timings and such, but no fb. The pfit case also hits this
> path (though currently only fastboots if you hack your mode clock to match).
But even when the BIOS fb is to small and the BIOS config uses the
pfit, we /should/ have an fb around. Looking a bit closer I think the
confusion stems from you reading out the adjusted mode, but treating
it like the requested mode. I think it'd be much more solid if we
store the read-out mode in the adjusted_mode (won't work otherwise
with hw state cross-checking anyway), and then do two comparisons
here:
- Does the request mode (plus everything else) match? If so, just do
an fb_set_base call.
- Does the adjusted mode match (plus the entire output routing ofc)?
That means there's either the vga plane or a pfit in-between which we
don't like. If possible we can then play some tricks to avoid the full
modeset.
The reason that I'm a bit freaked about about your change here is that
in a lot of places we treat crtc->fb == NULL as "no mode set". So I
fear we're breaking a few too many assumptions here with that hack,
and it simply needs more thought about what we should actually check.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set
2013-03-27 12:49 ` Daniel Vetter
@ 2013-03-27 13:40 ` Chris Wilson
2013-03-27 15:51 ` Jesse Barnes
1 sibling, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2013-03-27 13:40 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, Mar 27, 2013 at 01:49:47PM +0100, Daniel Vetter wrote:
> On Wed, Mar 27, 2013 at 3:09 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > If for example the BIOS fb is too small for the dual pipe config we detect,
> > we may have valid timings and such, but no fb. The pfit case also hits this
> > path (though currently only fastboots if you hack your mode clock to match).
>
> But even when the BIOS fb is to small and the BIOS config uses the
> pfit, we /should/ have an fb around. Looking a bit closer I think the
> confusion stems from you reading out the adjusted mode, but treating
> it like the requested mode. I think it'd be much more solid if we
> store the read-out mode in the adjusted_mode (won't work otherwise
> with hw state cross-checking anyway), and then do two comparisons
> here:
>
> - Does the request mode (plus everything else) match? If so, just do
> an fb_set_base call.
> - Does the adjusted mode match (plus the entire output routing ofc)?
> That means there's either the vga plane or a pfit in-between which we
> don't like. If possible we can then play some tricks to avoid the full
> modeset.
>
> The reason that I'm a bit freaked about about your change here is that
> in a lot of places we treat crtc->fb == NULL as "no mode set". So I
> fear we're breaking a few too many assumptions here with that hack,
> and it simply needs more thought about what we should actually check.
The check here is why we were so enthusiastic about moving to a
pipe_config framework that could match up a configured pipe with an
invalid fb and then just exchange the fb.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set
2013-03-27 12:49 ` Daniel Vetter
2013-03-27 13:40 ` Chris Wilson
@ 2013-03-27 15:51 ` Jesse Barnes
1 sibling, 0 replies; 6+ messages in thread
From: Jesse Barnes @ 2013-03-27 15:51 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, 27 Mar 2013 13:49:47 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Mar 27, 2013 at 3:09 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > If for example the BIOS fb is too small for the dual pipe config we detect,
> > we may have valid timings and such, but no fb. The pfit case also hits this
> > path (though currently only fastboots if you hack your mode clock to match).
>
> But even when the BIOS fb is to small and the BIOS config uses the
> pfit, we /should/ have an fb around. Looking a bit closer I think the
> confusion stems from you reading out the adjusted mode, but treating
> it like the requested mode. I think it'd be much more solid if we
> store the read-out mode in the adjusted_mode (won't work otherwise
> with hw state cross-checking anyway), and then do two comparisons
> here:
Yeah, there's an fb, but it may not be what we want. So we don't
allocate it and thus crtc->fb is empty.
> - Does the request mode (plus everything else) match? If so, just do
> an fb_set_base call.
> - Does the adjusted mode match (plus the entire output routing ofc)?
> That means there's either the vga plane or a pfit in-between which we
> don't like. If possible we can then play some tricks to avoid the full
> modeset.
Right, I'm trying to play tricks here for sure.
> The reason that I'm a bit freaked about about your change here is that
> in a lot of places we treat crtc->fb == NULL as "no mode set". So I
> fear we're breaking a few too many assumptions here with that hack,
> and it simply needs more thought about what we should actually check.
Yeah I was worried about that too. Maybe we should use crtc->active
for that everywhere?
With the modeset rework and pipe config stuff, we should be able to
drop this in favor of something that looks at the pipe_config and
figures out the minimal mode set sequence required.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread
* Updated fastboot patches
@ 2013-03-26 23:33 Jesse Barnes
2013-03-26 23:33 ` [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set Jesse Barnes
0 siblings, 1 reply; 6+ messages in thread
From: Jesse Barnes @ 2013-03-26 23:33 UTC (permalink / raw)
To: intel-gfx
They're slowly getting slurped in. These ones are rebased on top of the
bits Daniel pushed today, and include the comments from Imre.
There's one missing piece for getting fastboot to not slowboot on
laptops that don't use the native mode: overriding the pixel clock of a
panel fitted mode with the native one. The BIOS takeover code can
already adjust the width, height, and pitch, but for the clock we may
need to fetch it from the EDID and add a pipe_config flag or something.
But as they are, I think most of these patches are ready, with the r-b
from Imre.
Thanks,
Jesse
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set
2013-03-26 23:33 Updated fastboot patches Jesse Barnes
@ 2013-03-26 23:33 ` Jesse Barnes
2013-03-27 0:03 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Jesse Barnes @ 2013-03-26 23:33 UTC (permalink / raw)
To: intel-gfx
In case we don't get an fb from the BIOS, we may still be able to re-use
existing state and flip a new buffer.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/intel_display.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0f3c036..f24da1a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8109,10 +8109,8 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
/* We should be able to check here if the fb has the same properties
* and then just flip_or_move it */
if (set->crtc->fb != set->fb) {
- /* If we have no fb then treat it as a full mode set */
if (set->crtc->fb == NULL) {
- DRM_DEBUG_KMS("crtc has no fb, full mode set\n");
- config->mode_changed = true;
+ config->fb_changed = true;
} else if (set->fb == NULL) {
config->mode_changed = true;
} else if (set->fb->depth != set->crtc->fb->depth) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set
2013-03-26 23:33 ` [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set Jesse Barnes
@ 2013-03-27 0:03 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2013-03-27 0:03 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Tue, Mar 26, 2013 at 04:33:11PM -0700, Jesse Barnes wrote:
> In case we don't get an fb from the BIOS, we may still be able to re-use
> existing state and flip a new buffer.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
This hack here smells extremely fishy. Where do we come up with no fb, but
a real mode and want to actually fastboot?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0f3c036..f24da1a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8109,10 +8109,8 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
> /* We should be able to check here if the fb has the same properties
> * and then just flip_or_move it */
> if (set->crtc->fb != set->fb) {
> - /* If we have no fb then treat it as a full mode set */
> if (set->crtc->fb == NULL) {
> - DRM_DEBUG_KMS("crtc has no fb, full mode set\n");
> - config->mode_changed = true;
> + config->fb_changed = true;
> } else if (set->fb == NULL) {
> config->mode_changed = true;
> } else if (set->fb->depth != set->crtc->fb->depth) {
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-03-27 15:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-27 2:09 [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set Jesse Barnes
2013-03-27 12:49 ` Daniel Vetter
2013-03-27 13:40 ` Chris Wilson
2013-03-27 15:51 ` Jesse Barnes
-- strict thread matches above, loose matches on Subject: below --
2013-03-26 23:33 Updated fastboot patches Jesse Barnes
2013-03-26 23:33 ` [PATCH 8/9] drm/i915: treat no fb -> fb as simple flip instead of full mode set Jesse Barnes
2013-03-27 0:03 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox