Bug 23144

(In reply to comment #0)
> Created an attachment (id=28356) [details]
> Add 16 and 32 bit RGB formats to textured video. R6xx/R7xx only.
> 
> RGB formats would be useful for some applications.
I agree that they are useful sometimes. Should be possible for all chips easily though.
Also, any reason you restrict the interpolation parameter to rgb formats? If you really want ugly filtering (not sure this attribute really is a good idea), there doesn't seem to be a good reason why you'd not allow that for other formats. Looks completely orthogonal to rgb formats to me.

Created attachment 29345 [details] [review]
Add 16 and 32 bit RGB formats to textured video. R6xx/R7xx only.

(In reply to comment #1)
> (In reply to comment #0)
> > Created an attachment (id=28356) [details] [details]
> > Add 16 and 32 bit RGB formats to textured video. R6xx/R7xx only.
> > 
> > RGB formats would be useful for some applications.
> I agree that they are useful sometimes. Should be possible for all chips easily
> though.

Other chips should be supported as well as r6xx/r7xx. But I cannot test r100-r500 because I don't have them. So I didn't touch its part.


> Also, any reason you restrict the interpolation parameter to rgb formats? If
> you really want ugly filtering (not sure this attribute really is a good idea),
> there doesn't seem to be a good reason why you'd not allow that for other
> formats. Looks completely orthogonal to rgb formats to me.

The interpolation parameter could also be applied to YUV formats. But I dropped interpolation part because I thought that image format and interpolation method were two separate things.

Created attachment 41588 [details] [review]
Implement RGB formats for textured-Xv with pass-through shader (i.e. no hue/contrast/brightness/saturation adjustment)

Created attachment 41589 [details] [review]
Implement hue/contrast/brightness/saturation adjustments for RGB textured-Xv

Created attachment 41590 [details] [review]
Test program for testing RGB output via textured-Xv

My two patches add support for RGB video output with the textured-video Xv output driver. Patch 1 (should) add the basic support similar to (and based upon) the patches of Kusanagi, but for r3xx. Patch 2 extends the support with hue/contrast/brightness/saturation/gamma correction support. Attachment 3 [details] is not a patch but a small tool I wrote to test various output formats. It's not the greatest software ever but it works.  Patches developed & tested on Mobility Radeon 9600 M10 (RV350).

The following formats are supported:
FOURCC_RGBA32, e.g. A8R8G8B8
FOURCC_RGB16, e.g. R5G6B5
FOURCC_RGBT16, e.g. A1R5G5B5
FOURCC_RGB15, e.g. R5G5B5
FOURCC_BGR15, e.g. B5G5R5

FOURCC_BGR15 is especially useful since it appears to be the native pixel format for Playstation 1, and possibly other (old?) game consoles. This means that emulators can output directly without having to perform a costly conversion step in software. I have some proof-of-concept code for Pcsx working on my laptop.

FOURCC_RGB24 is notably missing because I could not find a way to output it in hardware directly (I think there were some comments on IRC stating that it is in fact not possible). Therefor I think it is best to leave the conversion to the application instead.

Finally I noticed that there appears to be a bug with rgb32 output when using certain image widths (I'll try to find an example later). I have not yet been able to find what causes this. Any help would be appreciated.

Created attachment 41593 [details]
This image displays wrong in RGB32 mode (something seems amiss with the image stride)

(In reply to comment #6)
> Finally I noticed that there appears to be a bug with rgb32 output when using
> certain image widths (I'll try to find an example later). I have not yet been
> able to find what causes this. Any help would be appreciated.

The below part of your patch incorrectly open-codes the RADEON_ALIGN macro for dstPitch. Using the macro instead might work better. :)

+    case FOURCC_RGBA32:
+	srcPitch = width << 2;
+	dstPitch = ((dst_width << 2) + hw_align) & ~hw_align;
+	break;

(In reply to comment #8)
> (In reply to comment #6)
> > Finally I noticed that there appears to be a bug with rgb32 output when using
> > certain image widths (I'll try to find an example later). I have not yet been
> > able to find what causes this. Any help would be appreciated.
> 
> The below part of your patch incorrectly open-codes the RADEON_ALIGN macro for
> dstPitch. Using the macro instead might work better. :)
> 
> +    case FOURCC_RGBA32:
> +    srcPitch = width << 2;
> +    dstPitch = ((dst_width << 2) + hw_align) & ~hw_align;
> +    break;

Yes, using the RADEON_ALIGN macro indeed fixed the issue. I think that piece of code was originally from kusanagi's patch, and looked good to me. I didn't look into the RADEON_ALIGN macro or why it was not used here, I kind of assumed that there was a reason for the difference. Thanks for pointing that out so fast! I see now why that wouldn't work. I'll refrain from posting a new patch since the change is so small, and this is not the final patch.

Created attachment 45133 [details] [review]
Xv: Add RGB formats to r600+

Can anyone test this patch? It's working on my Redwood.

Mass closure: This bug has been untouched for more than six years, and is not
obviously still valid. Please reopen this bug or file a new report if you continue to experience issues with current releases.