Re: [DPU PATCH 09/11] drm/msm/dpu: Remove unused code and move the header
On 2018-06-22 21:03, Jordan Crouse wrote:
On Fri, Jun 22, 2018 at 09:51:28AM -0400, Sean Paul wrote:
On Wed, May 30, 2018 at 10:50 AM Rajesh Yadav
wrote:
>
> From: Jordan Crouse
>
> Remove unused code from dpu_io_util.c. The functions are only
> used inside of the msm driver so remove the EXPORT_SYMBOL
> tags and move the header dpu_io_util.h from include/linux.
>
> Signed-off-by: Jordan Crouse
> [rya...@codeaurora.org: rebased and removed some extra unused code]
> Signed-off-by: Rajesh Yadav
Hi Rajesh and Jordan,
I'm backporting this series for testing, and have found that the
mdss-pll driver uses both dpu_io_util and a bunch of functions/struct
members removed in this patchset. Do we anticipate having to add those
back for mdss-pll?
At some point in the distant past some of the downstream folks
discovered that
they were using relatively similar functions so they decided to share
the code.
The code originally lived in the display driver so it happened that the
exported
code continued to be hosted there.
I'm not sure what the upstream status of the mdss-pll code is but I
can't
imagine that sharing this code would be in the long term plan and even
if it
was basic module guidelines would dictate that it would live with
mdss-pll
which would be more likely to be either be built in or act as a
dependency for
DPU.
Jordan
Hi Sean,
drivers/clk/qcom/mdss was needed for dsi-staging driver and can be
safely dropped since we switched to upstream DSI driver.
Please drop the following change to address this issue (except for the
driver/gpu/drm/msm/Kconfig):
https://gitlab.freedesktop.org/seanpaul/dpu-staging/commit/2e433b56489b3e53b9d89ce7bfa15fb4698c8c58
I had tested it on my side.
Thanks,
Rajesh
> ---
> drivers/gpu/drm/msm/dpu_io_util.c | 380
+-
> drivers/gpu/drm/msm/dpu_io_util.h | 61 ++
> drivers/gpu/drm/msm/msm_drv.h | 1 -
> include/linux/dpu_io_util.h | 115
> 4 files changed, 66 insertions(+), 491 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/dpu_io_util.h
> delete mode 100644 include/linux/dpu_io_util.h
>
> diff --git a/drivers/gpu/drm/msm/dpu_io_util.c
b/drivers/gpu/drm/msm/dpu_io_util.c
> index ecc297c..f7caec3 100644
> --- a/drivers/gpu/drm/msm/dpu_io_util.c
> +++ b/drivers/gpu/drm/msm/dpu_io_util.c
> @@ -13,318 +13,9 @@
>
> #include
> #include
> -#include
> -#include
> #include
> -#include
>
> -#define MAX_I2C_CMDS 16
> -void dss_reg_w(struct dss_io_data *io, u32 offset, u32 value, u32 debug)
> -{
> - u32 in_val;
> -
> - if (!io || !io->base) {
> - DEV_ERR("%pS->%s: invalid input\n",
> - __builtin_return_address(0), __func__);
> - return;
> - }
> -
> - if (offset > io->len) {
> - DEV_ERR("%pS->%s: offset out of range\n",
> - __builtin_return_address(0), __func__);
> - return;
> - }
> -
> - writel_relaxed(value, io->base + offset);
> - if (debug) {
> - in_val = readl_relaxed(io->base + offset);
> - DEV_DBG("[%08x] => %08x [%08x]\n",
> - (u32)(unsigned long)(io->base + offset),
> - value, in_val);
> - }
> -} /* dss_reg_w */
> -EXPORT_SYMBOL(dss_reg_w);
> -
> -u32 dss_reg_r(struct dss_io_data *io, u32 offset, u32 debug)
> -{
> - u32 value;
> -
> - if (!io || !io->base) {
> - DEV_ERR("%pS->%s: invalid input\n",
> - __builtin_return_address(0), __func__);
> - return -EINVAL;
> - }
> -
> - if (offset > io->len) {
> - DEV_ERR("%pS->%s: offset out of range\n",
> - __builtin_return_address(0), __func__);
> - return -EINVAL;
> - }
> -
> - value = readl_relaxed(io->base + offset);
> - if (debug)
> - DEV_DBG("[%08x] <= %08x\n",
> - (u32)(unsigned long)(io->base + offset), value);
> -
> - return value;
> -} /* dss_reg_r */
> -EXPORT_SYMBOL(dss_reg_r);
> -
> -void dss_reg_dump(void __iomem *base, u32 length, const char *prefix,
> - u32 debug)
> -{
> - if (debug)
> - print_hex_dump(KERN_INFO, prefix, DUMP_PREFIX_OFFSET, 32, 4,
> - (void *)base, length, false);
> -} /* dss_reg_dump */
> -EXPORT_SYMBOL(dss_reg_dump);
> -
> -static struct resource *msm_dss_get_res_byname(struct platform_device *pdev,
> - unsigned int type, const char *name)
> -{
> - struct resource *res = NULL;
> -
> - res = platform_get_resource_byname(pdev, type, name);
> - if (!res)
> - DEV_ERR("%s: '%s' resource not found\n", __func__, name);
> -
> - return res;
> -} /* msm_dss_get_res_byname */
> -EXPORT_SYMBOL(msm_dss_get_res_byname);
> -
> -int msm_dss_ioremap_byname(struct platform_device *pdev,
> - struct dss_io_data *io_data, const ch
Re: [DPU PATCH 09/11] drm/msm/dpu: Remove unused code and move the header
On Fri, Jun 22, 2018 at 09:51:28AM -0400, Sean Paul wrote:
> On Wed, May 30, 2018 at 10:50 AM Rajesh Yadav wrote:
> >
> > From: Jordan Crouse
> >
> > Remove unused code from dpu_io_util.c. The functions are only
> > used inside of the msm driver so remove the EXPORT_SYMBOL
> > tags and move the header dpu_io_util.h from include/linux.
> >
> > Signed-off-by: Jordan Crouse
> > [rya...@codeaurora.org: rebased and removed some extra unused code]
> > Signed-off-by: Rajesh Yadav
>
> Hi Rajesh and Jordan,
> I'm backporting this series for testing, and have found that the
> mdss-pll driver uses both dpu_io_util and a bunch of functions/struct
> members removed in this patchset. Do we anticipate having to add those
> back for mdss-pll?
At some point in the distant past some of the downstream folks discovered that
they were using relatively similar functions so they decided to share the code.
The code originally lived in the display driver so it happened that the exported
code continued to be hosted there.
I'm not sure what the upstream status of the mdss-pll code is but I can't
imagine that sharing this code would be in the long term plan and even if it
was basic module guidelines would dictate that it would live with mdss-pll
which would be more likely to be either be built in or act as a dependency for
DPU.
Jordan
> > ---
> > drivers/gpu/drm/msm/dpu_io_util.c | 380
> > +-
> > drivers/gpu/drm/msm/dpu_io_util.h | 61 ++
> > drivers/gpu/drm/msm/msm_drv.h | 1 -
> > include/linux/dpu_io_util.h | 115
> > 4 files changed, 66 insertions(+), 491 deletions(-)
> > create mode 100644 drivers/gpu/drm/msm/dpu_io_util.h
> > delete mode 100644 include/linux/dpu_io_util.h
> >
> > diff --git a/drivers/gpu/drm/msm/dpu_io_util.c
> > b/drivers/gpu/drm/msm/dpu_io_util.c
> > index ecc297c..f7caec3 100644
> > --- a/drivers/gpu/drm/msm/dpu_io_util.c
> > +++ b/drivers/gpu/drm/msm/dpu_io_util.c
> > @@ -13,318 +13,9 @@
> >
> > #include
> > #include
> > -#include
> > -#include
> > #include
> > -#include
> >
> > -#define MAX_I2C_CMDS 16
> > -void dss_reg_w(struct dss_io_data *io, u32 offset, u32 value, u32 debug)
> > -{
> > - u32 in_val;
> > -
> > - if (!io || !io->base) {
> > - DEV_ERR("%pS->%s: invalid input\n",
> > - __builtin_return_address(0), __func__);
> > - return;
> > - }
> > -
> > - if (offset > io->len) {
> > - DEV_ERR("%pS->%s: offset out of range\n",
> > - __builtin_return_address(0), __func__);
> > - return;
> > - }
> > -
> > - writel_relaxed(value, io->base + offset);
> > - if (debug) {
> > - in_val = readl_relaxed(io->base + offset);
> > - DEV_DBG("[%08x] => %08x [%08x]\n",
> > - (u32)(unsigned long)(io->base + offset),
> > - value, in_val);
> > - }
> > -} /* dss_reg_w */
> > -EXPORT_SYMBOL(dss_reg_w);
> > -
> > -u32 dss_reg_r(struct dss_io_data *io, u32 offset, u32 debug)
> > -{
> > - u32 value;
> > -
> > - if (!io || !io->base) {
> > - DEV_ERR("%pS->%s: invalid input\n",
> > - __builtin_return_address(0), __func__);
> > - return -EINVAL;
> > - }
> > -
> > - if (offset > io->len) {
> > - DEV_ERR("%pS->%s: offset out of range\n",
> > - __builtin_return_address(0), __func__);
> > - return -EINVAL;
> > - }
> > -
> > - value = readl_relaxed(io->base + offset);
> > - if (debug)
> > - DEV_DBG("[%08x] <= %08x\n",
> > - (u32)(unsigned long)(io->base + offset), value);
> > -
> > - return value;
> > -} /* dss_reg_r */
> > -EXPORT_SYMBOL(dss_reg_r);
> > -
> > -void dss_reg_dump(void __iomem *base, u32 length, const char *prefix,
> > - u32 debug)
> > -{
> > - if (debug)
> > - print_hex_dump(KERN_INFO, prefix, DUMP_PREFIX_OFFSET, 32, 4,
> > - (void *)base, length, false);
> > -} /* dss_reg_dump */
> > -EXPORT_SYMBOL(dss_reg_dump);
> > -
> > -static struct resource *msm_dss_get_res_byname(struct platform_device
> > *pdev,
> > - unsigned int type, const char *name)
> > -{
> > - struct resource *res = NULL;
> > -
> > - res = platform_get_resource_byname(pdev, type, name);
> > - if (!res)
> > - DEV_ERR("%s: '%s' resource not found\n", __func__, name);
> > -
> > - return res;
> > -} /* msm_dss_get_res_byname */
> > -EXPORT_SYMBOL(msm_dss_get_res_byname);
> > -
> > -int msm_dss_ioremap_byname(struct platform_device *pdev,
> > - struct dss_io_data *io_data, const char *name)
> > -{
> > - struct resource *res = NULL;
> > -
> > - if (!pdev || !io_data) {
> > - DEV_ERR("%pS->%s: invalid input\n",
> > - __builtin_ret
Re: [DPU PATCH 09/11] drm/msm/dpu: Remove unused code and move the header
On Wed, May 30, 2018 at 10:50 AM Rajesh Yadav wrote:
>
> From: Jordan Crouse
>
> Remove unused code from dpu_io_util.c. The functions are only
> used inside of the msm driver so remove the EXPORT_SYMBOL
> tags and move the header dpu_io_util.h from include/linux.
>
> Signed-off-by: Jordan Crouse
> [rya...@codeaurora.org: rebased and removed some extra unused code]
> Signed-off-by: Rajesh Yadav
Hi Rajesh and Jordan,
I'm backporting this series for testing, and have found that the
mdss-pll driver uses both dpu_io_util and a bunch of functions/struct
members removed in this patchset. Do we anticipate having to add those
back for mdss-pll?
Sean
> ---
> drivers/gpu/drm/msm/dpu_io_util.c | 380
> +-
> drivers/gpu/drm/msm/dpu_io_util.h | 61 ++
> drivers/gpu/drm/msm/msm_drv.h | 1 -
> include/linux/dpu_io_util.h | 115
> 4 files changed, 66 insertions(+), 491 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/dpu_io_util.h
> delete mode 100644 include/linux/dpu_io_util.h
>
> diff --git a/drivers/gpu/drm/msm/dpu_io_util.c
> b/drivers/gpu/drm/msm/dpu_io_util.c
> index ecc297c..f7caec3 100644
> --- a/drivers/gpu/drm/msm/dpu_io_util.c
> +++ b/drivers/gpu/drm/msm/dpu_io_util.c
> @@ -13,318 +13,9 @@
>
> #include
> #include
> -#include
> -#include
> #include
> -#include
>
> -#define MAX_I2C_CMDS 16
> -void dss_reg_w(struct dss_io_data *io, u32 offset, u32 value, u32 debug)
> -{
> - u32 in_val;
> -
> - if (!io || !io->base) {
> - DEV_ERR("%pS->%s: invalid input\n",
> - __builtin_return_address(0), __func__);
> - return;
> - }
> -
> - if (offset > io->len) {
> - DEV_ERR("%pS->%s: offset out of range\n",
> - __builtin_return_address(0), __func__);
> - return;
> - }
> -
> - writel_relaxed(value, io->base + offset);
> - if (debug) {
> - in_val = readl_relaxed(io->base + offset);
> - DEV_DBG("[%08x] => %08x [%08x]\n",
> - (u32)(unsigned long)(io->base + offset),
> - value, in_val);
> - }
> -} /* dss_reg_w */
> -EXPORT_SYMBOL(dss_reg_w);
> -
> -u32 dss_reg_r(struct dss_io_data *io, u32 offset, u32 debug)
> -{
> - u32 value;
> -
> - if (!io || !io->base) {
> - DEV_ERR("%pS->%s: invalid input\n",
> - __builtin_return_address(0), __func__);
> - return -EINVAL;
> - }
> -
> - if (offset > io->len) {
> - DEV_ERR("%pS->%s: offset out of range\n",
> - __builtin_return_address(0), __func__);
> - return -EINVAL;
> - }
> -
> - value = readl_relaxed(io->base + offset);
> - if (debug)
> - DEV_DBG("[%08x] <= %08x\n",
> - (u32)(unsigned long)(io->base + offset), value);
> -
> - return value;
> -} /* dss_reg_r */
> -EXPORT_SYMBOL(dss_reg_r);
> -
> -void dss_reg_dump(void __iomem *base, u32 length, const char *prefix,
> - u32 debug)
> -{
> - if (debug)
> - print_hex_dump(KERN_INFO, prefix, DUMP_PREFIX_OFFSET, 32, 4,
> - (void *)base, length, false);
> -} /* dss_reg_dump */
> -EXPORT_SYMBOL(dss_reg_dump);
> -
> -static struct resource *msm_dss_get_res_byname(struct platform_device *pdev,
> - unsigned int type, const char *name)
> -{
> - struct resource *res = NULL;
> -
> - res = platform_get_resource_byname(pdev, type, name);
> - if (!res)
> - DEV_ERR("%s: '%s' resource not found\n", __func__, name);
> -
> - return res;
> -} /* msm_dss_get_res_byname */
> -EXPORT_SYMBOL(msm_dss_get_res_byname);
> -
> -int msm_dss_ioremap_byname(struct platform_device *pdev,
> - struct dss_io_data *io_data, const char *name)
> -{
> - struct resource *res = NULL;
> -
> - if (!pdev || !io_data) {
> - DEV_ERR("%pS->%s: invalid input\n",
> - __builtin_return_address(0), __func__);
> - return -EINVAL;
> - }
> -
> - res = msm_dss_get_res_byname(pdev, IORESOURCE_MEM, name);
> - if (!res) {
> - DEV_ERR("%pS->%s: '%s' msm_dss_get_res_byname failed\n",
> - __builtin_return_address(0), __func__, name);
> - return -ENODEV;
> - }
> -
> - io_data->len = (u32)resource_size(res);
> - io_data->base = ioremap(res->start, io_data->len);
> - if (!io_data->base) {
> - DEV_ERR("%pS->%s: '%s' ioremap failed\n",
> - __builtin_return_address(0), __func__, name);
> - return -EIO;
> - }
> -
> - return 0;
> -} /* msm_dss_ioremap_byname */
> -EXPORT_SYMBOL(msm_dss_ioremap_byname);
> -
> -void msm_dss_iounmap(struct dss_io_data *io_data)
> -{
> - if (!io_data) {
> - DEV_ERR("%p
[DPU PATCH 09/11] drm/msm/dpu: Remove unused code and move the header
From: Jordan Crouse
Remove unused code from dpu_io_util.c. The functions are only
used inside of the msm driver so remove the EXPORT_SYMBOL
tags and move the header dpu_io_util.h from include/linux.
Signed-off-by: Jordan Crouse
[rya...@codeaurora.org: rebased and removed some extra unused code]
Signed-off-by: Rajesh Yadav
---
drivers/gpu/drm/msm/dpu_io_util.c | 380 +-
drivers/gpu/drm/msm/dpu_io_util.h | 61 ++
drivers/gpu/drm/msm/msm_drv.h | 1 -
include/linux/dpu_io_util.h | 115
4 files changed, 66 insertions(+), 491 deletions(-)
create mode 100644 drivers/gpu/drm/msm/dpu_io_util.h
delete mode 100644 include/linux/dpu_io_util.h
diff --git a/drivers/gpu/drm/msm/dpu_io_util.c
b/drivers/gpu/drm/msm/dpu_io_util.c
index ecc297c..f7caec3 100644
--- a/drivers/gpu/drm/msm/dpu_io_util.c
+++ b/drivers/gpu/drm/msm/dpu_io_util.c
@@ -13,318 +13,9 @@
#include
#include
-#include
-#include
#include
-#include
-#define MAX_I2C_CMDS 16
-void dss_reg_w(struct dss_io_data *io, u32 offset, u32 value, u32 debug)
-{
- u32 in_val;
-
- if (!io || !io->base) {
- DEV_ERR("%pS->%s: invalid input\n",
- __builtin_return_address(0), __func__);
- return;
- }
-
- if (offset > io->len) {
- DEV_ERR("%pS->%s: offset out of range\n",
- __builtin_return_address(0), __func__);
- return;
- }
-
- writel_relaxed(value, io->base + offset);
- if (debug) {
- in_val = readl_relaxed(io->base + offset);
- DEV_DBG("[%08x] => %08x [%08x]\n",
- (u32)(unsigned long)(io->base + offset),
- value, in_val);
- }
-} /* dss_reg_w */
-EXPORT_SYMBOL(dss_reg_w);
-
-u32 dss_reg_r(struct dss_io_data *io, u32 offset, u32 debug)
-{
- u32 value;
-
- if (!io || !io->base) {
- DEV_ERR("%pS->%s: invalid input\n",
- __builtin_return_address(0), __func__);
- return -EINVAL;
- }
-
- if (offset > io->len) {
- DEV_ERR("%pS->%s: offset out of range\n",
- __builtin_return_address(0), __func__);
- return -EINVAL;
- }
-
- value = readl_relaxed(io->base + offset);
- if (debug)
- DEV_DBG("[%08x] <= %08x\n",
- (u32)(unsigned long)(io->base + offset), value);
-
- return value;
-} /* dss_reg_r */
-EXPORT_SYMBOL(dss_reg_r);
-
-void dss_reg_dump(void __iomem *base, u32 length, const char *prefix,
- u32 debug)
-{
- if (debug)
- print_hex_dump(KERN_INFO, prefix, DUMP_PREFIX_OFFSET, 32, 4,
- (void *)base, length, false);
-} /* dss_reg_dump */
-EXPORT_SYMBOL(dss_reg_dump);
-
-static struct resource *msm_dss_get_res_byname(struct platform_device *pdev,
- unsigned int type, const char *name)
-{
- struct resource *res = NULL;
-
- res = platform_get_resource_byname(pdev, type, name);
- if (!res)
- DEV_ERR("%s: '%s' resource not found\n", __func__, name);
-
- return res;
-} /* msm_dss_get_res_byname */
-EXPORT_SYMBOL(msm_dss_get_res_byname);
-
-int msm_dss_ioremap_byname(struct platform_device *pdev,
- struct dss_io_data *io_data, const char *name)
-{
- struct resource *res = NULL;
-
- if (!pdev || !io_data) {
- DEV_ERR("%pS->%s: invalid input\n",
- __builtin_return_address(0), __func__);
- return -EINVAL;
- }
-
- res = msm_dss_get_res_byname(pdev, IORESOURCE_MEM, name);
- if (!res) {
- DEV_ERR("%pS->%s: '%s' msm_dss_get_res_byname failed\n",
- __builtin_return_address(0), __func__, name);
- return -ENODEV;
- }
-
- io_data->len = (u32)resource_size(res);
- io_data->base = ioremap(res->start, io_data->len);
- if (!io_data->base) {
- DEV_ERR("%pS->%s: '%s' ioremap failed\n",
- __builtin_return_address(0), __func__, name);
- return -EIO;
- }
-
- return 0;
-} /* msm_dss_ioremap_byname */
-EXPORT_SYMBOL(msm_dss_ioremap_byname);
-
-void msm_dss_iounmap(struct dss_io_data *io_data)
-{
- if (!io_data) {
- DEV_ERR("%pS->%s: invalid input\n",
- __builtin_return_address(0), __func__);
- return;
- }
-
- if (io_data->base) {
- iounmap(io_data->base);
- io_data->base = NULL;
- }
- io_data->len = 0;
-} /* msm_dss_iounmap */
-EXPORT_SYMBOL(msm_dss_iounmap);
-
-int msm_dss_config_vreg(struct device *dev, struct dss_vreg *in_vreg,
- int num_vreg, int config)
-{
- int i = 0, rc = 0;
- struct dss_vreg *curr_vreg = NULL;
- enum dss_vreg_type type;
-
- if (!in_vreg || !num_vreg)
-
