[PATCH] make xf86SetDesiredModes a bit more driver friendly

[Alex Deucher]

On Mon, Mar 10, 2008 at 7:14 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> Here's an updated intel half of the patchset.  This one takes advantage of the
>  earlier server patch to provide 0 flicker LVDS mode setting.
>
>  I'm not totally happy with it however, and I'm looking for suggestions on the
>  following points:
>   - getting crtc mapping & output status
>     Right now this is done in each output setup routine, which creates a lot
>     of duplication.  Maybe we should have a output function pointer
>     callback ->get_crtc() and move all that stuff into the server (output
>     status can already be done this way)?

In radeon we just iterate through the outputs after we create them
from the bios connector table and call detect.  We do this even
through the server already does it to make some decisions about output
fallbacks and such depending on what's connected.  How about a
set_crtc() callback instead.  then the driver can force the crtc to
avoid letting the server pick and it might also be useful on cards
where, for example, crtc0 has to be active to use crtc1, etc.  this
would also allow the driver to re-arrange crtcs if something changes
that it would know how to handle best.

>
>   - save/restore of hw state in the driver vs. core
>
>     I think this hunk:
>
>  @@ -970,6 +970,7 @@ PreInitCleanup(ScrnInfoPtr pScrn)
>   {
>     I830Ptr pI830 = I830PTR(pScrn);
>
>  +   RestoreHWState(pScrn);
>     if (I830IsPrimary(pScrn)) {
>        if (pI830->entityPrivate)
>          pI830->entityPrivate->pScrn_1 = NULL;
>  @@ -1547,11 +1548,9 @@ I830PreInit(ScrnInfoPtr pScrn, int flags)
>     if (!xf86InitialConfiguration (pScrn, FALSE))
>     {
>        xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "No valid modes.\n");
>  -      RestoreHWState(pScrn);
>        PreInitCleanup(pScrn);
>        return FALSE;
>     }
>  -   RestoreHWState(pScrn);
>
>  Makes sense (at least partially, we should be calling RestoreHWState only in
>  the PreInitCleanup routine, not by hand elsewhere), but it doesn't quite work
>  on its own, since xf86InitialConfiguration modifies hardware state, which is
>  then saved later on at EnterVT time:
>
>     /* XXX This should go away, replaced by xf86Crtc.c support for it */
>     pI830->rotation = RR_Rotate_0;
>  @@ -3157,15 +3156,6 @@ I830EnterVT(int scrnIndex, int flags)
>
>     DPRINTF(PFX, "Enter VT\n");
>
>  -   /*
>  -    * Only save state once per server generation since that's what most
>  -    * drivers do.  Could change this to save state at each VT enter.
>  -    */
>  -   if (pI830->SaveGeneration != serverGeneration) {
>  -      pI830->SaveGeneration = serverGeneration;
>  -      SaveHWState(pScrn);
>  -   }
>  -
>
>  However, removing this is the wrong thing to do since hw state might change
>  between LeaveVT and EnterVT.
>
>     pI830->leaving = FALSE;
>
>   #ifdef XF86DRI_MM
>  @@ -3200,10 +3190,13 @@ I830EnterVT(int scrnIndex, int flags)
>     memset(pI830->FbBase + pScrn->fbOffset, 0,
>           pScrn->virtualY * pScrn->displayWidth * pI830->cpp);
>
>  +   /* If coming back from an actual VT switch we may need this? */
>  +#if 0
>     for (o = 0; o < config->num_output; o++) {
>         xf86OutputPtr  output = config->output[o];
>         output->funcs->dpms(output, DPMSModeOff);
>     }
>  +#endif
>
>  But ultimately, this is what needs to be dealt with.  If xf86SetDesiredModes
>  got the current configuration from each output & crtc, we wouldn't need to
>  turn everything off to get a known state (I think).  Which means the above
>  removals of RestoreHWState wouldn't be necessary (aside from cleanliness).
>
>  Any ideas?  What works for radeon & nv?

For radeon we call have general save/restore functions.  We save the
console state at startup and then restore it in LeaveVT.  For EnterVT
we just call xf86SetDesiredModes().

Alex