diff mbox

[RFC] video: imx: Select VIDEOMODE_HELPERS

Message ID 1366666110-9585-1-git-send-email-marex@denx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Marek Vasut April 22, 2013, 9:28 p.m. UTC
Without this, I get the following problem when building kernel:

drivers/built-in.o: In function `imx_pd_connector_get_modes':
/linux-2.6/drivers/staging/imx-drm/parallel-display.c:78: undefined reference to `of_get_drm_display_mode'
make: *** [vmlinux] Error 1

NOTE: I think this patch is almost absolutely not correct.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/staging/imx-drm/Kconfig |    1 +
 1 file changed, 1 insertion(+)

Comments

Fabio Estevam April 22, 2013, 10:35 p.m. UTC | #1
Hi Marek,

On Mon, Apr 22, 2013 at 6:28 PM, Marek Vasut <marex@denx.de> wrote:
> Without this, I get the following problem when building kernel:
>
> drivers/built-in.o: In function `imx_pd_connector_get_modes':
> /linux-2.6/drivers/staging/imx-drm/parallel-display.c:78: undefined reference to `of_get_drm_display_mode'
> make: *** [vmlinux] Error 1
>
> NOTE: I think this patch is almost absolutely not correct.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>

Patch looks good, but you should have copied Greg Kroah-Hartman, as he
is the one who takes patches into staging.

Regards,

Fabio Estevam
Marek Vasut April 22, 2013, 11:57 p.m. UTC | #2
Dear Fabio Estevam,

> Hi Marek,
> 
> On Mon, Apr 22, 2013 at 6:28 PM, Marek Vasut <marex@denx.de> wrote:
> > Without this, I get the following problem when building kernel:
> > 
> > drivers/built-in.o: In function `imx_pd_connector_get_modes':
> > /linux-2.6/drivers/staging/imx-drm/parallel-display.c:78: undefined
> > reference to `of_get_drm_display_mode' make: *** [vmlinux] Error 1
> > 
> > NOTE: I think this patch is almost absolutely not correct.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Patch looks good, but you should have copied Greg Kroah-Hartman, as he
> is the one who takes patches into staging.

I'm not sure if the select is at correct symbol, I dont think it is.

Best regards,
Marek Vasut
Sascha Hauer April 23, 2013, 5:07 a.m. UTC | #3
On Tue, Apr 23, 2013 at 01:57:47AM +0200, Marek Vasut wrote:
> Dear Fabio Estevam,
> 
> > Hi Marek,
> > 
> > On Mon, Apr 22, 2013 at 6:28 PM, Marek Vasut <marex@denx.de> wrote:
> > > Without this, I get the following problem when building kernel:
> > > 
> > > drivers/built-in.o: In function `imx_pd_connector_get_modes':
> > > /linux-2.6/drivers/staging/imx-drm/parallel-display.c:78: undefined
> > > reference to `of_get_drm_display_mode' make: *** [vmlinux] Error 1
> > > 
> > > NOTE: I think this patch is almost absolutely not correct.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > 
> > Patch looks good, but you should have copied Greg Kroah-Hartman, as he
> > is the one who takes patches into staging.
> 
> I'm not sure if the select is at correct symbol, I dont think it is.

No it's not. The parallel display driver needs it, not the i.MX drm
support in general.

Sascha
Philipp Zabel April 23, 2013, 7:41 a.m. UTC | #4
Hi Marek,

Am Dienstag, den 23.04.2013, 01:57 +0200 schrieb Marek Vasut:
> Dear Fabio Estevam,
> 
> > Hi Marek,
> > 
> > On Mon, Apr 22, 2013 at 6:28 PM, Marek Vasut <marex@denx.de> wrote:
> > > Without this, I get the following problem when building kernel:
> > > 
> > > drivers/built-in.o: In function `imx_pd_connector_get_modes':
> > > /linux-2.6/drivers/staging/imx-drm/parallel-display.c:78: undefined
> > > reference to `of_get_drm_display_mode' make: *** [vmlinux] Error 1
> > > 
> > > NOTE: I think this patch is almost absolutely not correct.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > 
> > Patch looks good, but you should have copied Greg Kroah-Hartman, as he
> > is the one who takes patches into staging.
> 
> I'm not sure if the select is at correct symbol, I dont think it is.

OF_VIDEOMODE is the correct one. The implementation of
of_get_drm_display_mode in drivers/gpu/drm/drm_modes.c depends on it.

regards
Philipp
Marek Vasut April 23, 2013, 10:29 a.m. UTC | #5
Dear Sascha Hauer,

> On Tue, Apr 23, 2013 at 01:57:47AM +0200, Marek Vasut wrote:
> > Dear Fabio Estevam,
> > 
> > > Hi Marek,
> > > 
> > > On Mon, Apr 22, 2013 at 6:28 PM, Marek Vasut <marex@denx.de> wrote:
> > > > Without this, I get the following problem when building kernel:
> > > > 
> > > > drivers/built-in.o: In function `imx_pd_connector_get_modes':
> > > > /linux-2.6/drivers/staging/imx-drm/parallel-display.c:78: undefined
> > > > reference to `of_get_drm_display_mode' make: *** [vmlinux] Error 1
> > > > 
> > > > NOTE: I think this patch is almost absolutely not correct.
> > > > 
> > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > > > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > > 
> > > Patch looks good, but you should have copied Greg Kroah-Hartman, as he
> > > is the one who takes patches into staging.
> > 
> > I'm not sure if the select is at correct symbol, I dont think it is.
> 
> No it's not. The parallel display driver needs it, not the i.MX drm
> support in general.

Yep, I expected this coming, thus RFC. I'll roll out new patch and move this to 
parallel display, ok?

Best regards,
Marek Vasut
Sascha Hauer April 23, 2013, 5:43 p.m. UTC | #6
On Tue, Apr 23, 2013 at 12:29:48PM +0200, Marek Vasut wrote:
> Dear Sascha Hauer,
> 
> > On Tue, Apr 23, 2013 at 01:57:47AM +0200, Marek Vasut wrote:
> > > Dear Fabio Estevam,
> > > 
> > > > Hi Marek,
> > > > 
> > > > On Mon, Apr 22, 2013 at 6:28 PM, Marek Vasut <marex@denx.de> wrote:
> > > > > Without this, I get the following problem when building kernel:
> > > > > 
> > > > > drivers/built-in.o: In function `imx_pd_connector_get_modes':
> > > > > /linux-2.6/drivers/staging/imx-drm/parallel-display.c:78: undefined
> > > > > reference to `of_get_drm_display_mode' make: *** [vmlinux] Error 1
> > > > > 
> > > > > NOTE: I think this patch is almost absolutely not correct.
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > > > > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > > > 
> > > > Patch looks good, but you should have copied Greg Kroah-Hartman, as he
> > > > is the one who takes patches into staging.
> > > 
> > > I'm not sure if the select is at correct symbol, I dont think it is.
> > 
> > No it's not. The parallel display driver needs it, not the i.MX drm
> > support in general.
> 
> Yep, I expected this coming, thus RFC. I'll roll out new patch and move this to 
> parallel display, ok?

ok

Sascha
Marek Vasut April 24, 2013, 12:31 a.m. UTC | #7
Hi Philipp,

> Hi Marek,
> 
> Am Dienstag, den 23.04.2013, 01:57 +0200 schrieb Marek Vasut:
> > Dear Fabio Estevam,
> > 
> > > Hi Marek,
> > > 
> > > On Mon, Apr 22, 2013 at 6:28 PM, Marek Vasut <marex@denx.de> wrote:
> > > > Without this, I get the following problem when building kernel:
> > > > 
> > > > drivers/built-in.o: In function `imx_pd_connector_get_modes':
> > > > /linux-2.6/drivers/staging/imx-drm/parallel-display.c:78: undefined
> > > > reference to `of_get_drm_display_mode' make: *** [vmlinux] Error 1
> > > > 
> > > > NOTE: I think this patch is almost absolutely not correct.
> > > > 
> > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > > > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > > 
> > > Patch looks good, but you should have copied Greg Kroah-Hartman, as he
> > > is the one who takes patches into staging.
> > 
> > I'm not sure if the select is at correct symbol, I dont think it is.
> 
> OF_VIDEOMODE is the correct one. The implementation of
> of_get_drm_display_mode in drivers/gpu/drm/drm_modes.c depends on it.

I really need VIDEOMODE_HELPERS, not OF_VIDEOMODE. If I select only 
OF_VIDEOMODE, I still get the issue above.

Check drivers/gpu/drm/drm_modes.c , it's protected by CONFIG_VIDEOMODE_HELPERS

Best regards,
Marek Vasut
Fabio Estevam April 24, 2013, 2:48 a.m. UTC | #8
Hi Philipp,

On Tue, Apr 23, 2013 at 4:41 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:

> OF_VIDEOMODE is the correct one. The implementation of
> of_get_drm_display_mode in drivers/gpu/drm/drm_modes.c depends on it.

The symbol OF_VIDEOMODE has been deprecated.

Regards,

Fabio Estevam
Philipp Zabel April 24, 2013, 8:22 a.m. UTC | #9
Hi Marek,

Am Mittwoch, den 24.04.2013, 02:31 +0200 schrieb Marek Vasut:
> Hi Philipp,
> 
> > Hi Marek,
> > 
> > Am Dienstag, den 23.04.2013, 01:57 +0200 schrieb Marek Vasut:
> > > Dear Fabio Estevam,
> > > 
> > > > Hi Marek,
> > > > 
> > > > On Mon, Apr 22, 2013 at 6:28 PM, Marek Vasut <marex@denx.de> wrote:
> > > > > Without this, I get the following problem when building kernel:
> > > > > 
> > > > > drivers/built-in.o: In function `imx_pd_connector_get_modes':
> > > > > /linux-2.6/drivers/staging/imx-drm/parallel-display.c:78: undefined
> > > > > reference to `of_get_drm_display_mode' make: *** [vmlinux] Error 1
> > > > > 
> > > > > NOTE: I think this patch is almost absolutely not correct.
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > > > > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > > > 
> > > > Patch looks good, but you should have copied Greg Kroah-Hartman, as he
> > > > is the one who takes patches into staging.
> > > 
> > > I'm not sure if the select is at correct symbol, I dont think it is.
> > 
> > OF_VIDEOMODE is the correct one. The implementation of
> > of_get_drm_display_mode in drivers/gpu/drm/drm_modes.c depends on it.
> 
> I really need VIDEOMODE_HELPERS, not OF_VIDEOMODE. If I select only 
> OF_VIDEOMODE, I still get the issue above.
> 
> Check drivers/gpu/drm/drm_modes.c , it's protected by CONFIG_VIDEOMODE_HELPERS

Indeed, I was looking at the wrong checkout. Sorry for the noise.

regards
Philipp
diff mbox

Patch

diff --git a/drivers/staging/imx-drm/Kconfig b/drivers/staging/imx-drm/Kconfig
index 8c9e403..5ad9165 100644
--- a/drivers/staging/imx-drm/Kconfig
+++ b/drivers/staging/imx-drm/Kconfig
@@ -1,6 +1,7 @@ 
 config DRM_IMX
 	tristate "DRM Support for Freescale i.MX"
 	select DRM_KMS_HELPER
+	select VIDEOMODE_HELPERS
 	select DRM_GEM_CMA_HELPER
 	select DRM_KMS_CMA_HELPER
 	depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM)