Re: [PATCH 2/3] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage
On 2022-05-03 14:30, Dmitry Baryshkov wrote:
On Tue, 3 May 2022 at 13:57, Robin Murphy wrote:
On 2022-05-01 11:10, Dmitry Baryshkov wrote:
Move iommu_domain_alloc() in front of adress space/IOMMU initialization.
This allows us to drop final bits of struct mdp5_cfg_platform which
remained from the pre-DT days.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16
drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h | 6 --
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 --
3 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 1bf9ff5dbabc..714effb967ff 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
{ .revision = 3, .config = { .hw = _config } },
};
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
-
const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler
*cfg_handler)
{
return cfg_handler->config.hw;
@@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
uint32_t major, uint32_t minor)
{
struct drm_device *dev = mdp5_kms->dev;
- struct platform_device *pdev = to_platform_device(dev->dev);
struct mdp5_cfg_handler *cfg_handler;
const struct mdp5_cfg_handler *cfg_handlers;
- struct mdp5_cfg_platform *pconfig;
int i, ret = 0, num_handlers;
cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL);
@@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
cfg_handler->revision = minor;
cfg_handler->config.hw = mdp5_cfg;
- pconfig = mdp5_get_config(pdev);
- memcpy(_handler->config.platform, pconfig, sizeof(*pconfig));
-
DBG("MDP5: %s hw config selected", mdp5_cfg->name);
return cfg_handler;
@@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
return ERR_PTR(ret);
}
-
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
-{
- static struct mdp5_cfg_platform config = {};
-
- config.iommu = iommu_domain_alloc(_bus_type);
-
- return
-}
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
index 6b03d7899309..c2502cc33864 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
@@ -104,14 +104,8 @@ struct mdp5_cfg_hw {
uint32_t max_clk;
};
-/* platform config data (ie. from DT, or pdata) */
-struct mdp5_cfg_platform {
- struct iommu_domain *iommu;
-};
-
struct mdp5_cfg {
const struct mdp5_cfg_hw *hw;
- struct mdp5_cfg_platform platform;
};
struct mdp5_kms;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 9b7bbc3adb97..1c67c2c828cd 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev)
struct msm_gem_address_space *aspace;
int irq, i, ret;
struct device *iommu_dev;
+ struct iommu_domain *iommu;
ret = mdp5_init(to_platform_device(dev->dev), dev);
@@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev)
}
mdelay(16);
- if (config->platform.iommu) {
+ iommu = iommu_domain_alloc(_bus_type);
To preempt the next change down the line as well, could this be
rearranged to work as iommu_domain_alloc(iommu_dev->bus)?
I'd prefer to split this into the separate change, if you don't mind.
Oh, for sure, divide the patches however you see fit - I'm just hoping
to save your time overall by getting all the IOMMU-related refactoring
done now as a single series rather than risk me coming back and breaking
things again in a few months :)
Cheers,
Robin.
+ if (iommu) {
struct msm_mmu *mmu;
iommu_dev = >dev;
if (!dev_iommu_fwspec_get(iommu_dev))
The fwspec helpers are more of an internal thing between the IOMMU
drivers and the respective firmware code - I'd rather that external API
users stuck consistently to using device_iommu_mapped() (it should give
the same result).
Let me check that it works correctly and spin a v2 afterwards.
Otherwise, thanks for sorting this out!
Robin.
iommu_dev = iommu_dev->parent;
- mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
+ mmu = msm_iommu_new(iommu_dev, iommu);
aspace = msm_gem_address_space_create(mmu, "mdp5",
0x1000, 0x1 - 0x1000);
Re: [PATCH 2/3] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage
On Tue, 3 May 2022 at 13:57, Robin Murphy wrote:
>
> On 2022-05-01 11:10, Dmitry Baryshkov wrote:
> > Move iommu_domain_alloc() in front of adress space/IOMMU initialization.
> > This allows us to drop final bits of struct mdp5_cfg_platform which
> > remained from the pre-DT days.
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h | 6 --
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 --
> > 3 files changed, 4 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> > index 1bf9ff5dbabc..714effb967ff 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> > @@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler
> > cfg_handlers_v3[] = {
> > { .revision = 3, .config = { .hw = _config } },
> > };
> >
> > -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device
> > *dev);
> > -
> > const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler
> > *cfg_handler)
> > {
> > return cfg_handler->config.hw;
> > @@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct
> > mdp5_kms *mdp5_kms,
> > uint32_t major, uint32_t minor)
> > {
> > struct drm_device *dev = mdp5_kms->dev;
> > - struct platform_device *pdev = to_platform_device(dev->dev);
> > struct mdp5_cfg_handler *cfg_handler;
> > const struct mdp5_cfg_handler *cfg_handlers;
> > - struct mdp5_cfg_platform *pconfig;
> > int i, ret = 0, num_handlers;
> >
> > cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL);
> > @@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct
> > mdp5_kms *mdp5_kms,
> > cfg_handler->revision = minor;
> > cfg_handler->config.hw = mdp5_cfg;
> >
> > - pconfig = mdp5_get_config(pdev);
> > - memcpy(_handler->config.platform, pconfig, sizeof(*pconfig));
> > -
> > DBG("MDP5: %s hw config selected", mdp5_cfg->name);
> >
> > return cfg_handler;
> > @@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct
> > mdp5_kms *mdp5_kms,
> >
> > return ERR_PTR(ret);
> > }
> > -
> > -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device
> > *dev)
> > -{
> > - static struct mdp5_cfg_platform config = {};
> > -
> > - config.iommu = iommu_domain_alloc(_bus_type);
> > -
> > - return
> > -}
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> > index 6b03d7899309..c2502cc33864 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> > @@ -104,14 +104,8 @@ struct mdp5_cfg_hw {
> > uint32_t max_clk;
> > };
> >
> > -/* platform config data (ie. from DT, or pdata) */
> > -struct mdp5_cfg_platform {
> > - struct iommu_domain *iommu;
> > -};
> > -
> > struct mdp5_cfg {
> > const struct mdp5_cfg_hw *hw;
> > - struct mdp5_cfg_platform platform;
> > };
> >
> > struct mdp5_kms;
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > index 9b7bbc3adb97..1c67c2c828cd 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > @@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev)
> > struct msm_gem_address_space *aspace;
> > int irq, i, ret;
> > struct device *iommu_dev;
> > + struct iommu_domain *iommu;
> >
> > ret = mdp5_init(to_platform_device(dev->dev), dev);
> >
> > @@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev)
> > }
> > mdelay(16);
> >
> > - if (config->platform.iommu) {
> > + iommu = iommu_domain_alloc(_bus_type);
>
> To preempt the next change down the line as well, could this be
> rearranged to work as iommu_domain_alloc(iommu_dev->bus)?
I'd prefer to split this into the separate change, if you don't mind.
>
> > + if (iommu) {
> > struct msm_mmu *mmu;
> >
> > iommu_dev = >dev;
> > if (!dev_iommu_fwspec_get(iommu_dev))
>
> The fwspec helpers are more of an internal thing between the IOMMU
> drivers and the respective firmware code - I'd rather that external API
> users stuck consistently to using device_iommu_mapped() (it should give
> the same result).
Let me check that it works correctly and spin a v2 afterwards.
>
> Otherwise, thanks for sorting this out!
>
> Robin.
>
> > iommu_dev = iommu_dev->parent;
> >
> > - mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
> > + mmu = msm_iommu_new(iommu_dev, iommu);
> >
> > aspace = msm_gem_address_space_create(mmu, "mdp5",
> > 0x1000, 0x1 - 0x1000);
--
With best
Re: [PATCH 2/3] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage
On 2022-05-01 11:10, Dmitry Baryshkov wrote:
Move iommu_domain_alloc() in front of adress space/IOMMU initialization.
This allows us to drop final bits of struct mdp5_cfg_platform which
remained from the pre-DT days.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16
drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h | 6 --
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 --
3 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 1bf9ff5dbabc..714effb967ff 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
{ .revision = 3, .config = { .hw = _config } },
};
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
-
const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler
*cfg_handler)
{
return cfg_handler->config.hw;
@@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
uint32_t major, uint32_t minor)
{
struct drm_device *dev = mdp5_kms->dev;
- struct platform_device *pdev = to_platform_device(dev->dev);
struct mdp5_cfg_handler *cfg_handler;
const struct mdp5_cfg_handler *cfg_handlers;
- struct mdp5_cfg_platform *pconfig;
int i, ret = 0, num_handlers;
cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL);
@@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
cfg_handler->revision = minor;
cfg_handler->config.hw = mdp5_cfg;
- pconfig = mdp5_get_config(pdev);
- memcpy(_handler->config.platform, pconfig, sizeof(*pconfig));
-
DBG("MDP5: %s hw config selected", mdp5_cfg->name);
return cfg_handler;
@@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
return ERR_PTR(ret);
}
-
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
-{
- static struct mdp5_cfg_platform config = {};
-
- config.iommu = iommu_domain_alloc(_bus_type);
-
- return
-}
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
index 6b03d7899309..c2502cc33864 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
@@ -104,14 +104,8 @@ struct mdp5_cfg_hw {
uint32_t max_clk;
};
-/* platform config data (ie. from DT, or pdata) */
-struct mdp5_cfg_platform {
- struct iommu_domain *iommu;
-};
-
struct mdp5_cfg {
const struct mdp5_cfg_hw *hw;
- struct mdp5_cfg_platform platform;
};
struct mdp5_kms;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 9b7bbc3adb97..1c67c2c828cd 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev)
struct msm_gem_address_space *aspace;
int irq, i, ret;
struct device *iommu_dev;
+ struct iommu_domain *iommu;
ret = mdp5_init(to_platform_device(dev->dev), dev);
@@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev)
}
mdelay(16);
- if (config->platform.iommu) {
+ iommu = iommu_domain_alloc(_bus_type);
To preempt the next change down the line as well, could this be
rearranged to work as iommu_domain_alloc(iommu_dev->bus)?
+ if (iommu) {
struct msm_mmu *mmu;
iommu_dev = >dev;
if (!dev_iommu_fwspec_get(iommu_dev))
The fwspec helpers are more of an internal thing between the IOMMU
drivers and the respective firmware code - I'd rather that external API
users stuck consistently to using device_iommu_mapped() (it should give
the same result).
Otherwise, thanks for sorting this out!
Robin.
iommu_dev = iommu_dev->parent;
- mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
+ mmu = msm_iommu_new(iommu_dev, iommu);
aspace = msm_gem_address_space_create(mmu, "mdp5",
0x1000, 0x1 - 0x1000);
[PATCH 2/3] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage
Move iommu_domain_alloc() in front of adress space/IOMMU initialization.
This allows us to drop final bits of struct mdp5_cfg_platform which
remained from the pre-DT days.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16
drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h | 6 --
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 6 --
3 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 1bf9ff5dbabc..714effb967ff 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
{ .revision = 3, .config = { .hw = _config } },
};
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
-
const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler
*cfg_handler)
{
return cfg_handler->config.hw;
@@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
uint32_t major, uint32_t minor)
{
struct drm_device *dev = mdp5_kms->dev;
- struct platform_device *pdev = to_platform_device(dev->dev);
struct mdp5_cfg_handler *cfg_handler;
const struct mdp5_cfg_handler *cfg_handlers;
- struct mdp5_cfg_platform *pconfig;
int i, ret = 0, num_handlers;
cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL);
@@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
cfg_handler->revision = minor;
cfg_handler->config.hw = mdp5_cfg;
- pconfig = mdp5_get_config(pdev);
- memcpy(_handler->config.platform, pconfig, sizeof(*pconfig));
-
DBG("MDP5: %s hw config selected", mdp5_cfg->name);
return cfg_handler;
@@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms
*mdp5_kms,
return ERR_PTR(ret);
}
-
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
-{
- static struct mdp5_cfg_platform config = {};
-
- config.iommu = iommu_domain_alloc(_bus_type);
-
- return
-}
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
index 6b03d7899309..c2502cc33864 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
@@ -104,14 +104,8 @@ struct mdp5_cfg_hw {
uint32_t max_clk;
};
-/* platform config data (ie. from DT, or pdata) */
-struct mdp5_cfg_platform {
- struct iommu_domain *iommu;
-};
-
struct mdp5_cfg {
const struct mdp5_cfg_hw *hw;
- struct mdp5_cfg_platform platform;
};
struct mdp5_kms;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 9b7bbc3adb97..1c67c2c828cd 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev)
struct msm_gem_address_space *aspace;
int irq, i, ret;
struct device *iommu_dev;
+ struct iommu_domain *iommu;
ret = mdp5_init(to_platform_device(dev->dev), dev);
@@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev)
}
mdelay(16);
- if (config->platform.iommu) {
+ iommu = iommu_domain_alloc(_bus_type);
+ if (iommu) {
struct msm_mmu *mmu;
iommu_dev = >dev;
if (!dev_iommu_fwspec_get(iommu_dev))
iommu_dev = iommu_dev->parent;
- mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
+ mmu = msm_iommu_new(iommu_dev, iommu);
aspace = msm_gem_address_space_create(mmu, "mdp5",
0x1000, 0x1 - 0x1000);
--
2.35.1
