| Message ID | 20200106104339.215511-1-christian.gmeiner@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/etnaviv: rework perfmon query infrastructure | expand |
Am Mo., 6. Jan. 2020 um 11:43 Uhr schrieb Christian Gmeiner <christian.gmeiner@gmail.com>: > > Report the correct perfmon domains and signals depending > on the supported feature flags. > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query perf counter") > Cc: stable@vger.kernel.org > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > --- > drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 57 ++++++++++++++++++++--- > 1 file changed, 50 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c > index 8adbf2861bff..7ae8f347ca06 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c > @@ -32,6 +32,7 @@ struct etnaviv_pm_domain { > }; > > struct etnaviv_pm_domain_meta { > + unsigned int feature; > const struct etnaviv_pm_domain *domains; > u32 nr_domains; > }; > @@ -410,36 +411,78 @@ static const struct etnaviv_pm_domain doms_vg[] = { > > static const struct etnaviv_pm_domain_meta doms_meta[] = { > { > + .feature = chipFeatures_PIPE_3D, > .nr_domains = ARRAY_SIZE(doms_3d), > .domains = &doms_3d[0] > }, > { > + .feature = chipFeatures_PIPE_2D, > .nr_domains = ARRAY_SIZE(doms_2d), > .domains = &doms_2d[0] > }, > { > + .feature = chipFeatures_PIPE_VG, > .nr_domains = ARRAY_SIZE(doms_vg), > .domains = &doms_vg[0] > } > }; > > +static unsigned int num_pm_domains(const struct etnaviv_gpu *gpu) > +{ > + unsigned int num = 0, i; > + > + for (i = 0; i < ARRAY_SIZE(doms_meta); i++) { > + const struct etnaviv_pm_domain_meta *meta = &doms_meta[i]; > + > + if (gpu->identity.features & meta->feature) > + num += meta->nr_domains; > + } > + > + return num; > +} > + > +static const struct etnaviv_pm_domain *pm_domain(const struct etnaviv_gpu *gpu, > + unsigned int index) > +{ > + const struct etnaviv_pm_domain *domain = NULL; > + unsigned int offset = 0, i; > + > + for (i = 0; i < ARRAY_SIZE(doms_meta); i++) { > + const struct etnaviv_pm_domain_meta *meta = &doms_meta[i]; > + > + if (!(gpu->identity.features & meta->feature)) > + continue; > + > + if (meta->nr_domains < (index - offset)) { > + offset += meta->nr_domains; > + continue; > + } > + > + domain = meta->domains + (index - offset); > + } > + > + BUG_ON(!domain); > + > + return domain; > +} > + > int etnaviv_pm_query_dom(struct etnaviv_gpu *gpu, > struct drm_etnaviv_pm_domain *domain) > { > - const struct etnaviv_pm_domain_meta *meta = &doms_meta[domain->pipe]; > + const unsigned int nr_domains = num_pm_domains(gpu); > const struct etnaviv_pm_domain *dom; > > - if (domain->iter >= meta->nr_domains) > + if (domain->iter >= nr_domains) > return -EINVAL; > > - dom = meta->domains + domain->iter; > + dom = pm_domain(gpu, domain->iter); > > domain->id = domain->iter; > domain->nr_signals = dom->nr_signals; > strncpy(domain->name, dom->name, sizeof(domain->name)); > > domain->iter++; > - if (domain->iter == meta->nr_domains) > + if (domain->iter == nr_domains) > domain->iter = 0xff; > > return 0; > @@ -448,14 +491,14 @@ int etnaviv_pm_query_dom(struct etnaviv_gpu *gpu, > int etnaviv_pm_query_sig(struct etnaviv_gpu *gpu, > struct drm_etnaviv_pm_signal *signal) > { > - const struct etnaviv_pm_domain_meta *meta = &doms_meta[signal->pipe]; > + const unsigned int nr_domains = num_pm_domains(gpu); > const struct etnaviv_pm_domain *dom; > const struct etnaviv_pm_signal *sig; > > - if (signal->domain >= meta->nr_domains) > + if (signal->domain >= nr_domains) > return -EINVAL; > > - dom = meta->domains + signal->domain; > + dom = pm_domain(gpu, signal->domain); > > if (signal->iter >= dom->nr_signals) > return -EINVAL; > -- > 2.24.1 > ping
Hi Christian, sorry for taking so long to get around to this. On Mo, 2020-01-06 at 11:43 +0100, Christian Gmeiner wrote: > Report the correct perfmon domains and signals depending > on the supported feature flags. > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query perf counter") > Cc: stable@vger.kernel.org > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > --- > drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 57 ++++++++++++++++++++--- > 1 file changed, 50 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c > index 8adbf2861bff..7ae8f347ca06 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c > @@ -32,6 +32,7 @@ struct etnaviv_pm_domain { > }; > > struct etnaviv_pm_domain_meta { > + unsigned int feature; > const struct etnaviv_pm_domain *domains; > u32 nr_domains; > }; > @@ -410,36 +411,78 @@ static const struct etnaviv_pm_domain doms_vg[] = { > > static const struct etnaviv_pm_domain_meta doms_meta[] = { > { > + .feature = chipFeatures_PIPE_3D, > .nr_domains = ARRAY_SIZE(doms_3d), > .domains = &doms_3d[0] > }, > { > + .feature = chipFeatures_PIPE_2D, > .nr_domains = ARRAY_SIZE(doms_2d), > .domains = &doms_2d[0] > }, > { > + .feature = chipFeatures_PIPE_VG, > .nr_domains = ARRAY_SIZE(doms_vg), > .domains = &doms_vg[0] > } > }; > > +static unsigned int num_pm_domains(const struct etnaviv_gpu *gpu) > +{ > + unsigned int num = 0, i; > + > + for (i = 0; i < ARRAY_SIZE(doms_meta); i++) { > + const struct etnaviv_pm_domain_meta *meta = &doms_meta[i]; > + > + if (gpu->identity.features & meta->feature) > + num += meta->nr_domains; > + } > + > + return num; > +} > + > +static const struct etnaviv_pm_domain *pm_domain(const struct etnaviv_gpu *gpu, > + unsigned int index) > +{ > + const struct etnaviv_pm_domain *domain = NULL; > + unsigned int offset = 0, i; > + > + for (i = 0; i < ARRAY_SIZE(doms_meta); i++) { > + const struct etnaviv_pm_domain_meta *meta = &doms_meta[i]; > + > + if (!(gpu->identity.features & meta->feature)) > + continue; > + > + if (meta->nr_domains < (index - offset)) { > + offset += meta->nr_domains; > + continue; > + } > + > + domain = meta->domains + (index - offset); > + } > + > + BUG_ON(!domain); This is a no-go. BUG_ON is reserved for only the most severe kernel bugs where you can't possibly continue without risking a corruption of non-volatile state. This isn't the case here, please instead just make the callers handle a NULL return gracefully. Regards, Lucas > + > + return domain; > +} > + > int etnaviv_pm_query_dom(struct etnaviv_gpu *gpu, > struct drm_etnaviv_pm_domain *domain) > { > - const struct etnaviv_pm_domain_meta *meta = &doms_meta[domain->pipe]; > + const unsigned int nr_domains = num_pm_domains(gpu); > const struct etnaviv_pm_domain *dom; > > - if (domain->iter >= meta->nr_domains) > + if (domain->iter >= nr_domains) > return -EINVAL; > > - dom = meta->domains + domain->iter; > + dom = pm_domain(gpu, domain->iter); > > domain->id = domain->iter; > domain->nr_signals = dom->nr_signals; > strncpy(domain->name, dom->name, sizeof(domain->name)); > > domain->iter++; > - if (domain->iter == meta->nr_domains) > + if (domain->iter == nr_domains) > domain->iter = 0xff; > > return 0; > @@ -448,14 +491,14 @@ int etnaviv_pm_query_dom(struct etnaviv_gpu *gpu, > int etnaviv_pm_query_sig(struct etnaviv_gpu *gpu, > struct drm_etnaviv_pm_signal *signal) > { > - const struct etnaviv_pm_domain_meta *meta = &doms_meta[signal->pipe]; > + const unsigned int nr_domains = num_pm_domains(gpu); > const struct etnaviv_pm_domain *dom; > const struct etnaviv_pm_signal *sig; > > - if (signal->domain >= meta->nr_domains) > + if (signal->domain >= nr_domains) > return -EINVAL; > > - dom = meta->domains + signal->domain; > + dom = pm_domain(gpu, signal->domain); > > if (signal->iter >= dom->nr_signals) > return -EINVAL;
Hi Lucas, Am Mi., 26. Feb. 2020 um 16:19 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>: > > Hi Christian, > > sorry for taking so long to get around to this. > No problem... > On Mo, 2020-01-06 at 11:43 +0100, Christian Gmeiner wrote: > > Report the correct perfmon domains and signals depending > > on the supported feature flags. > > > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query perf counter") > > Cc: stable@vger.kernel.org > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > > --- > > drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 57 ++++++++++++++++++++--- > > 1 file changed, 50 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c > > index 8adbf2861bff..7ae8f347ca06 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c > > @@ -32,6 +32,7 @@ struct etnaviv_pm_domain { > > }; > > > > struct etnaviv_pm_domain_meta { > > + unsigned int feature; > > const struct etnaviv_pm_domain *domains; > > u32 nr_domains; > > }; > > @@ -410,36 +411,78 @@ static const struct etnaviv_pm_domain doms_vg[] = { > > > > static const struct etnaviv_pm_domain_meta doms_meta[] = { > > { > > + .feature = chipFeatures_PIPE_3D, > > .nr_domains = ARRAY_SIZE(doms_3d), > > .domains = &doms_3d[0] > > }, > > { > > + .feature = chipFeatures_PIPE_2D, > > .nr_domains = ARRAY_SIZE(doms_2d), > > .domains = &doms_2d[0] > > }, > > { > > + .feature = chipFeatures_PIPE_VG, > > .nr_domains = ARRAY_SIZE(doms_vg), > > .domains = &doms_vg[0] > > } > > }; > > > > +static unsigned int num_pm_domains(const struct etnaviv_gpu *gpu) > > +{ > > + unsigned int num = 0, i; > > + > > + for (i = 0; i < ARRAY_SIZE(doms_meta); i++) { > > + const struct etnaviv_pm_domain_meta *meta = &doms_meta[i]; > > + > > + if (gpu->identity.features & meta->feature) > > + num += meta->nr_domains; > > + } > > + > > + return num; > > +} > > + > > +static const struct etnaviv_pm_domain *pm_domain(const struct etnaviv_gpu *gpu, > > + unsigned int index) > > +{ > > + const struct etnaviv_pm_domain *domain = NULL; > > + unsigned int offset = 0, i; > > + > > + for (i = 0; i < ARRAY_SIZE(doms_meta); i++) { > > + const struct etnaviv_pm_domain_meta *meta = &doms_meta[i]; > > + > > + if (!(gpu->identity.features & meta->feature)) > > + continue; > > + > > + if (meta->nr_domains < (index - offset)) { > > + offset += meta->nr_domains; > > + continue; > > + } > > + > > + domain = meta->domains + (index - offset); > > + } > > + > > + BUG_ON(!domain); > > This is a no-go. BUG_ON is reserved for only the most severe kernel > bugs where you can't possibly continue without risking a corruption of > non-volatile state. This isn't the case here, please instead just make > the callers handle a NULL return gracefully. > Fixed it in V2.
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c index 8adbf2861bff..7ae8f347ca06 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c @@ -32,6 +32,7 @@ struct etnaviv_pm_domain { }; struct etnaviv_pm_domain_meta { + unsigned int feature; const struct etnaviv_pm_domain *domains; u32 nr_domains; }; @@ -410,36 +411,78 @@ static const struct etnaviv_pm_domain doms_vg[] = { static const struct etnaviv_pm_domain_meta doms_meta[] = { { + .feature = chipFeatures_PIPE_3D, .nr_domains = ARRAY_SIZE(doms_3d), .domains = &doms_3d[0] }, { + .feature = chipFeatures_PIPE_2D, .nr_domains = ARRAY_SIZE(doms_2d), .domains = &doms_2d[0] }, { + .feature = chipFeatures_PIPE_VG, .nr_domains = ARRAY_SIZE(doms_vg), .domains = &doms_vg[0] } }; +static unsigned int num_pm_domains(const struct etnaviv_gpu *gpu) +{ + unsigned int num = 0, i; + + for (i = 0; i < ARRAY_SIZE(doms_meta); i++) { + const struct etnaviv_pm_domain_meta *meta = &doms_meta[i]; + + if (gpu->identity.features & meta->feature) + num += meta->nr_domains; + } + + return num; +} + +static const struct etnaviv_pm_domain *pm_domain(const struct etnaviv_gpu *gpu, + unsigned int index) +{ + const struct etnaviv_pm_domain *domain = NULL; + unsigned int offset = 0, i; + + for (i = 0; i < ARRAY_SIZE(doms_meta); i++) { + const struct etnaviv_pm_domain_meta *meta = &doms_meta[i]; + + if (!(gpu->identity.features & meta->feature)) + continue; + + if (meta->nr_domains < (index - offset)) { + offset += meta->nr_domains; + continue; + } + + domain = meta->domains + (index - offset); + } + + BUG_ON(!domain); + + return domain; +} + int etnaviv_pm_query_dom(struct etnaviv_gpu *gpu, struct drm_etnaviv_pm_domain *domain) { - const struct etnaviv_pm_domain_meta *meta = &doms_meta[domain->pipe]; + const unsigned int nr_domains = num_pm_domains(gpu); const struct etnaviv_pm_domain *dom; - if (domain->iter >= meta->nr_domains) + if (domain->iter >= nr_domains) return -EINVAL; - dom = meta->domains + domain->iter; + dom = pm_domain(gpu, domain->iter); domain->id = domain->iter; domain->nr_signals = dom->nr_signals; strncpy(domain->name, dom->name, sizeof(domain->name)); domain->iter++; - if (domain->iter == meta->nr_domains) + if (domain->iter == nr_domains) domain->iter = 0xff; return 0; @@ -448,14 +491,14 @@ int etnaviv_pm_query_dom(struct etnaviv_gpu *gpu, int etnaviv_pm_query_sig(struct etnaviv_gpu *gpu, struct drm_etnaviv_pm_signal *signal) { - const struct etnaviv_pm_domain_meta *meta = &doms_meta[signal->pipe]; + const unsigned int nr_domains = num_pm_domains(gpu); const struct etnaviv_pm_domain *dom; const struct etnaviv_pm_signal *sig; - if (signal->domain >= meta->nr_domains) + if (signal->domain >= nr_domains) return -EINVAL; - dom = meta->domains + signal->domain; + dom = pm_domain(gpu, signal->domain); if (signal->iter >= dom->nr_signals) return -EINVAL;
Report the correct perfmon domains and signals depending on the supported feature flags. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query perf counter") Cc: stable@vger.kernel.org Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> --- drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 57 ++++++++++++++++++++--- 1 file changed, 50 insertions(+), 7 deletions(-)