Any feedback? Does the silence mean I'm to send it straight to Linus, first class? :-)
> It seems linux commit 0cc4d4300c broke i915 interlaced mode support > while fixing another issue (broken by my patch supporting interlaced > mode). Yep, I agree resetting the mode isn't the best idea (though it's > what several drivers do) and should not be needed in the first place. > I wonder if this is all working around a completely unneeded "feature". > > The "core" modesetting Linux code does the following: > drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V) > What is it good for, there in the core code? > > The (any) driver (or output) either supports interlaced mode, which > forces it to revert this core operation with > "drm_mode_set_crtcinfo(adjusted_mode, 0)" (which my i915 patch did and > which may and do break special customizations), or it doesn't support > interlaced mode and then this flag isn't used at all. > > Does the following patch (+ probably removing then unneeded calls in > e.g. radeon driver) fixes all these problems for good? > > At least makes my i915 working again in interlaced mode. I could also > test on radeon, though I don't think this change could break it. > > BTW interlaced mode on i915 requires X.org patch as well, see > http://www.mail-archive.com/[email protected]/msg11512.html > (only the userland driver patch and the kernel fix (below) is needed, > the main kernel part is already in place). > Perhaps someone in charge could apply the userland patch as well? > This is needed mainly for AV applications (e.g. connecting TV-alike > displays). > > Obviously I'm open for suggestions, I'm not an X.org nor drivers/gpu > expert. > > BTW2 is there some doc, note explaining all those "adjusted_mode" > magics? Why can't individual drivers mess with such things internally > when they need so? > > BTW3 :-) I think the drm_mode_set_crtcinfo(x, CRTC_INTERLACE_HALVE_V) > logic has another flaw: > > if (p->flags & DRM_MODE_FLAG_INTERLACE) { > if (adjust_flags & CRTC_INTERLACE_HALVE_V) { > p->crtc_vdisplay /= 2; > p->crtc_vsync_start /= 2; > p->crtc_vsync_end /= 2; > p->crtc_vtotal /= 2; > } > > p->crtc_vtotal |= 1; > } > > The last line should only be applied when we don't do > CRTC_INTERLACE_HALVE_V (i.e. the total number of lines in interlaced > mode has to be odd (X lines for odd field + X lines for even field + two > half-lines) - and only when we don't count these half-lines as full ones > (and we do, most of the time). If we do CRTC_INTERLACE_HALVE_V, we got > something like 288 or 240 field lines (instead of 576 or 480 for full > frame) and forcing the odd value makes no sense. > I'm not fixing this bug since I think we should remove this > CRTC_INTERLACE_HALVE_V completely (but I'll wait for comments to the > patch below first). > > Thanks. > > Signed-off-by: Krzysztof Hałasa <[email protected]> > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 57cea01..2acfc88 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1520,7 +1520,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, > > mode = drm_mode_create(dev); > drm_crtc_convert_umode(mode, &crtc_req->mode); > - drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V); > + drm_mode_set_crtcinfo(mode, 0); > } > > if (crtc_req->count_connectors == 0 && mode) { > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > b/drivers/gpu/drm/drm_crtc_helper.c > index 9b2a541..aa38c98 100644 > --- a/drivers/gpu/drm/drm_crtc_helper.c > +++ b/drivers/gpu/drm/drm_crtc_helper.c > @@ -146,7 +146,7 @@ prune: > list_for_each_entry_safe(mode, t, &connector->modes, head) { > mode->vrefresh = drm_mode_vrefresh(mode); > > - drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V); > + drm_mode_set_crtcinfo(mode, 0); > drm_mode_debug_printmodeline(mode); > } > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 7196620..e890c98 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1053,7 +1053,7 @@ create_mode: > cmdline_mode->refresh_specified ? > cmdline_mode->refresh : 60, > cmdline_mode->interlace, > cmdline_mode->margins); > - drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V); > + drm_mode_set_crtcinfo(mode, 0); > list_add(&mode->head, &fb_helper_conn->connector->modes); > return mode; > } -- Krzysztof Halasa _______________________________________________ [email protected]: X.Org support Archives: http://lists.freedesktop.org/archives/xorg Info: http://lists.freedesktop.org/mailman/listinfo/xorg Your subscription address: [email protected]
