Bug 96397

Created attachment 124341 [details]
Xorg log (non-working)

Created attachment 124342 [details]
regs dump (non-working)

Created attachment 124343 [details]
Xorg log (working)

Created attachment 124344 [details]
regs dump (working)

Created attachment 124345 [details] [review]
Old patch to fix new modesetting for the CLE266

Created attachment 124346 [details] [review]
Revert legacy modesetting removal on top of 0.4.0

Hi Xavier,

There is a reason why I did not observe the bug your reported here.
I do not own a functional monitor that can do 1920 X 1080 resolution at my place.
The best I got that is working is capable of 1680 X 1050 maximum (Note: I repaired this monitor by replacing the faculty "bulging" electrolytic capacitors. It was made by Samsung, and it appears that Samsung tends to cut corners on capacitor quality by using certain lousy Taiwanese vendor capacitors.), and I did not observe a bug at this screen resolution.
    Regarding the patch you have uploaded, I do not like the fact that you have put back in many elements of the old rotten code into the patch.
The only way I can personally agree to handle this bug is to figure out why did the so called "legacy mode setting" work in the first place, and borrow some portion of the code to specifically fix the current mode setting bug for CLE266.
I am not interested in bringing back alternative mode setting option into the Git repository code since this is a very bad design decision (i.e., going back to the UniChrome vs. OpenChrome schism that happened around Year 2005) that OpenChrome developers (i.e., you were one of them) made back then.
I think you need to let such design go since everything is moving into automatic display detection and configuration, and Luc Verhaegen was right on that point.
I tend to agree with him on that point, and my work has been going in that direction for the past 4 months.
    I do not know how hard it was to bring back the old code back into the current code (as of June 5th, 2016, Version 0.4.169), but if you merely reported the bug, I definitely would have told you not to try to bring back the old rotten code back into the current one I have been in the process of cleaning it up for the past 4 months.
That being said, I certainly would not have minded to learn why the "legacy mode setting" worked, and incorporate some aspects of the old code into the mode setting code path for CLE266.
I am hoping that you can agree to the way I will like to triage the bug rather than blindly bringing back the old rotten code you did with the proposed patch.
    Since I have been focused on cleaning up the code for the past 2 months, in addition to fixing the runtime screen resize bug, I have observed that UniChrome (CLE266, KM400, etc) appears to be very sensitive to the change of the display FIFO threshold values.
However, UniChrome Pro and Chrome9 appears far more robust, and based on this, I honestly will not be surprised that UniChrome is buggy from a hardware perspective.
    Just to let you know, I was planning to bump up the OpenChrome version to 0.4.901 (Release Candidate 2) in the next few days after I arrive at my destination in preparation for the release of OpenChrome Version 0.5.
Since I fixed the runtime screen resize bug, this alone justifies a new official version to be released.
I can consider fixing this bug with OpenChrome Version 0.6 since it affects very few users (i.e., people who own a 1920 X 1080 capable monitor which is you in this case).
    I am going for a 5 week trip tomorrow, and I will be leaving my place in the next 12 hours.
If you want me to bring a VIA Embedded EPIA-M mainboard (CLE266 + VT1622 TV encoder), I can, but if I do not bring it with me, I will not have access to it for the next 5 weeks.
Let me know in the next 10 hours if I need to bring it with me (I do not really have room in my suitcase, but I might be able to bring it if you really want me to.
I am already bringing 3 pieces of VIA hardware with me for this trip.
I guess it shows how committed I am to improving and fixing OpenChrome.

Hi Kevin,

Sorry if I was not clear, indeed I'm not asking for bringing back the old code, the patch I attached is just a quick and dirty fix as I needed my CLE266 (an Epia-M too) to be in a working state. And the patch is against 0.4.0, not master, porting that to master would probably be more work and would not hold much value anyway. I reported the bug on the mailing list a couple days after the commit, but it took me far too more time to file a bug with meaningful information, the patch reverting the megacy modesetting removal just happen to be part of the information. Let me know if anything else is needed.

Regards,
Xavier

I've experimented a bit more with this. Setting all of 3d5 6a, 6b and 6c to 0x00 restores the display with 0.4.169.

(In reply to Xavier Bachelot from comment #8)

Hi Xavier,

> Hi Kevin,
> 
> Sorry if I was not clear, indeed I'm not asking for bringing back the old
> code, the patch I attached is just a quick and dirty fix as I needed my
> CLE266 (an Epia-M too) to be in a working state. And the patch is against
> 0.4.0, not master, porting that to master would probably be more work and
> would not hold much value anyway. I reported the bug on the mailing list a
> couple days after the commit, but it took me far too more time to file a bug
> with meaningful information, the patch reverting the megacy modesetting
> removal just happen to be part of the information. Let me know if anything
> else is needed.
> 
> Regards,
> Xavier

Okay, thanks for letting me know what your intention is.
Yeah, the way I saw it is, the train has already left the station, so when I saw that the patch you created was against Version 0.4.0, I honestly could not believe that you were dealing with 2 months ago code that contained more problems than the current one.
I do remember from March that you were talking about creating a patch against Version 0.4.0, but I did not really see much activity on that front, so by now, I assumed that you gave up trying to do that.
Yesterday was my personal travel day, and since I saw the bug report about 12 hours before departure, I suddenly had to think about bringing EPIA-M mainboard with me in my suitcase.
I was not really planning to bring it, but now that you filed a bug report with a proposed patch that brought back old code, I had to ask what you were trying to accomplish other than proving that a very high screen resolution (1920 X 1080) used to work fine with CLE266 chipset.
As I have indicated in an announcement piece sent in late May (https://lists.freedesktop.org/archives/openchrome-devel/2016-May/002552.html) that there will be OpenChrome Version 0.5 released in the next few weeks, and the major reason for this was that a fatal bug when the screen is resized during runtime is now fixed, and as a result, multi-monitor capability is now working.
I was not planning to fix any more major problems prior to Version 0.5 release, and then you suddenly filed a bug report indicating that a code regression happened.
If you think the release of OpenChrome should be delay by a few weeks, I think we can do that.
Let me know, what if the new release should be delayed.
    Anyway, since you let me know what you were trying to accomplish within 12 hours of my departure, I decided to bring EPIA-M mainboard with me so that I can test the code change locally, although I do not have a monitor that can handle a screen resolution greater than 1280 X 1024.
Although this might not be an elegant way to workaround the problem, if you specify in xorg.conf of a resolution other than 1920 X 1080 like 1600 X 1200, the screen should work for now.
You should be able to resize the screen during runtime, and I have tested this with CLE266.
As I have already indicated in the previous e-mail, I have observed CLE266 to work with 1680 X 1050 resolution, although I do see some weird blue lines on the right edge of the screen (I will like to fix this bug eventually.).
I will do a file comparison of the register dump between the 2 different versions to see which registers are being set differently.

I've reduced the needed changes to both 3d5.6b bit 7 and 3d5.6c bit 0.
Initial registers setting is 3d5.6b = 0x84 and 3d5.6c = 0x01.
Working registers setting is 3d5.6b = 0x04 and 3d5.6c = 0x00.

According to CX700 documentation, the older chipset with doc available :

6b[7:6] is "First Display Channel Clock Mode Selection"
0x: Normal
1x: Division by 2

6c[0] is "LCDCK Source Selection"
0: LCDCK PLL output clock
1: LCDCK PLL reference clock

(In reply to Xavier Bachelot from comment #9)

Hi Xavier,

> I've experimented a bit more with this. Setting all of 3d5 6a, 6b and 6c to
> 0x00 restores the display with 0.4.169.

I will have to ask the question, what do you precisely mean "restores the display with 0.4.169?"
Does the screen suddenly start working properly at 1920 X 1080 resolution with Version 0.4.169 if 3X5.6A, 3X5.6B, and 3X5.6C are all set to 0x00?
    Now, comparing the 2 files.
This is from the non-working version (I assume OpenChrome Version 0.4.169.).

________________________________________________________________
. . .
    0x6a = 0x80 (Second Display Channel and LCD Enable)
    0x6b = 0x84 (Channel 1 and 2 Clock Mode Selection)
    0x6c = 0x01 (TV Clock Control)
. . .
________________________________________________________________


This is from the working version (OpenChrome Version 0.4.0 with your patch to revert the code change.).

________________________________________________________________
. . .
    0x6a = 0x00 (Second Display Channel and LCD Enable)
    0x6b = 0x00 (Channel 1 and 2 Clock Mode Selection)
    0x6c = 0x00 (TV Clock Control)
. . .
________________________________________________________________


I personally think 3X5.6C is not related to this bug (it is related to TV clock control), so we should leave that register alone.
That means 3X5.6A and 3X5.6B are contributing to the problem.
For now, I will ignore 3X5.6A.
This is due to the fact that 3X5.6A is mostly related to IGA2 (secondary display controller), and in your case, it is not running.
Looking at CX700 UniChrome Pro II programming guide, I suspect 3X5.6B[7:6] - First Display Channel Clock Mode Selection might be contributing to the problem.
Here is an edited version of 3X5.6B.

________________________________________________________________
3X5.6B Channel 1 and 2 Clock Mode Selection

3X5.6B[7:6] - First Display Channel Clock Mode Selection
              0x: Normal
              1x: Division by 2
. . .
________________________________________________________________


For stock OpenChrome Version 0.4.169, 3X5.6B is 0x84.
This implies that IGA1 clock mode is set to division by 2 mode.
Within OpenChrome, it appears that IGA1 is set to division by 2 mode only when TV out is used (i.e., VT1622).

(In reply to Xavier Bachelot from comment #11)

Hi Xavier,

> I've reduced the needed changes to both 3d5.6b bit 7 and 3d5.6c bit 0.
> Initial registers setting is 3d5.6b = 0x84 and 3d5.6c = 0x01.
> Working registers setting is 3d5.6b = 0x04 and 3d5.6c = 0x00.
> 
> According to CX700 documentation, the older chipset with doc available :
> 
> 6b[7:6] is "First Display Channel Clock Mode Selection"
> 0x: Normal
> 1x: Division by 2
> 
> 6c[0] is "LCDCK Source Selection"
> 0: LCDCK PLL output clock
> 1: LCDCK PLL reference clock

When I posted comment #12, you got ahead of me, and you posted comment #11.
Basically, you and I were working on the same problem at the same time. (^^)
I will post my CLE266 register dump I obtained around May 22nd.
I do not know the version of OpenChrome that was running when I did the register dump.
It was obtained with Ubuntu 10.04 LTS.

Created attachment 124393 [details]
EPIA-M (CLE266) Ubuntu 10.04 LTS register dump

The version of OpenChrome is unknown, but it is definitely after Version 0.4.140.
The data was obtained on May 22nd, 2016.

(In reply to Xavier Bachelot from comment #11)

Hi Xavier,

> I've reduced the needed changes to both 3d5.6b bit 7 and 3d5.6c bit 0.
> Initial registers setting is 3d5.6b = 0x84 and 3d5.6c = 0x01.
> Working registers setting is 3d5.6b = 0x04 and 3d5.6c = 0x00.
> 
> According to CX700 documentation, the older chipset with doc available :
> 
> 6b[7:6] is "First Display Channel Clock Mode Selection"
> 0x: Normal
> 1x: Division by 2
> 
> 6c[0] is "LCDCK Source Selection"
> 0: LCDCK PLL output clock
> 1: LCDCK PLL reference clock

Do you think 3X5.6C[0] is even a factor here?
LCDCK implies IGA2, but we are dealing with only IGA1 in this situation.
I suspect 3X5.6B[7:6] is suspect here.
    I know that EPIA-M BIOS has a way to specify which type of display devices are connected to the mainboard.
What is the current setting you have?
Is it set to CRT + TV setting?
    Looking at the OpenChrome source code, it appears that division by 2 mode is used when VT1622(A) or VT1623 are used.
However, in your case, OpenChrome appears not to be going through the VIA TV encoder initialization code within OpenChrome.
Is this a situation where OpenChrome is not initializing the register unless the TV encoder is actively being used, and OpenChrome is effectively depending on the BIOS to set the register?
It is my speculation that if EPIA-M BIOS is set to CRT + TV mode, this register field in question gets set by BIOS, but under the circumstance you have encountered, it is not being initialized by OpenChrome.
The only exception is if you tried to use TV with OpenChrome, but when I tried this several months ago, it caused a hang (lockup) when I connected a TV set to EPIA-M.
However, the TV out hardware is working since the screen behaves normally when I turn the hardware on (the stuff on the TV is cloned with the VGA out).

Just to clarify the TV out, it appears that my EPIA-M TV out is working fine from a hardware perspective.
In other words, the TV out hardware is not defective.
When I turn the hardware on, TV out displays the cloned image being displayed on a VGA monitor.
The problem is when I try to boot Ubuntu 10.04 LTS, it will hang if I try to use RCA composite (i.e., RCA composite is connected to a TV).
Of course, I consider this a bug, and will eventually fix this bug.

Hi Xavier,

I did recently create a function that initializes IGA1 registers to a certain state.
It is called viaIGA1Init.
Should I include initialization of 3X5.6B related to IGA1? (i.e., 3X5.6B[7:6] only and leave other fields intact)

This might indeed be related to TV out. If I disable the call to via_tv_init in via_outputs.c (with 0.4.169), I get a working display without manually setting crtc registers 6b and 6c.
In this case, 3d5.6b = 0x04 and 3d5.6c = 0x00.

The bios was set to CRT+TV. Setting to CRT only gave me a working VGA display with unmodified 0.4.169, so the driver depends on BIOS. I guess the fix would be to initialize 3x5.6b and 3x5.6c to 0x00 in viaIGAInitCommon (rather than viaIGA1Init as you suggested, as it seems to me it is not specifically related to IGA1). What do you think ?

(In reply to Xavier Bachelot from comment #18)

Hi Xavier,

> This might indeed be related to TV out. If I disable the call to via_tv_init
> in via_outputs.c (with 0.4.169), I get a working display without manually
> setting crtc registers 6b and 6c.
> In this case, 3d5.6b = 0x04 and 3d5.6c = 0x00.

I searched through OpenChrome source code, and I did not think the code ran through the portion that explicitly uses "division by 2" mode in your case.
3X5.6B[7:6] is set inside VT1621ModeCrtc and VT1622ModeCrtc function inside via_vt162x.c.
Since both of these functions should display a message identifying that the code is running those functions (i.e., Xorg.0.log message identifying VT1621ModeCrtc or VT1622ModeCrtc), but I did not see it in the log file you attached.
CH7xxxModeCrtc function inside via_ch7xxx.c also can change this field, but since you did not run the code with Chrontel encoder, it will not execute this code.
   In my situation, the log file I uploaded is with EPIA-M (same as yours) mainboard with only VGA.
TV out will cause a hang during boot, so I did not bother with it.
You may want to take a look at the BIOS setup, and find the area where you can specify what type of display device is connected to the chipset.
Currently OpenChrome relies on this BIOS based interface (the BIOS writes the settings to what VIA calls scratch pad register; CR3A - CR3F) for non-I2C bus FP screen resolution detection.
James Simmons using this in the sort of forgotten OpenChrome DRM / KMS module, so I fully activated this functionality in a commit I made around late March 2016.

https://cgit.freedesktop.org/openchrome/xf86-video-openchrome/commit/?id=aba5302d9017e3ed993909cafdd1ff5b7b39779d

This was really critical to use these registers since there are so many non-I2C bus FP out there, and this is the only reason OpenChrome is currently working okay without the use of that big "known device table" that used exist until Version 0.3.3.
    It appears that for non-TV out use (i.e., VGA), 3X5.6B[7:6] appears not to be touched by OpenChrome.
I think this register should be set when IGA1 mode setting is done regardless of what display device is connected.
As an alternative, we can let individual display output device mode setting routines to handle this (i.e., VGA and in the future, DVI).

(In reply to Xavier Bachelot from comment #19)

Hi Xavier,

> The bios was set to CRT+TV. Setting to CRT only gave me a working VGA
> display with unmodified 0.4.169, so the driver depends on BIOS. I guess the
> fix would be to initialize 3x5.6b and 3x5.6c to 0x00 in viaIGAInitCommon
> (rather than viaIGA1Init as you suggested, as it seems to me it is not
> specifically related to IGA1). What do you think ?

Yes, that is what I started to suspect.
3X5.6B[7:6] is not being set by non-TV portion of the code, and that is a problem.
I think for now, we can possibly put the fixes into viIGAInitCommon, viaIGA1Init , and viaIGA2Init functions.
This is how I will do it.


viaIGAInitCommon function:

3X5.6B[3] -> 0


viaIGA1Init function:

- 3X5.6B[7:6] -> 00
- 3X5.6C[7:5] -> 000 (VCLK (IGA1) related)
- 3X5.6C[4] -> 0 (VCLK (IGA1) related)

viaIGA2Init function:

- 3X5.6A[7:0] -> 0x00
- 3X5.6B[5:4] -> 00
- 3X5.6B[2] -> 0
- 3X5.6B[1] -> 0
- 3X5.6C[3:1] -> 000 (LCDCK (IGA2) related)
- 3X5.6C[0] -> 0 (LCDCK (IGA2) related)


The way these functions should work is that register bits common to IGA1 and IGA2 should be initialized inside viaIGAInitCommon function.
Register bits that belong to IGA1 should be initialized inside viaIGA1Init function and the same thing for IGA2.
When IGA2 mode setting is done, it may alter some of the IGA2 registers within viaIGA2SetDisplayRegister function to the final value.
What this means is that the final register value may be different from what was initialized in viaIGA2Init.
An example of this is 3X5.6A[5] (IGA2 6-bit / 8-bit LUT selection).

(In reply to Xavier Bachelot from comment #19)

Hi Xavier,

> The bios was set to CRT+TV. Setting to CRT only gave me a working VGA
> display with unmodified 0.4.169, so the driver depends on BIOS. I guess the
> fix would be to initialize 3x5.6b and 3x5.6c to 0x00 in viaIGAInitCommon
> (rather than viaIGA1Init as you suggested, as it seems to me it is not
> specifically related to IGA1). What do you think ?

In my case, I am pretty sure I set the CLE266 BIOS setting for display output device setting to CRT only.
Probably as a result of that, I did not catch this bug at my place.
Of course, I have EPIA-M with me now, but I will need to find a case and a hard drive before I can use it (i.e., it will take a few more days.)

Created attachment 124397 [details] [review]
[PATCH] Initializing CR6A, CR6B, and CR6C when mode setting is done

Not all the register fields of CR6A (3X5.6A), CR6B (3X5.6B), and
CR6C (3X5.6C) were being initialized, and as a result, specifying
the availability of TV out in the BIOS setup will lead to analog out
(VGA) not working properly. This bug was observed with VIA Embedded
EPIA-M mainboard (CLE266 chipset with VT1622 TV encoder).

Signed-off-by: Xavier Bachelot <"Xavier's e-mail address">
Signed-off-by: Kevin Brace <"Kevin's e-mail address">

Hi Xavier,

I created a patch that can be committed soon.
You can test it, and I hope it works.
I have confirmed that this patch works with a VN896 chipset laptop (FP + VGA).
Please note that I upped the OpenChrome version to 0.4.170, so you may want to test it against it.
Of course, when I commit it, the e-mail addresses will be the real ones.

Hi Xavier,

I did alter the 3X5.6A value being initialized since messing this register up leads to a hardware hang.
This is due to the fact that 3X5.6A contains a bit that resets IGA2.

So with the patch applied and the bios set to CRT only, it works.
With CRT + TV, 3x5.6b is properly set to 0x04, but 3x5.6c is still set to 0x01 rather than 0x00 and thus no display. Forcing 6c to 0x00 gives a working display.

I need to go out now, I'll be back in 2 to 3 hours.

(In reply to Xavier Bachelot from comment #26)

Hi Xavier,

> So with the patch applied and the bios set to CRT only, it works.
> With CRT + TV, 3x5.6b is properly set to 0x04, but 3x5.6c is still set to
> 0x01 rather than 0x00 and thus no display. Forcing 6c to 0x00 gives a
> working display.
> 
> I need to go out now, I'll be back in 2 to 3 hours.

Thanks for testing the patch.
If I think about it, with your hardware, IGA2 is probably not running at this point since you probably do not have TV connected, and you should keep it that way for now (I do know that TV out is currently broken. You can hook up a TV if you can, but when I tested it, it caused a hang. The OS was Ubuntu 10.04 LTS.).
This will likely mean that the IGA2 initialization function (viaIGA2Init) will never be executed.
I thought the situation was odd, so I looked at UniChrome Pro II documentation.
It says 3X5.6C[0] selects LCDCK PLL source.
LCDCK is for IGA2.
I also looked at viafbdev.c (VIA FB driver), and it appears that the role of 3X5.6C was changed starting with CX700.
I am currently analyzing the source code, and a few patch will probably not be ready in a few hours.
I hope to have a new patch ready by tomorrow.

Created attachment 124430 [details] [review]
Proposed CLE266 VGA TV out Bug Fix

Initializing CR6A, CR6B, and CR6C when mode setting is done

Not all the register fields of CR6A (3X5.6A), CR6B (3X5.6B), and 
CR6C (3X5.6C) were being initialized, and as a result, specifying 
the availability of TV out in the BIOS setup will lead to analog out 
(VGA) not working properly. Specifically, CR6B[7:6] and CR6C[0] 
were causing problems. CR6C controls DIP0 (Digital Interface Port 0) 
and exists at this address only in CLE266 (DIP0 is called DVP0 since 
KM400 chipset, and DVP0 control register is located at CR96.). 
This bug was observed with VIA Embedded EPIA-M mainboard (CLE266 chipset 
with VT1622 TV encoder), and likely only affects CLE266 chipset.

Signed-off-by: Xavier Bachelot <"Xavier's e-mail address">
Signed-off-by: Kevin Brace <"Kevin's e-mail address">

Latest patch works fine with bios set to CRT + TV. Tested both with and w/o tv out (s-video) connected.

Thanks for the fix, this was a tricky one.

(In reply to Xavier Bachelot from comment #29)

Hi Xavier,

> Latest patch works fine with bios set to CRT + TV. Tested both with and w/o
> tv out (s-video) connected.
> 
> Thanks for the fix, this was a tricky one.

Okay, I am glad the fix is working.
I will need to test the fix with my Foxconn KM400 mainboard when I get back, but for now, I think we are okay with the fix.
I will make the commit right now with your name in it.
    I am actually surprised that your S-Video is working.
When I tried it with RCA composite, it caused Ubuntu 10.04 LTS to hang during boot time.
As a test of the TV out code, can you actually try RCA composite?
I am wondering if OpenChrome is assigning TV out to IGA2 or if it is cloning it against IGA1.
I know that VT1622(A) is only capable around 800 X 600 resolution, so this may actually limit your VGA screen resolution.
I am glad you got the patch working since I have a few more commits I was planning to make.

Fixed by commit 97b17c6.

https://cgit.freedesktop.org/openchrome/xf86-video-openchrome/commit/?id=97b17c6823bdf2b0fe07574da2dea464d1ea3730