| Message ID | 20200620155752.21065-1-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 17f64701ea6f541db7eb5d7423a830cb929b3052 |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | [RESEND,v1] drm/meson: viu: fix setting the OSD burst length in VIU_OSD1_FIFO_CTRL_STAT | expand |
Hi Martin, On 20/06/2020 17:57, Martin Blumenstingl wrote: > The burst length is configured in VIU_OSD1_FIFO_CTRL_STAT[31] and > VIU_OSD1_FIFO_CTRL_STAT[11:10]. The public S905D3 datasheet describes > this as: > - 0x0 = up to 24 per burst > - 0x1 = up to 32 per burst > - 0x2 = up to 48 per burst > - 0x3 = up to 64 per burst > - 0x4 = up to 96 per burst > - 0x5 = up to 128 per burst > > The lower two bits map to VIU_OSD1_FIFO_CTRL_STAT[11:10] while the upper > bit maps to VIU_OSD1_FIFO_CTRL_STAT[31]. > > Replace meson_viu_osd_burst_length_reg() with pre-defined macros which > set these values. meson_viu_osd_burst_length_reg() always returned 0 > (for the two used values: 32 and 64 at least) and thus incorrectly set > the burst size to 24. > > Fixes: 147ae1cbaa1842 ("drm: meson: viu: use proper macros instead of magic constants") > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > re-send of v1 [0] with no changes as I still noticed that this is > sitting in my tree and I wasn't asked to change anything in v1. > > > [0] https://patchwork.kernel.org/patch/11510045/ > > > drivers/gpu/drm/meson/meson_registers.h | 6 ++++++ > drivers/gpu/drm/meson/meson_viu.c | 11 ++--------- > 2 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_registers.h b/drivers/gpu/drm/meson/meson_registers.h > index 8ea00546cd4e..049c4bfe2a3a 100644 > --- a/drivers/gpu/drm/meson/meson_registers.h > +++ b/drivers/gpu/drm/meson/meson_registers.h > @@ -261,6 +261,12 @@ > #define VIU_OSD_FIFO_DEPTH_VAL(val) ((val & 0x7f) << 12) > #define VIU_OSD_WORDS_PER_BURST(words) (((words & 0x4) >> 1) << 22) > #define VIU_OSD_FIFO_LIMITS(size) ((size & 0xf) << 24) > +#define VIU_OSD_BURST_LENGTH_24 (0x0 << 31 | 0x0 << 10) > +#define VIU_OSD_BURST_LENGTH_32 (0x0 << 31 | 0x1 << 10) > +#define VIU_OSD_BURST_LENGTH_48 (0x0 << 31 | 0x2 << 10) > +#define VIU_OSD_BURST_LENGTH_64 (0x0 << 31 | 0x3 << 10) > +#define VIU_OSD_BURST_LENGTH_96 (0x1 << 31 | 0x0 << 10) > +#define VIU_OSD_BURST_LENGTH_128 (0x1 << 31 | 0x1 << 10) > > #define VD1_IF0_GEN_REG 0x1a50 > #define VD1_IF0_CANVAS0 0x1a51 > diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c > index 304f8ff1339c..aede0c67a57f 100644 > --- a/drivers/gpu/drm/meson/meson_viu.c > +++ b/drivers/gpu/drm/meson/meson_viu.c > @@ -411,13 +411,6 @@ void meson_viu_gxm_disable_osd1_afbc(struct meson_drm *priv) > priv->io_base + _REG(VIU_MISC_CTRL1)); > } > > -static inline uint32_t meson_viu_osd_burst_length_reg(uint32_t length) > -{ > - uint32_t val = (((length & 0x80) % 24) / 12); > - > - return (((val & 0x3) << 10) | (((val & 0x4) >> 2) << 31)); > -} > - > void meson_viu_init(struct meson_drm *priv) > { > uint32_t reg; > @@ -444,9 +437,9 @@ void meson_viu_init(struct meson_drm *priv) > VIU_OSD_FIFO_LIMITS(2); /* fifo_lim: 2*16=32 */ > > if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) > - reg |= meson_viu_osd_burst_length_reg(32); > + reg |= VIU_OSD_BURST_LENGTH_32; > else > - reg |= meson_viu_osd_burst_length_reg(64); > + reg |= VIU_OSD_BURST_LENGTH_64; > > writel_relaxed(reg, priv->io_base + _REG(VIU_OSD1_FIFO_CTRL_STAT)); > writel_relaxed(reg, priv->io_base + _REG(VIU_OSD2_FIFO_CTRL_STAT)); > Completely forgot this one, will test asap. Neil
On 20/06/2020 17:57, Martin Blumenstingl wrote: > The burst length is configured in VIU_OSD1_FIFO_CTRL_STAT[31] and > VIU_OSD1_FIFO_CTRL_STAT[11:10]. The public S905D3 datasheet describes > this as: > - 0x0 = up to 24 per burst > - 0x1 = up to 32 per burst > - 0x2 = up to 48 per burst > - 0x3 = up to 64 per burst > - 0x4 = up to 96 per burst > - 0x5 = up to 128 per burst > > The lower two bits map to VIU_OSD1_FIFO_CTRL_STAT[11:10] while the upper > bit maps to VIU_OSD1_FIFO_CTRL_STAT[31]. > > Replace meson_viu_osd_burst_length_reg() with pre-defined macros which > set these values. meson_viu_osd_burst_length_reg() always returned 0 > (for the two used values: 32 and 64 at least) and thus incorrectly set > the burst size to 24. > > Fixes: 147ae1cbaa1842 ("drm: meson: viu: use proper macros instead of magic constants") > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> I also got an off-list: Tested-by: Christian Hewitt <christianshewitt@gmail.com> Applying to drm-misc-fixes Thanks, Neil > --- > re-send of v1 [0] with no changes as I still noticed that this is > sitting in my tree and I wasn't asked to change anything in v1. > > > [0] https://patchwork.kernel.org/patch/11510045/ > > > drivers/gpu/drm/meson/meson_registers.h | 6 ++++++ > drivers/gpu/drm/meson/meson_viu.c | 11 ++--------- > 2 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_registers.h b/drivers/gpu/drm/meson/meson_registers.h > index 8ea00546cd4e..049c4bfe2a3a 100644 > --- a/drivers/gpu/drm/meson/meson_registers.h > +++ b/drivers/gpu/drm/meson/meson_registers.h > @@ -261,6 +261,12 @@ > #define VIU_OSD_FIFO_DEPTH_VAL(val) ((val & 0x7f) << 12) > #define VIU_OSD_WORDS_PER_BURST(words) (((words & 0x4) >> 1) << 22) > #define VIU_OSD_FIFO_LIMITS(size) ((size & 0xf) << 24) > +#define VIU_OSD_BURST_LENGTH_24 (0x0 << 31 | 0x0 << 10) > +#define VIU_OSD_BURST_LENGTH_32 (0x0 << 31 | 0x1 << 10) > +#define VIU_OSD_BURST_LENGTH_48 (0x0 << 31 | 0x2 << 10) > +#define VIU_OSD_BURST_LENGTH_64 (0x0 << 31 | 0x3 << 10) > +#define VIU_OSD_BURST_LENGTH_96 (0x1 << 31 | 0x0 << 10) > +#define VIU_OSD_BURST_LENGTH_128 (0x1 << 31 | 0x1 << 10) > > #define VD1_IF0_GEN_REG 0x1a50 > #define VD1_IF0_CANVAS0 0x1a51 > diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c > index 304f8ff1339c..aede0c67a57f 100644 > --- a/drivers/gpu/drm/meson/meson_viu.c > +++ b/drivers/gpu/drm/meson/meson_viu.c > @@ -411,13 +411,6 @@ void meson_viu_gxm_disable_osd1_afbc(struct meson_drm *priv) > priv->io_base + _REG(VIU_MISC_CTRL1)); > } > > -static inline uint32_t meson_viu_osd_burst_length_reg(uint32_t length) > -{ > - uint32_t val = (((length & 0x80) % 24) / 12); > - > - return (((val & 0x3) << 10) | (((val & 0x4) >> 2) << 31)); > -} > - > void meson_viu_init(struct meson_drm *priv) > { > uint32_t reg; > @@ -444,9 +437,9 @@ void meson_viu_init(struct meson_drm *priv) > VIU_OSD_FIFO_LIMITS(2); /* fifo_lim: 2*16=32 */ > > if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) > - reg |= meson_viu_osd_burst_length_reg(32); > + reg |= VIU_OSD_BURST_LENGTH_32; > else > - reg |= meson_viu_osd_burst_length_reg(64); > + reg |= VIU_OSD_BURST_LENGTH_64; > > writel_relaxed(reg, priv->io_base + _REG(VIU_OSD1_FIFO_CTRL_STAT)); > writel_relaxed(reg, priv->io_base + _REG(VIU_OSD2_FIFO_CTRL_STAT)); >
Hello: This patch was applied to khilman/linux-amlogic.git (refs/heads/for-next). On Sat, 20 Jun 2020 17:57:52 +0200 you wrote: > The burst length is configured in VIU_OSD1_FIFO_CTRL_STAT[31] and > VIU_OSD1_FIFO_CTRL_STAT[11:10]. The public S905D3 datasheet describes > this as: > - 0x0 = up to 24 per burst > - 0x1 = up to 32 per burst > - 0x2 = up to 48 per burst > - 0x3 = up to 64 per burst > - 0x4 = up to 96 per burst > - 0x5 = up to 128 per burst > > [...] Here is a summary with links: - [RESEND,v1] drm/meson: viu: fix setting the OSD burst length in VIU_OSD1_FIFO_CTRL_STAT https://git.kernel.org/khilman/linux-amlogic/c/17f64701ea6f541db7eb5d7423a830cb929b3052 You are awesome, thank you!
diff --git a/drivers/gpu/drm/meson/meson_registers.h b/drivers/gpu/drm/meson/meson_registers.h index 8ea00546cd4e..049c4bfe2a3a 100644 --- a/drivers/gpu/drm/meson/meson_registers.h +++ b/drivers/gpu/drm/meson/meson_registers.h @@ -261,6 +261,12 @@ #define VIU_OSD_FIFO_DEPTH_VAL(val) ((val & 0x7f) << 12) #define VIU_OSD_WORDS_PER_BURST(words) (((words & 0x4) >> 1) << 22) #define VIU_OSD_FIFO_LIMITS(size) ((size & 0xf) << 24) +#define VIU_OSD_BURST_LENGTH_24 (0x0 << 31 | 0x0 << 10) +#define VIU_OSD_BURST_LENGTH_32 (0x0 << 31 | 0x1 << 10) +#define VIU_OSD_BURST_LENGTH_48 (0x0 << 31 | 0x2 << 10) +#define VIU_OSD_BURST_LENGTH_64 (0x0 << 31 | 0x3 << 10) +#define VIU_OSD_BURST_LENGTH_96 (0x1 << 31 | 0x0 << 10) +#define VIU_OSD_BURST_LENGTH_128 (0x1 << 31 | 0x1 << 10) #define VD1_IF0_GEN_REG 0x1a50 #define VD1_IF0_CANVAS0 0x1a51 diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c index 304f8ff1339c..aede0c67a57f 100644 --- a/drivers/gpu/drm/meson/meson_viu.c +++ b/drivers/gpu/drm/meson/meson_viu.c @@ -411,13 +411,6 @@ void meson_viu_gxm_disable_osd1_afbc(struct meson_drm *priv) priv->io_base + _REG(VIU_MISC_CTRL1)); } -static inline uint32_t meson_viu_osd_burst_length_reg(uint32_t length) -{ - uint32_t val = (((length & 0x80) % 24) / 12); - - return (((val & 0x3) << 10) | (((val & 0x4) >> 2) << 31)); -} - void meson_viu_init(struct meson_drm *priv) { uint32_t reg; @@ -444,9 +437,9 @@ void meson_viu_init(struct meson_drm *priv) VIU_OSD_FIFO_LIMITS(2); /* fifo_lim: 2*16=32 */ if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) - reg |= meson_viu_osd_burst_length_reg(32); + reg |= VIU_OSD_BURST_LENGTH_32; else - reg |= meson_viu_osd_burst_length_reg(64); + reg |= VIU_OSD_BURST_LENGTH_64; writel_relaxed(reg, priv->io_base + _REG(VIU_OSD1_FIFO_CTRL_STAT)); writel_relaxed(reg, priv->io_base + _REG(VIU_OSD2_FIFO_CTRL_STAT));
The burst length is configured in VIU_OSD1_FIFO_CTRL_STAT[31] and VIU_OSD1_FIFO_CTRL_STAT[11:10]. The public S905D3 datasheet describes this as: - 0x0 = up to 24 per burst - 0x1 = up to 32 per burst - 0x2 = up to 48 per burst - 0x3 = up to 64 per burst - 0x4 = up to 96 per burst - 0x5 = up to 128 per burst The lower two bits map to VIU_OSD1_FIFO_CTRL_STAT[11:10] while the upper bit maps to VIU_OSD1_FIFO_CTRL_STAT[31]. Replace meson_viu_osd_burst_length_reg() with pre-defined macros which set these values. meson_viu_osd_burst_length_reg() always returned 0 (for the two used values: 32 and 64 at least) and thus incorrectly set the burst size to 24. Fixes: 147ae1cbaa1842 ("drm: meson: viu: use proper macros instead of magic constants") Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- re-send of v1 [0] with no changes as I still noticed that this is sitting in my tree and I wasn't asked to change anything in v1. [0] https://patchwork.kernel.org/patch/11510045/ drivers/gpu/drm/meson/meson_registers.h | 6 ++++++ drivers/gpu/drm/meson/meson_viu.c | 11 ++--------- 2 files changed, 8 insertions(+), 9 deletions(-)