Message ID | 20230117135154.387208-5-tomi.valkeinen+renesas@ideasonboard.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm: rcar-du: Misc patches | expand |
Hi Tomi, Thank you for the patch. On Tue, Jan 17, 2023 at 03:51:52PM +0200, Tomi Valkeinen wrote: > rcar_du_crtc.c does a soc_device_match() in > rcar_du_crtc_set_display_timing() to find out if the SoC is H3 ES1, and s/ES1/ES1.x/ Same below. > if so, apply a WA. s/WA/workaround/ Same below. > We will need another H3 ES1 check in the following patch, so rather than > adding more soc_device_match() calls, let's add a rcar_du_device_info > entry for the ES1, and a quirk flag, > RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY, for the WA. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 8 +--- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 51 +++++++++++++++++++++++++- > drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 + > 3 files changed, 52 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > index 3619e1ddeb62..f2d3266509cc 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -10,7 +10,6 @@ > #include <linux/clk.h> > #include <linux/mutex.h> > #include <linux/platform_device.h> > -#include <linux/sys_soc.h> > > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > @@ -204,11 +203,6 @@ static void rcar_du_escr_divider(struct clk *clk, unsigned long target, > } > } > > -static const struct soc_device_attribute rcar_du_r8a7795_es1[] = { > - { .soc_id = "r8a7795", .revision = "ES1.*" }, > - { /* sentinel */ } > -}; > - > static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) > { > const struct drm_display_mode *mode = &rcrtc->crtc.state->adjusted_mode; > @@ -238,7 +232,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) > * no post-divider when a display PLL is present (as shown by > * the workaround breaking HDMI output on M3-W during testing). > */ > - if (soc_device_match(rcar_du_r8a7795_es1)) { > + if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY) { > target *= 2; > div = 1; > } > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index c7c5217cfc1a..ba2e069fc0f7 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -16,6 +16,7 @@ > #include <linux/platform_device.h> > #include <linux/pm.h> > #include <linux/slab.h> > +#include <linux/sys_soc.h> > #include <linux/wait.h> > > #include <drm/drm_atomic_helper.h> > @@ -386,6 +387,42 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = { > .dpll_mask = BIT(2) | BIT(1), > }; > > +static const struct rcar_du_device_info rcar_du_r8a7795_es1_info = { > + .gen = 3, > + .features = RCAR_DU_FEATURE_CRTC_IRQ > + | RCAR_DU_FEATURE_CRTC_CLOCK > + | RCAR_DU_FEATURE_VSP1_SOURCE > + | RCAR_DU_FEATURE_INTERLACED > + | RCAR_DU_FEATURE_TVM_SYNC, > + .quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY, > + .channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0), > + .routes = { > + /* > + * R8A7795 has one RGB output, two HDMI outputs and one > + * LVDS output. > + */ > + [RCAR_DU_OUTPUT_DPAD0] = { > + .possible_crtcs = BIT(3), > + .port = 0, > + }, > + [RCAR_DU_OUTPUT_HDMI0] = { > + .possible_crtcs = BIT(1), > + .port = 1, > + }, > + [RCAR_DU_OUTPUT_HDMI1] = { > + .possible_crtcs = BIT(2), > + .port = 2, > + }, > + [RCAR_DU_OUTPUT_LVDS0] = { > + .possible_crtcs = BIT(0), > + .port = 3, > + }, > + }, > + .num_lvds = 1, > + .num_rpf = 5, > + .dpll_mask = BIT(2) | BIT(1), > +}; > + > static const struct rcar_du_device_info rcar_du_r8a7796_info = { > .gen = 3, > .features = RCAR_DU_FEATURE_CRTC_IRQ > @@ -576,6 +613,11 @@ static const struct of_device_id rcar_du_of_table[] = { > > MODULE_DEVICE_TABLE(of, rcar_du_of_table); > > +static const struct soc_device_attribute rcar_du_soc_table[] = { > + { .soc_id = "r8a7795", .revision = "ES1.*", .data = &rcar_du_r8a7795_es1_info }, > + { /* sentinel */ } > +}; > + > const char *rcar_du_output_name(enum rcar_du_output output) > { > static const char * const names[] = { > @@ -670,6 +712,7 @@ static int rcar_du_probe(struct platform_device *pdev) > struct rcar_du_device *rcdu; > unsigned int mask; > int ret; > + const struct soc_device_attribute *soc_attr; Please move this up before rcdu. > > if (drm_firmware_drivers_only()) > return -ENODEV; > @@ -681,7 +724,13 @@ static int rcar_du_probe(struct platform_device *pdev) > return PTR_ERR(rcdu); > > rcdu->dev = &pdev->dev; > - rcdu->info = of_device_get_match_data(rcdu->dev); > + > + soc_attr = soc_device_match(rcar_du_soc_table); > + if (soc_attr) > + rcdu->info = soc_attr->data; > + > + if (!rcdu->info) > + rcdu->info = of_device_get_match_data(rcdu->dev); As Geert mentioned, rcdu->info = of_device_get_match_data(rcdu->dev); soc_attr = soc_device_match(rcar_du_soc_table); if (soc_attr) rcdu->info = soc_attr->data; Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > platform_set_drvdata(pdev, rcdu); > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h > index 5cfa2bb7ad93..df87ccab146f 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h > @@ -34,6 +34,7 @@ struct rcar_du_device; > #define RCAR_DU_FEATURE_NO_BLENDING BIT(5) /* PnMR.SPIM does not have ALP nor EOR bits */ > > #define RCAR_DU_QUIRK_ALIGN_128B BIT(0) /* Align pitches to 128 bytes */ > +#define RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY BIT(1) /* H3 ES1 has pclk stability issue */ > > enum rcar_du_output { > RCAR_DU_OUTPUT_DPAD0,
On 17/01/2023 18:11, Geert Uytterhoeven wrote: > Hi Tomi, > > On Tue, Jan 17, 2023 at 2:54 PM Tomi Valkeinen > <tomi.valkeinen+renesas@ideasonboard.com> wrote: >> rcar_du_crtc.c does a soc_device_match() in >> rcar_du_crtc_set_display_timing() to find out if the SoC is H3 ES1, and >> if so, apply a WA. >> >> We will need another H3 ES1 check in the following patch, so rather than >> adding more soc_device_match() calls, let's add a rcar_du_device_info >> entry for the ES1, and a quirk flag, >> RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY, for the WA. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> > > Thanks for your patch! > >> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > >> @@ -681,7 +724,13 @@ static int rcar_du_probe(struct platform_device *pdev) >> return PTR_ERR(rcdu); >> >> rcdu->dev = &pdev->dev; >> - rcdu->info = of_device_get_match_data(rcdu->dev); > > No need to remove this line... > >> + >> + soc_attr = soc_device_match(rcar_du_soc_table); >> + if (soc_attr) >> + rcdu->info = soc_attr->data; >> + >> + if (!rcdu->info) >> + rcdu->info = of_device_get_match_data(rcdu->dev); > > ... and no need to add these two lines. > The idiom is to set rcdu->info based on of_device_get_match_data() > first, and override based on of_device_get_match_data() when needed. Ok. Tomi
On 18/01/2023 23:19, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Tue, Jan 17, 2023 at 03:51:52PM +0200, Tomi Valkeinen wrote: >> rcar_du_crtc.c does a soc_device_match() in >> rcar_du_crtc_set_display_timing() to find out if the SoC is H3 ES1, and > > s/ES1/ES1.x/ > > Same below. > >> if so, apply a WA. > > s/WA/workaround/ > > Same below. Ok. >> We will need another H3 ES1 check in the following patch, so rather than >> adding more soc_device_match() calls, let's add a rcar_du_device_info >> entry for the ES1, and a quirk flag, >> RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY, for the WA. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> >> --- >> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 8 +--- >> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 51 +++++++++++++++++++++++++- >> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 + >> 3 files changed, 52 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c >> index 3619e1ddeb62..f2d3266509cc 100644 >> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c >> @@ -10,7 +10,6 @@ >> #include <linux/clk.h> >> #include <linux/mutex.h> >> #include <linux/platform_device.h> >> -#include <linux/sys_soc.h> >> >> #include <drm/drm_atomic.h> >> #include <drm/drm_atomic_helper.h> >> @@ -204,11 +203,6 @@ static void rcar_du_escr_divider(struct clk *clk, unsigned long target, >> } >> } >> >> -static const struct soc_device_attribute rcar_du_r8a7795_es1[] = { >> - { .soc_id = "r8a7795", .revision = "ES1.*" }, >> - { /* sentinel */ } >> -}; >> - >> static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) >> { >> const struct drm_display_mode *mode = &rcrtc->crtc.state->adjusted_mode; >> @@ -238,7 +232,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) >> * no post-divider when a display PLL is present (as shown by >> * the workaround breaking HDMI output on M3-W during testing). >> */ >> - if (soc_device_match(rcar_du_r8a7795_es1)) { >> + if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY) { >> target *= 2; >> div = 1; >> } >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> index c7c5217cfc1a..ba2e069fc0f7 100644 >> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> @@ -16,6 +16,7 @@ >> #include <linux/platform_device.h> >> #include <linux/pm.h> >> #include <linux/slab.h> >> +#include <linux/sys_soc.h> >> #include <linux/wait.h> >> >> #include <drm/drm_atomic_helper.h> >> @@ -386,6 +387,42 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = { >> .dpll_mask = BIT(2) | BIT(1), >> }; >> >> +static const struct rcar_du_device_info rcar_du_r8a7795_es1_info = { >> + .gen = 3, >> + .features = RCAR_DU_FEATURE_CRTC_IRQ >> + | RCAR_DU_FEATURE_CRTC_CLOCK >> + | RCAR_DU_FEATURE_VSP1_SOURCE >> + | RCAR_DU_FEATURE_INTERLACED >> + | RCAR_DU_FEATURE_TVM_SYNC, >> + .quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY, >> + .channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0), >> + .routes = { >> + /* >> + * R8A7795 has one RGB output, two HDMI outputs and one >> + * LVDS output. >> + */ >> + [RCAR_DU_OUTPUT_DPAD0] = { >> + .possible_crtcs = BIT(3), >> + .port = 0, >> + }, >> + [RCAR_DU_OUTPUT_HDMI0] = { >> + .possible_crtcs = BIT(1), >> + .port = 1, >> + }, >> + [RCAR_DU_OUTPUT_HDMI1] = { >> + .possible_crtcs = BIT(2), >> + .port = 2, >> + }, >> + [RCAR_DU_OUTPUT_LVDS0] = { >> + .possible_crtcs = BIT(0), >> + .port = 3, >> + }, >> + }, >> + .num_lvds = 1, >> + .num_rpf = 5, >> + .dpll_mask = BIT(2) | BIT(1), >> +}; >> + >> static const struct rcar_du_device_info rcar_du_r8a7796_info = { >> .gen = 3, >> .features = RCAR_DU_FEATURE_CRTC_IRQ >> @@ -576,6 +613,11 @@ static const struct of_device_id rcar_du_of_table[] = { >> >> MODULE_DEVICE_TABLE(of, rcar_du_of_table); >> >> +static const struct soc_device_attribute rcar_du_soc_table[] = { >> + { .soc_id = "r8a7795", .revision = "ES1.*", .data = &rcar_du_r8a7795_es1_info }, >> + { /* sentinel */ } >> +}; >> + >> const char *rcar_du_output_name(enum rcar_du_output output) >> { >> static const char * const names[] = { >> @@ -670,6 +712,7 @@ static int rcar_du_probe(struct platform_device *pdev) >> struct rcar_du_device *rcdu; >> unsigned int mask; >> int ret; >> + const struct soc_device_attribute *soc_attr; > > Please move this up before rcdu. Sure. >> >> if (drm_firmware_drivers_only()) >> return -ENODEV; >> @@ -681,7 +724,13 @@ static int rcar_du_probe(struct platform_device *pdev) >> return PTR_ERR(rcdu); >> >> rcdu->dev = &pdev->dev; >> - rcdu->info = of_device_get_match_data(rcdu->dev); >> + >> + soc_attr = soc_device_match(rcar_du_soc_table); >> + if (soc_attr) >> + rcdu->info = soc_attr->data; >> + >> + if (!rcdu->info) >> + rcdu->info = of_device_get_match_data(rcdu->dev); > > As Geert mentioned, > > rcdu->info = of_device_get_match_data(rcdu->dev); > > soc_attr = soc_device_match(rcar_du_soc_table); > if (soc_attr) > rcdu->info = soc_attr->data; > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Ok. Thanks! Tomi
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 3619e1ddeb62..f2d3266509cc 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -10,7 +10,6 @@ #include <linux/clk.h> #include <linux/mutex.h> #include <linux/platform_device.h> -#include <linux/sys_soc.h> #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> @@ -204,11 +203,6 @@ static void rcar_du_escr_divider(struct clk *clk, unsigned long target, } } -static const struct soc_device_attribute rcar_du_r8a7795_es1[] = { - { .soc_id = "r8a7795", .revision = "ES1.*" }, - { /* sentinel */ } -}; - static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) { const struct drm_display_mode *mode = &rcrtc->crtc.state->adjusted_mode; @@ -238,7 +232,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) * no post-divider when a display PLL is present (as shown by * the workaround breaking HDMI output on M3-W during testing). */ - if (soc_device_match(rcar_du_r8a7795_es1)) { + if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY) { target *= 2; div = 1; } diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index c7c5217cfc1a..ba2e069fc0f7 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -16,6 +16,7 @@ #include <linux/platform_device.h> #include <linux/pm.h> #include <linux/slab.h> +#include <linux/sys_soc.h> #include <linux/wait.h> #include <drm/drm_atomic_helper.h> @@ -386,6 +387,42 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = { .dpll_mask = BIT(2) | BIT(1), }; +static const struct rcar_du_device_info rcar_du_r8a7795_es1_info = { + .gen = 3, + .features = RCAR_DU_FEATURE_CRTC_IRQ + | RCAR_DU_FEATURE_CRTC_CLOCK + | RCAR_DU_FEATURE_VSP1_SOURCE + | RCAR_DU_FEATURE_INTERLACED + | RCAR_DU_FEATURE_TVM_SYNC, + .quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY, + .channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0), + .routes = { + /* + * R8A7795 has one RGB output, two HDMI outputs and one + * LVDS output. + */ + [RCAR_DU_OUTPUT_DPAD0] = { + .possible_crtcs = BIT(3), + .port = 0, + }, + [RCAR_DU_OUTPUT_HDMI0] = { + .possible_crtcs = BIT(1), + .port = 1, + }, + [RCAR_DU_OUTPUT_HDMI1] = { + .possible_crtcs = BIT(2), + .port = 2, + }, + [RCAR_DU_OUTPUT_LVDS0] = { + .possible_crtcs = BIT(0), + .port = 3, + }, + }, + .num_lvds = 1, + .num_rpf = 5, + .dpll_mask = BIT(2) | BIT(1), +}; + static const struct rcar_du_device_info rcar_du_r8a7796_info = { .gen = 3, .features = RCAR_DU_FEATURE_CRTC_IRQ @@ -576,6 +613,11 @@ static const struct of_device_id rcar_du_of_table[] = { MODULE_DEVICE_TABLE(of, rcar_du_of_table); +static const struct soc_device_attribute rcar_du_soc_table[] = { + { .soc_id = "r8a7795", .revision = "ES1.*", .data = &rcar_du_r8a7795_es1_info }, + { /* sentinel */ } +}; + const char *rcar_du_output_name(enum rcar_du_output output) { static const char * const names[] = { @@ -670,6 +712,7 @@ static int rcar_du_probe(struct platform_device *pdev) struct rcar_du_device *rcdu; unsigned int mask; int ret; + const struct soc_device_attribute *soc_attr; if (drm_firmware_drivers_only()) return -ENODEV; @@ -681,7 +724,13 @@ static int rcar_du_probe(struct platform_device *pdev) return PTR_ERR(rcdu); rcdu->dev = &pdev->dev; - rcdu->info = of_device_get_match_data(rcdu->dev); + + soc_attr = soc_device_match(rcar_du_soc_table); + if (soc_attr) + rcdu->info = soc_attr->data; + + if (!rcdu->info) + rcdu->info = of_device_get_match_data(rcdu->dev); platform_set_drvdata(pdev, rcdu); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 5cfa2bb7ad93..df87ccab146f 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h @@ -34,6 +34,7 @@ struct rcar_du_device; #define RCAR_DU_FEATURE_NO_BLENDING BIT(5) /* PnMR.SPIM does not have ALP nor EOR bits */ #define RCAR_DU_QUIRK_ALIGN_128B BIT(0) /* Align pitches to 128 bytes */ +#define RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY BIT(1) /* H3 ES1 has pclk stability issue */ enum rcar_du_output { RCAR_DU_OUTPUT_DPAD0,
rcar_du_crtc.c does a soc_device_match() in rcar_du_crtc_set_display_timing() to find out if the SoC is H3 ES1, and if so, apply a WA. We will need another H3 ES1 check in the following patch, so rather than adding more soc_device_match() calls, let's add a rcar_du_device_info entry for the ES1, and a quirk flag, RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY, for the WA. Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com> --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 8 +--- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 51 +++++++++++++++++++++++++- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 + 3 files changed, 52 insertions(+), 8 deletions(-)