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
