On Thu, Jun 13, 2013 at 7:19 AM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
And another issue...
What is drm_crtc_helper_set_mode() passed as the fb argument? Is it the old fb, or the new fb?
bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, int x, int y, struct drm_framebuffer *old_fb) ... int drm_crtc_helper_set_config(struct drm_mode_set *set) { ... save_set.fb = set->crtc->fb; ... old_fb = set->crtc->fb; set->crtc->fb = set->fb; if (!drm_crtc_helper_set_mode(set->crtc, set->mode, set->x, set->y, old_fb)) { ... /* Try to restore the config */ if (mode_changed && !drm_crtc_helper_set_mode(save_set.crtc, save_set.mode, save_set.x, save_set.y, save_set.fb)) } ... int drm_helper_resume_force_mode(struct drm_device *dev) { ... ret = drm_crtc_helper_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb);
The function prototype implies it's the old fb, as does the first use. The second and third uses of it clearly show it being the fb which we wish to be displayed.
most of the embedded drivers should ignore the old_fb.. the API is a bit odd but the purpose is to help drivers that need to pin/unpin the gem objects backing the fb. The ones that do, do something like:
foo_pin(new_fb); foo_unpin(old_fb);
if the two are the same, it works out.
Note that even for the non error paths, new_fb and old_fb could be the same.
Possibly something worth clarifying in the docs.
BR, -R
The deeper I look, the more bugs there seem to be in this DRM stuff, and I'm continuing to look because I'm chasing a framebuffer refcount bug.