Closed Bug 620526 Opened 14 years ago Closed 14 years ago

yv12_to_rgb565_neon should not be used on CPUs without NEON

Categories

(Core :: Graphics, defect)

ARM
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
fennec 2.0b4+ ---

People

(Reporter: mwu, Assigned: romaxa)

References

Details

(Keywords: crash)

Attachments

(3 files)

Not all modern ARM cpus have NEON. In particular, the tegra does not have it.
tracking-fennec: --- → ?
Keywords: crash
tracking-fennec: ? → 2.0b4+
Attached patch ARM neon check (deleted) — Splinter Review
Assignee: nobody → romaxa
Status: NEW → ASSIGNED
Attachment #498966 - Flags: review?(blassey.bugs)
Comment on attachment 498966 [details] [diff] [review]
ARM neon check

this certainly looks right to me, but its not my module
Attachment #498966 - Flags: review?(chris.double)
Attachment #498966 - Flags: review?(blassey.bugs)
Attachment #498966 - Flags: feedback+
(In reply to comment #1)
> Created attachment 498966 [details] [diff] [review]
> ARM neon check

I meant we need to have a runtime check.
(In reply to comment #3)
> (In reply to comment #1)
> > Created attachment 498966 [details] [diff] [review] [details]
> > ARM neon check
> 
> I meant we need to have a runtime check.

both would be good, but yea you'd need a runtime check for the tegra to work
yea. We currently do just about the same check in jstracer.cpp and jstracer.c.
(In reply to comment #5)
> something like this:
> http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/libpixman/src/pixman-cpu.c#223
> ?

Well, closer to http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/libpixman/src/pixman-cpu.c#247 but yeah, need some variant of it. There's also cpu detection in media/libvpx/vpx_ports/arm_cpudetect.c and another cpu detector in js. We have too many of them.
> Well, closer to
> http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/libpixman/src/pixman-cpu.c#247

How about something like this?

static pixman_bool_t
pixman_have_ycpcr_to_rgb565 ()
{
    int fd;
    Elf32_auxv_t aux;
	pixman_bool_t arm_has_neon = 0;
	
    fd = open ("/proc/self/auxv", O_RDONLY);
    if (fd >= 0)
    {
	while (read (fd, &aux, sizeof(Elf32_auxv_t)) == sizeof(Elf32_auxv_t))
	{
	    if (aux.a_type == AT_HWCAP)
	    {
			arm_has_neon = (hwcap & 4096) != 0;
			break;
	    }
	}
	close (fd);
    }
    
    return arm_has_neon;
}
tom could you attach working patch and ask for review?
Attached patch Patch - have_ycbcr_to_rgb565 (deleted) — Splinter Review
I copied the logic from piximan-cpu file and moved it 
here for runtime test in ycbcr folder.  This file could easily be moved
to a more central common location.

There is a test case in the following attachment.
Attachment #501747 - Flags: review?(mwu)
Attached file Test case for previous patch (deleted) —
Comment on attachment 501747 [details] [diff] [review]
Patch - have_ycbcr_to_rgb565

I suspect Chris Double is the right reviewer for this.
Attachment #501747 - Flags: review?(mwu) → review?(chris.double)
Attachment #498966 - Flags: review?(chris.double) → review?(jmuizelaar)
Attachment #501747 - Flags: review?(chris.double) → review?(jmuizelaar)
Attachment #498966 - Flags: review?(jmuizelaar) → review+
Attachment #501747 - Flags: review?(jmuizelaar) → review+
g++-4.2 -arch i386 -o ycbcr_to_rgb565.o -c  -fvisibility=hidden -D_IMPL_NS_GFX -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DSTATIC_EXPORTABLE_JS_API -DOSTYPE=\"Darwin\" -DOSARCH=Darwin  -I/builds/slave/try-osx64/build/gfx/ycbcr -I. -I../../dist/include -I../../dist/include/nsprpub  -I/builds/slave/try-osx64/build/obj-firefox/i386/dist/include/nspr -I/builds/slave/try-osx64/build/obj-firefox/i386/dist/include/nss       -fPIC  -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -gdwarf-2 -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fpascal-strings -fno-common -fshort-wchar -pthread -DNO_X11  -DNDEBUG -DTRIMMED -gdwarf-2 -O3 -fomit-frame-pointer   -DMOZILLA_CLIENT -include ../../mozilla-config.h /builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp
/builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:60:17: error: elf.h: No such file or directory
/builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp: In function 'int have_ycbcr_to_rgb565()':
/builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:96: error: 'Elf32_auxv_t' was not declared in this scope
/builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:96: error: expected `;' before 'aux'
/builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:101: error: 'aux' was not declared in this scope
/builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:103: error: 'AT_HWCAP' was not declared in this scope
/builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:105: error: 'uint32_t' was not declared in this scope
/builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:105: error: expected `;' before 'hwcap'
/builds/slave/try-osx64/build/gfx/ycbcr/ycbcr_to_rgb565.cpp:106: error: 'hwcap' was not declared in this scope



Does not compile on the mac.
http://hg.mozilla.org/mozilla-central/rev/c0db6f2597a2 with build bustage fixes.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment on attachment 501747 [details] [diff] [review]
Patch - have_ycbcr_to_rgb565

So, for those of you still building non-libxul:

(3x)
>+int have_ycbcr_to_rgb565 ()
NS_GFX_(int) have_ycbcr_to_rgb565 ()

>+int have_ycbcr_to_rgb565();
#include "gfxCore.h"
NS_GFX_(int) have_ycbcr_to_rgb565();
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: