Re: [Xpert]Radeon clone mode bug(s) + fix
Ok, as I still have no idea about the procedure used for working with the XFree86 code in general, and with ATI Radeon driver in particular, there are (hopefully) simple questions: - what should I do in order to have a patch which fixes obvious bugs applied? - what should I do if I want to work on much less obvious things like adding support for user-selectable output config (possibly including TV output, and fixing existing problems like the one with TV connected to TV-out)? -- Krzysztof Halasa Network Administrator ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
Another idea: Currently, DisplayType (and CloneType) is an enum. How about making them bitfields? Doing so would make it possible to have more than one physical display connected to a CRT controller. This is at least possible in combinations something + CRT, but might even be possible with 3 devices (if I found a clock circuits settings for such setup). I don't mean cloning here, of course. Another thing: wouldn't it be better to drop Clone* and just use a primary/secondary display, possibly using common frame buffer when cloning? It would greatly simplify the code. I don't know XFree86 internals much, so I don't know if it would be easier. -- Krzysztof Halasa Network Administrator ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
hy0 [EMAIL PROTECTED] writes: Yes, this is a bug. As Michel suggested, we should use a dedicated RADEONCloneAdjustFrame() function to separate CloneAdjustFrame from pScrn-AdjustFrame() completely. Ok. Is my patch going to be applied? -- Krzysztof Halasa Network Administrator ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
Le mer 18/09/2002 à 07:40, Vladimir Dergachev a écrit : Judging by the patch this is not anywhere near GATOS modifications and looks to be pretty reasonable.. I am not current on recent XFree86 CVS though. BTW, I suppose this is a FAQ but why aren't the GATOS modifications folded into main XFree86 driver ? Xav ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
On Mit, 2002-09-18 at 07:40, Vladimir Dergachev wrote: On 18 Sep 2002, Michel [ISO-8859-1] Dänzer wrote: On Die, 2002-09-17 at 23:39, Krzysztof Halasa wrote: 1. Cloning the display and using XVideo halts the system when moving the viewport. I've traced that to the fact XVideo routines change pScrn-AdjustFrame to their internal function, which isn't prepared to be called for clone display. I've changed all calls of pScrn-AdjustFrame() for clone display to just RADEONAdjustFrame() - clone display has nothing to do with XVideo anyway. I don't see anything about AdjustFrame in radeon_video.c, must be a GATOS thing? Anyway, wouldn't it be better to fix that other AdjustFrame function for clone? Judging by the patch this is not anywhere near GATOS modifications and looks to be pretty reasonable.. I just don't understand what the changes are trying to achieve, much less what they have to do with XVideo. I'd appreciate some insights. -- Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
On Wed, Sep 18, 2002 at 10:24:26AM +0200, Xavier Bestel wrote: Le mer 18/09/2002 a 07:40, Vladimir Dergachev a ecrit : Judging by the patch this is not anywhere near GATOS modifications and looks to be pretty reasonable.. I am not current on recent XFree86 CVS though. BTW, I suppose this is a FAQ but why aren't the GATOS modifications folded into main XFree86 driver ? As I've been told by someone on this list, GATOS functionality will get merged before next major release (ie, 4.3.0). ./danfe ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
Michel Dänzer [EMAIL PROTECTED] writes: 1. Cloning the display and using XVideo halts the system when moving the viewport. I've traced that to the fact XVideo routines change pScrn-AdjustFrame to their internal function, which isn't prepared to be called for clone display. I've changed all calls of pScrn-AdjustFrame() for clone display to just RADEONAdjustFrame() - clone display has nothing to do with XVideo anyway. I don't see anything about AdjustFrame in radeon_video.c, must be a GATOS thing? Anyway, wouldn't it be better to fix that other AdjustFrame function for clone? No, I'm using XFree86 CVS driver and not the GATOS one. In xc/programs/Xserver/hw/xfree86/common/xf86xv.c, xf86XVScreenInit() does pScrn-AdjustFrame = xf86XVAdjustFrame. xf86XVAdjustFrame() in turn call the original ScreenPriv-AdjustFrame() and then does things related to Xvideo. However, it thinks it operates on primary display, while in fact the original RADEONAdjustFrame would be called for cloned display (not the primary nor secondary - the cloned display has nothing to do with the rest of XFree86, except that share the frame buffer and cursor posision). IMHO we should just call RADEONAdjustFrame for cloned display as it is an internal task, not related to any other XFree86 activity. The rest of the server only knows about the primary (and secondary) screen. So this is the correct fix and not a workaround. -- Krzysztof Halasa Network Administrator ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
On Mit, 2002-09-18 at 14:05, Krzysztof Halasa wrote: Michel Dänzer [EMAIL PROTECTED] writes: 1. Cloning the display and using XVideo halts the system when moving the viewport. I've traced that to the fact XVideo routines change pScrn-AdjustFrame to their internal function, which isn't prepared to be called for clone display. I've changed all calls of pScrn-AdjustFrame() for clone display to just RADEONAdjustFrame() - clone display has nothing to do with XVideo anyway. I don't see anything about AdjustFrame in radeon_video.c, must be a GATOS thing? Anyway, wouldn't it be better to fix that other AdjustFrame function for clone? No, I'm using XFree86 CVS driver and not the GATOS one. In xc/programs/Xserver/hw/xfree86/common/xf86xv.c, xf86XVScreenInit() does pScrn-AdjustFrame = xf86XVAdjustFrame. xf86XVAdjustFrame() in turn call the original ScreenPriv-AdjustFrame() and then does things related to Xvideo. However, it thinks it operates on primary display, while in fact the original RADEONAdjustFrame would be called for cloned display (not the primary nor secondary - the cloned display has nothing to do with the rest of XFree86, except that share the frame buffer and cursor posision). So the second part of the code in xf86XVAdjustFrame() is the problem? IMHO we should just call RADEONAdjustFrame for cloned display as it is an internal task, not related to any other XFree86 activity. The rest of the server only knows about the primary (and secondary) screen. So this is the correct fix and not a workaround. But what if pScrn-AdjustFrame() is called from outside the driver? -- Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
On 18 Sep 2002, Michel [ISO-8859-1] Dänzer wrote: On Mit, 2002-09-18 at 07:40, Vladimir Dergachev wrote: On 18 Sep 2002, Michel [ISO-8859-1] Dänzer wrote: On Die, 2002-09-17 at 23:39, Krzysztof Halasa wrote: 1. Cloning the display and using XVideo halts the system when moving the viewport. I've traced that to the fact XVideo routines change pScrn-AdjustFrame to their internal function, which isn't prepared to be called for clone display. I've changed all calls of pScrn-AdjustFrame() for clone display to just RADEONAdjustFrame() - clone display has nothing to do with XVideo anyway. I don't see anything about AdjustFrame in radeon_video.c, must be a GATOS thing? Anyway, wouldn't it be better to fix that other AdjustFrame function for clone? Judging by the patch this is not anywhere near GATOS modifications and looks to be pretty reasonable.. I just don't understand what the changes are trying to achieve, much less what they have to do with XVideo. I'd appreciate some insights. Most likely what happens is that generic Xvideo layer wraps some functions in order to trace certain events. So calling pScrn-AdjustFrame results in calling generic functions (and not whatever was assigned to pScrn-AdjustFrame during initialization) resulting in conflict. Vladimir Dergachev -- Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
Michel Dänzer [EMAIL PROTECTED] writes: So the second part of the code in xf86XVAdjustFrame() is the problem? Yes, the problem is that part is called at all. IMHO we should just call RADEONAdjustFrame for cloned display as it is an internal task, not related to any other XFree86 activity. The rest of the server only knows about the primary (and secondary) screen. So this is the correct fix and not a workaround. But what if pScrn-AdjustFrame() is called from outside the driver? AdjustFrame for cloned screen should never be called from outside I think. The outside doesn't even know of the clone display. For primary and secondary displays, my patch doesn't change anything. -- Krzysztof Halasa Network Administrator ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
On Mit, 2002-09-18 at 15:23, Krzysztof Halasa wrote: Michel Dänzer [EMAIL PROTECTED] writes: IMHO we should just call RADEONAdjustFrame for cloned display as it is an internal task, not related to any other XFree86 activity. The rest of the server only knows about the primary (and secondary) screen. So this is the correct fix and not a workaround. But what if pScrn-AdjustFrame() is called from outside the driver? AdjustFrame for cloned screen should never be called from outside I think. The outside doesn't even know of the clone display. Ah, I was missing the 1 passed as a flag to RADEONAdjustFrame(), so I didn't see the difference to pScrn-AdjustFrame() being called by driver independent code. -- Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
On Mit, 2002-09-18 at 16:19, Michel Dänzer wrote: On Mit, 2002-09-18 at 15:23, Krzysztof Halasa wrote: Michel Dänzer [EMAIL PROTECTED] writes: IMHO we should just call RADEONAdjustFrame for cloned display as it is an internal task, not related to any other XFree86 activity. The rest of the server only knows about the primary (and secondary) screen. So this is the correct fix and not a workaround. But what if pScrn-AdjustFrame() is called from outside the driver? AdjustFrame for cloned screen should never be called from outside I think. The outside doesn't even know of the clone display. Ah, I was missing the 1 passed as a flag to RADEONAdjustFrame(), so I didn't see the difference to pScrn-AdjustFrame() being called by driver independent code. I think a dedicated RADEONCloneAdjustFrame() function would be less confusing though, and wouldn't break if 1 should ever have a driver independent meaning as a flag to AdjustFrame(). -- Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
Michel Dänzer [EMAIL PROTECTED] writes: So the second part of the code in xf86XVAdjustFrame() is the problem? Yes, the problem is that part is called at all. IMHO we should just call RADEONAdjustFrame for cloned display as it is an internal task, not related to any other XFree86 activity. The rest of the server only knows about the primary (and secondary) screen. So this is the correct fix and not a workaround. But what if pScrn-AdjustFrame() is called from outside the driver? AdjustFrame for cloned screen should never be called from outside I think. The outside doesn't even know of the clone display. Yes, this is a bug. As Michel suggested, we should use a dedicated RADEONCloneAdjustFrame() function to separate CloneAdjustFrame from pScrn-AdjustFrame() completely. For primary and secondary displays, my patch doesn't change anything. Some parts of clone mode and secondary display code were separated intentionally, though they are identical at present. Clone mode code can be extended to twinview like feature if there is a need in future. -- Krzysztof Halasa Network Administrator ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
[Xpert]Radeon clone mode bug(s) + fix
Hi,
I hope this is the correct place to report bugs and send bugfixes, if not,
please let me know.
The attached patch fixes few bugs in Radeon driver. Tested with R8500LE
on Linux with XFree86 4.2.0 (RedHat package) with current CVS radeon
driver.
1. Cloning the display and using XVideo halts the system when moving
the viewport. I've traced that to the fact XVideo routines change
pScrn-AdjustFrame to their internal function, which isn't prepared
to be called for clone display.
I've changed all calls of pScrn-AdjustFrame() for clone display
to just RADEONAdjustFrame() - clone display has nothing to do with
XVideo anyway.
2. I've simplified some conditional code.
3. User should now be able to specify sync polarity for clone modes
(using ModeLine).
4. Clone mode %s in config file is used is now being displayed
correctly.
Some of these changes might be useful for 4.2.0/4.2.1 as well, but I
haven't tested that.
Another question: I'm possibly going to implement some more changes
to radeon driver, mainly related to connecting different output
devices (DFP, CRT, TV, etc) to primary/secondary CRT controllers
(current driver is unusable if you have something connected to TV-out,
and you have only CRT and no DVI device). The other thing would be
merging (portions of) GATOS code.
How should I do that? Is anyone working on similar things?
--
Krzysztof Halasa
Network Administrator
--- xc/programs/Xserver/hw/xfree86/drivers/ati-new/radeon.h Thu Jul 11 22:11:51 2002
+++ xc/programs/Xserver/hw/xfree86/drivers/ati/radeon.h Tue Sep 17 22:40:28 2002
-511,6 +513,7
extern int RADEONMinBits(int val);
extern voidRADEONInitVideo(ScreenPtr);
+extern voidRADEONAdjustFrame(int scrnIndex, int x, int y, int flags);
#ifdef XF86DRI
extern BoolRADEONDRIScreenInit(ScreenPtr pScreen);
--- xc/programs/Xserver/hw/xfree86/drivers/ati-new/radeon_cursor.c Thu Jul 11 22:11:51 2002
+++ xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_cursor.c Tue Sep 17 22:42:49 2002
-154,7 +154,7
if ((X0 = 0 || Y0 = 0)
((info-CloneFrameX0 != X0) || (info-CloneFrameY0 != Y0))) {
- pScrn-AdjustFrame(pScrn-scrnIndex, X0, Y0, 1);
+ RADEONAdjustFrame(pScrn-scrnIndex, X0, Y0, 1);
info-CloneFrameX0 = X0;
info-CloneFrameY0 = Y0;
}
-168,18 +168,9
| ((xorigin ? 0 : x) 16)
| (yorigin ? 0 : y)));
OUTREG(RADEON_CUR_OFFSET, info-cursor_start + yorigin * 16);
-} else {
- OUTREG(RADEON_CUR2_HORZ_VERT_OFF, (RADEON_CUR2_LOCK
- | (xorigin 16)
- | yorigin));
- OUTREG(RADEON_CUR2_HORZ_VERT_POSN, (RADEON_CUR2_LOCK
- | ((xorigin ? 0 : x) 16)
- | (yorigin ? 0 : y)));
- OUTREG(RADEON_CUR2_OFFSET,
- info-cursor_start + pScrn-fbOffset + yorigin * 16);
}
-if (info-Clone) {
+if (info-IsSecondary || info-Clone) {
OUTREG(RADEON_CUR2_HORZ_VERT_OFF, (RADEON_CUR2_LOCK
| (xorigin 16)
| yorigin));
--- xc/programs/Xserver/hw/xfree86/drivers/ati-new/radeon_driver.c Tue Sep 17 18:33:16 2002
+++ xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_driver.c Tue Sep 17 23:00:45 2002
-616,33 +632,16
RADEONInfoPtr info = RADEONPTR(pScrn);
unsigned char *RADEONMMIO = info-MMIO;
-if (!info-IsSecondary) {
- switch(info-DisplayType) {
- case MT_LCD:
- case MT_CRT:
- case MT_DFP:
- OUTREGP(RADEON_CRTC_EXT_CNTL,
- RADEON_CRTC_DISPLAY_DIS |
- RADEON_CRTC_VSYNC_DIS |
- RADEON_CRTC_HSYNC_DIS,
- ~(RADEON_CRTC_DISPLAY_DIS |
- RADEON_CRTC_VSYNC_DIS |
- RADEON_CRTC_HSYNC_DIS));
- break;
+if (!info-IsSecondary)
+ OUTREGP(RADEON_CRTC_EXT_CNTL,
+ RADEON_CRTC_DISPLAY_DIS |
+ RADEON_CRTC_VSYNC_DIS |
+ RADEON_CRTC_HSYNC_DIS,
+ ~(RADEON_CRTC_DISPLAY_DIS |
+ RADEON_CRTC_VSYNC_DIS |
+ RADEON_CRTC_HSYNC_DIS));
- case MT_NONE:
- default:
- break;
- }
- if (info-Clone)
- OUTREGP(RADEON_CRTC2_GEN_CNTL,
- RADEON_CRTC2_DISP_DIS |
- RADEON_CRTC2_VSYNC_DIS |
- RADEON_CRTC2_HSYNC_DIS,
- ~(RADEON_CRTC2_DISP_DIS |
- RADEON_CRTC2_VSYNC_DIS |
- RADEON_CRTC2_HSYNC_DIS));
-} else {
+if (info-IsSecondary || info-Clone)
OUTREGP(RADEON_CRTC2_GEN_CNTL,
RADEON_CRTC2_DISP_DIS |
RADEON_CRTC2_VSYNC_DIS |
-650,7 +649,6
~(RADEON_CRTC2_DISP_DIS |
RADEON_CRTC2_VSYNC_DIS |
RADEON_CRTC2_HSYNC_DIS));
-}
}
/* Unblank screen */
-659,45 +657,19
RADEONInfoPtr info = RADEONPTR(pScrn);
unsigned char *RADEONMMIO = info-MMIO;
-if (!info-IsSecondary) {
- switch (info-DisplayType) {
- case MT_LCD:
- case MT_CRT:
- case MT_DFP:
- OUTREGP(RADEON_CRTC_EXT_CNTL,
- RADEON_CRTC_CRT_ON,
- ~(RADEON_CRTC_DISPLAY_DIS |
- RADEON_CRTC_VSYNC_DIS |
- RADEON_CRTC_HSYNC_DIS));
- break;
-
- case MT_NONE:
- default:
- break;
- }
- if (info-Clone)
- OUTREGP(RADEON_CRTC2_GEN_CNTL,
- 0,
- ~(RADEON_CRTC2_DISP_DIS |
- RADEON_CRTC2_VSYNC_DIS |
-
Re: [Xpert]Radeon clone mode bug(s) + fix
On Die, 2002-09-17 at 23:39, Krzysztof Halasa wrote: 1. Cloning the display and using XVideo halts the system when moving the viewport. I've traced that to the fact XVideo routines change pScrn-AdjustFrame to their internal function, which isn't prepared to be called for clone display. I've changed all calls of pScrn-AdjustFrame() for clone display to just RADEONAdjustFrame() - clone display has nothing to do with XVideo anyway. I don't see anything about AdjustFrame in radeon_video.c, must be a GATOS thing? Anyway, wouldn't it be better to fix that other AdjustFrame function for clone? Or is this about fbdevHWAdjustFrame? Slightly confused. -- Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
Re: [Xpert]Radeon clone mode bug(s) + fix
On 18 Sep 2002, Michel [ISO-8859-1] Dänzer wrote: On Die, 2002-09-17 at 23:39, Krzysztof Halasa wrote: 1. Cloning the display and using XVideo halts the system when moving the viewport. I've traced that to the fact XVideo routines change pScrn-AdjustFrame to their internal function, which isn't prepared to be called for clone display. I've changed all calls of pScrn-AdjustFrame() for clone display to just RADEONAdjustFrame() - clone display has nothing to do with XVideo anyway. I don't see anything about AdjustFrame in radeon_video.c, must be a GATOS thing? Anyway, wouldn't it be better to fix that other AdjustFrame function for clone? Judging by the patch this is not anywhere near GATOS modifications and looks to be pretty reasonable.. I am not current on recent XFree86 CVS though. Vladimir Dergachev Or is this about fbdevHWAdjustFrame? Slightly confused. -- Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert ___ Xpert mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/xpert
