| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ca6de04c-4547-f068-6b29-194d3d256926@foss.st.com>
Date: Mon, 27 Jun 2022 16:13:06 +0200
From: Philippe CORNU <philippe.cornu@...s.st.com>
To: Yannick Fertre <yannick.fertre@...s.st.com>,
Raphael Gallais-Pou <raphael.gallais-pou@...s.st.com>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
<dri-devel@...ts.freedesktop.org>,
<linux-stm32@...md-mailman.stormreply.com>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/stm: ltdc: update hardware error management
On 6/3/22 15:46, Yannick Fertre wrote:
> The latest hardware version (0x40100) supports a hardware threshold
> register (aka FUTR) to trigger a fifo underrun interrupt.
> A software threshold has been implemented for other hardware versions.
> The threshold is set to 128 by default.
>
> Signed-off-by: Yannick Fertre <yannick.fertre@...s.st.com>
> ---
> drivers/gpu/drm/stm/ltdc.c | 90 ++++++++++++++++++++++++++++++--------
> drivers/gpu/drm/stm/ltdc.h | 6 ++-
> 2 files changed, 77 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index ff2075dd9474..42a3bd515477 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -162,16 +162,20 @@
> #define BCCR_BCWHITE GENMASK(23, 0) /* Background Color WHITE */
>
> #define IER_LIE BIT(0) /* Line Interrupt Enable */
> -#define IER_FUIE BIT(1) /* Fifo Underrun Interrupt Enable */
> +#define IER_FUWIE BIT(1) /* Fifo Underrun Warning Interrupt Enable */
> #define IER_TERRIE BIT(2) /* Transfer ERRor Interrupt Enable */
> -#define IER_RRIE BIT(3) /* Register Reload Interrupt enable */
> +#define IER_RRIE BIT(3) /* Register Reload Interrupt Enable */
> +#define IER_FUEIE BIT(6) /* Fifo Underrun Error Interrupt Enable */
> +#define IER_CRCIE BIT(7) /* CRC Error Interrupt Enable */
>
> #define CPSR_CYPOS GENMASK(15, 0) /* Current Y position */
>
> #define ISR_LIF BIT(0) /* Line Interrupt Flag */
> -#define ISR_FUIF BIT(1) /* Fifo Underrun Interrupt Flag */
> +#define ISR_FUWIF BIT(1) /* Fifo Underrun Warning Interrupt Flag */
> #define ISR_TERRIF BIT(2) /* Transfer ERRor Interrupt Flag */
> #define ISR_RRIF BIT(3) /* Register Reload Interrupt Flag */
> +#define ISR_FUEIF BIT(6) /* Fifo Underrun Error Interrupt Flag */
> +#define ISR_CRCIF BIT(7) /* CRC Error Interrupt Flag */
>
> #define EDCR_OCYEN BIT(25) /* Output Conversion to YCbCr 422: ENable */
> #define EDCR_OCYSEL BIT(26) /* Output Conversion to YCbCr 422: SELection of the CCIR */
> @@ -231,6 +235,8 @@
>
> #define NB_PF 8 /* Max nb of HW pixel format */
>
> +#define FUT_DFT 128 /* Default value of fifo underrun threshold */
> +
> /*
> * Skip the first value and the second in case CRC was enabled during
> * the thread irq. This is to be sure CRC value is relevant for the
> @@ -711,12 +717,13 @@ static irqreturn_t ltdc_irq_thread(int irq, void *arg)
> ltdc_irq_crc_handle(ldev, crtc);
> }
>
> - /* Save FIFO Underrun & Transfer Error status */
> mutex_lock(&ldev->err_lock);
> - if (ldev->irq_status & ISR_FUIF)
> - ldev->error_status |= ISR_FUIF;
> if (ldev->irq_status & ISR_TERRIF)
> - ldev->error_status |= ISR_TERRIF;
> + ldev->transfer_err++;
> + if (ldev->irq_status & ISR_FUEIF)
> + ldev->fifo_err++;
> + if (ldev->irq_status & ISR_FUWIF)
> + ldev->fifo_warn++;
> mutex_unlock(&ldev->err_lock);
>
> return IRQ_HANDLED;
> @@ -775,7 +782,7 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc,
> regmap_write(ldev->regmap, LTDC_BCCR, BCCR_BCBLACK);
>
> /* Enable IRQ */
> - regmap_set_bits(ldev->regmap, LTDC_IER, IER_RRIE | IER_FUIE | IER_TERRIE);
> + regmap_set_bits(ldev->regmap, LTDC_IER, IER_FUWIE | IER_FUEIE | IER_RRIE | IER_TERRIE);
>
> /* Commit shadow registers = update planes at next vblank */
> if (!ldev->caps.plane_reg_shadow)
> @@ -801,13 +808,20 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc *crtc,
> LXCR_CLUTEN | LXCR_LEN, 0);
>
> /* disable IRQ */
> - regmap_clear_bits(ldev->regmap, LTDC_IER, IER_RRIE | IER_FUIE | IER_TERRIE);
> + regmap_clear_bits(ldev->regmap, LTDC_IER, IER_FUWIE | IER_FUEIE | IER_RRIE | IER_TERRIE);
>
> /* immediately commit disable of layers before switching off LTDC */
> if (!ldev->caps.plane_reg_shadow)
> regmap_set_bits(ldev->regmap, LTDC_SRCR, SRCR_IMR);
>
> pm_runtime_put_sync(ddev->dev);
> +
> + /* clear interrupt error counters */
> + mutex_lock(&ldev->err_lock);
> + ldev->transfer_err = 0;
> + ldev->fifo_err = 0;
> + ldev->fifo_warn = 0;
> + mutex_unlock(&ldev->err_lock);
> }
>
> #define CLK_TOLERANCE_HZ 50
> @@ -1168,6 +1182,18 @@ static int ltdc_crtc_verify_crc_source(struct drm_crtc *crtc,
> return 0;
> }
>
> +static void ltdc_crtc_atomic_print_state(struct drm_printer *p,
> + const struct drm_crtc_state *state)
> +{
> + struct drm_crtc *crtc = state->crtc;
> + struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> +
> + drm_printf(p, "\ttransfer_error=%d\n", ldev->transfer_err);
> + drm_printf(p, "\tfifo_underrun_error=%d\n", ldev->fifo_err);
> + drm_printf(p, "\tfifo_underrun_warning=%d\n", ldev->fifo_warn);
> + drm_printf(p, "\tfifo_underrun_threshold=%d\n", ldev->fifo_threshold);
> +}
> +
> static const struct drm_crtc_funcs ltdc_crtc_funcs = {
> .destroy = drm_crtc_cleanup,
> .set_config = drm_atomic_helper_set_config,
> @@ -1178,6 +1204,7 @@ static const struct drm_crtc_funcs ltdc_crtc_funcs = {
> .enable_vblank = ltdc_crtc_enable_vblank,
> .disable_vblank = ltdc_crtc_disable_vblank,
> .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
> + .atomic_print_state = ltdc_crtc_atomic_print_state,
> };
>
> static const struct drm_crtc_funcs ltdc_crtc_with_crc_support_funcs = {
> @@ -1192,6 +1219,7 @@ static const struct drm_crtc_funcs ltdc_crtc_with_crc_support_funcs = {
> .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
> .set_crc_source = ltdc_crtc_set_crc_source,
> .verify_crc_source = ltdc_crtc_verify_crc_source,
> + .atomic_print_state = ltdc_crtc_atomic_print_state,
> };
>
> /*
> @@ -1452,13 +1480,21 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
> ldev->plane_fpsi[plane->index].counter++;
>
> mutex_lock(&ldev->err_lock);
> - if (ldev->error_status & ISR_FUIF) {
> - DRM_WARN("ltdc fifo underrun: please verify display mode\n");
> - ldev->error_status &= ~ISR_FUIF;
> + if (ldev->transfer_err) {
> + DRM_WARN("ltdc transfer error: %d\n", ldev->transfer_err);
> + ldev->transfer_err = 0;
> }
> - if (ldev->error_status & ISR_TERRIF) {
> - DRM_WARN("ltdc transfer error\n");
> - ldev->error_status &= ~ISR_TERRIF;
> +
> + if (ldev->caps.fifo_threshold) {
> + if (ldev->fifo_err) {
> + DRM_WARN("ltdc fifo underrun: please verify display mode\n");
> + ldev->fifo_err = 0;
> + }
> + } else {
> + if (ldev->fifo_warn >= ldev->fifo_threshold) {
> + DRM_WARN("ltdc fifo underrun: please verify display mode\n");
> + ldev->fifo_warn = 0;
> + }
> }
> mutex_unlock(&ldev->err_lock);
> }
> @@ -1700,6 +1736,10 @@ static void ltdc_encoder_enable(struct drm_encoder *encoder)
>
> DRM_DEBUG_DRIVER("\n");
>
> + /* set fifo underrun threshold register */
> + if (ldev->caps.fifo_threshold)
> + regmap_write(ldev->regmap, LTDC_FUT, ldev->fifo_threshold);
> +
> /* Enable LTDC */
> regmap_set_bits(ldev->regmap, LTDC_GCR, GCR_LTDCEN);
> }
> @@ -1801,6 +1841,7 @@ static int ltdc_get_caps(struct drm_device *ddev)
> ldev->caps.crc = false;
> ldev->caps.dynamic_zorder = false;
> ldev->caps.plane_rotation = false;
> + ldev->caps.fifo_threshold = false;
> break;
> case HWVER_20101:
> ldev->caps.layer_ofs = LAY_OFS_0;
> @@ -1818,6 +1859,7 @@ static int ltdc_get_caps(struct drm_device *ddev)
> ldev->caps.crc = false;
> ldev->caps.dynamic_zorder = false;
> ldev->caps.plane_rotation = false;
> + ldev->caps.fifo_threshold = false;
> break;
> case HWVER_40100:
> ldev->caps.layer_ofs = LAY_OFS_1;
> @@ -1835,6 +1877,7 @@ static int ltdc_get_caps(struct drm_device *ddev)
> ldev->caps.crc = true;
> ldev->caps.dynamic_zorder = true;
> ldev->caps.plane_rotation = true;
> + ldev->caps.fifo_threshold = true;
> break;
> default:
> return -ENODEV;
> @@ -1959,9 +2002,6 @@ int ltdc_load(struct drm_device *ddev)
> goto err;
> }
>
> - /* Disable interrupts */
> - regmap_clear_bits(ldev->regmap, LTDC_IER, IER_LIE | IER_RRIE | IER_FUIE | IER_TERRIE);
> -
> ret = ltdc_get_caps(ddev);
> if (ret) {
> DRM_ERROR("hardware identifier (0x%08x) not supported!\n",
> @@ -1969,8 +2009,22 @@ int ltdc_load(struct drm_device *ddev)
> goto err;
> }
>
> + /* Disable interrupts */
> + if (ldev->caps.fifo_threshold)
> + regmap_clear_bits(ldev->regmap, LTDC_IER, IER_LIE | IER_RRIE | IER_FUWIE |
> + IER_TERRIE);
> + else
> + regmap_clear_bits(ldev->regmap, LTDC_IER, IER_LIE | IER_RRIE | IER_FUWIE |
> + IER_TERRIE | IER_FUEIE);
> +
> DRM_DEBUG_DRIVER("ltdc hw version 0x%08x\n", ldev->caps.hw_version);
>
> + /* initialize default value for fifo underrun threshold & clear interrupt error counters */
> + ldev->transfer_err = 0;
> + ldev->fifo_err = 0;
> + ldev->fifo_warn = 0;
> + ldev->fifo_threshold = FUT_DFT;
> +
> for (i = 0; i < ldev->caps.nb_irq; i++) {
> irq = platform_get_irq(pdev, i);
> if (irq < 0) {
> diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> index 15139980d8ea..9d488043ffdb 100644
> --- a/drivers/gpu/drm/stm/ltdc.h
> +++ b/drivers/gpu/drm/stm/ltdc.h
> @@ -30,6 +30,7 @@ struct ltdc_caps {
> bool crc; /* cyclic redundancy check supported */
> bool dynamic_zorder; /* dynamic z-order */
> bool plane_rotation; /* plane rotation */
> + bool fifo_threshold; /* fifo underrun threshold supported */
> };
>
> #define LTDC_MAX_LAYER 4
> @@ -45,8 +46,11 @@ struct ltdc_device {
> struct clk *pixel_clk; /* lcd pixel clock */
> struct mutex err_lock; /* protecting error_status */
> struct ltdc_caps caps;
> - u32 error_status;
> u32 irq_status;
> + u32 fifo_err; /* fifo underrun error counter */
> + u32 fifo_warn; /* fifo underrun warning counter */
> + u32 fifo_threshold; /* fifo underrun threshold */
> + u32 transfer_err; /* transfer error counter */
> struct fps_info plane_fpsi[LTDC_MAX_LAYER];
> struct drm_atomic_state *suspend_state;
> int crc_skip_count;
Dear Yannick,
Many thanks for your patch,
Applied on drm-misc-next.
Have a good day
Philippe :-)
Powered by blists - more mailing lists