On Fri, Jan 06, 2017 at 10:18:24AM +0000, Kevin Brace wrote:
> src/via_display.c | 244
> ++++++++++++++++++++++++++++++++++++++++++++----------
> src/via_driver.h | 3
> 2 files changed, 203 insertions(+), 44 deletions(-)
>
> New commits:
> commit f8551e0e07366a70cf6aa8c3801d2965a86e170f
> Author: Kevin Brace <kevinbr...@gmx.com>
> Date: Fri Jan 6 04:17:32 2017 -0600
>
> Preparation for the partial fix of save / restore functions
>
> Signed-off-by: Kevin Brace <kevinbr...@gmx.com>
>
> diff --git a/src/via_display.c b/src/via_display.c
> index a405ce2..9f3920a 100644
> --- a/src/via_display.c
> +++ b/src/via_display.c
> @@ -3342,41 +3342,111 @@ viaIGA2Save(ScrnInfoPtr pScrn)
> DEBUG(xf86DrvMsg(pScrn->scrnIndex, X_INFO,
> "Saving IGA2 registers.\n"));
>
> -/* Unlock and save extended registers. */
> + /* Unlock extended registers. */
> hwp->writeSeq(hwp, 0x10, 0x01);
>
> - /* Save LCD control registers (from CR 0x50 to 0x93). */
> - for (i = 0; i < 68; i++)
> - Regs->CRTCRegs[i] = hwp->readCrtc(hwp, i + 0x50);
> -
> - if (pVia->Chipset != VIA_CLE266 && pVia->Chipset != VIA_KM400) {
> - /* LVDS Channel 2 Function Select 0 / DVI Function Select */
> - Regs->CR97 = hwp->readCrtc(hwp, 0x97);
> - /* LVDS Channel 1 Function Select 0 */
> - Regs->CR99 = hwp->readCrtc(hwp, 0x99);
> - /* Digital Video Port 1 Function Select 0 */
> - Regs->CR9B = hwp->readCrtc(hwp, 0x9B);
> - /* Power Now Control 4 */
> - Regs->CR9F = hwp->readCrtc(hwp, 0x9F);
> -
> - /* Horizontal Scaling Initial Value */
> - Regs->CRA0 = hwp->readCrtc(hwp, 0xA0);
> - /* Vertical Scaling Initial Value */
> - Regs->CRA1 = hwp->readCrtc(hwp, 0xA1);
> - /* Scaling Enable Bit */
> - Regs->CRA2 = hwp->readCrtc(hwp, 0xA2);
> + for (i = 0; i < (0x88 - 0x50 + 1); i++) {
> + Regs->CR[i + 0x50] = hwp->readCrtc(hwp, i + 0x50);
> + }
> +
> + for (i = 0; i < (0x92 - 0x8A + 1); i++) {
> + Regs->CR[i + 0x8A] = hwp->readCrtc(hwp, i + 0x8A);
> + }
> +
> + for (i = 0; i < (0xA3 - 0x94 + 1); i++) {
> + Regs->CR[i + 0x94] = hwp->readCrtc(hwp, i + 0x94);
> }
This is a mistake, and a very slippery slope you started. I had some
very good reasons for not doing tables for save/restore.
1) Direct typed registers requires you to think very carefully about
which registers to restore. They require you to have very acute
knowledge of the display engine you are working with. Direct type
register names do not allow you to take shortcuts.
2) Synchronicity with a the real modesetting code. A missed register
in save/restore will usually also mean a missed register in the proper
modeset. Direct typed forces you to think about things more in depth and
makes it more natural to keep both the save/restore as the actual
modeset in sync.
2) what about timing and ordering? How are you going to deal with that
with the next abstraction step after you've introduced a table?
The fact that you missed CR6A/6B clearly shows that you missed something
big in save/restore. What did you do to stop saving restoring these
registers, which are very ordering critical.
My most recent customer was using register tables both in the capture
chip setup and for their poor excuse for a bootloader. In 2016. It was
amazing. Besides the fact that they were poor developers who couldn't
find or fix anything without JTAG, it also made it impossible for proper
developers to see the forest through the trees, or to try to use logic
to figure things out, and it was impossible to try to fix (apart from
the fact that the poor developers would have to change their idiotic
ways, which is impossible in itself).
Luc Verhaegen.
_______________________________________________
Openchrome-devel mailing list
Openchrome-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/openchrome-devel
