Re: [PATCH] drm/msm: Drop msm_read/writel
On Thu, Apr 11, 2024 at 08:27:22AM -0700, Bjorn Andersson wrote:
> On Thu, Apr 11, 2024 at 04:31:41AM +0300, Dmitry Baryshkov wrote:
> > On Wed, Apr 10, 2024 at 11:52:52PM +0200, Konrad Dybcio wrote:
> [..]
> > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > index e4275d3ad581..5a5dc3faa971 100644
> > > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > @@ -12,10 +12,10 @@
> > >
> > > #include "dsi.h"
> > >
> > > -#define dsi_phy_read(offset) msm_readl((offset))
> > > -#define dsi_phy_write(offset, data) msm_writel((data), (offset))
> > > -#define dsi_phy_write_udelay(offset, data, delay_us) {
> > > msm_writel((data), (offset)); udelay(delay_us); }
> > > -#define dsi_phy_write_ndelay(offset, data, delay_ns) {
> > > msm_writel((data), (offset)); ndelay(delay_ns); }
> > > +#define dsi_phy_read(offset) readl((offset))
> > > +#define dsi_phy_write(offset, data) writel((data), (offset))
> > > +#define dsi_phy_write_udelay(offset, data, delay_us) { writel((data),
> > > (offset)); udelay(delay_us); }
> > > +#define dsi_phy_write_ndelay(offset, data, delay_ns) { writel((data),
> > > (offset)); ndelay(delay_ns); }
> >
> > What about also inlining these wrappers?
> >
>
> But that should be done in a separate commit, no?
Yesm of course.
>
> PS. Too much scrolling to find your comments, please trim your replies.
Ack. I'm probably too used to GMail and Thunderbird extension which
collapses quotes.
>
> Thanks,
> Bjorn
--
With best wishes
Dmitry
Re: [PATCH] drm/msm: Drop msm_read/writel
On Thu, Apr 11, 2024 at 04:31:41AM +0300, Dmitry Baryshkov wrote:
> On Wed, Apr 10, 2024 at 11:52:52PM +0200, Konrad Dybcio wrote:
[..]
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > index e4275d3ad581..5a5dc3faa971 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > @@ -12,10 +12,10 @@
> >
> > #include "dsi.h"
> >
> > -#define dsi_phy_read(offset) msm_readl((offset))
> > -#define dsi_phy_write(offset, data) msm_writel((data), (offset))
> > -#define dsi_phy_write_udelay(offset, data, delay_us) { msm_writel((data),
> > (offset)); udelay(delay_us); }
> > -#define dsi_phy_write_ndelay(offset, data, delay_ns) { msm_writel((data),
> > (offset)); ndelay(delay_ns); }
> > +#define dsi_phy_read(offset) readl((offset))
> > +#define dsi_phy_write(offset, data) writel((data), (offset))
> > +#define dsi_phy_write_udelay(offset, data, delay_us) { writel((data),
> > (offset)); udelay(delay_us); }
> > +#define dsi_phy_write_ndelay(offset, data, delay_ns) { writel((data),
> > (offset)); ndelay(delay_ns); }
>
> What about also inlining these wrappers?
>
But that should be done in a separate commit, no?
PS. Too much scrolling to find your comments, please trim your replies.
Thanks,
Bjorn
Re: [PATCH] drm/msm: Drop msm_read/writel
On Wed, Apr 10, 2024 at 11:52:52PM +0200, Konrad Dybcio wrote:
> Totally useless.
>
> Signed-off-by: Konrad Dybcio
> ---
> only compile-tested
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 12 ++--
> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 4 ++--
> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 4 ++--
> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h| 4 ++--
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h| 4 ++--
> drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 8
> drivers/gpu/drm/msm/hdmi/hdmi.h | 10 +-
> drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c| 6 +++---
> drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c| 4 ++--
> drivers/gpu/drm/msm/msm_drv.h | 7 ++-
> drivers/gpu/drm/msm/msm_gpu.h | 12 ++--
> 13 files changed, 42 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 8bea8ef26f77..0e3dfd4c2bc8 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -507,7 +507,7 @@ static void a6xx_rpmh_stop(struct a6xx_gmu *gmu)
>
> static inline void pdc_write(void __iomem *ptr, u32 offset, u32 value)
> {
> - msm_writel(value, ptr + (offset << 2));
> + writel(value, ptr + (offset << 2));
> }
>
> static void __iomem *a6xx_gmu_get_mmio(struct platform_device *pdev,
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> index 592b296aab22..94b6c5cab6f4 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> @@ -103,12 +103,12 @@ struct a6xx_gmu {
>
> static inline u32 gmu_read(struct a6xx_gmu *gmu, u32 offset)
> {
> - return msm_readl(gmu->mmio + (offset << 2));
> + return readl(gmu->mmio + (offset << 2));
> }
>
> static inline void gmu_write(struct a6xx_gmu *gmu, u32 offset, u32 value)
> {
> - msm_writel(value, gmu->mmio + (offset << 2));
> + writel(value, gmu->mmio + (offset << 2));
> }
>
> static inline void
> @@ -131,8 +131,8 @@ static inline u64 gmu_read64(struct a6xx_gmu *gmu, u32
> lo, u32 hi)
> {
> u64 val;
>
> - val = (u64) msm_readl(gmu->mmio + (lo << 2));
> - val |= ((u64) msm_readl(gmu->mmio + (hi << 2)) << 32);
> + val = (u64) readl(gmu->mmio + (lo << 2));
> + val |= ((u64) readl(gmu->mmio + (hi << 2)) << 32);
>
> return val;
> }
> @@ -143,12 +143,12 @@ static inline u64 gmu_read64(struct a6xx_gmu *gmu, u32
> lo, u32 hi)
>
> static inline u32 gmu_read_rscc(struct a6xx_gmu *gmu, u32 offset)
> {
> - return msm_readl(gmu->rscc + (offset << 2));
> + return readl(gmu->rscc + (offset << 2));
> }
>
> static inline void gmu_write_rscc(struct a6xx_gmu *gmu, u32 offset, u32
> value)
> {
> - msm_writel(value, gmu->rscc + (offset << 2));
> + writel(value, gmu->rscc + (offset << 2));
> }
>
> #define gmu_poll_timeout_rscc(gmu, addr, val, cond, interval, timeout) \
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> index 34822b080759..8917032b7515 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> @@ -69,12 +69,12 @@ static inline void a6xx_llc_rmw(struct a6xx_gpu
> *a6xx_gpu, u32 reg, u32 mask, u3
>
> static inline u32 a6xx_llc_read(struct a6xx_gpu *a6xx_gpu, u32 reg)
> {
> - return msm_readl(a6xx_gpu->llc_mmio + (reg << 2));
> + return readl(a6xx_gpu->llc_mmio + (reg << 2));
> }
>
> static inline void a6xx_llc_write(struct a6xx_gpu *a6xx_gpu, u32 reg, u32
> value)
> {
> - msm_writel(value, a6xx_gpu->llc_mmio + (reg << 2));
> + writel(value, a6xx_gpu->llc_mmio + (reg << 2));
> }
>
> #define shadowptr(_a6xx_gpu, _ring) ((_a6xx_gpu)->shadow_iova + \
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> index a847a0f7a73c..83d7ee01c944 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> @@ -192,10 +192,10 @@ static int debugbus_read(struct msm_gpu *gpu, u32
> block, u32 offset,
> }
>
> #define cxdbg_write(ptr, offset, val) \
> - msm_writel((val), (ptr) + ((offset) << 2))
> + writel((val), (ptr) + ((offset) << 2))
>
> #define cxdbg_read(ptr, offset) \
> - msm_readl((ptr) + ((offset) << 2))
> + readl((ptr) + ((offset) << 2))
>
> /* read a value from the CX debug bus */
> static int cx_debugbus_read(void __iomem *cxdbg, u32 block, u32 offset,
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
> b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
> index 01179e764a29..94b1ba92785f 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
> @@ -44,12 +44,12 @@ struct mdp4_km
Re: [PATCH] drm/msm: Drop msm_read/writel
On Wed, Apr 10, 2024 at 11:52:52PM +0200, Konrad Dybcio wrote:
> Totally useless.
>
> Signed-off-by: Konrad Dybcio
A few more words in the description mentioning this just wraps readl/writel
with no, but that's a minor nit and is easy to find when you finally see
the removal in the end of the diff.
Reviewed-by: Andrew Halaney
> ---
> only compile-tested
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 12 ++--
> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 4 ++--
> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 4 ++--
> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h| 4 ++--
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h| 4 ++--
> drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 8
> drivers/gpu/drm/msm/hdmi/hdmi.h | 10 +-
> drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c| 6 +++---
> drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c| 4 ++--
> drivers/gpu/drm/msm/msm_drv.h | 7 ++-
> drivers/gpu/drm/msm/msm_gpu.h | 12 ++--
> 13 files changed, 42 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 8bea8ef26f77..0e3dfd4c2bc8 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -507,7 +507,7 @@ static void a6xx_rpmh_stop(struct a6xx_gmu *gmu)
>
> static inline void pdc_write(void __iomem *ptr, u32 offset, u32 value)
> {
> - msm_writel(value, ptr + (offset << 2));
> + writel(value, ptr + (offset << 2));
> }
>
> static void __iomem *a6xx_gmu_get_mmio(struct platform_device *pdev,
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> index 592b296aab22..94b6c5cab6f4 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> @@ -103,12 +103,12 @@ struct a6xx_gmu {
>
> static inline u32 gmu_read(struct a6xx_gmu *gmu, u32 offset)
> {
> - return msm_readl(gmu->mmio + (offset << 2));
> + return readl(gmu->mmio + (offset << 2));
> }
>
> static inline void gmu_write(struct a6xx_gmu *gmu, u32 offset, u32 value)
> {
> - msm_writel(value, gmu->mmio + (offset << 2));
> + writel(value, gmu->mmio + (offset << 2));
> }
>
> static inline void
> @@ -131,8 +131,8 @@ static inline u64 gmu_read64(struct a6xx_gmu *gmu, u32
> lo, u32 hi)
> {
> u64 val;
>
> - val = (u64) msm_readl(gmu->mmio + (lo << 2));
> - val |= ((u64) msm_readl(gmu->mmio + (hi << 2)) << 32);
> + val = (u64) readl(gmu->mmio + (lo << 2));
> + val |= ((u64) readl(gmu->mmio + (hi << 2)) << 32);
>
> return val;
> }
> @@ -143,12 +143,12 @@ static inline u64 gmu_read64(struct a6xx_gmu *gmu, u32
> lo, u32 hi)
>
> static inline u32 gmu_read_rscc(struct a6xx_gmu *gmu, u32 offset)
> {
> - return msm_readl(gmu->rscc + (offset << 2));
> + return readl(gmu->rscc + (offset << 2));
> }
>
> static inline void gmu_write_rscc(struct a6xx_gmu *gmu, u32 offset, u32
> value)
> {
> - msm_writel(value, gmu->rscc + (offset << 2));
> + writel(value, gmu->rscc + (offset << 2));
> }
>
> #define gmu_poll_timeout_rscc(gmu, addr, val, cond, interval, timeout) \
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> index 34822b080759..8917032b7515 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> @@ -69,12 +69,12 @@ static inline void a6xx_llc_rmw(struct a6xx_gpu
> *a6xx_gpu, u32 reg, u32 mask, u3
>
> static inline u32 a6xx_llc_read(struct a6xx_gpu *a6xx_gpu, u32 reg)
> {
> - return msm_readl(a6xx_gpu->llc_mmio + (reg << 2));
> + return readl(a6xx_gpu->llc_mmio + (reg << 2));
> }
>
> static inline void a6xx_llc_write(struct a6xx_gpu *a6xx_gpu, u32 reg, u32
> value)
> {
> - msm_writel(value, a6xx_gpu->llc_mmio + (reg << 2));
> + writel(value, a6xx_gpu->llc_mmio + (reg << 2));
> }
>
> #define shadowptr(_a6xx_gpu, _ring) ((_a6xx_gpu)->shadow_iova + \
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> index a847a0f7a73c..83d7ee01c944 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> @@ -192,10 +192,10 @@ static int debugbus_read(struct msm_gpu *gpu, u32
> block, u32 offset,
> }
>
> #define cxdbg_write(ptr, offset, val) \
> - msm_writel((val), (ptr) + ((offset) << 2))
> + writel((val), (ptr) + ((offset) << 2))
>
> #define cxdbg_read(ptr, offset) \
> - msm_readl((ptr) + ((offset) << 2))
> + readl((ptr) + ((offset) << 2))
>
> /* read a value from the CX debug bus */
> static int cx_debugbus_read(void __iomem *cxdbg, u32 block, u32 offset,
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
> b/
