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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
> 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@xxxxxxxxx>
> Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxx>

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



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux