| Message ID | 1352808910-31946-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 13, 2012 at 12:15:10PM +0000, Chris Wilson wrote: > In the slightly unusual case where the pipe is programmed to the same > modeline, but the framebuffer is a new size, we need to resetup the > panel fitter as appropriate and this requires a full modeset. This can > only occur currently as part of the BIOS takeover where there are > slightly different semantics governing how the panel fitter and > framebuffer is programmed relative to the modeline. Hmm. I don't get it. Why would the framebuffer size affect the panel fitter configuration?
On Tue, Nov 13, 2012 at 12:48:11PM +0000, Chris Wilson wrote: > On Tue, 13 Nov 2012 14:42:36 +0200, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Tue, Nov 13, 2012 at 12:15:10PM +0000, Chris Wilson wrote: > > > In the slightly unusual case where the pipe is programmed to the same > > > modeline, but the framebuffer is a new size, we need to resetup the > > > panel fitter as appropriate and this requires a full modeset. This can > > > only occur currently as part of the BIOS takeover where there are > > > slightly different semantics governing how the panel fitter and > > > framebuffer is programmed relative to the modeline. > > > > Hmm. I don't get it. Why would the framebuffer size affect the panel > > fitter configuration? > > The BIOS uses fb->(width,height) to program PIPESRC, we use > mode->[hv]display. The BIOS's semantics makes more sense I don't think so. That would make panning impossible. > and is > ultimately more flexible - especially if we do end up exposing more > information to userspace to solve the under/overscan issue using the > panel fitter. I've been known to bitch about this issue also. I'd like to have the mode be strictly the display mode. Then we'd need another way to configure PIPESRC (some kind of CRTC dimensions properties would do it). And then we also need to expose the primary plane as a drm_plane. Those measures would allow full control over the pipe, rather than relying on some magic connector properties that may or may not do what you want. Doing all that seems straightforward enough, but I suspect trying to maintain some kind of compatibility with the current mess is going to be the hard part.
On Tue, Nov 13, 2012 at 01:33:37PM +0000, Chris Wilson wrote: > On Tue, 13 Nov 2012 15:15:55 +0200, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Tue, Nov 13, 2012 at 12:48:11PM +0000, Chris Wilson wrote: > > > On Tue, 13 Nov 2012 14:42:36 +0200, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > > On Tue, Nov 13, 2012 at 12:15:10PM +0000, Chris Wilson wrote: > > > > > In the slightly unusual case where the pipe is programmed to the same > > > > > modeline, but the framebuffer is a new size, we need to resetup the > > > > > panel fitter as appropriate and this requires a full modeset. This can > > > > > only occur currently as part of the BIOS takeover where there are > > > > > slightly different semantics governing how the panel fitter and > > > > > framebuffer is programmed relative to the modeline. > > > > > > > > Hmm. I don't get it. Why would the framebuffer size affect the panel > > > > fitter configuration? > > > > > > The BIOS uses fb->(width,height) to program PIPESRC, we use > > > mode->[hv]display. The BIOS's semantics makes more sense > > > > I don't think so. That would make panning impossible. > > True. The issue really appears that we recreate the mode using the crtc > values and assume that the reverse mapping works. Further issues arise > if we want to recover any offsets in a multiple output configuration. I > will just have to double-check that the BIOS configuration matches our > usage. > > And I whole-heartedly agree with separating the display mode from the > plane configuration, I guess you have something planned along those > lines already ;-) Well, I have some ideas in my head, but so far I haven't found the time to do the work. Originally I was thinking that I'd try to combine this work with the atomic mode setting, but I fear that if I keep piling on more changes, it'll never be ready.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f3bec25..de00421 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8114,16 +8114,27 @@ 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) { + const struct drm_framebuffer *old = set->crtc->fb; + const struct drm_framebuffer *new = set->fb; + /* If we have no fb then treat it as a full mode set */ - if (set->crtc->fb == NULL) { + if (old == NULL) { DRM_DEBUG_KMS("crtc has no fb, full mode set\n"); config->mode_changed = true; - } else if (set->fb == NULL) { + } else if (new == NULL) { config->mode_changed = true; - } else if (set->fb->depth != set->crtc->fb->depth) { + /* check for a change in fb size, need to adjust fitting */ + } else if (new->width != old->width || + new->height != old->height) { + DRM_DEBUG_KMS("fb is changing size (%d, %d) -> (%d, %d), full mode set\n", + old->width, old->height, new->width, new->height); config->mode_changed = true; - } else if (set->fb->bits_per_pixel != - set->crtc->fb->bits_per_pixel) { + /* check for a change in fb format, need to reprogram the pipe */ + } else if (new->depth != old->depth || + new->bits_per_pixel != old->bits_per_pixel) { + DRM_DEBUG_KMS("fb is changing format (%d/%d) -> (%d/%d), full mode set\n", + old->depth, old->bits_per_pixel, + new->depth, new->bits_per_pixel); config->mode_changed = true; } else config->fb_changed = true;
In the slightly unusual case where the pipe is programmed to the same modeline, but the framebuffer is a new size, we need to resetup the panel fitter as appropriate and this requires a full modeset. This can only occur currently as part of the BIOS takeover where there are slightly different semantics governing how the panel fitter and framebuffer is programmed relative to the modeline. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)