| Message ID | 20231024224255.754779-2-bryan.odonoghue@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: qcom: camss: Introduce support for named power-domains | expand |
On 10/25/23 00:42, Bryan O'Donoghue wrote: > Right now we use the top-level camss structure to provide pointers via > VFE id index back to genpd linkages. > > In effect this hard-codes VFE indexes to power-domain indexes in the > dtsi and mandates a very particular ordering of power domains in the > dtsi, which bears no relationship to a real hardware dependency. > > As a first step to rationalising the VFE power-domain code and breaking > the magic indexing in dtsi use per-VFE pointers to genpd linkages. > > The top-level index in msm_vfe_subdev_init is still used to attain the > initial so no functional or logical change arises from this change. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- [...] > @@ -653,11 +653,11 @@ static int vfe_pm_domain_on(struct vfe_device *vfe) > if (id >= camss->res->vfe_num) > return 0; > > - camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id], > - DL_FLAG_STATELESS | > - DL_FLAG_PM_RUNTIME | > - DL_FLAG_RPM_ACTIVE); Good opportunity to inilne vfe->id and get rid of a local var! > - if (!camss->genpd_link[id]) > + vfe->genpd_link = device_link_add(camss->dev, vfe->genpd, > + DL_FLAG_STATELESS | > + DL_FLAG_PM_RUNTIME | > + DL_FLAG_RPM_ACTIVE); > + if (!vfe->genpd_link) > return -EINVAL; > [...] > /* > @@ -1128,10 +1124,10 @@ static int vfe_pm_domain_on(struct vfe_device *vfe) > struct camss *camss = vfe->camss; > enum vfe_line_id id = vfe->id; > > - camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id], DL_FLAG_STATELESS | > - DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); > + vfe->genpd_link = device_link_add(camss->dev, vfe->genpd, DL_FLAG_STATELESS | > + DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); > > - if (!camss->genpd_link[id]) { > + if (!vfe->genpd_link) { > dev_err(vfe->camss->dev, "Failed to add VFE#%d to power domain\n", id); > return -EINVAL; And here [...] > @@ -1113,10 +1111,10 @@ static int vfe_pm_domain_on(struct vfe_device *vfe) > struct camss *camss = vfe->camss; > enum vfe_line_id id = vfe->id; > > - camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id], DL_FLAG_STATELESS | > - DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); > + vfe->genpd_link = device_link_add(camss->dev, vfe->genpd, DL_FLAG_STATELESS | > + DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); > > - if (!camss->genpd_link[id]) { > + if (!vfe->genpd_link) { > dev_err(vfe->camss->dev, "Failed to add VFE#%d to power domain\n", id); > return -EINVAL; And here [...] > > /* > @@ -478,11 +478,11 @@ static int vfe_pm_domain_on(struct vfe_device *vfe) > if (id >= camss->res->vfe_num) > return 0; > > - camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id], > - DL_FLAG_STATELESS | > - DL_FLAG_PM_RUNTIME | > - DL_FLAG_RPM_ACTIVE); > - if (!camss->genpd_link[id]) > + vfe->genpd_link = device_link_add(camss->dev, vfe->genpd, > + DL_FLAG_STATELESS | > + DL_FLAG_PM_RUNTIME | > + DL_FLAG_RPM_ACTIVE); And here Konrad
On 25/10/2023 10:16, Konrad Dybcio wrote: >> @@ -653,11 +653,11 @@ static int vfe_pm_domain_on(struct vfe_device *vfe) >> if (id >= camss->res->vfe_num) >> return 0; >> - camss->genpd_link[id] = device_link_add(camss->dev, >> camss->genpd[id], >> - DL_FLAG_STATELESS | >> - DL_FLAG_PM_RUNTIME | >> - DL_FLAG_RPM_ACTIVE); > Good opportunity to inilne vfe->id and get rid of a local var! Yeah no objection to that, this is a progressive patchset so the index goes away in 1 or 2 patches later in the series anyway. --- bod
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-170.c b/drivers/media/platform/qcom/camss/camss-vfe-170.c index 0b211fed12760..59b0ea0aac48f 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe-170.c +++ b/drivers/media/platform/qcom/camss/camss-vfe-170.c @@ -638,7 +638,7 @@ static void vfe_pm_domain_off(struct vfe_device *vfe) if (vfe->id >= camss->res->vfe_num) return; - device_link_del(camss->genpd_link[vfe->id]); + device_link_del(vfe->genpd_link); } /* @@ -653,11 +653,11 @@ static int vfe_pm_domain_on(struct vfe_device *vfe) if (id >= camss->res->vfe_num) return 0; - camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id], - DL_FLAG_STATELESS | - DL_FLAG_PM_RUNTIME | - DL_FLAG_RPM_ACTIVE); - if (!camss->genpd_link[id]) + vfe->genpd_link = device_link_add(camss->dev, vfe->genpd, + DL_FLAG_STATELESS | + DL_FLAG_PM_RUNTIME | + DL_FLAG_RPM_ACTIVE); + if (!vfe->genpd_link) return -EINVAL; return 0; diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c index b65ed0fef595e..c668494ee1e98 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c @@ -1109,14 +1109,10 @@ static void vfe_isr_read(struct vfe_device *vfe, u32 *value0, u32 *value1) */ static void vfe_pm_domain_off(struct vfe_device *vfe) { - struct camss *camss; - if (!vfe) return; - camss = vfe->camss; - - device_link_del(camss->genpd_link[vfe->id]); + device_link_del(vfe->genpd_link); } /* @@ -1128,10 +1124,10 @@ static int vfe_pm_domain_on(struct vfe_device *vfe) struct camss *camss = vfe->camss; enum vfe_line_id id = vfe->id; - camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id], DL_FLAG_STATELESS | - DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); + vfe->genpd_link = device_link_add(camss->dev, vfe->genpd, DL_FLAG_STATELESS | + DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); - if (!camss->genpd_link[id]) { + if (!vfe->genpd_link) { dev_err(vfe->camss->dev, "Failed to add VFE#%d to power domain\n", id); return -EINVAL; } diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c index 7b3805177f037..5bd5b6b3c992a 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c @@ -1099,9 +1099,7 @@ static void vfe_isr_read(struct vfe_device *vfe, u32 *value0, u32 *value1) */ static void vfe_pm_domain_off(struct vfe_device *vfe) { - struct camss *camss = vfe->camss; - - device_link_del(camss->genpd_link[vfe->id]); + device_link_del(vfe->genpd_link); } /* @@ -1113,10 +1111,10 @@ static int vfe_pm_domain_on(struct vfe_device *vfe) struct camss *camss = vfe->camss; enum vfe_line_id id = vfe->id; - camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id], DL_FLAG_STATELESS | - DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); + vfe->genpd_link = device_link_add(camss->dev, vfe->genpd, DL_FLAG_STATELESS | + DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); - if (!camss->genpd_link[id]) { + if (!vfe->genpd_link) { dev_err(vfe->camss->dev, "Failed to add VFE#%d to power domain\n", id); return -EINVAL; } diff --git a/drivers/media/platform/qcom/camss/camss-vfe-480.c b/drivers/media/platform/qcom/camss/camss-vfe-480.c index f2368b77fc6d6..ca16a7ebb2903 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe-480.c +++ b/drivers/media/platform/qcom/camss/camss-vfe-480.c @@ -463,7 +463,7 @@ static void vfe_pm_domain_off(struct vfe_device *vfe) if (vfe->id >= camss->res->vfe_num) return; - device_link_del(camss->genpd_link[vfe->id]); + device_link_del(vfe->genpd_link); } /* @@ -478,11 +478,11 @@ static int vfe_pm_domain_on(struct vfe_device *vfe) if (id >= camss->res->vfe_num) return 0; - camss->genpd_link[id] = device_link_add(camss->dev, camss->genpd[id], - DL_FLAG_STATELESS | - DL_FLAG_PM_RUNTIME | - DL_FLAG_RPM_ACTIVE); - if (!camss->genpd_link[id]) + vfe->genpd_link = device_link_add(camss->dev, vfe->genpd, + DL_FLAG_STATELESS | + DL_FLAG_PM_RUNTIME | + DL_FLAG_RPM_ACTIVE); + if (!vfe->genpd_link) return -EINVAL; return 0; diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c index 4839e2cedfe58..3d31f4289b724 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe.c +++ b/drivers/media/platform/qcom/camss/camss-vfe.c @@ -1347,6 +1347,9 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe, if (!res->line_num) return -EINVAL; + if (camss->genpd) + vfe->genpd = camss->genpd[id]; + vfe->line_num = res->line_num; vfe->ops->subdev_init(dev, vfe); diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h index 09baded0dcdd6..c1c50023d4876 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe.h +++ b/drivers/media/platform/qcom/camss/camss-vfe.h @@ -150,6 +150,8 @@ struct vfe_device { const struct vfe_hw_ops_gen1 *ops_gen1; struct vfe_isr_ops isr_ops; struct camss_video_ops video_ops; + struct device *genpd; + struct device_link *genpd_link; }; struct camss_subdev_resources;
Right now we use the top-level camss structure to provide pointers via VFE id index back to genpd linkages. In effect this hard-codes VFE indexes to power-domain indexes in the dtsi and mandates a very particular ordering of power domains in the dtsi, which bears no relationship to a real hardware dependency. As a first step to rationalising the VFE power-domain code and breaking the magic indexing in dtsi use per-VFE pointers to genpd linkages. The top-level index in msm_vfe_subdev_init is still used to attain the initial so no functional or logical change arises from this change. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/media/platform/qcom/camss/camss-vfe-170.c | 12 ++++++------ drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 12 ++++-------- drivers/media/platform/qcom/camss/camss-vfe-4-8.c | 10 ++++------ drivers/media/platform/qcom/camss/camss-vfe-480.c | 12 ++++++------ drivers/media/platform/qcom/camss/camss-vfe.c | 3 +++ drivers/media/platform/qcom/camss/camss-vfe.h | 2 ++ 6 files changed, 25 insertions(+), 26 deletions(-)