RE: [PATCHv16 0/7] of: add display helper
Hi Steffen, On Thu, Jan 10, 2013 at 01:45:41, Steffen Trumtrar wrote: I think the series was tested at least with - imx6q: sabrelite, sabresd - imx53: tqma53/mba53 - omap: DA850 EVM, AM335x EVM, EVM-SK To clarify, This series was tested with DT boot (making use of the functionalities provided by this series) on AM335X EVM AM335X EVM-SK. DA850 EVM was tested with non DT boot along with this series (to make sure that non-DT boot on DA850 EVM is not broken, and it needed the additional change that was mentioned earlier) Sorry that my initial reply did not express what I wanted to. Regards Afzal
RE: [PATCHv16 5/7] fbmon: add of_videomode helpers
Hi Steffen, On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote: On Mon, Jan 07, 2013 at 06:10:13AM +, Mohammed, Afzal wrote: This breaks DaVinci (da8xx_omapl_defconfig), following change was required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS is not defined. There may be better solutions, following was the one that was used by me to test this series. I just did a quick make da8xx_omapl_defconfig make and it builds just fine. On what version did you apply the series? At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet. But fixing this shouldn't be a problem. You are right, me idiot, error will happen only upon try to make use of of_get_fb_videomode() (defined in this patch) in the da8xx-fb driver (with da8xx_omapl_defconfig), to be exact upon adding, video: da8xx-fb: obtain fb_videomode info from dt of my patch series. The change as I mentioned or something similar would be required as any driver that is going to make use of of_get_fb_videomode() would break if CONFIG_OF_VIDEOMODE or CONFIG_FB_MODE_HELPERS is not defined. And testing was done over v3.8-rc2. +#if IS_ENABLED(CONFIG_OF_VIDEOMODE) As _OF_VIDEOMODE is a bool type CONFIG, isn't, #ifdef CONFIG_OF_VIDEOMODE sufficient ? Yes, that is right. But I think IS_ENABLED is the preferred way to do it, isn't it? Now I realize it is. Regards Afzal
RE: [PATCHv16 5/7] fbmon: add of_videomode helpers
Hi Rob, On Tue, Jan 08, 2013 at 01:36:50, Rob Clark wrote: On Mon, Jan 7, 2013 at 2:46 AM, Mohammed, Afzal af...@ti.com wrote: On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote: I just did a quick make da8xx_omapl_defconfig make and it builds just fine. On what version did you apply the series? At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet. But fixing this shouldn't be a problem. The change as I mentioned or something similar would be required as any driver that is going to make use of of_get_fb_videomode() would break if CONFIG_OF_VIDEOMODE or CONFIG_FB_MODE_HELPERS is not defined. Shouldn't the driver that depends on CONFIG_OF_VIDEOMODE and CONFIG_FB_MODE_HELPERS, explicitly select them? I don't really see the point of having the static-inline fallbacks. But here da8xx-fb driver does not depend on _OF_VIDEOMODE and _FB_MODE_HELPERS, currently it works as a pure platform driver for DaVinci SoC's without those CONFIG's. It is only upon enhancing the driver to make use of of_get_fb_videomode() for DT support those CONFIG's are being made use of. As the driver can work w/o these CONFIG's and so as it is not a dependency for driver on non-DT boot (as in the case of DaVinci), I disagree in selecting those options always, but rather giving user an option to select. And selecting these options always will bring in some amount of code onto Kernel image w/o any purpose in the case of DaVinci builds. Another option would be to sprinkle driver with ifdef's to avoid inline fallbacks, which is not a good thing to do. Moreover having a static inline fallback is more in line with other of_*'s. fwiw, using 'select' is what I was doing for lcd panel support for lcdc/da8xx drm driver (which was using the of videomode helpers, albeit a slightly earlier version of the patches): In your case as it is a new driver is meant only for DT, that is fine, but here it is an existing driver that works w/o these. Regards Afzal
RE: [PATCHv16 5/7] fbmon: add of_videomode helpers
Hi Steffen,
On Tue, Dec 18, 2012 at 22:34:14, Steffen Trumtrar wrote:
Add helper to get fb_videomode from devicetree.
drivers/video/fbmon.c | 42 ++
include/linux/fb.h|4
This breaks DaVinci (da8xx_omapl_defconfig), following change was
required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS
is not defined. There may be better solutions, following was the
one that was used by me to test this series.
---8--
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 58b9860..0ce30d1 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -716,9 +716,19 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
+#if defined(CONFIG_OF_VIDEOMODE) defined(CONFIG_FB_MODE_HELPERS)
extern int of_get_fb_videomode(struct device_node *np,
struct fb_videomode *fb,
int index);
+#else
+static inline int of_get_fb_videomode(struct device_node *np,
+ struct fb_videomode *fb,
+ int index)
+{
+ return -EINVAL;
+}
+#endif
+
extern int fb_videomode_from_videomode(const struct videomode *vm,
struct fb_videomode *fbmode);
---8--
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
As _OF_VIDEOMODE is a bool type CONFIG, isn't,
#ifdef CONFIG_OF_VIDEOMODE
sufficient ?
Regards
Afzal
RE: [PATCHv16 0/7] of: add display helper
Hi Steffen,
On Tue, Dec 18, 2012 at 22:34:09, Steffen Trumtrar wrote:
Finally, right in time before the end of the world on friday, v16 of the
display helpers.
After another big bang, your series in the previous world was tried ;)
This series has been tested on DA850 EVM, AM335x EVM, EVM-SK along
with the fix mentioned in 5/7, there was a build breakage on default
config on DaVinci boards with this series, fix as well as more
details are mentioned as reply to 5/7.
With those changes or equivalent to achieve the same,
Tested-by: Afzal Mohammed af...@ti.com
Regards
Afzal
N�r��yb�X��ǧv�^�){.n�+{���bj)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
