| Message ID | 20221205174433.16847-1-quic_bjorande@quicinc.com |
|---|---|
| Headers | show |
| Series | drm/msm: Add SC8280XP support | expand |
On 05/12/2022 18:44, Bjorn Andersson wrote: > From: Bjorn Andersson <bjorn.andersson@linaro.org> > > The SA8295P ADP has, among other interfaces, six MiniDP connectors which > are connected to MDSS0 DP2 and DP3, and MDSS1 DP0 through DP3. > > Enable Display Clock controllers, MDSS instanced, MDPs, DP controllers, > DP PHYs and link them all together. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > --- > > Changes since v3: > - None > > arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 243 ++++++++++++++++++++++- > 1 file changed, 241 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts > index 6c29d7d757e0..d55c8c5304cc 100644 > --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts > +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts > @@ -23,6 +23,90 @@ aliases { > chosen { > stdout-path = "serial0:115200n8"; > }; > + > + dp2-connector { > + compatible = "dp-connector"; > + label = "DP2"; > + type = "mini"; > + > + hpd-gpios = <&tlmm 20 GPIO_ACTIVE_HIGH>; > + > + port { > + dp2_connector_in: endpoint { > + remote-endpoint = <&mdss1_dp0_phy_out>; > + }; > + }; > + }; > + > + dp3-connector { > + compatible = "dp-connector"; > + label = "DP3"; > + type = "mini"; > + > + hpd-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>; > + > + port { > + dp3_connector_in: endpoint { > + remote-endpoint = <&mdss1_dp1_phy_out>; > + }; > + }; > + }; > + > + edp0-connector { > + compatible = "dp-connector"; > + label = "EDP0"; > + type = "mini"; > + > + hpd-gpios = <&tlmm 2 GPIO_ACTIVE_HIGH>; > + > + port { > + edp0_connector_in: endpoint { > + remote-endpoint = <&mdss0_dp2_phy_out>; > + }; > + }; > + }; > + > + edp1-connector { > + compatible = "dp-connector"; > + label = "EDP1"; > + type = "mini"; > + > + hpd-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>; > + > + port { > + edp1_connector_in: endpoint { > + remote-endpoint = <&mdss0_dp3_phy_out>; > + }; > + }; > + }; > + > + edp2-connector { > + compatible = "dp-connector"; > + label = "EDP2"; > + type = "mini"; > + > + hpd-gpios = <&tlmm 7 GPIO_ACTIVE_HIGH>; > + > + port { > + edp2_connector_in: endpoint { > + remote-endpoint = <&mdss1_dp2_phy_out>; > + }; > + }; > + }; > + > + edp3-connector { > + compatible = "dp-connector"; > + label = "EDP3"; > + type = "mini"; > + > + hpd-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>; > + > + port { > + edp3_connector_in: endpoint { > + remote-endpoint = <&mdss1_dp3_phy_out>; > + }; > + }; > + }; > }; > > &apps_rsc { > @@ -163,13 +247,168 @@ vreg_l7g: ldo7 { > > vreg_l8g: ldo8 { > regulator-name = "vreg_l8g"; > - regulator-min-microvolt = <880000>; > - regulator-max-microvolt = <880000>; > + regulator-min-microvolt = <912000>; > + regulator-max-microvolt = <912000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + }; > + > + vreg_l11g: ldo11 { > + regulator-name = "vreg_l11g"; > + regulator-min-microvolt = <912000>; > + regulator-max-microvolt = <912000>; > regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > }; > }; > }; > > +&dispcc0 { > + status = "okay"; > +}; > + > +&dispcc1 { > + status = "okay"; > +}; > + > +&mdss0 { > + status = "okay"; > +}; > + > +&mdss0_dp2 { > + status = "okay"; status should go last. > + > + data-lanes = <0 1 2 3>; > + > + ports { > + port@1 { > + reg = <1>; > + mdss0_dp2_phy_out: endpoint { That's quite a lot of indentation.. couldn't these endpoints be defined in the SoC DT? Konrad > + remote-endpoint = <&edp0_connector_in>; > + }; > + }; > + }; > +}; > + > +&mdss0_dp2_phy { > + status = "okay"; > + > + vdda-phy-supply = <&vreg_l8g>; > + vdda-pll-supply = <&vreg_l3g>; > +}; > + > +&mdss0_dp3 { > + status = "okay"; > + > + data-lanes = <0 1 2 3>; > + > + ports { > + port@1 { > + reg = <1>; > + mdss0_dp3_phy_out: endpoint { > + remote-endpoint = <&edp1_connector_in>; > + }; > + }; > + }; > +}; > + > +&mdss0_dp3_phy { > + status = "okay"; > + > + vdda-phy-supply = <&vreg_l8g>; > + vdda-pll-supply = <&vreg_l3g>; > +}; > + > +&mdss1 { > + status = "okay"; > +}; > + > +&mdss1_dp0 { > + status = "okay"; > + > + data-lanes = <0 1 2 3>; > + > + ports { > + port@1 { > + reg = <1>; > + mdss1_dp0_phy_out: endpoint { > + remote-endpoint = <&dp2_connector_in>; > + }; > + }; > + }; > +}; > + > +&mdss1_dp0_phy { > + status = "okay"; > + > + vdda-phy-supply = <&vreg_l11g>; > + vdda-pll-supply = <&vreg_l3g>; > +}; > + > +&mdss1_dp1 { > + status = "okay"; > + > + data-lanes = <0 1 2 3>; > + > + ports { > + port@1 { > + reg = <1>; > + mdss1_dp1_phy_out: endpoint { > + remote-endpoint = <&dp3_connector_in>; > + }; > + }; > + }; > +}; > + > +&mdss1_dp1_phy { > + status = "okay"; > + > + vdda-phy-supply = <&vreg_l11g>; > + vdda-pll-supply = <&vreg_l3g>; > +}; > + > +&mdss1_dp2 { > + status = "okay"; > + > + data-lanes = <0 1 2 3>; > + > + ports { > + port@1 { > + reg = <1>; > + mdss1_dp2_phy_out: endpoint { > + remote-endpoint = <&edp2_connector_in>; > + }; > + }; > + }; > +}; > + > +&mdss1_dp2_phy { > + status = "okay"; > + > + vdda-phy-supply = <&vreg_l11g>; > + vdda-pll-supply = <&vreg_l3g>; > +}; > + > +&mdss1_dp3 { > + status = "okay"; > + > + data-lanes = <0 1 2 3>; > + > + ports { > + port@1 { > + reg = <1>; > + mdss1_dp3_phy_out: endpoint { > + remote-endpoint = <&edp3_connector_in>; > + }; > + }; > + }; > +}; > + > +&mdss1_dp3_phy { > + status = "okay"; > + > + vdda-phy-supply = <&vreg_l11g>; > + vdda-pll-supply = <&vreg_l3g>; > +}; > + > &pcie2a { > perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>; > wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
On Mon, Dec 05, 2022 at 07:09:45PM +0100, Konrad Dybcio wrote: > On 05/12/2022 18:44, Bjorn Andersson wrote: > > diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts [..] > > +&mdss0_dp2 { > > + status = "okay"; > status should go last. > Thanks for pointing that out. Would be nice if the computer told me that...somehow... > > + > > + data-lanes = <0 1 2 3>; > > + > > + ports { > > + port@1 { > > + reg = <1>; > > + mdss0_dp2_phy_out: endpoint { > That's quite a lot of indentation.. couldn't these endpoints be defined in > the SoC DT? > The alternative would be to have the description of each DP controller split over multiple nodes and rely on the reader to stitch together the view of the node based on the label. Based on the naming of these labels they would at least be adjacent, so it wouldn't be that bad. But I feel that there is enough DP-controller nodes in this board already; I've yet to describe the two USB Type-C controllers or the two DSI-DP bridges. So I don't know if it's worth optimizing indentation-level within each node like this. And we will end up mixing this optimization between DP controllers, USB Type-C nodes, QMP nodes, DSI-DP bridges. Regards, Bjorn
On 05/12/2022 21:02, Bjorn Andersson wrote: > On Mon, Dec 05, 2022 at 07:09:45PM +0100, Konrad Dybcio wrote: >> On 05/12/2022 18:44, Bjorn Andersson wrote: >>> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts > [..] >>> +&mdss0_dp2 { >>> + status = "okay"; >> status should go last. >> > > Thanks for pointing that out. Would be nice if the computer told me > that...somehow... > >>> + >>> + data-lanes = <0 1 2 3>; >>> + >>> + ports { >>> + port@1 { >>> + reg = <1>; >>> + mdss0_dp2_phy_out: endpoint { >> That's quite a lot of indentation.. couldn't these endpoints be defined in >> the SoC DT? >> > > The alternative would be to have the description of each DP controller > split over multiple nodes and rely on the reader to stitch together the > view of the node based on the label. > Based on the naming of these labels they would at least be adjacent, so > it wouldn't be that bad. > > But I feel that there is enough DP-controller nodes in this board > already; I've yet to describe the two USB Type-C controllers or the two > DSI-DP bridges. > So I don't know if it's worth optimizing indentation-level within each > node like this. > > > And we will end up mixing this optimization between DP controllers, USB > Type-C nodes, QMP nodes, DSI-DP bridges. Oh okay, I see, let's keep it as-is then. Konrad > > Regards, > Bjorn
On 5 December 2022 20:44:23 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: >From: Bjorn Andersson <bjorn.andersson@linaro.org> > >Add compatible for the SC8280XP Mobile Display Subsystem and >initialization for version 8.0.0. > >Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >---
On 5 December 2022 20:44:28 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: >From: Bjorn Andersson <bjorn.andersson@linaro.org> > >The DisplayPort controller's hot-plug mechanism is based on pinmuxing a >physical signal on a GPIO pin into the controller. This is not always >possible, either because there aren't dedicated GPIOs available or >because the hot-plug signal is a virtual notification, in cases such as >USB Type-C. > >For these cases, by implementing the hpd_notify() callback for the >DisplayPort controller's drm_bridge, a downstream drm_bridge >(next_bridge) can be used to track and signal the connection status >changes. > >This makes it possible to use downstream drm_bridges such as >display-connector or any virtual mechanism, as long as they are >implemented as a drm_bridge. > >Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >[bjorn: Drop connector->fwnode assignment and dev from struct msm_dp] >Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> >--- > >Changes since v3: >- None > > drivers/gpu/drm/msm/dp/dp_display.c | 22 ++++++++++++++++++++++ > drivers/gpu/drm/msm/dp/dp_drm.c | 1 + > drivers/gpu/drm/msm/dp/dp_drm.h | 2 ++ > 3 files changed, 25 insertions(+) > >diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >index 666b45c8ab80..17fcf8cd84cd 100644 >--- a/drivers/gpu/drm/msm/dp/dp_display.c >+++ b/drivers/gpu/drm/msm/dp/dp_display.c >@@ -1772,3 +1772,25 @@ void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > dp_display->dp_mode.h_active_low = > !!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC); > } >+ >+void dp_bridge_hpd_notify(struct drm_bridge *bridge, >+ enum drm_connector_status status) >+{ >+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(bridge); >+ struct msm_dp *dp_display = dp_bridge->dp_display; >+ struct dp_display_private *dp = container_of(dp_display, struct dp_display_private, dp_display); >+ >+ /* Without next_bridge interrupts are handled by the DP core directly */ >+ if (!dp_display->next_bridge) >+ return; Can we use hpd_notify in all the cases by dropping the corresponding piece of code from the core driver? >+ >+ if (!dp->core_initialized) { >+ drm_dbg_dp(dp->drm_dev, "not initialized\n"); >+ return; >+ } >+ >+ if (!dp_display->is_connected && status == connector_status_connected) >+ dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0); >+ else if (dp_display->is_connected && status == connector_status_disconnected) >+ dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0); >+} >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c >index 6db82f9b03af..3898366ebd5e 100644 >--- a/drivers/gpu/drm/msm/dp/dp_drm.c >+++ b/drivers/gpu/drm/msm/dp/dp_drm.c >@@ -102,6 +102,7 @@ static const struct drm_bridge_funcs dp_bridge_ops = { > .get_modes = dp_bridge_get_modes, > .detect = dp_bridge_detect, > .atomic_check = dp_bridge_atomic_check, >+ .hpd_notify = dp_bridge_hpd_notify, > }; > > struct drm_bridge *dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev, >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h >index 82035dbb0578..79e6b2cf2d25 100644 >--- a/drivers/gpu/drm/msm/dp/dp_drm.h >+++ b/drivers/gpu/drm/msm/dp/dp_drm.h >@@ -32,5 +32,7 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, > void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > const struct drm_display_mode *mode, > const struct drm_display_mode *adjusted_mode); >+void dp_bridge_hpd_notify(struct drm_bridge *bridge, >+ enum drm_connector_status status); > > #endif /* _DP_DRM_H_ */
On 5 December 2022 20:44:29 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: >From: Bjorn Andersson <bjorn.andersson@linaro.org> > >Most instances where HPD interrupts are masked and unmasked are guareded >by the presence of an EDP panel being connected, but not all. Extend >this to cover the last few places, as HPD interrupt handling is not used >for the EDP case. I don't remember whether I asked that or not. Would it be possible to move hpd irq enablement to bridge's hpd_enable() / hpd_disable() callbacks ? I think this would allow us to drop the is_edp checks. > >Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> >--- > >Changes since v3: >- None > > drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > >diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >index 17fcf8cd84cd..bb92c33beff8 100644 >--- a/drivers/gpu/drm/msm/dp/dp_display.c >+++ b/drivers/gpu/drm/msm/dp/dp_display.c >@@ -610,8 +610,10 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data) > } > > /* enable HDP irq_hpd/replug interrupt */ >- dp_catalog_hpd_config_intr(dp->catalog, >- DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, true); >+ if (!dp->dp_display.is_edp) >+ dp_catalog_hpd_config_intr(dp->catalog, >+ DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, >+ true); > > drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n", > dp->dp_display.connector_type, state); >@@ -651,8 +653,10 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) > dp->dp_display.connector_type, state); > > /* disable irq_hpd/replug interrupts */ >- dp_catalog_hpd_config_intr(dp->catalog, >- DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, false); >+ if (!dp->dp_display.is_edp) >+ dp_catalog_hpd_config_intr(dp->catalog, >+ DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, >+ false); > > /* unplugged, no more irq_hpd handle */ > dp_del_event(dp, EV_IRQ_HPD_INT); >@@ -678,7 +682,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) > } > > /* disable HPD plug interrupts */ >- dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false); >+ if (!dp->dp_display.is_edp) >+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false); > > /* > * We don't need separate work for disconnect as
On 5 December 2022 20:44:30 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: >From: Bjorn Andersson <bjorn.andersson@linaro.org> > >The DisplayPort controller's internal HPD interrupt handling is used for >cases where the HPD signal is connected to a GPIO which is pinmuxed into >the DisplayPort controller. In other configurations the HPD notification >might be delivered by the DRM framework from an associated bridge. > >This difference is not appropriately represented by the "is_edp" >boolean, but is properly represented by the frameworks invocation of the >hpd_enable() and hpd_disable() callbacks. Switch the current condition >to rely on these callbacks instead. > >This ensures appropriate handling of the three cases; no bridge >connected, a bridge without DRM_BRIDGE_OP_HPD and a bridge with >DRM_BRIDGE_OP_HPD. > >Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> >--- > >Worth mentioning, I did look into moving the HPD enablement/disablement >completely into these new callbacks, but that affect the entire power >management model of the driver, so I think it's worth to tackle that in >subsequent changes. It seems also reasonable to expect that we by such >modifications could leave the block unclocked until the external HPD >notification arrives... I see... I still suppose this is the way to go in the long term. For now: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >Changes since v3: >- Introduced reliance on hpd_enable/disable callbacks instead of next_bridge > > drivers/gpu/drm/msm/dp/dp_display.c | 35 ++++++++++++++++++++--------- > drivers/gpu/drm/msm/dp/dp_display.h | 1 + > drivers/gpu/drm/msm/dp/dp_drm.c | 2 ++ > drivers/gpu/drm/msm/dp/dp_drm.h | 2 ++ > 4 files changed, 30 insertions(+), 10 deletions(-) > >diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >index bb92c33beff8..3e464c33ff11 100644 >--- a/drivers/gpu/drm/msm/dp/dp_display.c >+++ b/drivers/gpu/drm/msm/dp/dp_display.c >@@ -610,7 +610,7 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data) > } > > /* enable HDP irq_hpd/replug interrupt */ >- if (!dp->dp_display.is_edp) >+ if (dp->dp_display.internal_hpd) > dp_catalog_hpd_config_intr(dp->catalog, > DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, > true); >@@ -653,7 +653,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) > dp->dp_display.connector_type, state); > > /* disable irq_hpd/replug interrupts */ >- if (!dp->dp_display.is_edp) >+ if (dp->dp_display.internal_hpd) > dp_catalog_hpd_config_intr(dp->catalog, > DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, > false); >@@ -682,7 +682,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) > } > > /* disable HPD plug interrupts */ >- if (!dp->dp_display.is_edp) >+ if (dp->dp_display.internal_hpd) > dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false); > > /* >@@ -701,7 +701,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) > dp_display_handle_plugged_change(&dp->dp_display, false); > > /* enable HDP plug interrupt to prepare for next plugin */ >- if (!dp->dp_display.is_edp) >+ if (dp->dp_display.internal_hpd) > dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, true); > > drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n", >@@ -1086,8 +1086,8 @@ static void dp_display_config_hpd(struct dp_display_private *dp) > dp_display_host_init(dp); > dp_catalog_ctrl_hpd_config(dp->catalog); > >- /* Enable plug and unplug interrupts only for external DisplayPort */ >- if (!dp->dp_display.is_edp) >+ /* Enable plug and unplug interrupts only if requested */ >+ if (dp->dp_display.internal_hpd) > dp_catalog_hpd_config_intr(dp->catalog, > DP_DP_HPD_PLUG_INT_MASK | > DP_DP_HPD_UNPLUG_INT_MASK, >@@ -1379,8 +1379,7 @@ static int dp_pm_resume(struct device *dev) > > dp_catalog_ctrl_hpd_config(dp->catalog); > >- >- if (!dp->dp_display.is_edp) >+ if (dp->dp_display.internal_hpd) > dp_catalog_hpd_config_intr(dp->catalog, > DP_DP_HPD_PLUG_INT_MASK | > DP_DP_HPD_UNPLUG_INT_MASK, >@@ -1778,6 +1777,22 @@ void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > !!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC); > } > >+void dp_bridge_hpd_enable(struct drm_bridge *bridge) >+{ >+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(bridge); >+ struct msm_dp *dp_display = dp_bridge->dp_display; >+ >+ dp_display->internal_hpd = true; >+} >+ >+void dp_bridge_hpd_disable(struct drm_bridge *bridge) >+{ >+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(bridge); >+ struct msm_dp *dp_display = dp_bridge->dp_display; >+ >+ dp_display->internal_hpd = false; >+} >+ > void dp_bridge_hpd_notify(struct drm_bridge *bridge, > enum drm_connector_status status) > { >@@ -1785,8 +1800,8 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge, > struct msm_dp *dp_display = dp_bridge->dp_display; > struct dp_display_private *dp = container_of(dp_display, struct dp_display_private, dp_display); > >- /* Without next_bridge interrupts are handled by the DP core directly */ >- if (!dp_display->next_bridge) >+ /* Plug events are generated by the dp_display_irq_handler() */ >+ if (dp_display->internal_hpd) > return; > > if (!dp->core_initialized) { >diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h >index dcedf021f7fe..371337d0fae2 100644 >--- a/drivers/gpu/drm/msm/dp/dp_display.h >+++ b/drivers/gpu/drm/msm/dp/dp_display.h >@@ -21,6 +21,7 @@ struct msm_dp { > bool power_on; > unsigned int connector_type; > bool is_edp; >+ bool internal_hpd; > > hdmi_codec_plugged_cb plugged_cb; > >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c >index 3898366ebd5e..275370f21115 100644 >--- a/drivers/gpu/drm/msm/dp/dp_drm.c >+++ b/drivers/gpu/drm/msm/dp/dp_drm.c >@@ -102,6 +102,8 @@ static const struct drm_bridge_funcs dp_bridge_ops = { > .get_modes = dp_bridge_get_modes, > .detect = dp_bridge_detect, > .atomic_check = dp_bridge_atomic_check, >+ .hpd_enable = dp_bridge_hpd_enable, >+ .hpd_disable = dp_bridge_hpd_disable, > .hpd_notify = dp_bridge_hpd_notify, > }; > >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h >index 79e6b2cf2d25..250f7c66201f 100644 >--- a/drivers/gpu/drm/msm/dp/dp_drm.h >+++ b/drivers/gpu/drm/msm/dp/dp_drm.h >@@ -32,6 +32,8 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, > void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > const struct drm_display_mode *mode, > const struct drm_display_mode *adjusted_mode); >+void dp_bridge_hpd_enable(struct drm_bridge *bridge); >+void dp_bridge_hpd_disable(struct drm_bridge *bridge); > void dp_bridge_hpd_notify(struct drm_bridge *bridge, > enum drm_connector_status status); >
On 6 December 2022 00:07:12 GMT+03:00, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > >On 5 December 2022 20:44:29 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: >>From: Bjorn Andersson <bjorn.andersson@linaro.org> >> >>Most instances where HPD interrupts are masked and unmasked are guareded >>by the presence of an EDP panel being connected, but not all. Extend >>this to cover the last few places, as HPD interrupt handling is not used >>for the EDP case. > >I don't remember whether I asked that or not. Would it be possible to move hpd irq enablement to bridge's hpd_enable() / hpd_disable() callbacks ? I think this would allow us to drop the is_edp checks. Ignore this. I should read the series carefully. > >> >>Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >>Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> >>--- >> >>Changes since v3: >>- None >> >> drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >>diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >>index 17fcf8cd84cd..bb92c33beff8 100644 >>--- a/drivers/gpu/drm/msm/dp/dp_display.c >>+++ b/drivers/gpu/drm/msm/dp/dp_display.c >>@@ -610,8 +610,10 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data) >> } >> >> /* enable HDP irq_hpd/replug interrupt */ >>- dp_catalog_hpd_config_intr(dp->catalog, >>- DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, true); >>+ if (!dp->dp_display.is_edp) >>+ dp_catalog_hpd_config_intr(dp->catalog, >>+ DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, >>+ true); >> >> drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n", >> dp->dp_display.connector_type, state); >>@@ -651,8 +653,10 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) >> dp->dp_display.connector_type, state); >> >> /* disable irq_hpd/replug interrupts */ >>- dp_catalog_hpd_config_intr(dp->catalog, >>- DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, false); >>+ if (!dp->dp_display.is_edp) >>+ dp_catalog_hpd_config_intr(dp->catalog, >>+ DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, >>+ false); >> >> /* unplugged, no more irq_hpd handle */ >> dp_del_event(dp, EV_IRQ_HPD_INT); >>@@ -678,7 +682,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) >> } >> >> /* disable HPD plug interrupts */ >>- dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false); >>+ if (!dp->dp_display.is_edp) >>+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false); >> >> /* >> * We don't need separate work for disconnect as >
On 5 December 2022 20:44:32 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: >From: Bjorn Andersson <bjorn.andersson@linaro.org> > >The SC8280XP CRD has a EDP display on MDSS0 DP3, enable relevant nodes >and link it together with the backlight control. > >Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> >--- > >Changes since v3: >- Added description of the regulator that powers the panel. > > arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 72 ++++++++++++++++++++++- > 1 file changed, 71 insertions(+), 1 deletion(-) > >diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts >index f09810e3d956..a7d2384cbbe8 100644 >--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts >+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts >@@ -20,7 +20,7 @@ aliases { > serial0 = &qup2_uart17; > }; > >- backlight { >+ backlight: backlight { > compatible = "pwm-backlight"; > pwms = <&pmc8280c_lpg 3 1000000>; > enable-gpios = <&pmc8280_1_gpios 8 GPIO_ACTIVE_HIGH>; >@@ -34,6 +34,22 @@ chosen { > stdout-path = "serial0:115200n8"; > }; > >+ vreg_edp_3p3: regulator-edp-3p3 { >+ compatible = "regulator-fixed"; >+ >+ regulator-name = "VREG_EDP_3P3"; >+ regulator-min-microvolt = <3300000>; >+ regulator-max-microvolt = <3300000>; >+ >+ gpio = <&tlmm 25 GPIO_ACTIVE_HIGH>; >+ enable-active-high; >+ >+ pinctrl-names = "default"; >+ pinctrl-0 = <&edp_reg_en>; >+ >+ regulator-boot-on; >+ }; >+ > vreg_edp_bl: regulator-edp-bl { > compatible = "regulator-fixed"; > >@@ -230,6 +246,54 @@ vreg_l9d: ldo9 { > }; > }; > >+&dispcc0 { >+ status = "okay"; >+}; >+ >+&mdss0 { >+ status = "okay"; >+}; >+ >+&mdss0_dp3 { >+ compatible = "qcom,sc8280xp-edp"; >+ status = "okay"; >+ >+ data-lanes = <0 1 2 3>; >+ >+ aux-bus { >+ panel { >+ compatible = "edp-panel"; >+ power-supply = <&vreg_edp_3p3>; >+ >+ backlight = <&backlight>; >+ >+ ports { >+ port { >+ edp_panel_in: endpoint { >+ remote-endpoint = <&mdss0_dp3_out>; >+ }; >+ }; >+ }; >+ }; >+ }; >+ >+ ports { >+ port@1 { >+ reg = <1>; You already have reg assignment in the SoC dtsi. >+ mdss0_dp3_out: endpoint { >+ remote-endpoint = <&edp_panel_in>; >+ }; >+ }; >+ }; >+}; >+ >+&mdss0_dp3_phy { >+ status = "okay"; >+ >+ vdda-phy-supply = <&vreg_l6b>; >+ vdda-pll-supply = <&vreg_l3b>; >+}; >+ > &pcie2a { > perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>; > wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>; >@@ -496,6 +560,12 @@ hastings_reg_en: hastings-reg-en-state { > &tlmm { > gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>; > >+ edp_reg_en: edp-reg-en-state { >+ pins = "gpio25"; >+ function = "gpio"; >+ output-enable; >+ }; >+ > kybd_default: kybd-default-state { > disable-pins { > pins = "gpio102";
On 5 December 2022 20:44:28 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: >From: Bjorn Andersson <bjorn.andersson@linaro.org> > >The DisplayPort controller's hot-plug mechanism is based on pinmuxing a >physical signal on a GPIO pin into the controller. This is not always >possible, either because there aren't dedicated GPIOs available or >because the hot-plug signal is a virtual notification, in cases such as >USB Type-C. > >For these cases, by implementing the hpd_notify() callback for the >DisplayPort controller's drm_bridge, a downstream drm_bridge >(next_bridge) can be used to track and signal the connection status >changes. > >This makes it possible to use downstream drm_bridges such as >display-connector or any virtual mechanism, as long as they are >implemented as a drm_bridge. > >Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >[bjorn: Drop connector->fwnode assignment and dev from struct msm_dp] >Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Minor nit: if for the next revision you reorder the patches to have hpd_enable first, then missing conditions, then this patch, it will look more logical. >--- > >Changes since v3: >- None > > drivers/gpu/drm/msm/dp/dp_display.c | 22 ++++++++++++++++++++++ > drivers/gpu/drm/msm/dp/dp_drm.c | 1 + > drivers/gpu/drm/msm/dp/dp_drm.h | 2 ++ > 3 files changed, 25 insertions(+) > >diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >index 666b45c8ab80..17fcf8cd84cd 100644 >--- a/drivers/gpu/drm/msm/dp/dp_display.c >+++ b/drivers/gpu/drm/msm/dp/dp_display.c >@@ -1772,3 +1772,25 @@ void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > dp_display->dp_mode.h_active_low = > !!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC); > } >+ >+void dp_bridge_hpd_notify(struct drm_bridge *bridge, >+ enum drm_connector_status status) >+{ >+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(bridge); >+ struct msm_dp *dp_display = dp_bridge->dp_display; >+ struct dp_display_private *dp = container_of(dp_display, struct dp_display_private, dp_display); >+ >+ /* Without next_bridge interrupts are handled by the DP core directly */ >+ if (!dp_display->next_bridge) >+ return; >+ >+ if (!dp->core_initialized) { >+ drm_dbg_dp(dp->drm_dev, "not initialized\n"); >+ return; >+ } >+ >+ if (!dp_display->is_connected && status == connector_status_connected) >+ dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0); >+ else if (dp_display->is_connected && status == connector_status_disconnected) >+ dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0); >+} >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c >index 6db82f9b03af..3898366ebd5e 100644 >--- a/drivers/gpu/drm/msm/dp/dp_drm.c >+++ b/drivers/gpu/drm/msm/dp/dp_drm.c >@@ -102,6 +102,7 @@ static const struct drm_bridge_funcs dp_bridge_ops = { > .get_modes = dp_bridge_get_modes, > .detect = dp_bridge_detect, > .atomic_check = dp_bridge_atomic_check, >+ .hpd_notify = dp_bridge_hpd_notify, > }; > > struct drm_bridge *dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev, >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h >index 82035dbb0578..79e6b2cf2d25 100644 >--- a/drivers/gpu/drm/msm/dp/dp_drm.h >+++ b/drivers/gpu/drm/msm/dp/dp_drm.h >@@ -32,5 +32,7 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, > void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > const struct drm_display_mode *mode, > const struct drm_display_mode *adjusted_mode); >+void dp_bridge_hpd_notify(struct drm_bridge *bridge, >+ enum drm_connector_status status); > > #endif /* _DP_DRM_H_ */
On Tue, Dec 06, 2022 at 12:29:13AM +0300, Dmitry Baryshkov wrote: > > > On 5 December 2022 20:44:28 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: > >From: Bjorn Andersson <bjorn.andersson@linaro.org> > > > >The DisplayPort controller's hot-plug mechanism is based on pinmuxing a > >physical signal on a GPIO pin into the controller. This is not always > >possible, either because there aren't dedicated GPIOs available or > >because the hot-plug signal is a virtual notification, in cases such as > >USB Type-C. > > > >For these cases, by implementing the hpd_notify() callback for the > >DisplayPort controller's drm_bridge, a downstream drm_bridge > >(next_bridge) can be used to track and signal the connection status > >changes. > > > >This makes it possible to use downstream drm_bridges such as > >display-connector or any virtual mechanism, as long as they are > >implemented as a drm_bridge. > > > >Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > >[bjorn: Drop connector->fwnode assignment and dev from struct msm_dp] > >Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Minor nit: if for the next revision you reorder the patches to have > hpd_enable first, then missing conditions, then this patch, it will > look more logical. You're right, that will look better. I'll do so. Thanks, Bjorn > > >--- > > > >Changes since v3: > >- None > > > > drivers/gpu/drm/msm/dp/dp_display.c | 22 ++++++++++++++++++++++ > > drivers/gpu/drm/msm/dp/dp_drm.c | 1 + > > drivers/gpu/drm/msm/dp/dp_drm.h | 2 ++ > > 3 files changed, 25 insertions(+) > > > >diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > >index 666b45c8ab80..17fcf8cd84cd 100644 > >--- a/drivers/gpu/drm/msm/dp/dp_display.c > >+++ b/drivers/gpu/drm/msm/dp/dp_display.c > >@@ -1772,3 +1772,25 @@ void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > > dp_display->dp_mode.h_active_low = > > !!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC); > > } > >+ > >+void dp_bridge_hpd_notify(struct drm_bridge *bridge, > >+ enum drm_connector_status status) > >+{ > >+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(bridge); > >+ struct msm_dp *dp_display = dp_bridge->dp_display; > >+ struct dp_display_private *dp = container_of(dp_display, struct dp_display_private, dp_display); > >+ > >+ /* Without next_bridge interrupts are handled by the DP core directly */ > >+ if (!dp_display->next_bridge) > >+ return; > >+ > >+ if (!dp->core_initialized) { > >+ drm_dbg_dp(dp->drm_dev, "not initialized\n"); > >+ return; > >+ } > >+ > >+ if (!dp_display->is_connected && status == connector_status_connected) > >+ dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0); > >+ else if (dp_display->is_connected && status == connector_status_disconnected) > >+ dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0); > >+} > >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c > >index 6db82f9b03af..3898366ebd5e 100644 > >--- a/drivers/gpu/drm/msm/dp/dp_drm.c > >+++ b/drivers/gpu/drm/msm/dp/dp_drm.c > >@@ -102,6 +102,7 @@ static const struct drm_bridge_funcs dp_bridge_ops = { > > .get_modes = dp_bridge_get_modes, > > .detect = dp_bridge_detect, > > .atomic_check = dp_bridge_atomic_check, > >+ .hpd_notify = dp_bridge_hpd_notify, > > }; > > > > struct drm_bridge *dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev, > >diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h > >index 82035dbb0578..79e6b2cf2d25 100644 > >--- a/drivers/gpu/drm/msm/dp/dp_drm.h > >+++ b/drivers/gpu/drm/msm/dp/dp_drm.h > >@@ -32,5 +32,7 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, > > void dp_bridge_mode_set(struct drm_bridge *drm_bridge, > > const struct drm_display_mode *mode, > > const struct drm_display_mode *adjusted_mode); > >+void dp_bridge_hpd_notify(struct drm_bridge *bridge, > >+ enum drm_connector_status status); > > > > #endif /* _DP_DRM_H_ */ > > -- > With best wishes > Dmitry
On 05/12/2022 19:44, Bjorn Andersson wrote: > From: Bjorn Andersson <bjorn.andersson@linaro.org> > > The Qualcomm SC8280XP platform contains DPU version 8.0.0, has 9 > interfaces, 2 DSI controllers and 4 DisplayPort controllers. Extend the > necessary definitions and describe the DPU in the SC8280XP. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > --- > > Changes since v3: > - Reuse existing masks, rather than duplicating > - Fixed qseed3lite vs qseed4 scaler bits > - Added source-split > - Splitted mdss to separate patch > > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 216 ++++++++++++++++++ > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 + > .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 18 ++ > .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 3 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 2 + > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 + > drivers/gpu/drm/msm/msm_drv.h | 1 + > 7 files changed, 242 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > index 2196e205efa5..429c9ae40b80 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > @@ -124,6 +124,19 @@ > BIT(MDP_AD4_0_INTR) | \ > BIT(MDP_AD4_1_INTR)) > > +#define IRQ_SC8280XP_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > + BIT(MDP_SSPP_TOP0_INTR2) | \ > + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > + BIT(MDP_INTF0_7xxx_INTR) | \ > + BIT(MDP_INTF1_7xxx_INTR) | \ > + BIT(MDP_INTF2_7xxx_INTR) | \ > + BIT(MDP_INTF3_7xxx_INTR) | \ > + BIT(MDP_INTF4_7xxx_INTR) | \ > + BIT(MDP_INTF5_7xxx_INTR) | \ > + BIT(MDP_INTF6_7xxx_INTR) | \ > + BIT(MDP_INTF7_7xxx_INTR) | \ > + BIT(MDP_INTF8_7xxx_INTR)) > + > #define WB_SM8250_MASK (BIT(DPU_WB_LINE_MODE) | \ > BIT(DPU_WB_UBWC) | \ > BIT(DPU_WB_YUV_CONFIG) | \ > @@ -365,6 +378,20 @@ static const struct dpu_caps sc8180x_dpu_caps = { > .max_vdeci_exp = MAX_VERT_DECIMATION, > }; > > +static const struct dpu_caps sc8280xp_dpu_caps = { > + .max_mixer_width = 2560, > + .max_mixer_blendstages = 11, > + .qseed_type = DPU_SSPP_SCALER_QSEED3LITE, > + .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */ > + .ubwc_version = DPU_HW_UBWC_VER_40, > + .has_src_split = true, > + .has_dim_layer = true, > + .has_idle_pc = true, > + .has_3d_merge = true, > + .max_linewidth = 5120, > + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > +}; > + > static const struct dpu_caps sm8250_dpu_caps = { > .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH, > .max_mixer_blendstages = 0xb, > @@ -545,6 +572,24 @@ static const struct dpu_mdp_cfg sc7280_mdp[] = { > }, > }; > > +static const struct dpu_mdp_cfg sc8280xp_mdp[] = { > + { > + .name = "top_0", .id = MDP_TOP, > + .base = 0x0, .len = 0x494, > + .features = 0, > + .highest_bank_bit = 0x3, /* TODO: 2 for LP_DDR4 */ ubwc_swizzle ? I'd suppose it's 6, but I'd bet on it. > + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0}, > + .clk_ctrls[DPU_CLK_CTRL_VIG1] = { .reg_off = 0x2b4, .bit_off = 0}, > + .clk_ctrls[DPU_CLK_CTRL_VIG2] = { .reg_off = 0x2bc, .bit_off = 0}, > + .clk_ctrls[DPU_CLK_CTRL_VIG3] = { .reg_off = 0x2c4, .bit_off = 0}, > + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8}, > + .clk_ctrls[DPU_CLK_CTRL_DMA1] = { .reg_off = 0x2b4, .bit_off = 8}, > + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { .reg_off = 0x2bc, .bit_off = 8}, > + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = { .reg_off = 0x2c4, .bit_off = 8}, > + .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20}, > + }, > +}; > + > static const struct dpu_mdp_cfg qcm2290_mdp[] = { > { > .name = "top_0", .id = MDP_TOP, > @@ -648,6 +693,45 @@ static const struct dpu_ctl_cfg sc7180_ctl[] = { > }, > }; > > +static const struct dpu_ctl_cfg sc8280xp_ctl[] = { > + { > + .name = "ctl_0", .id = CTL_0, > + .base = 0x15000, .len = 0x204, > + .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG), Please use CTL_SC7270_MASK instead, unless you have a strong reasong not to do it. > + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9), > + }, > + { > + .name = "ctl_1", .id = CTL_1, > + .base = 0x16000, .len = 0x204, > + .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG), > + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10), > + }, > + { > + .name = "ctl_2", .id = CTL_2, > + .base = 0x17000, .len = 0x204, > + .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG), > + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 11), > + }, > + { > + .name = "ctl_3", .id = CTL_3, > + .base = 0x18000, .len = 0x204, > + .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG), > + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 12), > + }, > + { > + .name = "ctl_4", .id = CTL_4, > + .base = 0x19000, .len = 0x204, > + .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG), > + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 13), > + }, > + { > + .name = "ctl_5", .id = CTL_5, > + .base = 0x1a000, .len = 0x204, > + .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG), > + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 23), > + }, > +}; > + > static const struct dpu_ctl_cfg sm8150_ctl[] = { > { > .name = "ctl_0", .id = CTL_0, > @@ -926,6 +1010,33 @@ static const struct dpu_sspp_cfg sc7280_sspp[] = { > sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1), > }; > > +static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_0 = > + _VIG_SBLK("0", 5, DPU_SSPP_SCALER_QSEED3LITE); > +static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_1 = > + _VIG_SBLK("1", 6, DPU_SSPP_SCALER_QSEED3LITE); > +static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_2 = > + _VIG_SBLK("2", 7, DPU_SSPP_SCALER_QSEED3LITE); > +static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_3 = > + _VIG_SBLK("3", 8, DPU_SSPP_SCALER_QSEED3LITE); > + > +static const struct dpu_sspp_cfg sc8280xp_sspp[] = { > + SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SM8250_MASK, > + sc8280xp_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0), > + SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SM8250_MASK, > + sc8280xp_vig_sblk_1, 4, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1), > + SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SM8250_MASK, > + sc8280xp_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2), > + SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SM8250_MASK, > + sc8280xp_vig_sblk_3, 12, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3), > + SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK, > + sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0), > + SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK, > + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1), > + SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK, > + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0), > + SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK, > + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1), > +}; > > #define _VIG_SBLK_NOSCALE(num, sdma_pri) \ > { \ > @@ -1034,6 +1145,17 @@ static const struct dpu_lm_cfg sc7180_lm[] = { > &sc7180_lm_sblk, PINGPONG_1, LM_0, 0), > }; > > +/* SC8280XP */ > + > +static const struct dpu_lm_cfg sc8280xp_lm[] = { > + LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK, &sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0), > + LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK, &sdm845_lm_sblk, PINGPONG_1, LM_0, DSPP_1), > + LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK, &sdm845_lm_sblk, PINGPONG_2, LM_3, DSPP_2), > + LM_BLK("lm_3", LM_3, 0x47000, MIXER_SDM845_MASK, &sdm845_lm_sblk, PINGPONG_3, LM_2, DSPP_3), > + LM_BLK("lm_4", LM_4, 0x48000, MIXER_SDM845_MASK, &sdm845_lm_sblk, PINGPONG_4, LM_5, 0), > + LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK, &sdm845_lm_sblk, PINGPONG_5, LM_4, 0), > +}; > + > /* SM8150 */ > > static const struct dpu_lm_cfg sm8150_lm[] = { > @@ -1192,6 +1314,21 @@ static struct dpu_pingpong_cfg sc7180_pp[] = { > PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, 0, sdm845_pp_sblk_te, -1, -1), > }; > > +static struct dpu_pingpong_cfg sc8280xp_pp[] = { > + PP_BLK_TE("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, sdm845_pp_sblk_te, > + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), -1), > + PP_BLK_TE("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, sdm845_pp_sblk_te, > + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), -1), > + PP_BLK_TE("pingpong_2", PINGPONG_2, 0x6b000, MERGE_3D_1, sdm845_pp_sblk_te, > + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), -1), > + PP_BLK_TE("pingpong_3", PINGPONG_3, 0x6c000, MERGE_3D_1, sdm845_pp_sblk_te, > + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), -1), > + PP_BLK_TE("pingpong_4", PINGPONG_4, 0x6d000, MERGE_3D_2, sdm845_pp_sblk_te, > + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), -1), > + PP_BLK_TE("pingpong_5", PINGPONG_5, 0x6e000, MERGE_3D_2, sdm845_pp_sblk_te, > + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), -1), > +}; > + > static const struct dpu_pingpong_cfg sm8150_pp[] = { > PP_BLK_TE("pingpong_0", PINGPONG_0, 0x70000, MERGE_3D_0, sdm845_pp_sblk_te, > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > @@ -1243,6 +1380,12 @@ static const struct dpu_merge_3d_cfg sm8150_merge_3d[] = { > MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200), > }; > > +static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = { > + MERGE_3D_BLK("merge_3d_0", MERGE_3D_0, 0x4e000), > + MERGE_3D_BLK("merge_3d_1", MERGE_3D_1, 0x4f000), > + MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000), > +}; > + > /************************************************************* > * DSC sub blocks config > *************************************************************/ > @@ -1317,6 +1460,19 @@ static const struct dpu_intf_cfg sc8180x_intf[] = { > INTF_BLK("intf_5", INTF_5, 0x6C800, INTF_DP, MSM_DP_CONTROLLER_2, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 22, 23), > }; > > +/* TODO: INTF 3, 8 and 7 are used for MST, marked as INTF_NONE for now */ > +static const struct dpu_intf_cfg sc8280xp_intf[] = { > + INTF_BLK("intf_0", INTF_0, 0x34000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > + INTF_BLK("intf_1", INTF_1, 0x35000, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > + INTF_BLK("intf_2", INTF_2, 0x36000, INTF_DSI, 1, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > + INTF_BLK("intf_3", INTF_3, 0x37000, INTF_NONE, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > + INTF_BLK("intf_4", INTF_4, 0x38000, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 20, 21), > + INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_3, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23), > + INTF_BLK("intf_6", INTF_6, 0x3a000, INTF_DP, MSM_DP_CONTROLLER_2, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 16, 17), > + INTF_BLK("intf_7", INTF_7, 0x3b000, INTF_NONE, MSM_DP_CONTROLLER_2, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 18, 19), > + INTF_BLK("intf_8", INTF_8, 0x3c000, INTF_NONE, MSM_DP_CONTROLLER_1, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 12, 13), > +}; > + > static const struct dpu_intf_cfg qcm2290_intf[] = { > INTF_BLK("intf_0", INTF_0, 0x00000, INTF_NONE, 0, 0, 0, 0, 0, 0), > INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > @@ -1419,6 +1575,14 @@ static const struct dpu_vbif_cfg sdm845_vbif[] = { > }, > }; > > +static const struct dpu_reg_dma_cfg sc8280xp_regdma = { > + .base = 0x0, > + .version = 0x00020000, > + .trigger_sel_off = 0x119c, > + .xin_id = 7, > + .clk_ctrl = DPU_CLK_CTRL_REG_DMA, > +}; > + > static const struct dpu_reg_dma_cfg sdm845_regdma = { > .base = 0x0, .version = 0x1, .trigger_sel_off = 0x119c > }; > @@ -1690,6 +1854,33 @@ static const struct dpu_perf_cfg sc8180x_perf_data = { > .min_llcc_ib = 800000, > .min_dram_ib = 800000, > .danger_lut_tbl = {0xf, 0xffff, 0x0}, > + .qos_lut_tbl = { > + {.nentry = ARRAY_SIZE(sc7180_qos_linear), > + .entries = sc7180_qos_linear > + }, > + {.nentry = ARRAY_SIZE(sc7180_qos_macrotile), > + .entries = sc7180_qos_macrotile > + }, > + {.nentry = ARRAY_SIZE(sc7180_qos_nrt), > + .entries = sc7180_qos_nrt > + }, > + /* TODO: macrotile-qseed is different from macrotile */ > + }, > + .cdp_cfg = { > + {.rd_enable = 1, .wr_enable = 1}, > + {.rd_enable = 1, .wr_enable = 0} > + }, > + .clk_inefficiency_factor = 105, > + .bw_inefficiency_factor = 120, > +}; > + > +static const struct dpu_perf_cfg sc8280xp_perf_data = { > + .max_bw_low = 13600000, > + .max_bw_high = 18200000, > + .min_core_ib = 2500000, > + .min_llcc_ib = 0, > + .min_dram_ib = 800000, > + .danger_lut_tbl = {0xf, 0xffff, 0x0}, > .qos_lut_tbl = { > {.nentry = ARRAY_SIZE(sc8180x_qos_linear), > .entries = sc8180x_qos_linear > @@ -1937,6 +2128,30 @@ static const struct dpu_mdss_cfg sc8180x_dpu_cfg = { > .mdss_irqs = IRQ_SC8180X_MASK, > }; > > +static const struct dpu_mdss_cfg sc8280xp_dpu_cfg = { > + .caps = &sc8280xp_dpu_caps, > + .mdp_count = ARRAY_SIZE(sc8280xp_mdp), > + .mdp = sc8280xp_mdp, > + .ctl_count = ARRAY_SIZE(sc8280xp_ctl), > + .ctl = sc8280xp_ctl, > + .sspp_count = ARRAY_SIZE(sc8280xp_sspp), > + .sspp = sc8280xp_sspp, > + .mixer_count = ARRAY_SIZE(sc8280xp_lm), > + .mixer = sc8280xp_lm, > + .dspp_count = ARRAY_SIZE(sm8150_dspp), > + .dspp = sm8150_dspp, > + .pingpong_count = ARRAY_SIZE(sc8280xp_pp), > + .pingpong = sc8280xp_pp, > + .merge_3d_count = ARRAY_SIZE(sc8280xp_merge_3d), > + .merge_3d = sc8280xp_merge_3d, > + .intf_count = ARRAY_SIZE(sc8280xp_intf), > + .intf = sc8280xp_intf, > + .vbif_count = ARRAY_SIZE(sdm845_vbif), > + .vbif = sdm845_vbif, > + .perf = &sc8280xp_perf_data, > + .mdss_irqs = IRQ_SC8280XP_MASK, > +}; > + > static const struct dpu_mdss_cfg sm8250_dpu_cfg = { > .caps = &sm8250_dpu_caps, > .mdp_count = ARRAY_SIZE(sm8250_mdp), > @@ -2024,6 +2239,7 @@ static const struct dpu_mdss_hw_cfg_handler cfg_handler[] = { > { .hw_rev = DPU_HW_VER_630, .dpu_cfg = &sm6115_dpu_cfg}, > { .hw_rev = DPU_HW_VER_650, .dpu_cfg = &qcm2290_dpu_cfg}, > { .hw_rev = DPU_HW_VER_720, .dpu_cfg = &sc7280_dpu_cfg}, > + { .hw_rev = DPU_HW_VER_800, .dpu_cfg = &sc8280xp_dpu_cfg}, > }; > > const struct dpu_mdss_cfg *dpu_hw_catalog_init(u32 hw_rev) > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > index 3b645d5aa9aa..6897b35c18fa 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > @@ -47,6 +47,7 @@ > #define DPU_HW_VER_630 DPU_HW_VER(6, 3, 0) /* sm6115|sm4250 */ > #define DPU_HW_VER_650 DPU_HW_VER(6, 5, 0) /* qcm2290|sm4125 */ > #define DPU_HW_VER_720 DPU_HW_VER(7, 2, 0) /* sc7280 */ > +#define DPU_HW_VER_800 DPU_HW_VER(8, 0, 0) /* sc8280xp */ > > #define IS_MSM8996_TARGET(rev) IS_DPU_MAJOR_MINOR_SAME((rev), DPU_HW_VER_170) > #define IS_MSM8998_TARGET(rev) IS_DPU_MAJOR_MINOR_SAME((rev), DPU_HW_VER_300) > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > index cf1b6d84c18a..27d74c4d8a98 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > @@ -35,6 +35,9 @@ > #define MDP_INTF_3_OFF_REV_7xxx 0x37000 > #define MDP_INTF_4_OFF_REV_7xxx 0x38000 > #define MDP_INTF_5_OFF_REV_7xxx 0x39000 > +#define MDP_INTF_6_OFF_REV_7xxx 0x3a000 > +#define MDP_INTF_7_OFF_REV_7xxx 0x3b000 > +#define MDP_INTF_8_OFF_REV_7xxx 0x3c000 > > /** > * struct dpu_intr_reg - array of DPU register sets > @@ -139,6 +142,21 @@ static const struct dpu_intr_reg dpu_intr_set[] = { > MDP_INTF_5_OFF_REV_7xxx+INTF_INTR_EN, > MDP_INTF_5_OFF_REV_7xxx+INTF_INTR_STATUS > }, > + [MDP_INTF6_7xxx_INTR] = { > + MDP_INTF_6_OFF_REV_7xxx+INTF_INTR_CLEAR, > + MDP_INTF_6_OFF_REV_7xxx+INTF_INTR_EN, > + MDP_INTF_6_OFF_REV_7xxx+INTF_INTR_STATUS > + }, > + [MDP_INTF7_7xxx_INTR] = { > + MDP_INTF_7_OFF_REV_7xxx+INTF_INTR_CLEAR, > + MDP_INTF_7_OFF_REV_7xxx+INTF_INTR_EN, > + MDP_INTF_7_OFF_REV_7xxx+INTF_INTR_STATUS > + }, > + [MDP_INTF8_7xxx_INTR] = { > + MDP_INTF_8_OFF_REV_7xxx+INTF_INTR_CLEAR, > + MDP_INTF_8_OFF_REV_7xxx+INTF_INTR_EN, > + MDP_INTF_8_OFF_REV_7xxx+INTF_INTR_STATUS > + }, > }; > > #define DPU_IRQ_REG(irq_idx) (irq_idx / 32) > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h > index 46443955443c..425465011c80 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h > @@ -31,6 +31,9 @@ enum dpu_hw_intr_reg { > MDP_INTF3_7xxx_INTR, > MDP_INTF4_7xxx_INTR, > MDP_INTF5_7xxx_INTR, > + MDP_INTF6_7xxx_INTR, > + MDP_INTF7_7xxx_INTR, > + MDP_INTF8_7xxx_INTR, > MDP_INTR_MAX, > }; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > index d3b0ed0a9c6c..d595096a4b1f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > @@ -214,6 +214,8 @@ enum dpu_intf { > INTF_4, > INTF_5, > INTF_6, > + INTF_7, > + INTF_8, > INTF_MAX > }; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index b71199511a52..30f894864cca 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -1292,6 +1292,7 @@ static const struct of_device_id dpu_dt_match[] = { > { .compatible = "qcom,sc7180-dpu", }, > { .compatible = "qcom,sc7280-dpu", }, > { .compatible = "qcom,sc8180x-dpu", }, > + { .compatible = "qcom,sc8280xp-dpu", }, > { .compatible = "qcom,sm6115-dpu", }, > { .compatible = "qcom,sm8150-dpu", }, > { .compatible = "qcom,sm8250-dpu", }, > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h > index d4e0ef608950..b2789efd59e8 100644 > --- a/drivers/gpu/drm/msm/msm_drv.h > +++ b/drivers/gpu/drm/msm/msm_drv.h > @@ -61,6 +61,7 @@ enum msm_dp_controller { > MSM_DP_CONTROLLER_0, > MSM_DP_CONTROLLER_1, > MSM_DP_CONTROLLER_2, > + MSM_DP_CONTROLLER_3, > MSM_DP_CONTROLLER_COUNT, > }; >
On Wed, Dec 07, 2022 at 04:49:07PM +0200, Dmitry Baryshkov wrote: > On 05/12/2022 19:44, Bjorn Andersson wrote: > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c [..] > > +static const struct dpu_mdp_cfg sc8280xp_mdp[] = { > > + { > > + .name = "top_0", .id = MDP_TOP, > > + .base = 0x0, .len = 0x494, > > + .features = 0, > > + .highest_bank_bit = 0x3, /* TODO: 2 for LP_DDR4 */ > > ubwc_swizzle ? I'd suppose it's 6, but I'd bet on it. > I don't see ubwc_swizzle defined for any other platform, and it seems to be unused for DPU_HW_UBWC_VER_40. Am I perhaps missing something? > > + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0}, > > + .clk_ctrls[DPU_CLK_CTRL_VIG1] = { .reg_off = 0x2b4, .bit_off = 0}, > > + .clk_ctrls[DPU_CLK_CTRL_VIG2] = { .reg_off = 0x2bc, .bit_off = 0}, > > + .clk_ctrls[DPU_CLK_CTRL_VIG3] = { .reg_off = 0x2c4, .bit_off = 0}, > > + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8}, > > + .clk_ctrls[DPU_CLK_CTRL_DMA1] = { .reg_off = 0x2b4, .bit_off = 8}, > > + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { .reg_off = 0x2bc, .bit_off = 8}, > > + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = { .reg_off = 0x2c4, .bit_off = 8}, > > + .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20}, > > + }, > > +}; > > + > > static const struct dpu_mdp_cfg qcm2290_mdp[] = { > > { > > .name = "top_0", .id = MDP_TOP, > > @@ -648,6 +693,45 @@ static const struct dpu_ctl_cfg sc7180_ctl[] = { > > }, > > }; > > +static const struct dpu_ctl_cfg sc8280xp_ctl[] = { > > + { > > + .name = "ctl_0", .id = CTL_0, > > + .base = 0x15000, .len = 0x204, > > + .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG), > > Please use CTL_SC7270_MASK instead, unless you have a strong reasong not to > do it. > No strong reason, will update. Thanks, Bjorn
On 07/12/2022 18:28, Bjorn Andersson wrote: > On Wed, Dec 07, 2022 at 04:49:07PM +0200, Dmitry Baryshkov wrote: >> On 05/12/2022 19:44, Bjorn Andersson wrote: >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > [..] >>> +static const struct dpu_mdp_cfg sc8280xp_mdp[] = { >>> + { >>> + .name = "top_0", .id = MDP_TOP, >>> + .base = 0x0, .len = 0x494, >>> + .features = 0, >>> + .highest_bank_bit = 0x3, /* TODO: 2 for LP_DDR4 */ >> >> ubwc_swizzle ? I'd suppose it's 6, but I'd bet on it. >> > > I don't see ubwc_swizzle defined for any other platform, and it seems to > be unused for DPU_HW_UBWC_VER_40. Am I perhaps missing something? Yes, it doesn't seem to be used for VER_40, just wanted to have it for the sake of completeness. See https://lore.kernel.org/linux-arm-msm/20221207142833.204193-4-dmitry.baryshkov@linaro.org/T/#u > >>> + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0}, >>> + .clk_ctrls[DPU_CLK_CTRL_VIG1] = { .reg_off = 0x2b4, .bit_off = 0}, >>> + .clk_ctrls[DPU_CLK_CTRL_VIG2] = { .reg_off = 0x2bc, .bit_off = 0}, >>> + .clk_ctrls[DPU_CLK_CTRL_VIG3] = { .reg_off = 0x2c4, .bit_off = 0}, >>> + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8}, >>> + .clk_ctrls[DPU_CLK_CTRL_DMA1] = { .reg_off = 0x2b4, .bit_off = 8}, >>> + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { .reg_off = 0x2bc, .bit_off = 8}, >>> + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = { .reg_off = 0x2c4, .bit_off = 8}, >>> + .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20}, >>> + }, >>> +}; >>> + >>> static const struct dpu_mdp_cfg qcm2290_mdp[] = { >>> { >>> .name = "top_0", .id = MDP_TOP, >>> @@ -648,6 +693,45 @@ static const struct dpu_ctl_cfg sc7180_ctl[] = { >>> }, >>> }; >>> +static const struct dpu_ctl_cfg sc8280xp_ctl[] = { >>> + { >>> + .name = "ctl_0", .id = CTL_0, >>> + .base = 0x15000, .len = 0x204, >>> + .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG), >> >> Please use CTL_SC7270_MASK instead, unless you have a strong reasong not to >> do it. >> > > No strong reason, will update. Thanks. The logic for me is to be able to update a single mask when new features are added instead of going all over the code. E.g. I think sc8280xp will benefit from hierarchical DSPP support, will it not? > > Thanks, > Bjorn