Hi Sravanthi,
On Wed, Oct 10, 2018 at 02:54:33PM +0530, Sravanthi Kollukuduru wrote:
> The interconnect framework is designed to provide a
> standard kernel interface to control the settings of
> the interconnects on a SoC.
>
> The interconnect API uses a consumer/provider-based model,
> where the providers are the interconnect buses and the
> consumers could be various drivers.
>
> MDSS is one of the interconnect consumers which uses the
> interconnect APIs to get the path between endpoints and
> set its bandwidth/latency/QoS requirements for the given
> interconnected path.
>
> Changes in v2:
> - Remove error log and unnecessary check (Jordan Crouse)
>
> Signed-off-by: Sravanthi Kollukuduru <skolluku@xxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 50 +++++++++++++++++++++++++++++---
> 1 file changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> index 2235ef8129f4..27c2594e5133 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> @@ -4,6 +4,7 @@
> */
>
> #include "dpu_kms.h"
> +#include <linux/interconnect.h>
>
> #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
>
> @@ -16,8 +17,33 @@ struct dpu_mdss {
> u32 hwversion;
> struct dss_module_power mp;
> struct dpu_irq_controller irq_controller;
> + struct icc_path *path[2];
> + u32 num_paths;
> };
>
> +static int dpu_mdss_parse_data_bus_icc_path(
> + struct drm_device *dev, struct dpu_mdss *dpu_mdss)
> +{
> + struct icc_path *path0 = of_icc_get(dev->dev, "port0");
> + struct icc_path *path1 = of_icc_get(dev->dev, "port1");
> + int total_num_paths = 0;
initialization is not needed
nit: the 'total_' prefix doesn't add any value here, just 'num_paths'
would be easier to parse. Actually you could get rid of the variable
completely by just initializing and incrementing dpu_mdss->num_paths.
> static irqreturn_t dpu_mdss_irq(int irq, void *arg)
> {
> struct dpu_mdss *dpu_mdss = arg;
> @@ -127,7 +153,12 @@ static int dpu_mdss_enable(struct msm_mdss *mdss)
> {
> struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
> struct dss_module_power *mp = &dpu_mdss->mp;
> - int ret;
> + int ret, i;
> + u64 ab = (dpu_mdss->num_paths) ? 6800000000/dpu_mdss->num_paths : 0;
parentheses are not needed
> + u64 ib = 6800000000;
There should probably be a define for 6800000000.
nit: names like avg_bw and peak_pw would be clearer than ab and ib,
without being excessively verbose.
Cheers
Matthias
