On 07/03/2012 16:53, Eric Nelson wrote:
Hi Stefano,
Hi Eric,
I did the same search but was apparently not as persistent as you were. The symptom is simple: Video won't play back on Android (R13.1 ICS) without the system revision but play nicely with it.
Ok, we know the symptoms...
I think I just found the culprit, and it's in userspace, not in the kernel. In package imx-lib-11.11.01, file vpu/vpu.c, there's this routine that sets a global system_rev based on /proc/cpuinfo:
static int get_system_rev(void) { FILE *fp; char buf[1024]; int nread; char *tmp, *rev; int ret = -1;
fp = fopen("/proc/cpuinfo", "r"); if (fp == NULL) { perror("/proc/cpuinfo\n"); return ret; } nread = fread(buf, 1, sizeof(buf), fp); fclose(fp); if ((nread == 0) || (nread == sizeof(buf))) { fclose(fp); return ret; } buf[nread] = '\0'; tmp = strstr(buf, "Revision"); if (tmp != NULL) { rev = index(tmp, ':'); if (rev != NULL) { rev++; system_rev = strtoul(rev, NULL, 16); ret = 0; } } return ret;}
The global is then exported via macros: vpu/vpu_io.c:unsigned int system_rev; vpu/vpu_io.c:static int get_system_rev(void) vpu/vpu_io.c: system_rev = strtoul(rev, NULL, 16); vpu/vpu_io.c: ret = get_system_rev(); vpu/vpu_lib.h:extern unsigned int system_rev; vpu/vpu_lib.h:#define mxc_cpu() (system_rev >> 12) vpu/vpu_lib.h:#define mxc_cpu_rev() (system_rev & 0xFF)
and used to find the firmware file: vpu/vpu_util.c: sprintf(temp_str, "vpu_fw_imx%2x.bin", mxc_cpu());
...all to support the userspace I/O for the VPU.
Understood - this is really crappy, because it makes so absurd dependencies that is very easy to break - and when it happens, nobody knows why, as we find now. Really this is a problem neither in u-boot nor in kernel...
We are way off topic here,
Well, we have now the cause...
but I certainly hope we can address this in the future and get a real driver written for the VPU.
..in the mainline kernel...
Anyway, the ATAG is supported and as I already said quite common for U-Boot boards. The commit message " Freescale 2.6.38 (Non-DT) kernels require the revision atag to enable the VPU." should be extended explaining the real cause (if known) or changed dropping VPU because there is no clear relationship between the ATAG and the issue.
How about something more generic like this? "Freescale Linux distributions depend on system_rev".
Because you deeply investigated and found the reason, I propose you add a full description indicating that the imx lib libraries depend on the system_rev in kernel to transfer the correct firmware. So we know it is neither a problem in u-boot nor in kernel, but we as u-bootlers are fair with some bad implemented libraries....
Normally I would say that the fix should be done where the bug is - we are introducing a work-around for a problem in user space. But as I stated previously, the revision tag is used on a lot of ARM boards, and there is no reason to reject it only on this board.
Best regards, Stefano Babic