[PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Geert Uytterhoeven]

Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
problem has been fixed in newer revisions.

However, the code didn't take into account the actual hardware revision,
thus applying the quirk also on newer hardware revisions, causing green
color reversals.

Fix this by applying the quirk when running on R-Car H2 ES1.x only.

Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?

While the issue was present before aforementioned commit, I do not think
there is a real need to fix the older code variant, as the new LVDS
encoder was backported to v4.14-ltsi.
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 3fc7e6899cab5843..50c11a7f0467f746 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -16,6 +16,7 @@
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/sys_soc.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
 	return 0;
 }
 
+static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
+	.gen = 2,
+	.quirks = RCAR_LVDS_QUIRK_LANES,
+	.pll_setup = rcar_lvds_pll_setup_gen2,
+};
+
+static const struct soc_device_attribute lvds_quirk_matches[] = {
+	{
+		.soc_id = "r8a7790", .revision = "ES1.*",
+		.data = &rcar_lvds_r8a7790es1_info,
+	},
+	{ /* sentinel */ }
+};
+
 static int rcar_lvds_probe(struct platform_device *pdev)
 {
+	const struct soc_device_attribute *attr;
 	struct rcar_lvds *lvds;
 	struct resource *mem;
 	int ret;
@@ -857,6 +873,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
 	lvds->dev = &pdev->dev;
 	lvds->info = of_device_get_match_data(&pdev->dev);
 
+	attr = soc_device_match(lvds_quirk_matches);
+	if (attr)
+		lvds->info = attr->data;
+
 	ret = rcar_lvds_parse_dt(lvds);
 	if (ret < 0)
 		return ret;
@@ -893,12 +913,6 @@ static const struct rcar_lvds_device_info rcar_lvds_gen2_info = {
 	.pll_setup = rcar_lvds_pll_setup_gen2,
 };
 
-static const struct rcar_lvds_device_info rcar_lvds_r8a7790_info = {
-	.gen = 2,
-	.quirks = RCAR_LVDS_QUIRK_LANES,
-	.pll_setup = rcar_lvds_pll_setup_gen2,
-};
-
 static const struct rcar_lvds_device_info rcar_lvds_gen3_info = {
 	.gen = 3,
 	.quirks = RCAR_LVDS_QUIRK_PWD,
@@ -930,7 +944,7 @@ static const struct of_device_id rcar_lvds_of_table[] = {
 	{ .compatible = "renesas,r8a7744-lvds", .data = &rcar_lvds_gen2_info },
 	{ .compatible = "renesas,r8a774a1-lvds", .data = &rcar_lvds_gen3_info },
 	{ .compatible = "renesas,r8a774c0-lvds", .data = &rcar_lvds_r8a77990_info },
-	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info },
+	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_gen2_info },
 	{ .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
 	{ .compatible = "renesas,r8a7793-lvds", .data = &rcar_lvds_gen2_info },
 	{ .compatible = "renesas,r8a7795-lvds", .data = &rcar_lvds_gen3_info },
-- 
2.17.1

[PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Geert Uytterhoeven]

Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
problem has been fixed in newer revisions.

However, the code didn't take into account the actual hardware revision,
thus applying the quirk also on newer hardware revisions, causing green
color reversals.

Fix this by applying the quirk when running on R-Car H2 ES1.x only.

Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?

While the issue was present before aforementioned commit, I do not think
there is a real need to fix the older code variant, as the new LVDS
encoder was backported to v4.14-ltsi.
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 3fc7e6899cab5843..50c11a7f0467f746 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -16,6 +16,7 @@
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/sys_soc.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
 	return 0;
 }
 
+static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
+	.gen = 2,
+	.quirks = RCAR_LVDS_QUIRK_LANES,
+	.pll_setup = rcar_lvds_pll_setup_gen2,
+};
+
+static const struct soc_device_attribute lvds_quirk_matches[] = {
+	{
+		.soc_id = "r8a7790", .revision = "ES1.*",
+		.data = &rcar_lvds_r8a7790es1_info,
+	},
+	{ /* sentinel */ }
+};
+
 static int rcar_lvds_probe(struct platform_device *pdev)
 {
+	const struct soc_device_attribute *attr;
 	struct rcar_lvds *lvds;
 	struct resource *mem;
 	int ret;
@@ -857,6 +873,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
 	lvds->dev = &pdev->dev;
 	lvds->info = of_device_get_match_data(&pdev->dev);
 
+	attr = soc_device_match(lvds_quirk_matches);
+	if (attr)
+		lvds->info = attr->data;
+
 	ret = rcar_lvds_parse_dt(lvds);
 	if (ret < 0)
 		return ret;
@@ -893,12 +913,6 @@ static const struct rcar_lvds_device_info rcar_lvds_gen2_info = {
 	.pll_setup = rcar_lvds_pll_setup_gen2,
 };
 
-static const struct rcar_lvds_device_info rcar_lvds_r8a7790_info = {
-	.gen = 2,
-	.quirks = RCAR_LVDS_QUIRK_LANES,
-	.pll_setup = rcar_lvds_pll_setup_gen2,
-};
-
 static const struct rcar_lvds_device_info rcar_lvds_gen3_info = {
 	.gen = 3,
 	.quirks = RCAR_LVDS_QUIRK_PWD,
@@ -930,7 +944,7 @@ static const struct of_device_id rcar_lvds_of_table[] = {
 	{ .compatible = "renesas,r8a7744-lvds", .data = &rcar_lvds_gen2_info },
 	{ .compatible = "renesas,r8a774a1-lvds", .data = &rcar_lvds_gen3_info },
 	{ .compatible = "renesas,r8a774c0-lvds", .data = &rcar_lvds_r8a77990_info },
-	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info },
+	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_gen2_info },
 	{ .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
 	{ .compatible = "renesas,r8a7793-lvds", .data = &rcar_lvds_gen2_info },
 	{ .compatible = "renesas,r8a7795-lvds", .data = &rcar_lvds_gen3_info },
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Ulrich Hecht]

> On September 17, 2019 at 8:23 AM Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> 
> 
> Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
> that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
> problem has been fixed in newer revisions.
> 
> However, the code didn't take into account the actual hardware revision,
> thus applying the quirk also on newer hardware revisions, causing green
> color reversals.
> 
> Fix this by applying the quirk when running on R-Car H2 ES1.x only.
> 
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?
> 
> While the issue was present before aforementioned commit, I do not think
> there is a real need to fix the older code variant, as the new LVDS
> encoder was backported to v4.14-ltsi.
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 3fc7e6899cab5843..50c11a7f0467f746 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/sys_soc.h>
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
>  	return 0;
>  }
>  
> +static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
> +	.gen = 2,
> +	.quirks = RCAR_LVDS_QUIRK_LANES,
> +	.pll_setup = rcar_lvds_pll_setup_gen2,
> +};
> +
> +static const struct soc_device_attribute lvds_quirk_matches[] = {
> +	{
> +		.soc_id = "r8a7790", .revision = "ES1.*",
> +		.data = &rcar_lvds_r8a7790es1_info,
> +	},
> +	{ /* sentinel */ }
> +};
> +
>  static int rcar_lvds_probe(struct platform_device *pdev)
>  {
> +	const struct soc_device_attribute *attr;
>  	struct rcar_lvds *lvds;
>  	struct resource *mem;
>  	int ret;
> @@ -857,6 +873,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	lvds->dev = &pdev->dev;
>  	lvds->info = of_device_get_match_data(&pdev->dev);
>  
> +	attr = soc_device_match(lvds_quirk_matches);
> +	if (attr)
> +		lvds->info = attr->data;
> +
>  	ret = rcar_lvds_parse_dt(lvds);
>  	if (ret < 0)
>  		return ret;
> @@ -893,12 +913,6 @@ static const struct rcar_lvds_device_info rcar_lvds_gen2_info = {
>  	.pll_setup = rcar_lvds_pll_setup_gen2,
>  };
>  
> -static const struct rcar_lvds_device_info rcar_lvds_r8a7790_info = {
> -	.gen = 2,
> -	.quirks = RCAR_LVDS_QUIRK_LANES,
> -	.pll_setup = rcar_lvds_pll_setup_gen2,
> -};
> -
>  static const struct rcar_lvds_device_info rcar_lvds_gen3_info = {
>  	.gen = 3,
>  	.quirks = RCAR_LVDS_QUIRK_PWD,
> @@ -930,7 +944,7 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  	{ .compatible = "renesas,r8a7744-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a774a1-lvds", .data = &rcar_lvds_gen3_info },
>  	{ .compatible = "renesas,r8a774c0-lvds", .data = &rcar_lvds_r8a77990_info },
> -	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info },
> +	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7793-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7795-lvds", .data = &rcar_lvds_gen3_info },
> -- 
> 2.17.1

Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>

CU
Uli

Re: [PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Ulrich Hecht]

> On September 17, 2019 at 8:23 AM Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> 
> 
> Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
> that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
> problem has been fixed in newer revisions.
> 
> However, the code didn't take into account the actual hardware revision,
> thus applying the quirk also on newer hardware revisions, causing green
> color reversals.
> 
> Fix this by applying the quirk when running on R-Car H2 ES1.x only.
> 
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?
> 
> While the issue was present before aforementioned commit, I do not think
> there is a real need to fix the older code variant, as the new LVDS
> encoder was backported to v4.14-ltsi.
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 3fc7e6899cab5843..50c11a7f0467f746 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/sys_soc.h>
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
>  	return 0;
>  }
>  
> +static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
> +	.gen = 2,
> +	.quirks = RCAR_LVDS_QUIRK_LANES,
> +	.pll_setup = rcar_lvds_pll_setup_gen2,
> +};
> +
> +static const struct soc_device_attribute lvds_quirk_matches[] = {
> +	{
> +		.soc_id = "r8a7790", .revision = "ES1.*",
> +		.data = &rcar_lvds_r8a7790es1_info,
> +	},
> +	{ /* sentinel */ }
> +};
> +
>  static int rcar_lvds_probe(struct platform_device *pdev)
>  {
> +	const struct soc_device_attribute *attr;
>  	struct rcar_lvds *lvds;
>  	struct resource *mem;
>  	int ret;
> @@ -857,6 +873,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	lvds->dev = &pdev->dev;
>  	lvds->info = of_device_get_match_data(&pdev->dev);
>  
> +	attr = soc_device_match(lvds_quirk_matches);
> +	if (attr)
> +		lvds->info = attr->data;
> +
>  	ret = rcar_lvds_parse_dt(lvds);
>  	if (ret < 0)
>  		return ret;
> @@ -893,12 +913,6 @@ static const struct rcar_lvds_device_info rcar_lvds_gen2_info = {
>  	.pll_setup = rcar_lvds_pll_setup_gen2,
>  };
>  
> -static const struct rcar_lvds_device_info rcar_lvds_r8a7790_info = {
> -	.gen = 2,
> -	.quirks = RCAR_LVDS_QUIRK_LANES,
> -	.pll_setup = rcar_lvds_pll_setup_gen2,
> -};
> -
>  static const struct rcar_lvds_device_info rcar_lvds_gen3_info = {
>  	.gen = 3,
>  	.quirks = RCAR_LVDS_QUIRK_PWD,
> @@ -930,7 +944,7 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  	{ .compatible = "renesas,r8a7744-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a774a1-lvds", .data = &rcar_lvds_gen3_info },
>  	{ .compatible = "renesas,r8a774c0-lvds", .data = &rcar_lvds_r8a77990_info },
> -	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info },
> +	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7793-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7795-lvds", .data = &rcar_lvds_gen3_info },
> -- 
> 2.17.1

Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>

CU
Uli
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Tue, Sep 17, 2019 at 08:23:53AM +0200, Geert Uytterhoeven wrote:
> Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
> that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
> problem has been fixed in newer revisions.
> 
> However, the code didn't take into account the actual hardware revision,
> thus applying the quirk also on newer hardware revisions, causing green
> color reversals.

Oops :-S

> Fix this by applying the quirk when running on R-Car H2 ES1.x only.
> 
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")

Shouldn't this be

Fixes: 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk")

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?

Or if there's any ES1.x other than ES1.0 ? :-)

> While the issue was present before aforementioned commit, I do not think
> there is a real need to fix the older code variant, as the new LVDS
> encoder was backported to v4.14-ltsi.

Probably not, but I think there's still value in pointing to the right
erroneous commit. It's a Fixes: tag, not a Backport-up-to: tag :-)

> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 3fc7e6899cab5843..50c11a7f0467f746 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/sys_soc.h>
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
>  	return 0;
>  }
>  
> +static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
> +	.gen = 2,
> +	.quirks = RCAR_LVDS_QUIRK_LANES,
> +	.pll_setup = rcar_lvds_pll_setup_gen2,
> +};
> +
> +static const struct soc_device_attribute lvds_quirk_matches[] = {
> +	{
> +		.soc_id = "r8a7790", .revision = "ES1.*",

Do you mind splitting this in two lines ?

With these small issues fixes,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Please let me know if I should fix while applying or if you want to send
a new version.

> +		.data = &rcar_lvds_r8a7790es1_info,
> +	},
> +	{ /* sentinel */ }
> +};
> +
>  static int rcar_lvds_probe(struct platform_device *pdev)
>  {
> +	const struct soc_device_attribute *attr;
>  	struct rcar_lvds *lvds;
>  	struct resource *mem;
>  	int ret;
> @@ -857,6 +873,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	lvds->dev = &pdev->dev;
>  	lvds->info = of_device_get_match_data(&pdev->dev);
>  
> +	attr = soc_device_match(lvds_quirk_matches);
> +	if (attr)
> +		lvds->info = attr->data;
> +
>  	ret = rcar_lvds_parse_dt(lvds);
>  	if (ret < 0)
>  		return ret;
> @@ -893,12 +913,6 @@ static const struct rcar_lvds_device_info rcar_lvds_gen2_info = {
>  	.pll_setup = rcar_lvds_pll_setup_gen2,
>  };
>  
> -static const struct rcar_lvds_device_info rcar_lvds_r8a7790_info = {
> -	.gen = 2,
> -	.quirks = RCAR_LVDS_QUIRK_LANES,
> -	.pll_setup = rcar_lvds_pll_setup_gen2,
> -};
> -
>  static const struct rcar_lvds_device_info rcar_lvds_gen3_info = {
>  	.gen = 3,
>  	.quirks = RCAR_LVDS_QUIRK_PWD,
> @@ -930,7 +944,7 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  	{ .compatible = "renesas,r8a7744-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a774a1-lvds", .data = &rcar_lvds_gen3_info },
>  	{ .compatible = "renesas,r8a774c0-lvds", .data = &rcar_lvds_r8a77990_info },
> -	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info },
> +	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7793-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7795-lvds", .data = &rcar_lvds_gen3_info },

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Tue, Sep 17, 2019 at 08:23:53AM +0200, Geert Uytterhoeven wrote:
> Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
> that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
> problem has been fixed in newer revisions.
> 
> However, the code didn't take into account the actual hardware revision,
> thus applying the quirk also on newer hardware revisions, causing green
> color reversals.

Oops :-S

> Fix this by applying the quirk when running on R-Car H2 ES1.x only.
> 
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")

Shouldn't this be

Fixes: 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk")

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?

Or if there's any ES1.x other than ES1.0 ? :-)

> While the issue was present before aforementioned commit, I do not think
> there is a real need to fix the older code variant, as the new LVDS
> encoder was backported to v4.14-ltsi.

Probably not, but I think there's still value in pointing to the right
erroneous commit. It's a Fixes: tag, not a Backport-up-to: tag :-)

> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 3fc7e6899cab5843..50c11a7f0467f746 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/sys_soc.h>
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
>  	return 0;
>  }
>  
> +static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
> +	.gen = 2,
> +	.quirks = RCAR_LVDS_QUIRK_LANES,
> +	.pll_setup = rcar_lvds_pll_setup_gen2,
> +};
> +
> +static const struct soc_device_attribute lvds_quirk_matches[] = {
> +	{
> +		.soc_id = "r8a7790", .revision = "ES1.*",

Do you mind splitting this in two lines ?

With these small issues fixes,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Please let me know if I should fix while applying or if you want to send
a new version.

> +		.data = &rcar_lvds_r8a7790es1_info,
> +	},
> +	{ /* sentinel */ }
> +};
> +
>  static int rcar_lvds_probe(struct platform_device *pdev)
>  {
> +	const struct soc_device_attribute *attr;
>  	struct rcar_lvds *lvds;
>  	struct resource *mem;
>  	int ret;
> @@ -857,6 +873,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	lvds->dev = &pdev->dev;
>  	lvds->info = of_device_get_match_data(&pdev->dev);
>  
> +	attr = soc_device_match(lvds_quirk_matches);
> +	if (attr)
> +		lvds->info = attr->data;
> +
>  	ret = rcar_lvds_parse_dt(lvds);
>  	if (ret < 0)
>  		return ret;
> @@ -893,12 +913,6 @@ static const struct rcar_lvds_device_info rcar_lvds_gen2_info = {
>  	.pll_setup = rcar_lvds_pll_setup_gen2,
>  };
>  
> -static const struct rcar_lvds_device_info rcar_lvds_r8a7790_info = {
> -	.gen = 2,
> -	.quirks = RCAR_LVDS_QUIRK_LANES,
> -	.pll_setup = rcar_lvds_pll_setup_gen2,
> -};
> -
>  static const struct rcar_lvds_device_info rcar_lvds_gen3_info = {
>  	.gen = 3,
>  	.quirks = RCAR_LVDS_QUIRK_PWD,
> @@ -930,7 +944,7 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  	{ .compatible = "renesas,r8a7744-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a774a1-lvds", .data = &rcar_lvds_gen3_info },
>  	{ .compatible = "renesas,r8a774c0-lvds", .data = &rcar_lvds_r8a77990_info },
> -	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info },
> +	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7793-lvds", .data = &rcar_lvds_gen2_info },
>  	{ .compatible = "renesas,r8a7795-lvds", .data = &rcar_lvds_gen3_info },

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Laurent Pinchart]

On Sat, Sep 21, 2019 at 02:40:03AM +0300, Laurent Pinchart wrote:
> On Tue, Sep 17, 2019 at 08:23:53AM +0200, Geert Uytterhoeven wrote:
> > Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
> > that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
> > problem has been fixed in newer revisions.
> > 
> > However, the code didn't take into account the actual hardware revision,
> > thus applying the quirk also on newer hardware revisions, causing green
> > color reversals.
> 
> Oops :-S
> 
> > Fix this by applying the quirk when running on R-Car H2 ES1.x only.
> > 
> > Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
> 
> Shouldn't this be
> 
> Fixes: 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk")
> 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> > Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?
> 
> Or if there's any ES1.x other than ES1.0 ? :-)
> 
> > While the issue was present before aforementioned commit, I do not think
> > there is a real need to fix the older code variant, as the new LVDS
> > encoder was backported to v4.14-ltsi.
> 
> Probably not, but I think there's still value in pointing to the right
> erroneous commit. It's a Fixes: tag, not a Backport-up-to: tag :-)
> 
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 3fc7e6899cab5843..50c11a7f0467f746 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/of_graph.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/slab.h>
> > +#include <linux/sys_soc.h>
> >  
> >  #include <drm/drm_atomic.h>
> >  #include <drm/drm_atomic_helper.h>
> > @@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
> >  	return 0;
> >  }
> >  
> > +static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
> > +	.gen = 2,
> > +	.quirks = RCAR_LVDS_QUIRK_LANES,
> > +	.pll_setup = rcar_lvds_pll_setup_gen2,
> > +};
> > +
> > +static const struct soc_device_attribute lvds_quirk_matches[] = {
> > +	{
> > +		.soc_id = "r8a7790", .revision = "ES1.*",
> 
> Do you mind splitting this in two lines ?

Actually, it could be argued that having both on the same line is more
readable. I'll let you decide what you like best.

> With these small issues fixes,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Please let me know if I should fix while applying or if you want to send
> a new version.
> 
> > +		.data = &rcar_lvds_r8a7790es1_info,
> > +	},
> > +	{ /* sentinel */ }
> > +};
> > +
> >  static int rcar_lvds_probe(struct platform_device *pdev)
> >  {
> > +	const struct soc_device_attribute *attr;
> >  	struct rcar_lvds *lvds;
> >  	struct resource *mem;
> >  	int ret;
> > @@ -857,6 +873,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
> >  	lvds->dev = &pdev->dev;
> >  	lvds->info = of_device_get_match_data(&pdev->dev);
> >  
> > +	attr = soc_device_match(lvds_quirk_matches);
> > +	if (attr)
> > +		lvds->info = attr->data;
> > +
> >  	ret = rcar_lvds_parse_dt(lvds);
> >  	if (ret < 0)
> >  		return ret;
> > @@ -893,12 +913,6 @@ static const struct rcar_lvds_device_info rcar_lvds_gen2_info = {
> >  	.pll_setup = rcar_lvds_pll_setup_gen2,
> >  };
> >  
> > -static const struct rcar_lvds_device_info rcar_lvds_r8a7790_info = {
> > -	.gen = 2,
> > -	.quirks = RCAR_LVDS_QUIRK_LANES,
> > -	.pll_setup = rcar_lvds_pll_setup_gen2,
> > -};
> > -
> >  static const struct rcar_lvds_device_info rcar_lvds_gen3_info = {
> >  	.gen = 3,
> >  	.quirks = RCAR_LVDS_QUIRK_PWD,
> > @@ -930,7 +944,7 @@ static const struct of_device_id rcar_lvds_of_table[] = {
> >  	{ .compatible = "renesas,r8a7744-lvds", .data = &rcar_lvds_gen2_info },
> >  	{ .compatible = "renesas,r8a774a1-lvds", .data = &rcar_lvds_gen3_info },
> >  	{ .compatible = "renesas,r8a774c0-lvds", .data = &rcar_lvds_r8a77990_info },
> > -	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info },
> > +	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_gen2_info },
> >  	{ .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
> >  	{ .compatible = "renesas,r8a7793-lvds", .data = &rcar_lvds_gen2_info },
> >  	{ .compatible = "renesas,r8a7795-lvds", .data = &rcar_lvds_gen3_info },
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Laurent Pinchart]

On Sat, Sep 21, 2019 at 02:40:03AM +0300, Laurent Pinchart wrote:
> On Tue, Sep 17, 2019 at 08:23:53AM +0200, Geert Uytterhoeven wrote:
> > Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
> > that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
> > problem has been fixed in newer revisions.
> > 
> > However, the code didn't take into account the actual hardware revision,
> > thus applying the quirk also on newer hardware revisions, causing green
> > color reversals.
> 
> Oops :-S
> 
> > Fix this by applying the quirk when running on R-Car H2 ES1.x only.
> > 
> > Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
> 
> Shouldn't this be
> 
> Fixes: 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk")
> 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> > Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?
> 
> Or if there's any ES1.x other than ES1.0 ? :-)
> 
> > While the issue was present before aforementioned commit, I do not think
> > there is a real need to fix the older code variant, as the new LVDS
> > encoder was backported to v4.14-ltsi.
> 
> Probably not, but I think there's still value in pointing to the right
> erroneous commit. It's a Fixes: tag, not a Backport-up-to: tag :-)
> 
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 3fc7e6899cab5843..50c11a7f0467f746 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/of_graph.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/slab.h>
> > +#include <linux/sys_soc.h>
> >  
> >  #include <drm/drm_atomic.h>
> >  #include <drm/drm_atomic_helper.h>
> > @@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
> >  	return 0;
> >  }
> >  
> > +static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
> > +	.gen = 2,
> > +	.quirks = RCAR_LVDS_QUIRK_LANES,
> > +	.pll_setup = rcar_lvds_pll_setup_gen2,
> > +};
> > +
> > +static const struct soc_device_attribute lvds_quirk_matches[] = {
> > +	{
> > +		.soc_id = "r8a7790", .revision = "ES1.*",
> 
> Do you mind splitting this in two lines ?

Actually, it could be argued that having both on the same line is more
readable. I'll let you decide what you like best.

> With these small issues fixes,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Please let me know if I should fix while applying or if you want to send
> a new version.
> 
> > +		.data = &rcar_lvds_r8a7790es1_info,
> > +	},
> > +	{ /* sentinel */ }
> > +};
> > +
> >  static int rcar_lvds_probe(struct platform_device *pdev)
> >  {
> > +	const struct soc_device_attribute *attr;
> >  	struct rcar_lvds *lvds;
> >  	struct resource *mem;
> >  	int ret;
> > @@ -857,6 +873,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
> >  	lvds->dev = &pdev->dev;
> >  	lvds->info = of_device_get_match_data(&pdev->dev);
> >  
> > +	attr = soc_device_match(lvds_quirk_matches);
> > +	if (attr)
> > +		lvds->info = attr->data;
> > +
> >  	ret = rcar_lvds_parse_dt(lvds);
> >  	if (ret < 0)
> >  		return ret;
> > @@ -893,12 +913,6 @@ static const struct rcar_lvds_device_info rcar_lvds_gen2_info = {
> >  	.pll_setup = rcar_lvds_pll_setup_gen2,
> >  };
> >  
> > -static const struct rcar_lvds_device_info rcar_lvds_r8a7790_info = {
> > -	.gen = 2,
> > -	.quirks = RCAR_LVDS_QUIRK_LANES,
> > -	.pll_setup = rcar_lvds_pll_setup_gen2,
> > -};
> > -
> >  static const struct rcar_lvds_device_info rcar_lvds_gen3_info = {
> >  	.gen = 3,
> >  	.quirks = RCAR_LVDS_QUIRK_PWD,
> > @@ -930,7 +944,7 @@ static const struct of_device_id rcar_lvds_of_table[] = {
> >  	{ .compatible = "renesas,r8a7744-lvds", .data = &rcar_lvds_gen2_info },
> >  	{ .compatible = "renesas,r8a774a1-lvds", .data = &rcar_lvds_gen3_info },
> >  	{ .compatible = "renesas,r8a774c0-lvds", .data = &rcar_lvds_r8a77990_info },
> > -	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info },
> > +	{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_gen2_info },
> >  	{ .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
> >  	{ .compatible = "renesas,r8a7793-lvds", .data = &rcar_lvds_gen2_info },
> >  	{ .compatible = "renesas,r8a7795-lvds", .data = &rcar_lvds_gen3_info },
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Sep 21, 2019 at 1:43 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Sat, Sep 21, 2019 at 02:40:03AM +0300, Laurent Pinchart wrote:
> > On Tue, Sep 17, 2019 at 08:23:53AM +0200, Geert Uytterhoeven wrote:
> > > Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
> > > that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
> > > problem has been fixed in newer revisions.
> > >
> > > However, the code didn't take into account the actual hardware revision,
> > > thus applying the quirk also on newer hardware revisions, causing green
> > > color reversals.
> >
> > Oops :-S

Quite understandable, as there was no soc_device_match() in 2013...

> > > Fix this by applying the quirk when running on R-Car H2 ES1.x only.
> > >
> > > Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
> >
> > Shouldn't this be
> >
> > Fixes: 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk")

Yes, that's where the issue was introduced. But see my original comment
about backporting below.

> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > > Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?
> >
> > Or if there's any ES1.x other than ES1.0 ? :-)
> >
> > > While the issue was present before aforementioned commit, I do not think
> > > there is a real need to fix the older code variant, as the new LVDS
> > > encoder was backported to v4.14-ltsi.
> >
> > Probably not, but I think there's still value in pointing to the right
> > erroneous commit. It's a Fixes: tag, not a Backport-up-to: tag :-)

OK.

> > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > @@ -16,6 +16,7 @@
> > >  #include <linux/of_graph.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/sys_soc.h>
> > >
> > >  #include <drm/drm_atomic.h>
> > >  #include <drm/drm_atomic_helper.h>
> > > @@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
> > >     return 0;
> > >  }
> > >
> > > +static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
> > > +   .gen = 2,
> > > +   .quirks = RCAR_LVDS_QUIRK_LANES,
> > > +   .pll_setup = rcar_lvds_pll_setup_gen2,
> > > +};
> > > +
> > > +static const struct soc_device_attribute lvds_quirk_matches[] = {
> > > +   {
> > > +           .soc_id = "r8a7790", .revision = "ES1.*",
> >
> > Do you mind splitting this in two lines ?

Yes I do: it makes it easier to locate fixes for early silicon.

> Actually, it could be argued that having both on the same line is more
> readable. I'll let you decide what you like best.

I'm happy to hear you're reconsidering!

> > With these small issues fixes,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Please let me know if I should fix while applying or if you want to send
> > a new version.

Feel free to fix (replace Fixes or add a second Fixes tag) while applying.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] drm: rcar_lvds: Fix color mismatches on R-Car H2 ES2.0 and later

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Sep 21, 2019 at 1:43 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Sat, Sep 21, 2019 at 02:40:03AM +0300, Laurent Pinchart wrote:
> > On Tue, Sep 17, 2019 at 08:23:53AM +0200, Geert Uytterhoeven wrote:
> > > Commit 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk") states
> > > that LVDS lanes 1 and 3 are inverted on R-Car H2 ES1 only, and that the
> > > problem has been fixed in newer revisions.
> > >
> > > However, the code didn't take into account the actual hardware revision,
> > > thus applying the quirk also on newer hardware revisions, causing green
> > > color reversals.
> >
> > Oops :-S

Quite understandable, as there was no soc_device_match() in 2013...

> > > Fix this by applying the quirk when running on R-Car H2 ES1.x only.
> > >
> > > Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > Fixes: c6a27fa41fabb35f ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
> >
> > Shouldn't this be
> >
> > Fixes: 5cca30ebe089be23 ("drm/rcar-du: Add LVDS_LANES quirk")

Yes, that's where the issue was introduced. But see my original comment
about backporting below.

> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > > Does anyone know if this was fixed in ES2.0, or in any earlier ES1.x?
> >
> > Or if there's any ES1.x other than ES1.0 ? :-)
> >
> > > While the issue was present before aforementioned commit, I do not think
> > > there is a real need to fix the older code variant, as the new LVDS
> > > encoder was backported to v4.14-ltsi.
> >
> > Probably not, but I think there's still value in pointing to the right
> > erroneous commit. It's a Fixes: tag, not a Backport-up-to: tag :-)

OK.

> > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > @@ -16,6 +16,7 @@
> > >  #include <linux/of_graph.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/sys_soc.h>
> > >
> > >  #include <drm/drm_atomic.h>
> > >  #include <drm/drm_atomic_helper.h>
> > > @@ -842,8 +843,23 @@ static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
> > >     return 0;
> > >  }
> > >
> > > +static const struct rcar_lvds_device_info rcar_lvds_r8a7790es1_info = {
> > > +   .gen = 2,
> > > +   .quirks = RCAR_LVDS_QUIRK_LANES,
> > > +   .pll_setup = rcar_lvds_pll_setup_gen2,
> > > +};
> > > +
> > > +static const struct soc_device_attribute lvds_quirk_matches[] = {
> > > +   {
> > > +           .soc_id = "r8a7790", .revision = "ES1.*",
> >
> > Do you mind splitting this in two lines ?

Yes I do: it makes it easier to locate fixes for early silicon.

> Actually, it could be argued that having both on the same line is more
> readable. I'll let you decide what you like best.

I'm happy to hear you're reconsidering!

> > With these small issues fixes,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Please let me know if I should fix while applying or if you want to send
> > a new version.

Feel free to fix (replace Fixes or add a second Fixes tag) while applying.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel