Add display-timing node parsing to drm fimd and depends on the display helper patchset at http://lists.freedesktop.org/archives/dri-devel/2013-January/033998.html
It also adds pinctrl support for drm fimd.
changes since v5: - addressed comments from Inki Dae inki.dae@samsung.com, to remove the allocation of 'fbmode' and replaced '-1'in "of_get_fb_videomode(dev->of_node, fbmode, -1)" with OF_USE_NATIVE_MODE.
changes since v4: - addressed comments from Paul Menzel paulepanter@users.sourceforge.net, to modify the commit message
changes since v3: - addressed comments from Sean Paul seanpaul@chromium.org, to modify the return values and print messages.
changes since v2: - moved 'devm_pinctrl_get_select_default' function call under 'if (pdev->dev.of_node)', this makes NON-DT code unchanged. (reported by: Rahul Sharma r.sh.open@gmail.com)
changes since v1: - addressed comments from Sean Paul seanpaul@chromium.org
Vikas Sajjan (1): video: drm: exynos: Add display-timing node parsing using video helper function
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)
Add support for parsing the display-timing node using video helper function.
The DT node parsing and pinctrl selection is done only if 'dev.of_node' exists and the NON-DT logic is still maintained under the 'else' part.
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Signed-off-by: Vikas Sajjan vikas.sajjan@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..8b2c0ff 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,7 +19,9 @@ #include <linux/clk.h> #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <linux/pinctrl/consumer.h>
+#include <video/of_display_timing.h> #include <video/samsung_fimd.h> #include <drm/exynos_drm.h>
@@ -877,16 +879,43 @@ static int fimd_probe(struct platform_device *pdev) struct exynos_drm_subdrv *subdrv; struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel; + struct fb_videomode *fbmode; + struct pinctrl *pctrl; struct resource *res; int win; int ret = -EINVAL;
DRM_DEBUG_KMS("%s\n", __FILE__);
- pdata = pdev->dev.platform_data; - if (!pdata) { - dev_err(dev, "no platform data specified\n"); - return -EINVAL; + if (pdev->dev.of_node) { + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) { + DRM_ERROR("memory allocation for pdata failed\n"); + return -ENOMEM; + } + + fbmode = &pdata->panel.timing; + ret = of_get_fb_videomode(dev->of_node, fbmode, + OF_USE_NATIVE_MODE); + if (ret) { + DRM_ERROR("failed: of_get_fb_videomode()\n" + "with return value: %d\n", ret); + return ret; + } + + pctrl = devm_pinctrl_get_select_default(dev); + if (IS_ERR_OR_NULL(pctrl)) { + DRM_ERROR("failed: devm_pinctrl_get_select_default()\n" + "with return value: %d\n", PTR_RET(pctrl)); + return PTR_RET(pctrl); + } + + } else { + pdata = pdev->dev.platform_data; + if (!pdata) { + DRM_ERROR("no platform data specified\n"); + return -EINVAL; + } }
panel = &pdata->panel;
-----Original Message----- From: Vikas Sajjan [mailto:vikas.sajjan@linaro.org] Sent: Friday, February 15, 2013 3:43 PM To: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org; kgene.kim@samsung.com; inki.dae@samsung.com; l.krishna@samsung.com; patches@linaro.org Subject: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function
Add support for parsing the display-timing node using video helper function.
The DT node parsing and pinctrl selection is done only if 'dev.of_node' exists and the NON-DT logic is still maintained under the 'else' part.
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Signed-off-by: Vikas Sajjan vikas.sajjan@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..8b2c0ff 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,7 +19,9 @@ #include <linux/clk.h> #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <linux/pinctrl/consumer.h>
+#include <video/of_display_timing.h> #include <video/samsung_fimd.h> #include <drm/exynos_drm.h>
@@ -877,16 +879,43 @@ static int fimd_probe(struct platform_device *pdev) struct exynos_drm_subdrv *subdrv; struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel;
struct fb_videomode *fbmode;
struct pinctrl *pctrl; struct resource *res; int win; int ret = -EINVAL;
DRM_DEBUG_KMS("%s\n", __FILE__);
- pdata = pdev->dev.platform_data;
- if (!pdata) {
dev_err(dev, "no platform data specified\n");return -EINVAL;
- if (pdev->dev.of_node) {
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);if (!pdata) {DRM_ERROR("memory allocation for pdata failed\n");return -ENOMEM;}fbmode = &pdata->panel.timing;ret = of_get_fb_videomode(dev->of_node, fbmode,OF_USE_NATIVE_MODE);if (ret) {DRM_ERROR("failed: of_get_fb_videomode()\n""with return value: %d\n", ret);return ret;}pctrl = devm_pinctrl_get_select_default(dev);
Why does it need pinctrl? and even though needed, I think this should be separated into another one.
Thanks, Inki Dae
if (IS_ERR_OR_NULL(pctrl)) {DRM_ERROR("failed:devm_pinctrl_get_select_default()\n"
"with return value: %d\n", PTR_RET(pctrl));return PTR_RET(pctrl);}} else {
pdata = pdev->dev.platform_data;if (!pdata) {DRM_ERROR("no platform data specified\n");return -EINVAL;}}
panel = &pdata->panel;
-- 1.7.9.5
Hi Mr. Inki Dae,
On 20 February 2013 16:45, Inki Dae inki.dae@samsung.com wrote:
-----Original Message----- From: Vikas Sajjan [mailto:vikas.sajjan@linaro.org] Sent: Friday, February 15, 2013 3:43 PM To: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org; kgene.kim@samsung.com; inki.dae@samsung.com; l.krishna@samsung.com; patches@linaro.org Subject: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function
Add support for parsing the display-timing node using video helper function.
The DT node parsing and pinctrl selection is done only if 'dev.of_node' exists and the NON-DT logic is still maintained under the 'else' part.
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Signed-off-by: Vikas Sajjan vikas.sajjan@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..8b2c0ff 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,7 +19,9 @@ #include <linux/clk.h> #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <linux/pinctrl/consumer.h>
+#include <video/of_display_timing.h> #include <video/samsung_fimd.h> #include <drm/exynos_drm.h>
@@ -877,16 +879,43 @@ static int fimd_probe(struct platform_device *pdev) struct exynos_drm_subdrv *subdrv; struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel;
struct fb_videomode *fbmode;struct pinctrl *pctrl; struct resource *res; int win; int ret = -EINVAL; DRM_DEBUG_KMS("%s\n", __FILE__);
pdata = pdev->dev.platform_data;if (!pdata) {dev_err(dev, "no platform data specified\n");return -EINVAL;
if (pdev->dev.of_node) {pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);if (!pdata) {DRM_ERROR("memory allocation for pdata failed\n");return -ENOMEM;}fbmode = &pdata->panel.timing;ret = of_get_fb_videomode(dev->of_node, fbmode,OF_USE_NATIVE_MODE);if (ret) {DRM_ERROR("failed: of_get_fb_videomode()\n""with return value: %d\n", ret);return ret;}pctrl = devm_pinctrl_get_select_default(dev);Why does it need pinctrl? and even though needed, I think this should be separated into another one.
Will separate it out and send it in a separate patch.
Thanks, Inki Dae
if (IS_ERR_OR_NULL(pctrl)) {DRM_ERROR("failed:devm_pinctrl_get_select_default()\n"
"with return value: %d\n", PTR_RET(pctrl));return PTR_RET(pctrl);}} else {pdata = pdev->dev.platform_data;if (!pdata) {DRM_ERROR("no platform data specified\n");return -EINVAL;} } panel = &pdata->panel;-- 1.7.9.5
Hi,
On 02/15/2013 03:43 PM, Vikas Sajjan wrote:
Add support for parsing the display-timing node using video helper function.
The DT node parsing and pinctrl selection is done only if 'dev.of_node' exists and the NON-DT logic is still maintained under the 'else' part.
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Signed-off-by: Vikas Sajjan vikas.sajjan@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..8b2c0ff 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,7 +19,9 @@ #include <linux/clk.h> #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <linux/pinctrl/consumer.h>
+#include <video/of_display_timing.h> #include <video/samsung_fimd.h> #include <drm/exynos_drm.h>
@@ -877,16 +879,43 @@ static int fimd_probe(struct platform_device *pdev) struct exynos_drm_subdrv *subdrv; struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel;
struct fb_videomode *fbmode;
struct pinctrl *pctrl; struct resource *res; int win; int ret = -EINVAL;
DRM_DEBUG_KMS("%s\n", __FILE__);
- pdata = pdev->dev.platform_data;
- if (!pdata) {
dev_err(dev, "no platform data specified\n");return -EINVAL;
- if (pdev->dev.of_node) {
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);if (!pdata) {DRM_ERROR("memory allocation for pdata failed\n");return -ENOMEM;}fbmode = &pdata->panel.timing;ret = of_get_fb_videomode(dev->of_node, fbmode,OF_USE_NATIVE_MODE);
fbmode variable can be substituted to &pdata->panel.timing directly then can remove fbmode variable.
if (ret) {DRM_ERROR("failed: of_get_fb_videomode()\n""with return value: %d\n", ret);return ret;}pctrl = devm_pinctrl_get_select_default(dev);if (IS_ERR_OR_NULL(pctrl)) {
It's enough to "if (IS_ERR(pctrl)) {".
DRM_ERROR("failed: devm_pinctrl_get_select_default()\n""with return value: %d\n", PTR_RET(pctrl));return PTR_RET(pctrl);
It's enough to "return PTR_ERR(pctrl);"
}
If this needs, parts for pinctrl should go to another patch.
} else {
pdata = pdev->dev.platform_data;if (!pdata) {DRM_ERROR("no platform data specified\n");return -EINVAL;}}
panel = &pdata->panel;
Thanks.
Hi,
On 21 February 2013 12:25, Joonyoung Shim jy0922.shim@samsung.com wrote:
Hi,
On 02/15/2013 03:43 PM, Vikas Sajjan wrote:
Add support for parsing the display-timing node using video helper function.
The DT node parsing and pinctrl selection is done only if 'dev.of_node' exists and the NON-DT logic is still maintained under the 'else' part.
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Signed-off-by: Vikas Sajjan vikas.sajjan@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..8b2c0ff 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,7 +19,9 @@ #include <linux/clk.h> #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <linux/pinctrl/consumer.h> +#include <video/of_display_timing.h> #include <video/samsung_fimd.h> #include <drm/exynos_drm.h> @@ -877,16 +879,43 @@ static int fimd_probe(struct platform_device *pdev) struct exynos_drm_subdrv *subdrv; struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel;
struct fb_videomode *fbmode;struct pinctrl *pctrl; struct resource *res; int win; int ret = -EINVAL; DRM_DEBUG_KMS("%s\n", __FILE__);
pdata = pdev->dev.platform_data;
if (!pdata) {dev_err(dev, "no platform data specified\n");return -EINVAL;
if (pdev->dev.of_node) {pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);if (!pdata) {DRM_ERROR("memory allocation for pdata failed\n");return -ENOMEM;}fbmode = &pdata->panel.timing;ret = of_get_fb_videomode(dev->of_node, fbmode,OF_USE_NATIVE_MODE);fbmode variable can be substituted to &pdata->panel.timing directly then can remove fbmode variable.
this is can be done.
if (ret) {DRM_ERROR("failed: of_get_fb_videomode()\n""with return value: %d\n", ret);return ret;}pctrl = devm_pinctrl_get_select_default(dev);if (IS_ERR_OR_NULL(pctrl)) {It's enough to "if (IS_ERR(pctrl)) {".
what if it returns NULL?
DRM_ERROR("failed:devm_pinctrl_get_select_default()\n"
"with return value: %d\n",PTR_RET(pctrl));
return PTR_RET(pctrl);It's enough to "return PTR_ERR(pctrl);"
ok.
}If this needs, parts for pinctrl should go to another patch.
I have it as a separate patch already. [PATCH v7 2/2] video: drm: exynos: Add pinctrl support to fimd
} else {pdata = pdev->dev.platform_data;if (!pdata) {DRM_ERROR("no platform data specified\n");return -EINVAL;} } panel = &pdata->panel;Thanks.
On 02/21/2013 04:12 PM, Vikas Sajjan wrote:
Hi,
On 21 February 2013 12:25, Joonyoung Shim jy0922.shim@samsung.com wrote:
Hi,
On 02/15/2013 03:43 PM, Vikas Sajjan wrote:
Add support for parsing the display-timing node using video helper function.
The DT node parsing and pinctrl selection is done only if 'dev.of_node' exists and the NON-DT logic is still maintained under the 'else' part.
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Signed-off-by: Vikas Sajjan vikas.sajjan@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..8b2c0ff 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,7 +19,9 @@ #include <linux/clk.h> #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <linux/pinctrl/consumer.h> +#include <video/of_display_timing.h> #include <video/samsung_fimd.h> #include <drm/exynos_drm.h> @@ -877,16 +879,43 @@ static int fimd_probe(struct platform_device *pdev) struct exynos_drm_subdrv *subdrv; struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel;
struct fb_videomode *fbmode;struct pinctrl *pctrl; struct resource *res; int win; int ret = -EINVAL; DRM_DEBUG_KMS("%s\n", __FILE__);
pdata = pdev->dev.platform_data;
if (!pdata) {dev_err(dev, "no platform data specified\n");return -EINVAL;
if (pdev->dev.of_node) {pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);if (!pdata) {DRM_ERROR("memory allocation for pdata failed\n");return -ENOMEM;}fbmode = &pdata->panel.timing;ret = of_get_fb_videomode(dev->of_node, fbmode,OF_USE_NATIVE_MODE);fbmode variable can be substituted to &pdata->panel.timing directly then can remove fbmode variable.
this is can be done.
if (ret) {DRM_ERROR("failed: of_get_fb_videomode()\n""with return value: %d\n", ret);return ret;}pctrl = devm_pinctrl_get_select_default(dev);if (IS_ERR_OR_NULL(pctrl)) {It's enough to "if (IS_ERR(pctrl)) {".
what if it returns NULL?
devm_pinctrl_get_select_default() never return NULL.
DRM_ERROR("failed:devm_pinctrl_get_select_default()\n"
"with return value: %d\n",PTR_RET(pctrl));
return PTR_RET(pctrl);It's enough to "return PTR_ERR(pctrl);"
ok.
}If this needs, parts for pinctrl should go to another patch.
I have it as a separate patch already. [PATCH v7 2/2] video: drm: exynos: Add pinctrl support to fimd
} else {pdata = pdev->dev.platform_data;if (!pdata) {DRM_ERROR("no platform data specified\n");return -EINVAL;} } panel = &pdata->panel;Thanks.
Hi,
On 02/21/2013 04:18 PM, Joonyoung Shim wrote:
On 02/21/2013 04:12 PM, Vikas Sajjan wrote:
Hi,
On 21 February 2013 12:25, Joonyoung Shim jy0922.shim@samsung.com wrote:
Hi,
On 02/15/2013 03:43 PM, Vikas Sajjan wrote:
Add support for parsing the display-timing node using video helper function.
The DT node parsing and pinctrl selection is done only if 'dev.of_node' exists and the NON-DT logic is still maintained under the 'else' part.
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Signed-off-by: Vikas Sajjan vikas.sajjan@linaro.org
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..8b2c0ff 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,7 +19,9 @@ #include <linux/clk.h> #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <linux/pinctrl/consumer.h> +#include <video/of_display_timing.h> #include <video/samsung_fimd.h> #include <drm/exynos_drm.h> @@ -877,16 +879,43 @@ static int fimd_probe(struct platform_device *pdev) struct exynos_drm_subdrv *subdrv; struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel;
struct fb_videomode *fbmode;struct pinctrl *pctrl; struct resource *res; int win; int ret = -EINVAL; DRM_DEBUG_KMS("%s\n", __FILE__);
pdata = pdev->dev.platform_data;
if (!pdata) {dev_err(dev, "no platform data specified\n");return -EINVAL;
if (pdev->dev.of_node) {pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);if (!pdata) {DRM_ERROR("memory allocation for pdatafailed\n");
return -ENOMEM;}fbmode = &pdata->panel.timing;ret = of_get_fb_videomode(dev->of_node, fbmode,OF_USE_NATIVE_MODE);fbmode variable can be substituted to &pdata->panel.timing directly then can remove fbmode variable.
this is can be done.
if (ret) {DRM_ERROR("failed: of_get_fb_videomode()\n""with return value: %d\n", ret);return ret;}pctrl = devm_pinctrl_get_select_default(dev);if (IS_ERR_OR_NULL(pctrl)) {It's enough to "if (IS_ERR(pctrl)) {".
what if it returns NULL?
devm_pinctrl_get_select_default() never return NULL.
My mistake. If CONFIG_PINCTRL is disabled, devm_pinctrl_get_select_default can return NULL.
Linus,
devm_pinctrl_get_select() and pinctrl_get_select() also need IS_ERR_OR_NULL instead of IS_ERR? And many drivers using above functions don't check NULL, right?
Thanks.
- DRM_ERROR("failed:
devm_pinctrl_get_select_default()\n"
"with return value: %d\n",PTR_RET(pctrl));
return PTR_RET(pctrl);It's enough to "return PTR_ERR(pctrl);"
ok.
}If this needs, parts for pinctrl should go to another patch.
I have it as a separate patch already. [PATCH v7 2/2] video: drm: exynos: Add pinctrl support to fimd
} else {pdata = pdev->dev.platform_data;if (!pdata) {DRM_ERROR("no platform data specified\n");return -EINVAL;} } panel = &pdata->panel;Thanks.
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Feb 28, 2013 at 2:51 AM, Joonyoung Shim jy0922.shim@samsung.com wrote:
My mistake. If CONFIG_PINCTRL is disabled, devm_pinctrl_get_select_default can return NULL.
Yes, and that is perfectly legal and *NOT* an error.
devm_pinctrl_get_select() and pinctrl_get_select() also need IS_ERR_OR_NULL instead of IS_ERR?
No.
In fact, IS_ERR_OR_NULL() shall not be used at all.
Check the LKML mailinglist for Russells recent struggle to purge it from the kernel.
And many drivers using above functions don't check NULL, right?
No, and they should not. Stub pinctrl handles, just like stub clocks and stub regulators, are perfectly legal, just that they have no effect.
Yours, Linus Walleij
dri-devel@lists.freedesktop.org
-
Inki Dae -
Joonyoung Shim -
Linus Walleij -
Vikas Sajjan