On Tue, Feb 05, 2013 at 03:51:55PM +0800, Aaron.Chen 陈俊杰 wrote:
> This is an initial patch for Siliconmotion Graphics chips. It add SM750
> chip support with 2d accelerate and hw curser support. It is a complete
> new driver. So the patch is a little bit big. Is it OK for review?
Adding a new fbdev driver while established consensus pretty much boils
down to fbdev being a dead subsystem which should only be used for legacy
applications and drivers: Not good. You should implement this as a drm
modesetting driver, and if required base your 2d acceleration on top of
the drm fb helpers. Or just implement a drm/gem driver to expose this.
Additionally you should include the appropriate mailing lists, which is
linux-fbdev for fbdev drivers.
On a very quick read the patch has serious issues:
- checkpatch.pl:
total: 2 errors, 779 warnings, 7961 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
scripts/cleanfile
- Remants of hungarian notation (dunno whether checkpatch checks for
those, too much other stuff flying by, but it really stuck out).
- Not using the linux i2c layer in the ddk750_hwi2c.c file.
- Reimplementing i2c bit-banging code in the ddk750_swi2c.c
- Adding a new implementation of a sil 164 dvi driver, we already have at
least two in drivers/gpu/drm/i2c/sil164_drv.c and
drivers/gpu/drm/i915/dvo_sil164.c. This should be unified and probably
use the drm encoder slave framework.
At that point I've given up on further review, since there's clearly a lot
of work left to do. Please review SubmittingPatches from the kernel
documentation carefully. Also cc'ing our dear staging maintainer, he
should be able to point you at further useful resources for getting this
going.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
