All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] omapfb: dss: Do not duplicate features data
@ 2017-11-29 12:33 Ladislav Michl
  2017-11-29 15:12 ` Adam Ford
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Ladislav Michl @ 2017-11-29 12:33 UTC (permalink / raw)
  To: linux-fbdev

As features data are read only, there is no need to allocate their
copy on the heap.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
Suggested-by: Markus Elfring <elfring@users.sourceforge.net>
---
 Note: This patch is not runtime tested. If you have hardware and can test
       it, please do so and eventually add you Tested-by tag. Thank you.
 Note2: Marcus, I hope it is okay to add your Suggested-by tag. Please
        let me know, if I'm wrong.

 drivers/video/fbdev/omap2/omapfb/dss/dispc.c    | 39 ++++++---------------
 drivers/video/fbdev/omap2/omapfb/dss/dss.c      | 45 +++++++------------------
 drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c | 31 ++++-------------
 3 files changed, 29 insertions(+), 86 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
index 7a75dfda9845..6be13a106ece 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
@@ -3976,52 +3976,33 @@ static const struct dispc_features omap54xx_dispc_feats = {
 	.has_writeback		=	true,
 };
 
-static int dispc_init_features(struct platform_device *pdev)
+static const struct dispc_features* dispc_get_features(void)
 {
-	const struct dispc_features *src;
-	struct dispc_features *dst;
-
-	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
-	if (!dst) {
-		dev_err(&pdev->dev, "Failed to allocate DISPC Features\n");
-		return -ENOMEM;
-	}
-
 	switch (omapdss_get_version()) {
 	case OMAPDSS_VER_OMAP24xx:
-		src = &omap24xx_dispc_feats;
-		break;
+		return &omap24xx_dispc_feats;
 
 	case OMAPDSS_VER_OMAP34xx_ES1:
-		src = &omap34xx_rev1_0_dispc_feats;
-		break;
+		return &omap34xx_rev1_0_dispc_feats;
 
 	case OMAPDSS_VER_OMAP34xx_ES3:
 	case OMAPDSS_VER_OMAP3630:
 	case OMAPDSS_VER_AM35xx:
 	case OMAPDSS_VER_AM43xx:
-		src = &omap34xx_rev3_0_dispc_feats;
-		break;
+		return &omap34xx_rev3_0_dispc_feats;
 
 	case OMAPDSS_VER_OMAP4430_ES1:
 	case OMAPDSS_VER_OMAP4430_ES2:
 	case OMAPDSS_VER_OMAP4:
-		src = &omap44xx_dispc_feats;
-		break;
+		return &omap44xx_dispc_feats;
 
 	case OMAPDSS_VER_OMAP5:
 	case OMAPDSS_VER_DRA7xx:
-		src = &omap54xx_dispc_feats;
-		break;
+		return &omap54xx_dispc_feats;
 
 	default:
-		return -ENODEV;
+		return NULL;
 	}
-
-	memcpy(dst, src, sizeof(*dst));
-	dispc.feat = dst;
-
-	return 0;
 }
 
 static irqreturn_t dispc_irq_handler(int irq, void *arg)
@@ -4078,9 +4059,9 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
 
 	spin_lock_init(&dispc.control_lock);
 
-	r = dispc_init_features(dispc.pdev);
-	if (r)
-		return r;
+	dispc.feat = dispc_get_features();
+	if (!dispc.feat)
+		return -ENODEV;
 
 	dispc_mem = platform_get_resource(dispc.pdev, IORESOURCE_MEM, 0);
 	if (!dispc_mem) {
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
index 48c6500c24e1..9a255ffe77c5 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
@@ -887,58 +887,37 @@ static const struct dss_features dra7xx_dss_feats = {
 	.num_ports		=	ARRAY_SIZE(dra7xx_ports),
 };
 
-static int dss_init_features(struct platform_device *pdev)
+static const struct dss_features* dss_get_features(void)
 {
-	const struct dss_features *src;
-	struct dss_features *dst;
-
-	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
-	if (!dst) {
-		dev_err(&pdev->dev, "Failed to allocate local DSS Features\n");
-		return -ENOMEM;
-	}
-
 	switch (omapdss_get_version()) {
 	case OMAPDSS_VER_OMAP24xx:
-		src = &omap24xx_dss_feats;
-		break;
+		return &omap24xx_dss_feats;
 
 	case OMAPDSS_VER_OMAP34xx_ES1:
 	case OMAPDSS_VER_OMAP34xx_ES3:
 	case OMAPDSS_VER_AM35xx:
-		src = &omap34xx_dss_feats;
-		break;
+		return &omap34xx_dss_feats;
 
 	case OMAPDSS_VER_OMAP3630:
-		src = &omap3630_dss_feats;
-		break;
+		return &omap3630_dss_feats;
 
 	case OMAPDSS_VER_OMAP4430_ES1:
 	case OMAPDSS_VER_OMAP4430_ES2:
 	case OMAPDSS_VER_OMAP4:
-		src = &omap44xx_dss_feats;
-		break;
+		return &omap44xx_dss_feats;
 
 	case OMAPDSS_VER_OMAP5:
-		src = &omap54xx_dss_feats;
-		break;
+		return &omap54xx_dss_feats;
 
 	case OMAPDSS_VER_AM43xx:
-		src = &am43xx_dss_feats;
-		break;
+		return &am43xx_dss_feats;
 
 	case OMAPDSS_VER_DRA7xx:
-		src = &dra7xx_dss_feats;
-		break;
+		return &dra7xx_dss_feats;
 
 	default:
-		return -ENODEV;
+		return NULL;
 	}
-
-	memcpy(dst, src, sizeof(*dst));
-	dss.feat = dst;
-
-	return 0;
 }
 
 static void dss_uninit_ports(struct platform_device *pdev);
@@ -1104,9 +1083,9 @@ static int dss_bind(struct device *dev)
 
 	dss.pdev = pdev;
 
-	r = dss_init_features(dss.pdev);
-	if (r)
-		return r;
+	dss.feat = dss_get_features();
+	if (!dss.feat)
+		return -ENODEV;
 
 	dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0);
 	if (!dss_mem) {
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
index 9a13c35fd6d8..07d46e14cea4 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
@@ -189,47 +189,30 @@ static const struct hdmi_phy_features omap54xx_phy_feats = {
 	.max_phy	=	186000000,
 };
 
-static int hdmi_phy_init_features(struct platform_device *pdev)
+static const struct hdmi_phy_features* hdmi_phy_get_features(void)
 {
-	struct hdmi_phy_features *dst;
-	const struct hdmi_phy_features *src;
-
-	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
-	if (!dst) {
-		dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n");
-		return -ENOMEM;
-	}
-
 	switch (omapdss_get_version()) {
 	case OMAPDSS_VER_OMAP4430_ES1:
 	case OMAPDSS_VER_OMAP4430_ES2:
 	case OMAPDSS_VER_OMAP4:
-		src = &omap44xx_phy_feats;
-		break;
+		return &omap44xx_phy_feats;
 
 	case OMAPDSS_VER_OMAP5:
 	case OMAPDSS_VER_DRA7xx:
-		src = &omap54xx_phy_feats;
-		break;
+		return &omap54xx_phy_feats;
 
 	default:
-		return -ENODEV;
+		return NULL;
 	}
-
-	memcpy(dst, src, sizeof(*dst));
-	phy_feat = dst;
-
-	return 0;
 }
 
 int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy)
 {
-	int r;
 	struct resource *res;
 
-	r = hdmi_phy_init_features(pdev);
-	if (r)
-		return r;
+	phy_feat = hdmi_phy_get_features();
+	if (!phy_feat)
+		return -ENODEV;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
 	if (!res) {
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] omapfb: dss: Do not duplicate features data
  2017-11-29 12:33 [PATCH] omapfb: dss: Do not duplicate features data Ladislav Michl
@ 2017-11-29 15:12 ` Adam Ford
  2017-11-29 16:08 ` Ladislav Michl
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Adam Ford @ 2017-11-29 15:12 UTC (permalink / raw)
  To: linux-fbdev

On Wed, Nov 29, 2017 at 6:33 AM, Ladislav Michl <ladis@linux-mips.org> wrote:
> As features data are read only, there is no need to allocate their
> copy on the heap.
>
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> Suggested-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  Note: This patch is not runtime tested. If you have hardware and can test
>        it, please do so and eventually add you Tested-by tag. Thank you.

Unfortunately, this fails to load properly on a Logic PD Torpedo kit
(omap3630/DM3730).

[    0.975097] omapdss_dss 48050000.dss: master bind failed: -517
[    3.821807] omapdss_dispc 48050400.dispc: Failed to allocate DISPC Features
[    3.829345] omapdss_dss 48050000.dss: failed to bind 48050400.dispc
(ops dispc_component_ops): -12
[    3.842254] omapdss_dss 48050000.dss: master bind failed: -12
[    3.848724] omapdss_dispc: probe of 48050400.dispc failed with error -12

I also get
[   12.258056] panel-dpi display: failed to find video source





>  Note2: Marcus, I hope it is okay to add your Suggested-by tag. Please
>         let me know, if I'm wrong.
>
>  drivers/video/fbdev/omap2/omapfb/dss/dispc.c    | 39 ++++++---------------
>  drivers/video/fbdev/omap2/omapfb/dss/dss.c      | 45 +++++++------------------
>  drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c | 31 ++++-------------
>  3 files changed, 29 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> index 7a75dfda9845..6be13a106ece 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> @@ -3976,52 +3976,33 @@ static const struct dispc_features omap54xx_dispc_feats = {
>         .has_writeback          =       true,
>  };
>
> -static int dispc_init_features(struct platform_device *pdev)
> +static const struct dispc_features* dispc_get_features(void)
>  {
> -       const struct dispc_features *src;
> -       struct dispc_features *dst;
> -
> -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
> -       if (!dst) {
> -               dev_err(&pdev->dev, "Failed to allocate DISPC Features\n");
> -               return -ENOMEM;
> -       }
> -
>         switch (omapdss_get_version()) {
>         case OMAPDSS_VER_OMAP24xx:
> -               src = &omap24xx_dispc_feats;
> -               break;
> +               return &omap24xx_dispc_feats;
>
>         case OMAPDSS_VER_OMAP34xx_ES1:
> -               src = &omap34xx_rev1_0_dispc_feats;
> -               break;
> +               return &omap34xx_rev1_0_dispc_feats;
>
>         case OMAPDSS_VER_OMAP34xx_ES3:
>         case OMAPDSS_VER_OMAP3630:
>         case OMAPDSS_VER_AM35xx:
>         case OMAPDSS_VER_AM43xx:
> -               src = &omap34xx_rev3_0_dispc_feats;
> -               break;
> +               return &omap34xx_rev3_0_dispc_feats;
>
>         case OMAPDSS_VER_OMAP4430_ES1:
>         case OMAPDSS_VER_OMAP4430_ES2:
>         case OMAPDSS_VER_OMAP4:
> -               src = &omap44xx_dispc_feats;
> -               break;
> +               return &omap44xx_dispc_feats;
>
>         case OMAPDSS_VER_OMAP5:
>         case OMAPDSS_VER_DRA7xx:
> -               src = &omap54xx_dispc_feats;
> -               break;
> +               return &omap54xx_dispc_feats;
>
>         default:
> -               return -ENODEV;
> +               return NULL;
>         }
> -
> -       memcpy(dst, src, sizeof(*dst));
> -       dispc.feat = dst;
> -
> -       return 0;
>  }
>
>  static irqreturn_t dispc_irq_handler(int irq, void *arg)
> @@ -4078,9 +4059,9 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
>
>         spin_lock_init(&dispc.control_lock);
>
> -       r = dispc_init_features(dispc.pdev);
> -       if (r)
> -               return r;
> +       dispc.feat = dispc_get_features();
> +       if (!dispc.feat)
> +               return -ENODEV;
>
>         dispc_mem = platform_get_resource(dispc.pdev, IORESOURCE_MEM, 0);
>         if (!dispc_mem) {
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> index 48c6500c24e1..9a255ffe77c5 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> @@ -887,58 +887,37 @@ static const struct dss_features dra7xx_dss_feats = {
>         .num_ports              =       ARRAY_SIZE(dra7xx_ports),
>  };
>
> -static int dss_init_features(struct platform_device *pdev)
> +static const struct dss_features* dss_get_features(void)
>  {
> -       const struct dss_features *src;
> -       struct dss_features *dst;
> -
> -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
> -       if (!dst) {
> -               dev_err(&pdev->dev, "Failed to allocate local DSS Features\n");
> -               return -ENOMEM;
> -       }
> -
>         switch (omapdss_get_version()) {
>         case OMAPDSS_VER_OMAP24xx:
> -               src = &omap24xx_dss_feats;
> -               break;
> +               return &omap24xx_dss_feats;
>
>         case OMAPDSS_VER_OMAP34xx_ES1:
>         case OMAPDSS_VER_OMAP34xx_ES3:
>         case OMAPDSS_VER_AM35xx:
> -               src = &omap34xx_dss_feats;
> -               break;
> +               return &omap34xx_dss_feats;
>
>         case OMAPDSS_VER_OMAP3630:
> -               src = &omap3630_dss_feats;
> -               break;
> +               return &omap3630_dss_feats;
>
>         case OMAPDSS_VER_OMAP4430_ES1:
>         case OMAPDSS_VER_OMAP4430_ES2:
>         case OMAPDSS_VER_OMAP4:
> -               src = &omap44xx_dss_feats;
> -               break;
> +               return &omap44xx_dss_feats;
>
>         case OMAPDSS_VER_OMAP5:
> -               src = &omap54xx_dss_feats;
> -               break;
> +               return &omap54xx_dss_feats;
>
>         case OMAPDSS_VER_AM43xx:
> -               src = &am43xx_dss_feats;
> -               break;
> +               return &am43xx_dss_feats;
>
>         case OMAPDSS_VER_DRA7xx:
> -               src = &dra7xx_dss_feats;
> -               break;
> +               return &dra7xx_dss_feats;
>
>         default:
> -               return -ENODEV;
> +               return NULL;
>         }
> -
> -       memcpy(dst, src, sizeof(*dst));
> -       dss.feat = dst;
> -
> -       return 0;
>  }
>
>  static void dss_uninit_ports(struct platform_device *pdev);
> @@ -1104,9 +1083,9 @@ static int dss_bind(struct device *dev)
>
>         dss.pdev = pdev;
>
> -       r = dss_init_features(dss.pdev);
> -       if (r)
> -               return r;
> +       dss.feat = dss_get_features();
> +       if (!dss.feat)
> +               return -ENODEV;
>
>         dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0);
>         if (!dss_mem) {
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
> index 9a13c35fd6d8..07d46e14cea4 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
> @@ -189,47 +189,30 @@ static const struct hdmi_phy_features omap54xx_phy_feats = {
>         .max_phy        =       186000000,
>  };
>
> -static int hdmi_phy_init_features(struct platform_device *pdev)
> +static const struct hdmi_phy_features* hdmi_phy_get_features(void)
>  {
> -       struct hdmi_phy_features *dst;
> -       const struct hdmi_phy_features *src;
> -
> -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
> -       if (!dst) {
> -               dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n");
> -               return -ENOMEM;
> -       }
> -
>         switch (omapdss_get_version()) {
>         case OMAPDSS_VER_OMAP4430_ES1:
>         case OMAPDSS_VER_OMAP4430_ES2:
>         case OMAPDSS_VER_OMAP4:
> -               src = &omap44xx_phy_feats;
> -               break;
> +               return &omap44xx_phy_feats;
>
>         case OMAPDSS_VER_OMAP5:
>         case OMAPDSS_VER_DRA7xx:
> -               src = &omap54xx_phy_feats;
> -               break;
> +               return &omap54xx_phy_feats;
>
>         default:
> -               return -ENODEV;
> +               return NULL;
>         }
> -
> -       memcpy(dst, src, sizeof(*dst));
> -       phy_feat = dst;
> -
> -       return 0;
>  }
>
>  int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy)
>  {
> -       int r;
>         struct resource *res;
>
> -       r = hdmi_phy_init_features(pdev);
> -       if (r)
> -               return r;
> +       phy_feat = hdmi_phy_get_features();
> +       if (!phy_feat)
> +               return -ENODEV;
>
>         res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
>         if (!res) {
> --
> 2.15.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] omapfb: dss: Do not duplicate features data
  2017-11-29 12:33 [PATCH] omapfb: dss: Do not duplicate features data Ladislav Michl
  2017-11-29 15:12 ` Adam Ford
@ 2017-11-29 16:08 ` Ladislav Michl
  2017-11-29 16:36 ` Adam Ford
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ladislav Michl @ 2017-11-29 16:08 UTC (permalink / raw)
  To: linux-fbdev

Hi Adam,

On Wed, Nov 29, 2017 at 09:12:34AM -0600, Adam Ford wrote:
> On Wed, Nov 29, 2017 at 6:33 AM, Ladislav Michl <ladis@linux-mips.org> wrote:
> > As features data are read only, there is no need to allocate their
> > copy on the heap.
> >
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > Suggested-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  Note: This patch is not runtime tested. If you have hardware and can test
> >        it, please do so and eventually add you Tested-by tag. Thank you.
> 
> Unfortunately, this fails to load properly on a Logic PD Torpedo kit
> (omap3630/DM3730).

Thank you for testing.

> [    0.975097] omapdss_dss 48050000.dss: master bind failed: -517

This is -EPROBE_DEFER, you'll probably get that also without patch.

> [    3.821807] omapdss_dispc 48050400.dispc: Failed to allocate DISPC Features

I wonder how could you get this as this is the message patch removes...

> [    3.829345] omapdss_dss 48050000.dss: failed to bind 48050400.dispc
> (ops dispc_component_ops): -12
> [    3.842254] omapdss_dss 48050000.dss: master bind failed: -12
> [    3.848724] omapdss_dispc: probe of 48050400.dispc failed with error -12
> 
> I also get
> [   12.258056] panel-dpi display: failed to find video source

These are only consequences of above error.

> >  Note2: Marcus, I hope it is okay to add your Suggested-by tag. Please
> >         let me know, if I'm wrong.
> >
> >  drivers/video/fbdev/omap2/omapfb/dss/dispc.c    | 39 ++++++---------------
> >  drivers/video/fbdev/omap2/omapfb/dss/dss.c      | 45 +++++++------------------
> >  drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c | 31 ++++-------------
> >  3 files changed, 29 insertions(+), 86 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> > index 7a75dfda9845..6be13a106ece 100644
> > --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> > @@ -3976,52 +3976,33 @@ static const struct dispc_features omap54xx_dispc_feats = {
> >         .has_writeback          =       true,
> >  };
> >
> > -static int dispc_init_features(struct platform_device *pdev)
> > +static const struct dispc_features* dispc_get_features(void)
> >  {
> > -       const struct dispc_features *src;
> > -       struct dispc_features *dst;
> > -
> > -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
> > -       if (!dst) {
> > -               dev_err(&pdev->dev, "Failed to allocate DISPC Features\n");

Here  -------------------------------------^

> > -               return -ENOMEM;
> > -       }
> > -
> >         switch (omapdss_get_version()) {
> >         case OMAPDSS_VER_OMAP24xx:
> > -               src = &omap24xx_dispc_feats;
> > -               break;
> > +               return &omap24xx_dispc_feats;
> >
> >         case OMAPDSS_VER_OMAP34xx_ES1:
> > -               src = &omap34xx_rev1_0_dispc_feats;
> > -               break;
> > +               return &omap34xx_rev1_0_dispc_feats;
> >
> >         case OMAPDSS_VER_OMAP34xx_ES3:
> >         case OMAPDSS_VER_OMAP3630:
> >         case OMAPDSS_VER_AM35xx:
> >         case OMAPDSS_VER_AM43xx:
> > -               src = &omap34xx_rev3_0_dispc_feats;
> > -               break;
> > +               return &omap34xx_rev3_0_dispc_feats;
> >
> >         case OMAPDSS_VER_OMAP4430_ES1:
> >         case OMAPDSS_VER_OMAP4430_ES2:
> >         case OMAPDSS_VER_OMAP4:
> > -               src = &omap44xx_dispc_feats;
> > -               break;
> > +               return &omap44xx_dispc_feats;
> >
> >         case OMAPDSS_VER_OMAP5:
> >         case OMAPDSS_VER_DRA7xx:
> > -               src = &omap54xx_dispc_feats;
> > -               break;
> > +               return &omap54xx_dispc_feats;
> >
> >         default:
> > -               return -ENODEV;
> > +               return NULL;
> >         }
> > -
> > -       memcpy(dst, src, sizeof(*dst));
> > -       dispc.feat = dst;
> > -
> > -       return 0;
> >  }
> >
> >  static irqreturn_t dispc_irq_handler(int irq, void *arg)
> > @@ -4078,9 +4059,9 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
> >
> >         spin_lock_init(&dispc.control_lock);
> >
> > -       r = dispc_init_features(dispc.pdev);
> > -       if (r)
> > -               return r;
> > +       dispc.feat = dispc_get_features();
> > +       if (!dispc.feat)
> > +               return -ENODEV;
> >
> >         dispc_mem = platform_get_resource(dispc.pdev, IORESOURCE_MEM, 0);
> >         if (!dispc_mem) {
> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> > index 48c6500c24e1..9a255ffe77c5 100644
> > --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> > @@ -887,58 +887,37 @@ static const struct dss_features dra7xx_dss_feats = {
> >         .num_ports              =       ARRAY_SIZE(dra7xx_ports),
> >  };
> >
> > -static int dss_init_features(struct platform_device *pdev)
> > +static const struct dss_features* dss_get_features(void)
> >  {
> > -       const struct dss_features *src;
> > -       struct dss_features *dst;
> > -
> > -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
> > -       if (!dst) {
> > -               dev_err(&pdev->dev, "Failed to allocate local DSS Features\n");
> > -               return -ENOMEM;
> > -       }
> > -
> >         switch (omapdss_get_version()) {
> >         case OMAPDSS_VER_OMAP24xx:
> > -               src = &omap24xx_dss_feats;
> > -               break;
> > +               return &omap24xx_dss_feats;
> >
> >         case OMAPDSS_VER_OMAP34xx_ES1:
> >         case OMAPDSS_VER_OMAP34xx_ES3:
> >         case OMAPDSS_VER_AM35xx:
> > -               src = &omap34xx_dss_feats;
> > -               break;
> > +               return &omap34xx_dss_feats;
> >
> >         case OMAPDSS_VER_OMAP3630:
> > -               src = &omap3630_dss_feats;
> > -               break;
> > +               return &omap3630_dss_feats;
> >
> >         case OMAPDSS_VER_OMAP4430_ES1:
> >         case OMAPDSS_VER_OMAP4430_ES2:
> >         case OMAPDSS_VER_OMAP4:
> > -               src = &omap44xx_dss_feats;
> > -               break;
> > +               return &omap44xx_dss_feats;
> >
> >         case OMAPDSS_VER_OMAP5:
> > -               src = &omap54xx_dss_feats;
> > -               break;
> > +               return &omap54xx_dss_feats;
> >
> >         case OMAPDSS_VER_AM43xx:
> > -               src = &am43xx_dss_feats;
> > -               break;
> > +               return &am43xx_dss_feats;
> >
> >         case OMAPDSS_VER_DRA7xx:
> > -               src = &dra7xx_dss_feats;
> > -               break;
> > +               return &dra7xx_dss_feats;
> >
> >         default:
> > -               return -ENODEV;
> > +               return NULL;
> >         }
> > -
> > -       memcpy(dst, src, sizeof(*dst));
> > -       dss.feat = dst;
> > -
> > -       return 0;
> >  }
> >
> >  static void dss_uninit_ports(struct platform_device *pdev);
> > @@ -1104,9 +1083,9 @@ static int dss_bind(struct device *dev)
> >
> >         dss.pdev = pdev;
> >
> > -       r = dss_init_features(dss.pdev);
> > -       if (r)
> > -               return r;
> > +       dss.feat = dss_get_features();
> > +       if (!dss.feat)
> > +               return -ENODEV;
> >
> >         dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0);
> >         if (!dss_mem) {
> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
> > index 9a13c35fd6d8..07d46e14cea4 100644
> > --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
> > @@ -189,47 +189,30 @@ static const struct hdmi_phy_features omap54xx_phy_feats = {
> >         .max_phy        =       186000000,
> >  };
> >
> > -static int hdmi_phy_init_features(struct platform_device *pdev)
> > +static const struct hdmi_phy_features* hdmi_phy_get_features(void)
> >  {
> > -       struct hdmi_phy_features *dst;
> > -       const struct hdmi_phy_features *src;
> > -
> > -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
> > -       if (!dst) {
> > -               dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n");
> > -               return -ENOMEM;
> > -       }
> > -
> >         switch (omapdss_get_version()) {
> >         case OMAPDSS_VER_OMAP4430_ES1:
> >         case OMAPDSS_VER_OMAP4430_ES2:
> >         case OMAPDSS_VER_OMAP4:
> > -               src = &omap44xx_phy_feats;
> > -               break;
> > +               return &omap44xx_phy_feats;
> >
> >         case OMAPDSS_VER_OMAP5:
> >         case OMAPDSS_VER_DRA7xx:
> > -               src = &omap54xx_phy_feats;
> > -               break;
> > +               return &omap54xx_phy_feats;
> >
> >         default:
> > -               return -ENODEV;
> > +               return NULL;
> >         }
> > -
> > -       memcpy(dst, src, sizeof(*dst));
> > -       phy_feat = dst;
> > -
> > -       return 0;
> >  }
> >
> >  int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy)
> >  {
> > -       int r;
> >         struct resource *res;
> >
> > -       r = hdmi_phy_init_features(pdev);
> > -       if (r)
> > -               return r;
> > +       phy_feat = hdmi_phy_get_features();
> > +       if (!phy_feat)
> > +               return -ENODEV;
> >
> >         res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
> >         if (!res) {
> > --
> > 2.15.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] omapfb: dss: Do not duplicate features data
  2017-11-29 12:33 [PATCH] omapfb: dss: Do not duplicate features data Ladislav Michl
  2017-11-29 15:12 ` Adam Ford
  2017-11-29 16:08 ` Ladislav Michl
@ 2017-11-29 16:36 ` Adam Ford
  2017-11-29 16:45 ` Adam Ford
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Adam Ford @ 2017-11-29 16:36 UTC (permalink / raw)
  To: linux-fbdev

On Wed, Nov 29, 2017 at 10:08 AM, Ladislav Michl <ladis@linux-mips.org> wrote:
> Hi Adam,
>
> On Wed, Nov 29, 2017 at 09:12:34AM -0600, Adam Ford wrote:
>> On Wed, Nov 29, 2017 at 6:33 AM, Ladislav Michl <ladis@linux-mips.org> wrote:
>> > As features data are read only, there is no need to allocate their
>> > copy on the heap.
>> >
>> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
>> > Suggested-by: Markus Elfring <elfring@users.sourceforge.net>
>> > ---
>> >  Note: This patch is not runtime tested. If you have hardware and can test
>> >        it, please do so and eventually add you Tested-by tag. Thank you.
>>
>> Unfortunately, this fails to load properly on a Logic PD Torpedo kit
>> (omap3630/DM3730).
>
> Thank you for testing.
>
>> [    0.975097] omapdss_dss 48050000.dss: master bind failed: -517
>
> This is -EPROBE_DEFER, you'll probably get that also without patch.
>
>> [    3.821807] omapdss_dispc 48050400.dispc: Failed to allocate DISPC Features
>
> I wonder how could you get this as this is the message patch removes...
>
>> [    3.829345] omapdss_dss 48050000.dss: failed to bind 48050400.dispc
>> (ops dispc_component_ops): -12
>> [    3.842254] omapdss_dss 48050000.dss: master bind failed: -12
>> [    3.848724] omapdss_dispc: probe of 48050400.dispc failed with error -12
>>
>> I also get
>> [   12.258056] panel-dpi display: failed to find video source
>
> These are only consequences of above error.
>
>> >  Note2: Marcus, I hope it is okay to add your Suggested-by tag. Please
>> >         let me know, if I'm wrong.
>> >
>> >  drivers/video/fbdev/omap2/omapfb/dss/dispc.c    | 39 ++++++---------------
>> >  drivers/video/fbdev/omap2/omapfb/dss/dss.c      | 45 +++++++------------------
>> >  drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c | 31 ++++-------------
>> >  3 files changed, 29 insertions(+), 86 deletions(-)
>> >
>> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
>> > index 7a75dfda9845..6be13a106ece 100644
>> > --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
>> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
>> > @@ -3976,52 +3976,33 @@ static const struct dispc_features omap54xx_dispc_feats = {
>> >         .has_writeback          =       true,
>> >  };
>> >
>> > -static int dispc_init_features(struct platform_device *pdev)
>> > +static const struct dispc_features* dispc_get_features(void)
>> >  {
>> > -       const struct dispc_features *src;
>> > -       struct dispc_features *dst;
>> > -
>> > -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
>> > -       if (!dst) {
>> > -               dev_err(&pdev->dev, "Failed to allocate DISPC Features\n");
>
> Here  -------------------------------------^
>

I copy-pasted the code from the URL. I assumed they were the same.  I
used it since copy-pasting tabs and spacing gets messaged up from my
e-mail.  Sorry about that.

I tried to apply the patch in the e-mail to 4.14.2 and it failed. (it
could also be the fact that my e-mail program sucks and it's messing
up characters)

Do you have a version or trunk you want me to use as the basis?

thanks

adam

>> > -               return -ENOMEM;
>> > -       }
>> > -
>> >         switch (omapdss_get_version()) {
>> >         case OMAPDSS_VER_OMAP24xx:
>> > -               src = &omap24xx_dispc_feats;
>> > -               break;
>> > +               return &omap24xx_dispc_feats;
>> >
>> >         case OMAPDSS_VER_OMAP34xx_ES1:
>> > -               src = &omap34xx_rev1_0_dispc_feats;
>> > -               break;
>> > +               return &omap34xx_rev1_0_dispc_feats;
>> >
>> >         case OMAPDSS_VER_OMAP34xx_ES3:
>> >         case OMAPDSS_VER_OMAP3630:
>> >         case OMAPDSS_VER_AM35xx:
>> >         case OMAPDSS_VER_AM43xx:
>> > -               src = &omap34xx_rev3_0_dispc_feats;
>> > -               break;
>> > +               return &omap34xx_rev3_0_dispc_feats;
>> >
>> >         case OMAPDSS_VER_OMAP4430_ES1:
>> >         case OMAPDSS_VER_OMAP4430_ES2:
>> >         case OMAPDSS_VER_OMAP4:
>> > -               src = &omap44xx_dispc_feats;
>> > -               break;
>> > +               return &omap44xx_dispc_feats;
>> >
>> >         case OMAPDSS_VER_OMAP5:
>> >         case OMAPDSS_VER_DRA7xx:
>> > -               src = &omap54xx_dispc_feats;
>> > -               break;
>> > +               return &omap54xx_dispc_feats;
>> >
>> >         default:
>> > -               return -ENODEV;
>> > +               return NULL;
>> >         }
>> > -
>> > -       memcpy(dst, src, sizeof(*dst));
>> > -       dispc.feat = dst;
>> > -
>> > -       return 0;
>> >  }
>> >
>> >  static irqreturn_t dispc_irq_handler(int irq, void *arg)
>> > @@ -4078,9 +4059,9 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
>> >
>> >         spin_lock_init(&dispc.control_lock);
>> >
>> > -       r = dispc_init_features(dispc.pdev);
>> > -       if (r)
>> > -               return r;
>> > +       dispc.feat = dispc_get_features();
>> > +       if (!dispc.feat)
>> > +               return -ENODEV;
>> >
>> >         dispc_mem = platform_get_resource(dispc.pdev, IORESOURCE_MEM, 0);
>> >         if (!dispc_mem) {
>> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
>> > index 48c6500c24e1..9a255ffe77c5 100644
>> > --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
>> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
>> > @@ -887,58 +887,37 @@ static const struct dss_features dra7xx_dss_feats = {
>> >         .num_ports              =       ARRAY_SIZE(dra7xx_ports),
>> >  };
>> >
>> > -static int dss_init_features(struct platform_device *pdev)
>> > +static const struct dss_features* dss_get_features(void)
>> >  {
>> > -       const struct dss_features *src;
>> > -       struct dss_features *dst;
>> > -
>> > -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
>> > -       if (!dst) {
>> > -               dev_err(&pdev->dev, "Failed to allocate local DSS Features\n");
>> > -               return -ENOMEM;
>> > -       }
>> > -
>> >         switch (omapdss_get_version()) {
>> >         case OMAPDSS_VER_OMAP24xx:
>> > -               src = &omap24xx_dss_feats;
>> > -               break;
>> > +               return &omap24xx_dss_feats;
>> >
>> >         case OMAPDSS_VER_OMAP34xx_ES1:
>> >         case OMAPDSS_VER_OMAP34xx_ES3:
>> >         case OMAPDSS_VER_AM35xx:
>> > -               src = &omap34xx_dss_feats;
>> > -               break;
>> > +               return &omap34xx_dss_feats;
>> >
>> >         case OMAPDSS_VER_OMAP3630:
>> > -               src = &omap3630_dss_feats;
>> > -               break;
>> > +               return &omap3630_dss_feats;
>> >
>> >         case OMAPDSS_VER_OMAP4430_ES1:
>> >         case OMAPDSS_VER_OMAP4430_ES2:
>> >         case OMAPDSS_VER_OMAP4:
>> > -               src = &omap44xx_dss_feats;
>> > -               break;
>> > +               return &omap44xx_dss_feats;
>> >
>> >         case OMAPDSS_VER_OMAP5:
>> > -               src = &omap54xx_dss_feats;
>> > -               break;
>> > +               return &omap54xx_dss_feats;
>> >
>> >         case OMAPDSS_VER_AM43xx:
>> > -               src = &am43xx_dss_feats;
>> > -               break;
>> > +               return &am43xx_dss_feats;
>> >
>> >         case OMAPDSS_VER_DRA7xx:
>> > -               src = &dra7xx_dss_feats;
>> > -               break;
>> > +               return &dra7xx_dss_feats;
>> >
>> >         default:
>> > -               return -ENODEV;
>> > +               return NULL;
>> >         }
>> > -
>> > -       memcpy(dst, src, sizeof(*dst));
>> > -       dss.feat = dst;
>> > -
>> > -       return 0;
>> >  }
>> >
>> >  static void dss_uninit_ports(struct platform_device *pdev);
>> > @@ -1104,9 +1083,9 @@ static int dss_bind(struct device *dev)
>> >
>> >         dss.pdev = pdev;
>> >
>> > -       r = dss_init_features(dss.pdev);
>> > -       if (r)
>> > -               return r;
>> > +       dss.feat = dss_get_features();
>> > +       if (!dss.feat)
>> > +               return -ENODEV;
>> >
>> >         dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0);
>> >         if (!dss_mem) {
>> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
>> > index 9a13c35fd6d8..07d46e14cea4 100644
>> > --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
>> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
>> > @@ -189,47 +189,30 @@ static const struct hdmi_phy_features omap54xx_phy_feats = {
>> >         .max_phy        =       186000000,
>> >  };
>> >
>> > -static int hdmi_phy_init_features(struct platform_device *pdev)
>> > +static const struct hdmi_phy_features* hdmi_phy_get_features(void)
>> >  {
>> > -       struct hdmi_phy_features *dst;
>> > -       const struct hdmi_phy_features *src;
>> > -
>> > -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
>> > -       if (!dst) {
>> > -               dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n");
>> > -               return -ENOMEM;
>> > -       }
>> > -
>> >         switch (omapdss_get_version()) {
>> >         case OMAPDSS_VER_OMAP4430_ES1:
>> >         case OMAPDSS_VER_OMAP4430_ES2:
>> >         case OMAPDSS_VER_OMAP4:
>> > -               src = &omap44xx_phy_feats;
>> > -               break;
>> > +               return &omap44xx_phy_feats;
>> >
>> >         case OMAPDSS_VER_OMAP5:
>> >         case OMAPDSS_VER_DRA7xx:
>> > -               src = &omap54xx_phy_feats;
>> > -               break;
>> > +               return &omap54xx_phy_feats;
>> >
>> >         default:
>> > -               return -ENODEV;
>> > +               return NULL;
>> >         }
>> > -
>> > -       memcpy(dst, src, sizeof(*dst));
>> > -       phy_feat = dst;
>> > -
>> > -       return 0;
>> >  }
>> >
>> >  int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy)
>> >  {
>> > -       int r;
>> >         struct resource *res;
>> >
>> > -       r = hdmi_phy_init_features(pdev);
>> > -       if (r)
>> > -               return r;
>> > +       phy_feat = hdmi_phy_get_features();
>> > +       if (!phy_feat)
>> > +               return -ENODEV;
>> >
>> >         res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
>> >         if (!res) {
>> > --
>> > 2.15.0
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] omapfb: dss: Do not duplicate features data
  2017-11-29 12:33 [PATCH] omapfb: dss: Do not duplicate features data Ladislav Michl
                   ` (2 preceding siblings ...)
  2017-11-29 16:36 ` Adam Ford
@ 2017-11-29 16:45 ` Adam Ford
  2017-11-29 17:03 ` Ladislav Michl
  2018-01-04 14:10 ` Bartlomiej Zolnierkiewicz
  5 siblings, 0 replies; 7+ messages in thread
From: Adam Ford @ 2017-11-29 16:45 UTC (permalink / raw)
  To: linux-fbdev

On Wed, Nov 29, 2017 at 10:36 AM, Adam Ford <aford173@gmail.com> wrote:
> On Wed, Nov 29, 2017 at 10:08 AM, Ladislav Michl <ladis@linux-mips.org> wrote:
>> Hi Adam,
>>
>> On Wed, Nov 29, 2017 at 09:12:34AM -0600, Adam Ford wrote:
>>> On Wed, Nov 29, 2017 at 6:33 AM, Ladislav Michl <ladis@linux-mips.org> wrote:
>>> > As features data are read only, there is no need to allocate their
>>> > copy on the heap.
>>> >
>>> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
>>> > Suggested-by: Markus Elfring <elfring@users.sourceforge.net>
>>> > ---
>>> >  Note: This patch is not runtime tested. If you have hardware and can test
>>> >        it, please do so and eventually add you Tested-by tag. Thank you.
>>>
>>> Unfortunately, this fails to load properly on a Logic PD Torpedo kit
>>> (omap3630/DM3730).
>>
>> Thank you for testing.
>>
>>> [    0.975097] omapdss_dss 48050000.dss: master bind failed: -517
>>
>> This is -EPROBE_DEFER, you'll probably get that also without patch.
>>
>>> [    3.821807] omapdss_dispc 48050400.dispc: Failed to allocate DISPC Features
>>
>> I wonder how could you get this as this is the message patch removes...
>>
>>> [    3.829345] omapdss_dss 48050000.dss: failed to bind 48050400.dispc
>>> (ops dispc_component_ops): -12
>>> [    3.842254] omapdss_dss 48050000.dss: master bind failed: -12
>>> [    3.848724] omapdss_dispc: probe of 48050400.dispc failed with error -12
>>>
>>> I also get
>>> [   12.258056] panel-dpi display: failed to find video source
>>
>> These are only consequences of above error.
>>
>>> >  Note2: Marcus, I hope it is okay to add your Suggested-by tag. Please
>>> >         let me know, if I'm wrong.
>>> >
>>> >  drivers/video/fbdev/omap2/omapfb/dss/dispc.c    | 39 ++++++---------------
>>> >  drivers/video/fbdev/omap2/omapfb/dss/dss.c      | 45 +++++++------------------
>>> >  drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c | 31 ++++-------------
>>> >  3 files changed, 29 insertions(+), 86 deletions(-)
>>> >
>>> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
>>> > index 7a75dfda9845..6be13a106ece 100644
>>> > --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
>>> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
>>> > @@ -3976,52 +3976,33 @@ static const struct dispc_features omap54xx_dispc_feats = {
>>> >         .has_writeback          =       true,
>>> >  };
>>> >
>>> > -static int dispc_init_features(struct platform_device *pdev)
>>> > +static const struct dispc_features* dispc_get_features(void)
>>> >  {
>>> > -       const struct dispc_features *src;
>>> > -       struct dispc_features *dst;
>>> > -
>>> > -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
>>> > -       if (!dst) {
>>> > -               dev_err(&pdev->dev, "Failed to allocate DISPC Features\n");
>>
>> Here  -------------------------------------^
>>
>
> I copy-pasted the code from the URL. I assumed they were the same.  I
> used it since copy-pasting tabs and spacing gets messaged up from my
> e-mail.  Sorry about that.
>
> I tried to apply the patch in the e-mail to 4.14.2 and it failed. (it
> could also be the fact that my e-mail program sucks and it's messing
> up characters)
>
> Do you have a version or trunk you want me to use as the basis?
>

If it's correct and the patch is this one:
https://patchwork.kernel.org/patch/10082025/

Then you can mark me as:

Tested-by: Adam Ford <aford173@gmail.com> #omap3630


> thanks
>
> adam
>
>>> > -               return -ENOMEM;
>>> > -       }
>>> > -
>>> >         switch (omapdss_get_version()) {
>>> >         case OMAPDSS_VER_OMAP24xx:
>>> > -               src = &omap24xx_dispc_feats;
>>> > -               break;
>>> > +               return &omap24xx_dispc_feats;
>>> >
>>> >         case OMAPDSS_VER_OMAP34xx_ES1:
>>> > -               src = &omap34xx_rev1_0_dispc_feats;
>>> > -               break;
>>> > +               return &omap34xx_rev1_0_dispc_feats;
>>> >
>>> >         case OMAPDSS_VER_OMAP34xx_ES3:
>>> >         case OMAPDSS_VER_OMAP3630:
>>> >         case OMAPDSS_VER_AM35xx:
>>> >         case OMAPDSS_VER_AM43xx:
>>> > -               src = &omap34xx_rev3_0_dispc_feats;
>>> > -               break;
>>> > +               return &omap34xx_rev3_0_dispc_feats;
>>> >
>>> >         case OMAPDSS_VER_OMAP4430_ES1:
>>> >         case OMAPDSS_VER_OMAP4430_ES2:
>>> >         case OMAPDSS_VER_OMAP4:
>>> > -               src = &omap44xx_dispc_feats;
>>> > -               break;
>>> > +               return &omap44xx_dispc_feats;
>>> >
>>> >         case OMAPDSS_VER_OMAP5:
>>> >         case OMAPDSS_VER_DRA7xx:
>>> > -               src = &omap54xx_dispc_feats;
>>> > -               break;
>>> > +               return &omap54xx_dispc_feats;
>>> >
>>> >         default:
>>> > -               return -ENODEV;
>>> > +               return NULL;
>>> >         }
>>> > -
>>> > -       memcpy(dst, src, sizeof(*dst));
>>> > -       dispc.feat = dst;
>>> > -
>>> > -       return 0;
>>> >  }
>>> >
>>> >  static irqreturn_t dispc_irq_handler(int irq, void *arg)
>>> > @@ -4078,9 +4059,9 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
>>> >
>>> >         spin_lock_init(&dispc.control_lock);
>>> >
>>> > -       r = dispc_init_features(dispc.pdev);
>>> > -       if (r)
>>> > -               return r;
>>> > +       dispc.feat = dispc_get_features();
>>> > +       if (!dispc.feat)
>>> > +               return -ENODEV;
>>> >
>>> >         dispc_mem = platform_get_resource(dispc.pdev, IORESOURCE_MEM, 0);
>>> >         if (!dispc_mem) {
>>> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
>>> > index 48c6500c24e1..9a255ffe77c5 100644
>>> > --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
>>> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
>>> > @@ -887,58 +887,37 @@ static const struct dss_features dra7xx_dss_feats = {
>>> >         .num_ports              =       ARRAY_SIZE(dra7xx_ports),
>>> >  };
>>> >
>>> > -static int dss_init_features(struct platform_device *pdev)
>>> > +static const struct dss_features* dss_get_features(void)
>>> >  {
>>> > -       const struct dss_features *src;
>>> > -       struct dss_features *dst;
>>> > -
>>> > -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
>>> > -       if (!dst) {
>>> > -               dev_err(&pdev->dev, "Failed to allocate local DSS Features\n");
>>> > -               return -ENOMEM;
>>> > -       }
>>> > -
>>> >         switch (omapdss_get_version()) {
>>> >         case OMAPDSS_VER_OMAP24xx:
>>> > -               src = &omap24xx_dss_feats;
>>> > -               break;
>>> > +               return &omap24xx_dss_feats;
>>> >
>>> >         case OMAPDSS_VER_OMAP34xx_ES1:
>>> >         case OMAPDSS_VER_OMAP34xx_ES3:
>>> >         case OMAPDSS_VER_AM35xx:
>>> > -               src = &omap34xx_dss_feats;
>>> > -               break;
>>> > +               return &omap34xx_dss_feats;
>>> >
>>> >         case OMAPDSS_VER_OMAP3630:
>>> > -               src = &omap3630_dss_feats;
>>> > -               break;
>>> > +               return &omap3630_dss_feats;
>>> >
>>> >         case OMAPDSS_VER_OMAP4430_ES1:
>>> >         case OMAPDSS_VER_OMAP4430_ES2:
>>> >         case OMAPDSS_VER_OMAP4:
>>> > -               src = &omap44xx_dss_feats;
>>> > -               break;
>>> > +               return &omap44xx_dss_feats;
>>> >
>>> >         case OMAPDSS_VER_OMAP5:
>>> > -               src = &omap54xx_dss_feats;
>>> > -               break;
>>> > +               return &omap54xx_dss_feats;
>>> >
>>> >         case OMAPDSS_VER_AM43xx:
>>> > -               src = &am43xx_dss_feats;
>>> > -               break;
>>> > +               return &am43xx_dss_feats;
>>> >
>>> >         case OMAPDSS_VER_DRA7xx:
>>> > -               src = &dra7xx_dss_feats;
>>> > -               break;
>>> > +               return &dra7xx_dss_feats;
>>> >
>>> >         default:
>>> > -               return -ENODEV;
>>> > +               return NULL;
>>> >         }
>>> > -
>>> > -       memcpy(dst, src, sizeof(*dst));
>>> > -       dss.feat = dst;
>>> > -
>>> > -       return 0;
>>> >  }
>>> >
>>> >  static void dss_uninit_ports(struct platform_device *pdev);
>>> > @@ -1104,9 +1083,9 @@ static int dss_bind(struct device *dev)
>>> >
>>> >         dss.pdev = pdev;
>>> >
>>> > -       r = dss_init_features(dss.pdev);
>>> > -       if (r)
>>> > -               return r;
>>> > +       dss.feat = dss_get_features();
>>> > +       if (!dss.feat)
>>> > +               return -ENODEV;
>>> >
>>> >         dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0);
>>> >         if (!dss_mem) {
>>> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
>>> > index 9a13c35fd6d8..07d46e14cea4 100644
>>> > --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
>>> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
>>> > @@ -189,47 +189,30 @@ static const struct hdmi_phy_features omap54xx_phy_feats = {
>>> >         .max_phy        =       186000000,
>>> >  };
>>> >
>>> > -static int hdmi_phy_init_features(struct platform_device *pdev)
>>> > +static const struct hdmi_phy_features* hdmi_phy_get_features(void)
>>> >  {
>>> > -       struct hdmi_phy_features *dst;
>>> > -       const struct hdmi_phy_features *src;
>>> > -
>>> > -       dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
>>> > -       if (!dst) {
>>> > -               dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n");
>>> > -               return -ENOMEM;
>>> > -       }
>>> > -
>>> >         switch (omapdss_get_version()) {
>>> >         case OMAPDSS_VER_OMAP4430_ES1:
>>> >         case OMAPDSS_VER_OMAP4430_ES2:
>>> >         case OMAPDSS_VER_OMAP4:
>>> > -               src = &omap44xx_phy_feats;
>>> > -               break;
>>> > +               return &omap44xx_phy_feats;
>>> >
>>> >         case OMAPDSS_VER_OMAP5:
>>> >         case OMAPDSS_VER_DRA7xx:
>>> > -               src = &omap54xx_phy_feats;
>>> > -               break;
>>> > +               return &omap54xx_phy_feats;
>>> >
>>> >         default:
>>> > -               return -ENODEV;
>>> > +               return NULL;
>>> >         }
>>> > -
>>> > -       memcpy(dst, src, sizeof(*dst));
>>> > -       phy_feat = dst;
>>> > -
>>> > -       return 0;
>>> >  }
>>> >
>>> >  int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy)
>>> >  {
>>> > -       int r;
>>> >         struct resource *res;
>>> >
>>> > -       r = hdmi_phy_init_features(pdev);
>>> > -       if (r)
>>> > -               return r;
>>> > +       phy_feat = hdmi_phy_get_features();
>>> > +       if (!phy_feat)
>>> > +               return -ENODEV;
>>> >
>>> >         res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
>>> >         if (!res) {
>>> > --
>>> > 2.15.0
>>> >
>>> > --
>>> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>> > the body of a message to majordomo@vger.kernel.org
>>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] omapfb: dss: Do not duplicate features data
  2017-11-29 12:33 [PATCH] omapfb: dss: Do not duplicate features data Ladislav Michl
                   ` (3 preceding siblings ...)
  2017-11-29 16:45 ` Adam Ford
@ 2017-11-29 17:03 ` Ladislav Michl
  2018-01-04 14:10 ` Bartlomiej Zolnierkiewicz
  5 siblings, 0 replies; 7+ messages in thread
From: Ladislav Michl @ 2017-11-29 17:03 UTC (permalink / raw)
  To: linux-fbdev

On Wed, Nov 29, 2017 at 10:45:52AM -0600, Adam Ford wrote:
[snip]
> If it's correct and the patch is this one:
> https://patchwork.kernel.org/patch/10082025/

Yes.

> Then you can mark me as:
> 
> Tested-by: Adam Ford <aford173@gmail.com> #omap3630

Thank you for testing. I'll wait a bit more for eventual objections
and Marcus' decision about his credit before sending v2 :)

Best regards,
	ladis

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] omapfb: dss: Do not duplicate features data
  2017-11-29 12:33 [PATCH] omapfb: dss: Do not duplicate features data Ladislav Michl
                   ` (4 preceding siblings ...)
  2017-11-29 17:03 ` Ladislav Michl
@ 2018-01-04 14:10 ` Bartlomiej Zolnierkiewicz
  5 siblings, 0 replies; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-01-04 14:10 UTC (permalink / raw)
  To: linux-fbdev

On Wednesday, November 29, 2017 06:03:14 PM Ladislav Michl wrote:
> On Wed, Nov 29, 2017 at 10:45:52AM -0600, Adam Ford wrote:
> [snip]
> > If it's correct and the patch is this one:
> > https://patchwork.kernel.org/patch/10082025/
> 
> Yes.
> 
> > Then you can mark me as:
> > 
> > Tested-by: Adam Ford <aford173@gmail.com> #omap3630
> 
> Thank you for testing. I'll wait a bit more for eventual objections
> and Marcus' decision about his credit before sending v2 :)

After adding Adam's Tested-by tag and fixing minor CodingStyle
errors reported by checkpatch.pl:

ERROR: "foo* bar" should be "foo *bar"
#37: FILE: drivers/video/fbdev/omap2/omapfb/dss/dispc.c:3979:
+static const struct dispc_features* dispc_get_features(void)

ERROR: "foo* bar" should be "foo *bar"
#114: FILE: drivers/video/fbdev/omap2/omapfb/dss/dss.c:890:
+static const struct dss_features* dss_get_features(void)

ERROR: "foo* bar" should be "foo *bar"
#199: FILE: drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c:192:
+static const struct hdmi_phy_features* hdmi_phy_get_features(void)

I've queued your patch for 4.16, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-01-04 14:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-29 12:33 [PATCH] omapfb: dss: Do not duplicate features data Ladislav Michl
2017-11-29 15:12 ` Adam Ford
2017-11-29 16:08 ` Ladislav Michl
2017-11-29 16:36 ` Adam Ford
2017-11-29 16:45 ` Adam Ford
2017-11-29 17:03 ` Ladislav Michl
2018-01-04 14:10 ` Bartlomiej Zolnierkiewicz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.