Re: [Freedreno] [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions
Hi Ville, On Tuesday, 4 December 2018 21:13:20 EET Ville Syrjälä wrote: > On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote: > > On 03.12.2018 22:38, Ville Syrjälä wrote: > >> On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote: > >>> On 21.11.2018 19:19, Laurent Pinchart wrote: > On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Make life easier for drivers by simply passing the connector > > to drm_hdmi_avi_infoframe_from_display_mode() and > > drm_hdmi_avi_infoframe_quant_range(). That way drivers don't > > need to worry about is_hdmi2_sink mess. > > While this is good for display controller drivers, the change isn't > great for bridge drivers. Down the road we're looking at moving > connector support out of the bridge drivers. Adding an additional > dependency to connectors in the bridges will make that more > difficult. Ideally bridges should retrieve the information from their > sink, regardless of whether it is a connector or another bridge. > >>> > >>> I agree with it, and case of sii8620 shows that there are cases where > >>> bridge has no direct access to the connector. > >> > >> It's just a matter of plumbing it through. > > > > What do you mean exactly? > > void bridge_foo(... > + ,struct drm_connector *connector); > > >>> On the other side, since you are passing connector to > >>> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode > >>> parameter and rename the function to > >>> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and > >>> mode set on the connector differs? > >> > >> Connectors don't have a mode. > > > > As they are passing video stream they should have it, even if not > > directly, for example: > > > > connector->state->crtc->mode > > That's not really how atomic works. One shouldn't go digging > through the obj->state pointers when we're not holding the > relevant locks anymore. The atomic way would be to pass either > both crtc state and connector state, or drm_atomic_state + > crtc/connector. Or a bridge state ? With chained bridges the mode can vary along the pipeline, the CRTC adjusted mode will only cover the link between the CRTC and the first bridge. It's only a matter of time until we need to store other intermediate modes in states. I'd rather prepare for that instead of passing the CRTC state to bridges. > > In moment of creating infoframe it should be set properly. > > > Please see below for an additional comment. > > > Cc: Alex Deucher > > Cc: "Christian König" > > Cc: "David (ChunMing) Zhou" > > Cc: Archit Taneja > > Cc: Andrzej Hajda > > Cc: Laurent Pinchart > > Cc: Inki Dae > > Cc: Joonyoung Shim > Cc: Seung-Woo Kim > > Cc: Kyungmin Park > > Cc: Russell King > > Cc: CK Hu > > Cc: Philipp Zabel > > Cc: Rob Clark > > Cc: Ben Skeggs > > Cc: Tomi Valkeinen > > Cc: Sandy Huang > > Cc: "Heiko Stübner" > > Cc: Benjamin Gaignard > > Cc: Vincent Abriou > > Cc: Thierry Reding > > Cc: Eric Anholt > > Cc: Shawn Guo > > Cc: Ilia Mirkin > > Cc: amd-...@lists.freedesktop.org > > Cc: linux-arm-...@vger.kernel.org > > Cc: freedreno@lists.freedesktop.org > > Cc: nouv...@lists.freedesktop.org > > Cc: linux-te...@vger.kernel.org > > Signed-off-by: Ville Syrjälä > > --- > > > > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- > > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- > > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 3 ++- > > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > > drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 ++-- > > drivers/gpu/drm/bridge/sii902x.c | 3 ++- > > drivers/gpu/drm/bridge/sil-sii8620.c | 3 +-- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 ++- > > drivers/gpu/drm/drm_edid.c| 33 > > drivers/gpu/drm/exynos/exynos_hdmi.c | 3 ++- > > drivers/gpu/drm/i2c/tda998x_drv.c | 3 ++- > > drivers/gpu/drm/i915/intel_hdmi.c | 14 +- > > drivers/gpu/drm/i915/intel_lspcon.c | 15 ++- > > drivers/gpu/drm/i915/intel_sdvo.c | 10 --- > > drivers/gpu/drm/mediatek/mtk_hdmi.c | 3 ++- > > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c| 3 ++- > > drivers/gpu/drm/nouveau/dispnv50/disp.c | 7 +++-- > > drivers/gpu/drm/omapdrm/omap_encoder.c| 5 ++-- > > drivers/gpu/drm/radeon/radeon_audio.c | 2 +- > > drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++- > > drivers/gpu/drm/sti/sti_hdmi.c| 3 ++- > > drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c| 3 ++- > > drivers/gpu/drm/tegra/hdmi.c | 3 ++- > > drivers/gpu/drm/tegra/sor.c | 3 ++- > >
Re: [Freedreno] [PATCH v6] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file
Hi, On Tue, Dec 4, 2018 at 3:54 PM Jeykumar Sankaran wrote: > > DPU is short for the Display Processing Unit. It is the display > controller on Qualcomm SDM845 chips. > > This change adds MDSS and DSI nodes to enable display on the > target device. > > Changes in v2: > - Beefed up commit message > - Use SoC specific compatibles for mdss and dpu (Rob H) > - Use assigned-clocks to set initial clock frequency(Rob H) > Changes in v3: > - added IOMMU node > - Fix device naming (remove _phys) > - Use correct IRQ_TYPE in interrupt specifiers > Changes in v4: > - move mdss node to preserve the unit address sort order > - remove _clk suffix from dsi clocks > (both the comments are from Doug Anderson) > Changes in v5: > - Keep the device status "disabled" by default (Bjorn Andersson) > - Use MDSS_GDSC macro (Jordan) > - Fix phy-names (Jordan) > - List reg ranges in numerical order (Jordan) > Changes in v6: > - Separating this patch out of the series > - fix phy-names > > Signed-off-by: Jeykumar Sankaran > Signed-off-by: Sean Paul > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 203 > +++ > 1 file changed, 203 insertions(+) With my admittedly limited understanding of the device tree for graphics, this looks good to me. Reviewed-by: Douglas Anderson ...this works for me on sdm845-cheza. Specifically I tested it on our 4.19 branch which has some display backports and a few picks from the mailing list. Anyone specifically interested in the tree I tested with can see https://crrev.com/c/1327901, which includes this patch. Thus: Tested-by: Douglas Anderson ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v6] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file
DPU is short for the Display Processing Unit. It is the display
controller on Qualcomm SDM845 chips.
This change adds MDSS and DSI nodes to enable display on the
target device.
Changes in v2:
- Beefed up commit message
- Use SoC specific compatibles for mdss and dpu (Rob H)
- Use assigned-clocks to set initial clock frequency(Rob H)
Changes in v3:
- added IOMMU node
- Fix device naming (remove _phys)
- Use correct IRQ_TYPE in interrupt specifiers
Changes in v4:
- move mdss node to preserve the unit address sort order
- remove _clk suffix from dsi clocks
(both the comments are from Doug Anderson)
Changes in v5:
- Keep the device status "disabled" by default (Bjorn Andersson)
- Use MDSS_GDSC macro (Jordan)
- Fix phy-names (Jordan)
- List reg ranges in numerical order (Jordan)
Changes in v6:
- Separating this patch out of the series
- fix phy-names
Signed-off-by: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 203 +++
1 file changed, 203 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 1419b00..fa7023e 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1256,6 +1256,209 @@
};
};
+ mdss: mdss@ae0 {
+ compatible = "qcom,sdm845-mdss";
+ reg = <0xae0 0x1000>;
+ reg-names = "mdss";
+
+ power-domains = < MDSS_GDSC>;
+
+ clocks = < GCC_DISP_AHB_CLK>,
+< GCC_DISP_AXI_CLK>,
+< DISP_CC_MDSS_MDP_CLK>;
+ clock-names = "iface", "bus", "core";
+
+ assigned-clocks = < DISP_CC_MDSS_MDP_CLK>;
+ assigned-clock-rates = <3>;
+
+ interrupts = ;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ iommus = <_smmu 0x880 0x8>,
+<_smmu 0xc80 0x8>;
+
+ status = "disabled";
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ mdss_mdp: mdp@ae01000 {
+ compatible = "qcom,sdm845-dpu";
+ reg = <0x0ae01000 0x8f000>,
+ <0x0aeb 0x2008>;
+ reg-names = "mdp", "vbif";
+
+ clocks = < DISP_CC_MDSS_AHB_CLK>,
+< DISP_CC_MDSS_AXI_CLK>,
+< DISP_CC_MDSS_MDP_CLK>,
+< DISP_CC_MDSS_VSYNC_CLK>;
+ clock-names = "iface", "bus", "core", "vsync";
+
+ assigned-clocks = <
DISP_CC_MDSS_MDP_CLK>,
+ <
DISP_CC_MDSS_VSYNC_CLK>;
+ assigned-clock-rates = <3>,
+ <1920>;
+
+ interrupt-parent = <>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dpu_intf1_out: endpoint {
+ remote-endpoint =
<_in>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dpu_intf2_out: endpoint {
+ remote-endpoint =
<_in>;
+ };
+ };
+ };
+ };
+
+ dsi0: dsi@ae94000 {
+ compatible = "qcom,mdss-dsi-ctrl";
+ reg = <0xae94000 0x400>;
+ reg-names = "dsi_ctrl";
+
+ interrupt-parent = <>;
+ interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+
+ clocks = < DISP_CC_MDSS_BYTE0_CLK>,
+<
Re: [Freedreno] [PATCH v2] drm/msm/dpu: add display port support in DPU
On 2018-12-03 06:47, Sean Paul wrote:
On Tue, Nov 27, 2018 at 02:28:30PM -0800, Jeykumar Sankaran wrote:
Add display port support in DPU by creating hooks
for DP encoder enumeration and encoder mode
initialization.
This change is based on the SDM845 Display port
driver changes[1].
changes in v2:
- rebase on [2] (Sean Paul)
- remove unwanted error checks and
switch cases (Jordan Crouse)
[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 ++---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 47
+
2 files changed, 45 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d3f4501..1f6b4b1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2015,7 +2015,7 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
{
int ret = 0;
int i = 0;
- enum dpu_intf_type intf_type;
+ enum dpu_intf_type intf_type = INTF_NONE;
dpu_intf_type seems unnecessary, you could just use the
DRM_MODE_ENCODER_*
value
directly?
enum dpu_intf_type enumerates HW interface types the SOC has. Below
switch
case maps the DRM_MODE_ENCODER_* to HW dpu_intf_type it should reserve.
Note that DRM_MODE_ENCODER_* and dpu_intf_type are not mapped 1-to-1.
e.g. DRM_MODE_ENCODER_TMDS can be mapped to HDMI or DisplayPort.
Thanks,
Jeykumar S.
struct dpu_enc_phys_init_params phys_params;
if (!dpu_enc || !dpu_kms) {
@@ -2038,9 +2038,9 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
case DRM_MODE_ENCODER_DSI:
intf_type = INTF_DSI;
break;
- default:
- DPU_ERROR_ENC(dpu_enc, "unsupported display interface
type\n");
- return -EINVAL;
+ case DRM_MODE_ENCODER_TMDS:
+ intf_type = INTF_DP;
+ break;
}
WARN_ON(disp_info->num_of_h_tiles < 1);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 985c855..7d931ae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -473,6 +473,32 @@ static void _dpu_kms_initialize_dsi(struct
drm_device *dev,
}
}
+static void _dpu_kms_initialize_displayport(struct drm_device *dev,
+ struct msm_drm_private *priv,
+ struct dpu_kms *dpu_kms)
+{
+ struct drm_encoder *encoder = NULL;
+ int rc;
+
+ if (!priv->dp)
+ return;
+
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+ if (IS_ERR(encoder)) {
+ DPU_ERROR("encoder init failed for dsi display\n");
+ return;
+ }
+
+ rc = msm_dp_modeset_init(priv->dp, dev, encoder);
+ if (rc) {
+ DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
+ drm_encoder_cleanup(encoder);
+ return;
+ }
+
+ priv->encoders[priv->num_encoders++] = encoder;
No need to keep track of drm resources at the driver level, the core
will
do
this for you. So can you please add a patch preceding this one to
remove
the
priv->encoders/crtc/planes/connectors arrays?
+}
+
/**
* _dpu_kms_setup_displays - create encoders, bridges and connectors
* for underlying displays
@@ -487,6 +513,8 @@ static void _dpu_kms_setup_displays(struct
drm_device *dev,
Why are these functions voids? Seems like there are plenty of places
for
them to
fail :)
Let's add a patch to the beginning of this series to properly handle
failures in
setup_displays and initialize_dsi
{
_dpu_kms_initialize_dsi(dev, priv, dpu_kms);
+ _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
+
/**
* Extend this function to initialize other
* types of displays
@@ -723,13 +751,20 @@ static void _dpu_kms_set_encoder_mode(struct
msm_kms *kms,
info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
MSM_DISPLAY_CAP_VID_MODE;
- /* TODO: No support for DSI swap */
- for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
- if (priv->dsi[i]) {
- info.h_tile_instance[info.num_of_h_tiles] = i;
- info.num_of_h_tiles++;
+ switch (info.intf_type) {
+ case DRM_MODE_ENCODER_DSI:
+ /* TODO: No support for DSI swap */
+ for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+ if (priv->dsi[i]) {
+ info.h_tile_instance[info.num_of_h_tiles]
= i;
+ info.num_of_h_tiles++;
+ }
}
-
[Freedreno] [PATCH v4 3/8] drm/msm/dsi: 28nm PHY: Get ref clock from the DT
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.
Signed-off-by: Matthias Kaehlcke
---
Changes in v4:
- always use parent rate in dsi_pll_28nm_clk_set_rate() and
dsi_pll_28nm_clk_recalc_rate()
- pass name of VCO ref clock to pll_28nm_register() instead of
storing it in a struct field
- updated commit message
Changes in v3:
- use default name and rate if the ref clock is not specified
in the DT
- store vco_ref_clk_name instead of vco_ref_clk
- dsi_pll_28nm_clk_set_rate: changed data type of ref_clk_rate to
unsigned long
- fixed check for EPROBE_DEFER
- renamed VCO_REF_CLK_RATE to VCO_REF_CLK_DEFAULT_RATE
Changes in v2:
- patch added to the series
---
drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c | 36 +++---
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
index 26e3a01a99c2b..340b03e8d 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
@@ -40,7 +40,6 @@
#define NUM_PROVIDED_CLKS 2
-#define VCO_REF_CLK_RATE 1920
#define VCO_MIN_RATE 35000
#define VCO_MAX_RATE 75000
@@ -166,17 +165,17 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw,
unsigned long rate,
pll_write(base + REG_DSI_28nm_PHY_PLL_LPFC1_CFG, 0x70);
pll_write(base + REG_DSI_28nm_PHY_PLL_LPFC2_CFG, 0x15);
- rem = rate % VCO_REF_CLK_RATE;
+ rem = rate % parent_rate;
if (rem) {
refclk_cfg = DSI_28nm_PHY_PLL_REFCLK_CFG_DBLR;
frac_n_mode = 1;
- div_fbx1000 = rate / (VCO_REF_CLK_RATE / 500);
- gen_vco_clk = div_fbx1000 * (VCO_REF_CLK_RATE / 500);
+ div_fbx1000 = rate / (parent_rate / 500);
+ gen_vco_clk = div_fbx1000 * (parent_rate / 500);
} else {
refclk_cfg = 0x0;
frac_n_mode = 0;
- div_fbx1000 = rate / (VCO_REF_CLK_RATE / 1000);
- gen_vco_clk = div_fbx1000 * (VCO_REF_CLK_RATE / 1000);
+ div_fbx1000 = rate / (parent_rate / 1000);
+ gen_vco_clk = div_fbx1000 * (parent_rate / 1000);
}
DBG("refclk_cfg = %d", refclk_cfg);
@@ -265,7 +264,7 @@ static unsigned long dsi_pll_28nm_clk_recalc_rate(struct
clk_hw *hw,
void __iomem *base = pll_28nm->mmio;
u32 sdm0, doubler, sdm_byp_div;
u32 sdm_dc_off, sdm_freq_seed, sdm2, sdm3;
- u32 ref_clk = VCO_REF_CLK_RATE;
+ u32 ref_clk = parent_rate;
unsigned long vco_rate;
VERB("parent_rate=%lu", parent_rate);
@@ -273,7 +272,7 @@ static unsigned long dsi_pll_28nm_clk_recalc_rate(struct
clk_hw *hw,
/* Check to see if the ref clk doubler is enabled */
doubler = pll_read(base + REG_DSI_28nm_PHY_PLL_REFCLK_CFG) &
DSI_28nm_PHY_PLL_REFCLK_CFG_DBLR;
- ref_clk += (doubler * VCO_REF_CLK_RATE);
+ ref_clk += (doubler * ref_clk);
/* see if it is integer mode or sdm mode */
sdm0 = pll_read(base + REG_DSI_28nm_PHY_PLL_SDM_CFG0);
@@ -514,11 +513,12 @@ static void dsi_pll_28nm_destroy(struct msm_dsi_pll *pll)
pll_28nm->clk_data.clk_num = 0;
}
-static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm)
+static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm,
+const char *ref_clk_name)
{
char clk_name[32], parent1[32], parent2[32], vco_name[32];
struct clk_init_data vco_init = {
- .parent_names = (const char *[]){ "xo" },
+ .parent_names = _clk_name,
.num_parents = 1,
.name = vco_name,
.flags = CLK_IGNORE_UNUSED,
@@ -593,6 +593,8 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_init(struct
platform_device *pdev,
{
struct dsi_pll_28nm *pll_28nm;
struct msm_dsi_pll *pll;
+ struct clk *vco_ref_clk;
+ const char *vco_ref_clk_name;
int ret;
if (!pdev)
@@ -605,6 +607,18 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_init(struct
platform_device *pdev,
pll_28nm->pdev = pdev;
pll_28nm->id = id;
+ vco_ref_clk = devm_clk_get(>dev, "ref");
+ if (!IS_ERR(vco_ref_clk)) {
+ vco_ref_clk_name = __clk_get_name(vco_ref_clk);
+ } else {
+ ret = PTR_ERR(vco_ref_clk);
+ if (ret == -EPROBE_DEFER)
+ ERR_PTR(ret);
+
+ dev_warn(>dev, "'ref' clock is not specified, using
default name\n");
+ vco_ref_clk_name = "xo";
+ }
+
pll_28nm->mmio = msm_ioremap(pdev, "dsi_pll", "DSI_PLL");
if (IS_ERR_OR_NULL(pll_28nm->mmio)) {
dev_err(>dev, "%s: failed to map pll base\n", __func__);
@@ -637,7 +651,7 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_init(struct
platform_device *pdev,
[Freedreno] [PATCH v4 2/8] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.
Signed-off-by: Matthias Kaehlcke
---
Changes in v4:
- always use parent rate in dsi_pll_28nm_clk_set_rate()
- pass name of VCO ref clock to pll_28nm_register() instead of
storing it in a struct field
- updated commit message
Changes in v3:
- use default name and rate if the ref clock is not specified
in the DT
- store vco_ref_clk_name instead of vco_ref_clk
- fixed check for EPROBE_DEFER
- renamed VCO_REF_CLK_RATE to VCO_REF_CLK_DEFAULT_RATE
Changes in v2:
- patch added to the series
---
.../gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c | 24 +++
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
index 49008451085b8..76e5188169b91 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
@@ -47,7 +47,6 @@
#define NUM_PROVIDED_CLKS 2
-#define VCO_REF_CLK_RATE 2700
#define VCO_MIN_RATE 6
#define VCO_MAX_RATE 12
@@ -125,7 +124,7 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw,
unsigned long rate,
DBG("rate=%lu, parent's=%lu", rate, parent_rate);
temp = rate / 10;
- val = VCO_REF_CLK_RATE / 10;
+ val = parent_rate / 10;
fb_divider = (temp * VCO_PREF_DIV_RATIO) / val;
fb_divider = fb_divider / 2 - 1;
pll_write(base + REG_DSI_28nm_8960_PHY_PLL_CTRL_1,
@@ -406,11 +405,12 @@ static void dsi_pll_28nm_destroy(struct msm_dsi_pll *pll)
pll_28nm->clks, pll_28nm->num_clks);
}
-static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm)
+static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm,
+const char *ref_clk_name)
{
char *clk_name, *parent_name, *vco_name;
struct clk_init_data vco_init = {
- .parent_names = (const char *[]){ "pxo" },
+ .parent_names = _clk_name,
.num_parents = 1,
.flags = CLK_IGNORE_UNUSED,
.ops = _ops_dsi_pll_28nm_vco,
@@ -494,6 +494,8 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init(struct
platform_device *pdev,
{
struct dsi_pll_28nm *pll_28nm;
struct msm_dsi_pll *pll;
+ struct clk *vco_ref_clk;
+ const char *vco_ref_clk_name;
int ret;
if (!pdev)
@@ -506,6 +508,18 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init(struct
platform_device *pdev,
pll_28nm->pdev = pdev;
pll_28nm->id = id + 1;
+ vco_ref_clk = devm_clk_get(>dev, "ref");
+ if (!IS_ERR(vco_ref_clk)) {
+ vco_ref_clk_name = __clk_get_name(vco_ref_clk);
+ } else {
+ ret = PTR_ERR(vco_ref_clk);
+ if (ret == -EPROBE_DEFER)
+ return ERR_PTR(ret);
+
+ dev_warn(>dev, "'ref' clock is not specified, using
default name\n");
+ vco_ref_clk_name = "pxo";
+ }
+
pll_28nm->mmio = msm_ioremap(pdev, "dsi_pll", "DSI_PLL");
if (IS_ERR_OR_NULL(pll_28nm->mmio)) {
dev_err(>dev, "%s: failed to map pll base\n", __func__);
@@ -524,7 +538,7 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init(struct
platform_device *pdev,
pll->en_seq_cnt = 1;
pll->enable_seqs[0] = dsi_pll_28nm_enable_seq;
- ret = pll_28nm_register(pll_28nm);
+ ret = pll_28nm_register(pll_28nm, vco_ref_clk_name);
if (ret) {
dev_err(>dev, "failed to register PLL: %d\n", ret);
return ERR_PTR(ret);
--
2.20.0.rc1.387.gf8505762e3-goog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4 0/8] drm/msm/dsi: Get PHY ref clocks from the DT
The MSM DSI PHY drivers currently hardcode the name and the rate of the PHY ref clock. Get the ref clock from the device tree instead. Note: testing of this series was limited to SDM845 and the 10nm PHY Major changes in v4: - always use parent rate for 28nm and 28nm 8960 PHYs Major changes in v3: - keep supporting DTs without ref clock for the 28nm and the 28nm 8960 PHYs - added patch to add ref clock to qcom-apq8064.dtsi Major changes in v2: - apply to all MSM DSI PHY drivers, not only 10nm Matthias Kaehlcke (8): dt-bindings: msm/dsi: Add ref clock for PHYs drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT drm/msm/dsi: 28nm PHY: Get ref clock from the DT drm/msm/dsi: 14nm PHY: Get ref clock from the DT drm/msm/dsi: 10nm PHY: Get ref clock from the DT arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs ARM: dts: qcom-apq8064: Set 'xo_board' as ref clock of the DSI PHY .../devicetree/bindings/display/msm/dsi.txt | 1 + arch/arm/boot/dts/qcom-apq8064.dtsi | 5 +-- arch/arm64/boot/dts/qcom/msm8916.dtsi | 5 +-- arch/arm64/boot/dts/qcom/sdm845.dtsi | 10 +++--- drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c| 13 ++- drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c| 16 +++-- drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c| 36 +-- .../gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c | 24 ++--- 8 files changed, 82 insertions(+), 28 deletions(-) -- 2.20.0.rc1.387.gf8505762e3-goog ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4 5/8] drm/msm/dsi: 10nm PHY: Get ref clock from the DT
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.
Note: This change could break old out-of-tree DTS files that
use the 10nm PHY
Signed-off-by: Matthias Kaehlcke
Reviewed-by: Douglas Anderson
---
Changes in v4:
- none
Changes in v3:
- fixed check for EPROBE_DEFER
- added note to commit message about breaking old DTS files
- added 'Reviewed-by: Douglas Anderson ' tag
Changes in v2:
- remove anonymous array in clk_init_data assignment
- log error code if devm_clk_get() fails
- don't log devm_clk_get() failures for -EPROBE_DEFER
- updated commit message
---
drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
index 4c03f0b7343ed..2d23372acd20d 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
@@ -91,6 +91,7 @@ struct dsi_pll_10nm {
void __iomem *phy_cmn_mmio;
void __iomem *mmio;
+ struct clk *vco_ref_clk;
u64 vco_ref_clk_rate;
u64 vco_current_rate;
@@ -629,8 +630,9 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm)
{
char clk_name[32], parent[32], vco_name[32];
char parent2[32], parent3[32], parent4[32];
+ const char *ref_clk_name = __clk_get_name(pll_10nm->vco_ref_clk);
struct clk_init_data vco_init = {
- .parent_names = (const char *[]){ "xo" },
+ .parent_names = _clk_name,
.num_parents = 1,
.name = vco_name,
.flags = CLK_IGNORE_UNUSED,
@@ -786,6 +788,15 @@ struct msm_dsi_pll *msm_dsi_pll_10nm_init(struct
platform_device *pdev, int id)
pll_10nm->id = id;
pll_10nm_list[id] = pll_10nm;
+ pll_10nm->vco_ref_clk = devm_clk_get(>dev, "ref");
+ if (IS_ERR(pll_10nm->vco_ref_clk)) {
+ ret = PTR_ERR(pll_10nm->vco_ref_clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(>dev, "couldn't get 'ref' clock: %d\n",
+ ret);
+ return ERR_PTR(ret);
+ }
+
pll_10nm->phy_cmn_mmio = msm_ioremap(pdev, "dsi_phy", "DSI_PHY");
if (IS_ERR_OR_NULL(pll_10nm->phy_cmn_mmio)) {
dev_err(>dev, "failed to map CMN PHY base\n");
--
2.20.0.rc1.387.gf8505762e3-goog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4 7/8] arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs
Add 'bi_tcxo' as ref clock for the DSI PHYs, it was previously
hardcoded in the PLL 'driver' for the 10nm PHY.
Signed-off-by: Matthias Kaehlcke
Reviewed-by: Douglas Anderson
Reviewed-by: Stephen Boyd
---
based on "[v4,1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file"
(https://patchwork.kernel.org/patch/10666253/)
Changes in v4:
- added 'Reviewed-by: Stephen Boyd ' tag
Changes in v3:
- added 'Reviewed-by: Douglas Anderson ' tag
Changes in v2:
- patch added to the series
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 5728b4cfae269..cdb5a9bb23e69 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1372,8 +1372,9 @@
#clock-cells = <1>;
#phy-cells = <0>;
- clocks = < DISP_CC_MDSS_AHB_CLK>;
- clock-names = "iface";
+ clocks = < DISP_CC_MDSS_AHB_CLK>,
+< RPMH_CXO_CLK>;
+ clock-names = "iface", "ref";
};
dsi1: dsi@ae96000 {
@@ -1434,8 +1435,9 @@
#clock-cells = <1>;
#phy-cells = <0>;
- clocks = < DISP_CC_MDSS_AHB_CLK>;
- clock-names = "iface";
+ clocks = < DISP_CC_MDSS_AHB_CLK>,
+< RPMH_CXO_CLK>;
+ clock-names = "iface", "ref";
};
};
--
2.20.0.rc1.387.gf8505762e3-goog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4 8/8] ARM: dts: qcom-apq8064: Set 'xo_board' as ref clock of the DSI PHY
Add 'xo_board' as ref clock for the DSI PHY, it was previously hardcoded in the PLL 'driver' for the 28nm 8960 PHY. Signed-off-by: Matthias Kaehlcke Reviewed-by: Stephen Boyd --- Changes in v4: - added 'Reviewed-by: Stephen Boyd ' tag Changes in v3: - patch added to the series --- arch/arm/boot/dts/qcom-apq8064.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi index 48c3cf4276101..d337ae9326cd8 100644 --- a/arch/arm/boot/dts/qcom-apq8064.dtsi +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi @@ -1338,8 +1338,9 @@ <0x04700300 0x200>, <0x04700500 0x5c>; reg-names = "dsi_pll", "dsi_phy", "dsi_phy_regulator"; - clock-names = "iface_clk"; - clocks = < DSI_M_AHB_CLK>; + clock-names = "iface_clk", "ref"; + clocks = < DSI_M_AHB_CLK>, +<_board>; }; -- 2.20.0.rc1.387.gf8505762e3-goog ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4 1/8] dt-bindings: msm/dsi: Add ref clock for PHYs
Allow the PHY drivers to get the ref clock from the DT. Signed-off-by: Matthias Kaehlcke Reviewed-by: Stephen Boyd Reviewed-by: Douglas Anderson --- Chnages in v4: - added "Reviewed-by" tags from Stephen and Doug Changes in v3: - added note that the ref clock is only required for new DTS files/entries Changes in v2: - add the ref clock for all PHYs, not only the 10nm one - updated commit message --- Documentation/devicetree/bindings/display/msm/dsi.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index dfc743219bd88..9ae9469427207 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -106,6 +106,7 @@ Required properties: - clocks: Phandles to device clocks. See [1] for details on clock bindings. - clock-names: the following clocks are required: * "iface" + * "ref" (only required for new DTS files/entries) For 28nm HPM/LP, 28nm 8960 PHYs: - vddio-supply: phandle to vdd-io regulator device node For 20nm PHY: -- 2.20.0.rc1.387.gf8505762e3-goog ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4 4/8] drm/msm/dsi: 14nm PHY: Get ref clock from the DT
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.
Note: This change could break old out-of-tree DTS files that
use the 14nm PHY.
Signed-off-by: Matthias Kaehlcke
Reviewed-by: Douglas Anderson
---
Changes in v4:
- none
Changes in v3:
- fixed check for EPROBE_DEFER
- added note to commit message about breaking old DTS files
- added 'Reviewed-by: Douglas Anderson ' tag
Changes in v2:
- patch added to the series
---
drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
index 71fe60e5f01f1..032bf3e8614bd 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
@@ -40,7 +40,6 @@
#define NUM_PROVIDED_CLKS 2
-#define VCO_REF_CLK_RATE 1920
#define VCO_MIN_RATE 13UL
#define VCO_MAX_RATE 26UL
@@ -139,6 +138,7 @@ struct dsi_pll_14nm {
/* protects REG_DSI_14nm_PHY_CMN_CLK_CFG0 register */
spinlock_t postdiv_lock;
+ struct clk *vco_ref_clk;
u64 vco_current_rate;
u64 vco_ref_clk_rate;
@@ -591,7 +591,7 @@ static int dsi_pll_14nm_vco_set_rate(struct clk_hw *hw,
unsigned long rate,
parent_rate);
pll_14nm->vco_current_rate = rate;
- pll_14nm->vco_ref_clk_rate = VCO_REF_CLK_RATE;
+ pll_14nm->vco_ref_clk_rate = parent_rate;
dsi_pll_14nm_input_init(pll_14nm);
@@ -950,8 +950,9 @@ static struct clk_hw *pll_14nm_postdiv_register(struct
dsi_pll_14nm *pll_14nm,
static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm)
{
char clk_name[32], parent[32], vco_name[32];
+ const char *ref_clk_name = __clk_get_name(pll_14nm->vco_ref_clk);
struct clk_init_data vco_init = {
- .parent_names = (const char *[]){ "xo" },
+ .parent_names = _clk_name,
.num_parents = 1,
.name = vco_name,
.flags = CLK_IGNORE_UNUSED,
@@ -1065,6 +1066,15 @@ struct msm_dsi_pll *msm_dsi_pll_14nm_init(struct
platform_device *pdev, int id)
pll_14nm->id = id;
pll_14nm_list[id] = pll_14nm;
+ pll_14nm->vco_ref_clk = devm_clk_get(>dev, "ref");
+ if (IS_ERR(pll_14nm->vco_ref_clk)) {
+ ret = PTR_ERR(pll_14nm->vco_ref_clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(>dev, "couldn't get 'ref' clock: %d\n",
+ ret);
+ return ERR_PTR(ret);
+ }
+
pll_14nm->phy_cmn_mmio = msm_ioremap(pdev, "dsi_phy", "DSI_PHY");
if (IS_ERR_OR_NULL(pll_14nm->phy_cmn_mmio)) {
dev_err(>dev, "failed to map CMN PHY base\n");
--
2.20.0.rc1.387.gf8505762e3-goog
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4 6/8] arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY
Add 'xo_board' as ref clock for the DSI PHYs, it was previously hardcoded in the PLL 'driver' for the 28nm PHY. Signed-off-by: Matthias Kaehlcke Reviewed-by: Douglas Anderson Reviewed-by: Stephen Boyd --- Changes in v4: - added 'Reviewed-by: Stephen Boyd ' tag Changes in v3: - added 'Reviewed-by: Douglas Anderson ' tag Changes in v2: - patch added to the series --- arch/arm64/boot/dts/qcom/msm8916.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi index d302d8d639a12..89f30f34ff896 100644 --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi @@ -959,8 +959,9 @@ #clock-cells = <1>; #phy-cells = <0>; - clocks = < GCC_MDSS_AHB_CLK>; - clock-names = "iface"; + clocks = < GCC_MDSS_AHB_CLK>, +<_board>; + clock-names = "iface", "ref"; }; }; -- 2.20.0.rc1.387.gf8505762e3-goog ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] of/device: add blacklist for iommu dma_ops
On Sat, Dec 1, 2018 at 10:54 AM Rob Clark wrote:
>
> This solves a problem we see with drm/msm, caused by getting
> iommu_dma_ops while we attach our own domain and manage it directly at
> the iommu API level:
>
> [0038] user address but active_mm is swapper
> Internal error: Oops: 9605 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 7 PID: 70 Comm: kworker/7:1 Tainted: GW 4.19.3 #90
> Hardware name: xxx (DT)
> Workqueue: events deferred_probe_work_func
> pstate: 80c9 (Nzcv daif +PAN +UAO)
> pc : iommu_dma_map_sg+0x7c/0x2c8
> lr : iommu_dma_map_sg+0x40/0x2c8
> sp : ff80095eb4f0
> x29: ff80095eb4f0 x28:
> x27: ffc0f9431578 x26:
> x25: x24: 0003
> x23: 0001 x22: ffc0fa9ac010
> x21: x20: ffc0fab40980
> x19: ffc0fab40980 x18: 0003
> x17: 01c4 x16: 0007
> x15: 000e x14:
> x13: x12: 0028
> x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
> x9 : x8 : ffc0fab409a0
> x7 : x6 : 0002
> x5 : 0001 x4 :
> x3 : 0001 x2 : 0002
> x1 : ffc0f9431578 x0 :
> Process kworker/7:1 (pid: 70, stack limit = 0x17d08ffb)
> Call trace:
>iommu_dma_map_sg+0x7c/0x2c8
>__iommu_map_sg_attrs+0x70/0x84
>get_pages+0x170/0x1e8
>msm_gem_get_iova+0x8c/0x128
>_msm_gem_kernel_new+0x6c/0xc8
>msm_gem_kernel_new+0x4c/0x58
>dsi_tx_buf_alloc_6g+0x4c/0x8c
>msm_dsi_host_modeset_init+0xc8/0x108
>msm_dsi_modeset_init+0x54/0x18c
>_dpu_kms_drm_obj_init+0x430/0x474
>dpu_kms_hw_init+0x5f8/0x6b4
>msm_drm_bind+0x360/0x6c8
>try_to_bring_up_master.part.7+0x28/0x70
>component_master_add_with_match+0xe8/0x124
>msm_pdev_probe+0x294/0x2b4
>platform_drv_probe+0x58/0xa4
>really_probe+0x150/0x294
>driver_probe_device+0xac/0xe8
>__device_attach_driver+0xa4/0xb4
>bus_for_each_drv+0x98/0xc8
>__device_attach+0xac/0x12c
>device_initial_probe+0x24/0x30
>bus_probe_device+0x38/0x98
>deferred_probe_work_func+0x78/0xa4
>process_one_work+0x24c/0x3dc
>worker_thread+0x280/0x360
>kthread+0x134/0x13c
>ret_from_fork+0x10/0x18
> Code: d284 91000725 6b17039f 5400048a (f9401f40)
> ---[ end trace f22dda57f3648e2c ]---
> Kernel panic - not syncing: Fatal exception
> SMP: stopping secondary CPUs
> Kernel Offset: disabled
> CPU features: 0x0,22802a18
> Memory Limit: none
>
> The problem is that when drm/msm does it's own iommu_attach_device(),
> now the domain returned by iommu_get_domain_for_dev() is drm/msm's
> domain, and it doesn't have domain->iova_cookie.
>
> We kind of avoided this problem prior to sdm845/dpu because the iommu
> was attached to the mdp node in dt, which is a child of the toplevel
> mdss node (which corresponds to the dev passed in dma_map_sg()). But
> with sdm845, now the iommu is attached at the mdss level so we hit the
> iommu_dma_ops in dma_map_sg().
>
> But auto allocating/attaching a domain before the driver is probed was
> already a blocking problem for enabling per-context pagetables for the
> GPU. This problem is also now solved with this patch.
>
> Fixes: 97890ba9289c dma-mapping: detect and configure IOMMU in
> of_dma_configure
> Tested-by: Douglas Anderson
> Signed-off-by: Rob Clark
> ---
> This is an alternative/replacement for [1]. What it lacks in elegance
> it makes up for in practicality ;-)
>
> [1] https://patchwork.freedesktop.org/patch/264930/
>
> drivers/of/device.c | 22 ++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 5957cd4fa262..15ffee00fb22 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -72,6 +72,14 @@ int of_device_add(struct platform_device *ofdev)
> return device_add(>dev);
> }
>
> +static const struct of_device_id iommu_blacklist[] = {
> + { .compatible = "qcom,mdp4" },
> + { .compatible = "qcom,mdss" },
> + { .compatible = "qcom,sdm845-mdss" },
> + { .compatible = "qcom,adreno" },
> + {}
> +};
Not completely clear to whether this is still needed or not, but this
really won't scale. Why can't the driver for these devices override
whatever has been setup by default?
Rob
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [RESEND PATCH v3] drm/msm: Move fence put to where failure occurs
On Tue, Dec 4, 2018 at 11:56 AM Robert Foss wrote:
>
> If dma_fence_wait fails to wait for a supplied in-fence in
> msm_ioctl_gem_submit, make sure we release that in-fence.
>
> Also remove this dma_fence_put() from the 'out' label.
>
> Signed-off-by: Robert Foss
> Reviewed-by: Chris Wilson
> Cc: sta...@vger.kernel.org
Fyi, this is queued up in msm-next/fixes
BR,
-R
> ---
> drivers/gpu/drm/msm/msm_gem_submit.c | 15 ---
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index a90aedd6883a..d5e6665a4c8f 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -411,7 +411,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> *data,
> struct msm_file_private *ctx = file->driver_priv;
> struct msm_gem_submit *submit;
> struct msm_gpu *gpu = priv->gpu;
> - struct dma_fence *in_fence = NULL;
> struct sync_file *sync_file = NULL;
> struct msm_gpu_submitqueue *queue;
> struct msm_ringbuffer *ring;
> @@ -444,6 +443,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> *data,
> ring = gpu->rb[queue->prio];
>
> if (args->flags & MSM_SUBMIT_FENCE_FD_IN) {
> + struct dma_fence *in_fence;
> +
> in_fence = sync_file_get_fence(args->fence_fd);
>
> if (!in_fence)
> @@ -453,11 +454,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> *data,
> * Wait if the fence is from a foreign context, or if the
> fence
> * array contains any fence from a foreign context.
> */
> - if (!dma_fence_match_context(in_fence, ring->fctx->context)) {
> + ret = 0;
> + if (!dma_fence_match_context(in_fence, ring->fctx->context))
> ret = dma_fence_wait(in_fence, true);
> - if (ret)
> - return ret;
> - }
> +
> + dma_fence_put(in_fence);
> + if (ret)
> + return ret;
> }
>
> ret = mutex_lock_interruptible(>struct_mutex);
> @@ -583,8 +586,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> *data,
> }
>
> out:
> - if (in_fence)
> - dma_fence_put(in_fence);
> submit_cleanup(submit);
> if (ret)
> msm_gem_submit_free(submit);
> --
> 2.17.1
>
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] drm/msm: Only add available components
On Tue, Dec 4, 2018 at 1:04 PM Douglas Anderson wrote: > > When trying to get the display up on my sdm845 board I noticed that > the display wouldn't probe if I had the dsi1 node marked as "disabled" > even though my board doesn't use dsi1. It looks like the msm code > adds all nodes to its list of components even if they are disabled. I > believe this doesn't work because all registered components need to > come up before we finish probing. Let's do like other DRM code and > only add available components. > > Signed-off-by: Douglas Anderson yeah, that seems like a reasonable thing to do Reviewed-by: Rob Clark > --- > > drivers/gpu/drm/msm/msm_drv.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index b1577e960889..0b828822117b 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -1190,8 +1190,10 @@ static int add_components_mdp(struct device *mdp_dev, > if (!intf) > continue; > > - drm_of_component_match_add(master_dev, matchptr, compare_of, > - intf); > + if (of_device_is_available(intf)) > + drm_of_component_match_add(master_dev, matchptr, > + compare_of, intf); > + > of_node_put(intf); > } > > -- > 2.20.0.rc1.387.gf8505762e3-goog > ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions
On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote: > On 03.12.2018 22:38, Ville Syrjälä wrote: > > On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote: > >> On 21.11.2018 19:19, Laurent Pinchart wrote: > >>> Hi Ville, > >>> > >>> Thank you for the patch. > >>> > >>> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote: > From: Ville Syrjälä > > Make life easier for drivers by simply passing the connector > to drm_hdmi_avi_infoframe_from_display_mode() and > drm_hdmi_avi_infoframe_quant_range(). That way drivers don't > need to worry about is_hdmi2_sink mess. > >>> While this is good for display controller drivers, the change isn't great > >>> for > >>> bridge drivers. Down the road we're looking at moving connector support > >>> out of > >>> the bridge drivers. Adding an additional dependency to connectors in the > >>> bridges will make that more difficult. Ideally bridges should retrieve > >>> the > >>> information from their sink, regardless of whether it is a connector or > >>> another bridge. > >> > >> I agree with it, and case of sii8620 shows that there are cases where > >> bridge has no direct access to the connector. > > It's just a matter of plumbing it through. > > > What do you mean exactly? void bridge_foo(... + ,struct drm_connector *connector); > > > > > >> On the other side, since you are passing connector to > >> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode > >> parameter and rename the function to > >> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and > >> mode set on the connector differs? > > Connectors don't have a mode. > > > As they are passing video stream they should have it, even if not > directly, for example: > > connector->state->crtc->mode That's not really how atomic works. One shouldn't go digging through the obj->state pointers when we're not holding the relevant locks anymore. The atomic way would be to pass either both crtc state and connector state, or drm_atomic_state + crtc/connector. > > In moment of creating infoframe it should be set properly. > > > Regards > > Andrzej > > > > > >> > >> Regards > >> > >> Andrzej > >> > >> > >>> Please see below for an additional comment. > >>> > Cc: Alex Deucher > Cc: "Christian König" > Cc: "David (ChunMing) Zhou" > Cc: Archit Taneja > Cc: Andrzej Hajda > Cc: Laurent Pinchart > Cc: Inki Dae > Cc: Joonyoung Shim > Cc: Seung-Woo Kim > Cc: Kyungmin Park > Cc: Russell King > Cc: CK Hu > Cc: Philipp Zabel > Cc: Rob Clark > Cc: Ben Skeggs > Cc: Tomi Valkeinen > Cc: Sandy Huang > Cc: "Heiko Stübner" > Cc: Benjamin Gaignard > Cc: Vincent Abriou > Cc: Thierry Reding > Cc: Eric Anholt > Cc: Shawn Guo > Cc: Ilia Mirkin > Cc: amd-...@lists.freedesktop.org > Cc: linux-arm-...@vger.kernel.org > Cc: freedreno@lists.freedesktop.org > Cc: nouv...@lists.freedesktop.org > Cc: linux-te...@vger.kernel.org > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 3 ++- > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 ++-- > drivers/gpu/drm/bridge/sii902x.c | 3 ++- > drivers/gpu/drm/bridge/sil-sii8620.c | 3 +-- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 ++- > drivers/gpu/drm/drm_edid.c| 33 ++- > drivers/gpu/drm/exynos/exynos_hdmi.c | 3 ++- > drivers/gpu/drm/i2c/tda998x_drv.c | 3 ++- > drivers/gpu/drm/i915/intel_hdmi.c | 14 +- > drivers/gpu/drm/i915/intel_lspcon.c | 15 ++- > drivers/gpu/drm/i915/intel_sdvo.c | 10 --- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 3 ++- > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c| 3 ++- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 7 +++-- > drivers/gpu/drm/omapdrm/omap_encoder.c| 5 ++-- > drivers/gpu/drm/radeon/radeon_audio.c | 2 +- > drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++- > drivers/gpu/drm/sti/sti_hdmi.c| 3 ++- > drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c| 3 ++- > drivers/gpu/drm/tegra/hdmi.c | 3 ++- > drivers/gpu/drm/tegra/sor.c | 3 ++- > drivers/gpu/drm/vc4/vc4_hdmi.c| 11 +--- > drivers/gpu/drm/zte/zx_hdmi.c | 4 ++- > include/drm/drm_edid.h| 8 +++--- > 27 files changed, 94 insertions(+), 66 deletions(-) > >>> For dw-hdmi and omapdrm, > >>> > >>> Reviewed-by: Laurent Pinchart > >>> -- Ville Syrjälä Intel
Re: [Freedreno] [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions
On Tue, Dec 04, 2018 at 08:03:53AM +0100, Andrzej Hajda wrote:
> On 03.12.2018 22:48, Ville Syrjälä wrote:
> > On Thu, Nov 29, 2018 at 09:46:16AM +0100, Andrzej Hajda wrote:
> >> Quite late, hopefully not too late.
> >>
> >>
> >> On 21.11.2018 12:51, Ville Syrjälä wrote:
> >>> On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
> > return;
> > diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> > b/drivers/gpu/drm/bridge/sil-sii8620.c
> > index a6e8f4591e63..0cc293a6ac24 100644
> > --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> > +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> > @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620
> > *ctx,
> > int ret;
> >
> > ret = drm_hdmi_avi_infoframe_from_display_mode(,
> > - mode,
> > - true);
> > + NULL, mode);
> > if (ctx->use_packed_pixel)
> > frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index 64c3cf027518..88b720b63126 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi,
> > struct drm_display_mode *mode)
> > u8 val;
> >
> > /* Initialise info frame from DRM mode */
> > - drm_hdmi_avi_infoframe_from_display_mode(, mode, false);
> > + drm_hdmi_avi_infoframe_from_display_mode(,
> > +>connector,
> > mode);
> >
> > if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
> > frame.colorspace = HDMI_COLORSPACE_YUV444;
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index b506e3622b08..501ac05ba7da 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct
> > drm_connector *connector,
> > }
> > EXPORT_SYMBOL(drm_set_preferred_mode);
> >
> > +static bool is_hdmi2_sink(struct drm_connector *connector)
> You're usually known for adding const all around, why not const pointer
> here and in all the other drm_* functions that call this?
> >>> My current approach is to constify states/fbs/etc. but not so much
> >>> crtcs/connectors/etc. Too much const can sometimes get in the way
> >>> of things requiring that you remove the const later. But I guess
> >>> in this case the const shouldn't really get in the way of anything
> >>> because these are pretty much supposed to be pure functions.
> >>>
> > +{
> > + /*
> > +* FIXME: sil-sii8620 doesn't have a connector around when
> > +* we need one, so we have to be prepared for a NULL connector.
> > +*/
> > + if (!connector)
> > + return false;
> This actually changes the is_hdmi2_sink value for sil-sii8620.
> >>> Hmm. No idea why they would have set that to true when everyone else is
> >>> passing false.
> >>
> >> Because false does not work :) More precisely MHLv3 (used in Sii8620)
> >> uses CTA-861-F standard for infoframes, which is specific to HDMI2.0.
> >>
> >> Unfortunately I have no access to MHL specs, but my experiments and
> >> vendor drivers strongly suggests it is done this way.
> >>
> >> This is important in case of 4K modes which are handled differently by
> >> HDMI 1.4 and HDMI2.0.
> > HDMI 2.0 handles 4k just like 1.4 handled it when you use one of
> > the 4k modes defined in 1.4. Only if you use features beyond 1.4 do we
> > switch over to the HDMI 2.0 specific signalling.
>
>
> The difference is in infoframes:
>
> HDMI 1.4 sets AVI infoframe VIC to 0, and sends HDMI_VIC in VSI.
>
> HDMI 2.0 sets AVI infoframe to non zero VICs introduced by
> HDMI2.0/CEA-861-F, VSI can be omitted if I remember correctly, unless 3d
> is in use.
Like I said, The HDMI 1.4 method is used even with HDMI 2.0 sinks unless
some feature gets used which can't be signalled via the HDMI 1.4 vendor
specific infoframe.
>
>
> So setting VICs to non-zero in case of HDMI1.4 sinks and 4k modes seems
> risky.
That is not what I was proposing.
>
>
> Regards
>
> Andrzej
>
>
> >
> >> The pipeline looks like (in parenthesis HDMI version on the stream):
> >>
> >> exynos_hdmi --(1.4)--> SII8620 --(2.0)--> MHL_dongle --(1.4)--> TV
> >>
> >>
> >>> I guess I can change this to true to not change it. IIRC
> >>> that was the only driver that didn't have a connector around.
> >>>
> >>> That said, I was actually thinking of removing this hdmi2 vs. not
> >>> stuff from
Re: [Freedreno] [PATCH v3 2/8] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT
Quoting Matthias Kaehlcke (2018-12-04 09:35:49)
> On Tue, Dec 04, 2018 at 08:44:00AM -0800, Stephen Boyd wrote:
> > Quoting Matthias Kaehlcke (2018-11-30 16:52:48)
> > > +
> > > /* custom byte clock divider */
> > > struct clk_bytediv *bytediv;
> > >
> > > @@ -125,7 +127,10 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw
> > > *hw, unsigned long rate,
> > > DBG("rate=%lu, parent's=%lu", rate, parent_rate);
> > >
> > > temp = rate / 10;
> > > - val = VCO_REF_CLK_RATE / 10;
> > > + if (parent_rate)
> > > + val = parent_rate / 10;
> > > + else
> > > + val = VCO_REF_CLK_DEFAULT_RATE / 10;
> >
> > Is the clk not properly hooked up to a parent sometimes so parent_rate
> > is 0? That sounds odd given the fact that it used to be 'pxo' and that
> > has always existed on the system as 27 MHz. So I'd remove this and just
> > use parent_rate all the time.
>
> I wondered about this, but since I don't have hardware for testing I
> kept the previous hardcoded rate. If we know for sure that 'pxo'
> always exists it should indeed be fine to use the parent rate.
Yes we know for sure. The 'pxo' board clk is there on apq8064 dtsi file
which seems to be the only place this is used. The pxo_board clk is sent
through a 'pxo' clk that's created in drivers/clk/qcom/common.c
qcom_cc_register_board_clk().
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v5 1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file
Hi,
On Mon, Dec 3, 2018 at 6:41 PM Jeykumar Sankaran wrote:
> >> + dsi1: dsi@ae96000 {
> >> + compatible = "qcom,mdss-dsi-ctrl";
> >> + reg = <0xae96000 0x400>;
> >> + reg-names = "dsi_ctrl";
> >> +
> >> + interrupt-parent = <>;
> >> + interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> >> +
> >> + clocks = <
> >> DISP_CC_MDSS_BYTE1_CLK>,
> >> +<
> >> DISP_CC_MDSS_BYTE1_INTF_CLK>,
> >> +<
> >> DISP_CC_MDSS_PCLK1_CLK>,
> >> +<
> >> DISP_CC_MDSS_ESC1_CLK>,
> >> +<
> >> DISP_CC_MDSS_AHB_CLK>,
> >> +<
> >> DISP_CC_MDSS_AXI_CLK>;
> >> + clock-names = "byte",
> >> + "byte_intf",
> >> + "pixel",
> >> + "core",
> >> + "iface",
> >> + "bus";
> >> +
> >> + phys = <_phy>;
> >> + phy-names = "dsi1";
> >> +
> >> + status = "disabled";
> >
> > This "disabled" is causing me problems. I don't actually need "dsi1"
> > but if I don't enable "dsi1" then my display doesn't come up. :( I
> > ran out of time to debug but I wonder if this is this the standard
> > thing where DRM needs to wait for all the components to probe until it
> > can finish? If nobody on this list just knows I'll dig tomorrow and
> > confirm that my memory isn't faulty and see what we've done about this
> > in the past.
> >
> https://patchwork.kernel.org/patch/10467895/
>
> Can you try out with this change (reviewed but not merged yet). It
> validates
> the nodes before adding to the DSI list.
No, that doesn't fix it. I also don't see your printout.
OK, found the problem and posted a patch. See
.
Please test and review if you are able.
> > One last note: it's pretty weird that you sent out only 1/3 and not
> > 2/3 and 3/3. If you're not ready to send out MTP stuff yet then you
> > should send out v6 as just a singleton patch.
> Yes. I was trying to separate this one out as an independent change.
> Sandeep
> is working on the comments on removing the pinctrl nodes and updated
> mtp nodes. He should be posting 2/3 and 3/3 in the next couple of days.
OK, good to know. Probably 2/3 and 3/3 will be squashed anyway since
(as I suggested in the review) they should both be touching the MTP
device tree file.
...in any case, you should send yours out as a singleton patch and
then you don't have to guess how many patches might or might not be
sent out later. Sandeep can send out his patch and say it depends on
yours.
-Doug
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH] drm/msm: Only add available components
When trying to get the display up on my sdm845 board I noticed that the display wouldn't probe if I had the dsi1 node marked as "disabled" even though my board doesn't use dsi1. It looks like the msm code adds all nodes to its list of components even if they are disabled. I believe this doesn't work because all registered components need to come up before we finish probing. Let's do like other DRM code and only add available components. Signed-off-by: Douglas Anderson --- drivers/gpu/drm/msm/msm_drv.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index b1577e960889..0b828822117b 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1190,8 +1190,10 @@ static int add_components_mdp(struct device *mdp_dev, if (!intf) continue; - drm_of_component_match_add(master_dev, matchptr, compare_of, - intf); + if (of_device_is_available(intf)) + drm_of_component_match_add(master_dev, matchptr, + compare_of, intf); + of_node_put(intf); } -- 2.20.0.rc1.387.gf8505762e3-goog ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 2/8] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT
On Tue, Dec 04, 2018 at 08:44:00AM -0800, Stephen Boyd wrote:
> Quoting Matthias Kaehlcke (2018-11-30 16:52:48)
> > Get the ref clock of the PHY from the device tree instead of
> > hardcoding its name and rate. Use default values if the ref
> > clock is not specified.
> >
> > Signed-off-by: Matthias Kaehlcke
> > ---
> > Changes in v3:
> > - use default name and rate if the ref clock is not specified
> > in the DT
> > - store vco_ref_clk_name instead of vco_ref_clk
> > - fixed check for EPROBE_DEFER
> > - renamed VCO_REF_CLK_RATE to VCO_REF_CLK_DEFAULT_RATE
> >
> > Changes in v2:
> > - patch added to the series
> > ---
> > .../gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c | 28 +++
> > 1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> > b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> > index 49008451085b8..3af678d3317f6 100644
> > --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> > +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> > @@ -47,9 +47,9 @@
> >
> > #define NUM_PROVIDED_CLKS 2
> >
> > -#define VCO_REF_CLK_RATE 2700
> > -#define VCO_MIN_RATE 6
> > -#define VCO_MAX_RATE 12
> > +#define VCO_REF_CLK_DEFAULT_RATE 2700
> > +#define VCO_MIN_RATE 6
> > +#define VCO_MAX_RATE 12
> >
> > #define DSI_BYTE_PLL_CLK 0
> > #define DSI_PIXEL_PLL_CLK 1
> > @@ -75,6 +75,8 @@ struct dsi_pll_28nm {
> > struct platform_device *pdev;
> > void __iomem *mmio;
> >
> > + const char *vco_ref_clk_name;
>
> Can this be passed around during clk registration so we don't have to
> store it away in the structure?
makes sense, will do
> > +
> > /* custom byte clock divider */
> > struct clk_bytediv *bytediv;
> >
> > @@ -125,7 +127,10 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw
> > *hw, unsigned long rate,
> > DBG("rate=%lu, parent's=%lu", rate, parent_rate);
> >
> > temp = rate / 10;
> > - val = VCO_REF_CLK_RATE / 10;
> > + if (parent_rate)
> > + val = parent_rate / 10;
> > + else
> > + val = VCO_REF_CLK_DEFAULT_RATE / 10;
>
> Is the clk not properly hooked up to a parent sometimes so parent_rate
> is 0? That sounds odd given the fact that it used to be 'pxo' and that
> has always existed on the system as 27 MHz. So I'd remove this and just
> use parent_rate all the time.
I wondered about this, but since I don't have hardware for testing I
kept the previous hardcoded rate. If we know for sure that 'pxo'
always exists it should indeed be fine to use the parent rate.
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 8/8] ARM: dts: qcom-apq8064: Set 'xo_board' as ref clock of the DSI PHY
On Tue, Dec 04, 2018 at 08:48:22AM -0800, Stephen Boyd wrote: > Quoting Matthias Kaehlcke (2018-11-30 16:52:54) > > Add 'xo_board' as ref clock for the DSI PHY, it was previously > > hardcoded in the PLL 'driver' for the 28nm 8960 PHY. > > Why is driver in quotes? It's not really a full fledged driver, but part of the 28nm 8960 PHY driver. > > > > Signed-off-by: Matthias Kaehlcke > > Reviewed-by: Stephen Boyd Thanks for the review! ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 1/8] dt-bindings: msm/dsi: Add ref clock for PHYs
Hi, On Fri, Nov 30, 2018 at 4:53 PM Matthias Kaehlcke wrote: > > Allow the PHY drivers to get the ref clock from the DT. > > Signed-off-by: Matthias Kaehlcke > --- > Changes in V3: > - added note that the ref clock is only required for new DTS > files/entries > > Changes in v2: > - add the ref clock for all PHYs, not only the 10nm one > - updated commit message > --- > Documentation/devicetree/bindings/display/msm/dsi.txt | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Douglas Anderson ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [RESEND PATCH v3] drm/msm: Move fence put to where failure occurs
If dma_fence_wait fails to wait for a supplied in-fence in
msm_ioctl_gem_submit, make sure we release that in-fence.
Also remove this dma_fence_put() from the 'out' label.
Signed-off-by: Robert Foss
Reviewed-by: Chris Wilson
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/msm/msm_gem_submit.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index a90aedd6883a..d5e6665a4c8f 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -411,7 +411,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
struct msm_file_private *ctx = file->driver_priv;
struct msm_gem_submit *submit;
struct msm_gpu *gpu = priv->gpu;
- struct dma_fence *in_fence = NULL;
struct sync_file *sync_file = NULL;
struct msm_gpu_submitqueue *queue;
struct msm_ringbuffer *ring;
@@ -444,6 +443,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
ring = gpu->rb[queue->prio];
if (args->flags & MSM_SUBMIT_FENCE_FD_IN) {
+ struct dma_fence *in_fence;
+
in_fence = sync_file_get_fence(args->fence_fd);
if (!in_fence)
@@ -453,11 +454,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
*data,
* Wait if the fence is from a foreign context, or if the fence
* array contains any fence from a foreign context.
*/
- if (!dma_fence_match_context(in_fence, ring->fctx->context)) {
+ ret = 0;
+ if (!dma_fence_match_context(in_fence, ring->fctx->context))
ret = dma_fence_wait(in_fence, true);
- if (ret)
- return ret;
- }
+
+ dma_fence_put(in_fence);
+ if (ret)
+ return ret;
}
ret = mutex_lock_interruptible(>struct_mutex);
@@ -583,8 +586,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
}
out:
- if (in_fence)
- dma_fence_put(in_fence);
submit_cleanup(submit);
if (ret)
msm_gem_submit_free(submit);
--
2.17.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 2/8] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT
Quoting Matthias Kaehlcke (2018-11-30 16:52:48)
> Get the ref clock of the PHY from the device tree instead of
> hardcoding its name and rate. Use default values if the ref
> clock is not specified.
>
> Signed-off-by: Matthias Kaehlcke
> ---
> Changes in v3:
> - use default name and rate if the ref clock is not specified
> in the DT
> - store vco_ref_clk_name instead of vco_ref_clk
> - fixed check for EPROBE_DEFER
> - renamed VCO_REF_CLK_RATE to VCO_REF_CLK_DEFAULT_RATE
>
> Changes in v2:
> - patch added to the series
> ---
> .../gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c | 28 +++
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> index 49008451085b8..3af678d3317f6 100644
> --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> @@ -47,9 +47,9 @@
>
> #define NUM_PROVIDED_CLKS 2
>
> -#define VCO_REF_CLK_RATE 2700
> -#define VCO_MIN_RATE 6
> -#define VCO_MAX_RATE 12
> +#define VCO_REF_CLK_DEFAULT_RATE 2700
> +#define VCO_MIN_RATE 6
> +#define VCO_MAX_RATE 12
>
> #define DSI_BYTE_PLL_CLK 0
> #define DSI_PIXEL_PLL_CLK 1
> @@ -75,6 +75,8 @@ struct dsi_pll_28nm {
> struct platform_device *pdev;
> void __iomem *mmio;
>
> + const char *vco_ref_clk_name;
Can this be passed around during clk registration so we don't have to
store it away in the structure?
> +
> /* custom byte clock divider */
> struct clk_bytediv *bytediv;
>
> @@ -125,7 +127,10 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw,
> unsigned long rate,
> DBG("rate=%lu, parent's=%lu", rate, parent_rate);
>
> temp = rate / 10;
> - val = VCO_REF_CLK_RATE / 10;
> + if (parent_rate)
> + val = parent_rate / 10;
> + else
> + val = VCO_REF_CLK_DEFAULT_RATE / 10;
Is the clk not properly hooked up to a parent sometimes so parent_rate
is 0? That sounds odd given the fact that it used to be 'pxo' and that
has always existed on the system as 27 MHz. So I'd remove this and just
use parent_rate all the time.
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 8/8] ARM: dts: qcom-apq8064: Set 'xo_board' as ref clock of the DSI PHY
Quoting Matthias Kaehlcke (2018-11-30 16:52:54) > Add 'xo_board' as ref clock for the DSI PHY, it was previously > hardcoded in the PLL 'driver' for the 28nm 8960 PHY. Why is driver in quotes? > > Signed-off-by: Matthias Kaehlcke Reviewed-by: Stephen Boyd ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 7/8] arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs
Quoting Matthias Kaehlcke (2018-11-30 16:52:53) > Add 'bi_tcxo' as ref clock for the DSI PHYs, it was previously > hardcoded in the PLL 'driver' for the 10nm PHY. > > Signed-off-by: Matthias Kaehlcke > Reviewed-by: Douglas Anderson > --- Reviewed-by: Stephen Boyd ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 3/8] drm/msm/dsi: 28nm PHY: Get ref clock from the DT
Quoting Matthias Kaehlcke (2018-11-30 16:52:49)
> diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
> b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
> index 26e3a01a99c2b..4a84c69ca0b2b 100644
> --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
> +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
> @@ -81,6 +81,7 @@ struct dsi_pll_28nm {
> struct platform_device *pdev;
> void __iomem *mmio;
>
> + const char *vco_ref_clk_name;
> int vco_delay;
>
> /* private clocks: */
> @@ -265,7 +268,8 @@ static unsigned long dsi_pll_28nm_clk_recalc_rate(struct
> clk_hw *hw,
> void __iomem *base = pll_28nm->mmio;
> u32 sdm0, doubler, sdm_byp_div;
> u32 sdm_dc_off, sdm_freq_seed, sdm2, sdm3;
> - u32 ref_clk = VCO_REF_CLK_RATE;
> + u32 ref_clk = parent_rate ?
> + parent_rate : VCO_REF_CLK_DEFAULT_RATE;
Same comments apply here.
> unsigned long vco_rate;
>
> VERB("parent_rate=%lu", parent_rate);
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 6/8] arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY
Quoting Matthias Kaehlcke (2018-11-30 16:52:52) > Add 'xo_board' as ref clock for the DSI PHYs, it was previously > hardcoded in the PLL 'driver' for the 28nm PHY. > > Signed-off-by: Matthias Kaehlcke > Reviewed-by: Douglas Anderson > --- Reviewed-by: Stephen Boyd ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 1/8] dt-bindings: msm/dsi: Add ref clock for PHYs
Quoting Matthias Kaehlcke (2018-11-30 16:52:47) > Allow the PHY drivers to get the ref clock from the DT. > > Signed-off-by: Matthias Kaehlcke > --- Reviewed-by: Stephen Boyd ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] drm/msm: dpu: Allocate proper amount for dpu_crtc_state
On Tue, Dec 04, 2018 at 10:51:42AM -0500, Bruce Wang wrote:
> On Mon, Dec 3, 2018 at 2:56 PM Sean Paul wrote:
> >
> > From: Sean Paul
> >
> > Since dpu_crtc subclasses crtc_state, we need a custom .reset hook in
> > order to allocate the right amount of memory to accommodate the
> > additional struct members in dpu_crtc_state. So bring it [partially]
> > back.
> >
> > Relevant KASAN splat:
> > [ 10.82]
> > ==
> > [ 10.344288] BUG: KASAN: slab-out-of-bounds in kmemdup+0x50/0x80
> > [ 10.350390] Read of size 736 at addr ffc0d9f06080 by task frecon/394
> >
> > [ 10.358861] CPU: 6 PID: 394 Comm: frecon Tainted: GW
> > 4.19.4 #121
> > [ 10.366476] Hardware name: Google Cheza (rev2) (DT)
> > [ 10.371514] Call trace:
> > [ 10.374087] dump_backtrace+0x0/0x194
> > [ 10.377878] show_stack+0x20/0x28
> > [ 10.381330] dump_stack+0xa0/0xc8
> > [ 10.384783] print_address_description+0x78/0x2e0
> > [ 10.389639] kasan_report+0x290/0x2d0
> > [ 10.393428] check_memory_region+0x20/0x14c
> > [ 10.397740] __asan_loadN+0x14/0x1c
> > [ 10.401345] kmemdup+0x50/0x80
> > [ 10.404524] dpu_crtc_duplicate_state+0x58/0xa0
> > [ 10.409228] drm_atomic_get_crtc_state+0xac/0x178
> > [ 10.414095] __drm_atomic_helper_set_config+0x54/0x4a4
> > [ 10.419393] drm_atomic_helper_set_config+0x60/0xb4
> > [ 10.424435] drm_mode_setcrtc+0x720/0x760
> > [ 10.428570] drm_ioctl_kernel+0xd8/0x13c
> > [ 10.432617] drm_ioctl+0x380/0x4f4
> > [ 10.436150] drm_compat_ioctl+0x54/0x13c
> > [ 10.440219] __arm64_compat_sys_ioctl+0x1d8/0xef4
> > [ 10.445086] el0_svc_common+0xd8/0x138
> > [ 10.448961] el0_svc_compat_handler+0x58/0x68
> > [ 10.453463] el0_svc_compat+0x8/0x18
> >
> > [ 10.458712] Allocated by task 56:
> > [ 10.462148] kasan_kmalloc.part.4+0x48/0xf4
> > [ 10.466465] kasan_kmalloc+0x8c/0xa0
> > [ 10.470165] kmem_cache_alloc_trace+0x25c/0x27c
> > [ 10.474848] drm_atomic_helper_crtc_reset+0x68/0x98
> > [ 10.479877] drm_mode_config_reset+0xc4/0x19c
> > [ 10.484383] msm_drm_bind+0x814/0x8dc
> > [ 10.488169] try_to_bring_up_master.part.7+0x48/0xac
> > [ 10.493282] component_master_add_with_match+0x158/0x198
> > [ 10.498758] msm_pdev_probe+0x328/0x348
> > [ 10.502736] platform_drv_probe+0x74/0xc8
> > [ 10.506877] really_probe+0x1ac/0x35c
> > [ 10.510659] driver_probe_device+0xd4/0x118
> > [ 10.514975] __device_attach_driver+0xc8/0xf4
> > [ 10.519477] bus_for_each_drv+0xb4/0xe4
> > [ 10.523439] __device_attach+0xd0/0x158
> > [ 10.527394] device_initial_probe+0x24/0x30
> > [ 10.531715] bus_probe_device+0x50/0xe4
> > [ 10.535681] deferred_probe_work_func+0xac/0xdc
> > [ 10.540376] process_one_work+0x3f0/0x6d4
> > [ 10.544521] worker_thread+0x3f4/0x520
> > [ 10.548399] kthread+0x1b4/0x1c8
> > [ 10.551740] ret_from_fork+0x10/0x18
> >
> > [ 10.556986] Freed by task 0:
> > [ 10.559967] (stack is not available)
> >
> > [ 10.565216] The buggy address belongs to the object at ffc0d9f06080
> > which belongs to the cache kmalloc-1024 of size 1024
> > [ 10.578268] The buggy address is located 0 bytes inside of
> > 1024-byte region [ffc0d9f06080, ffc0d9f06480)
> > [ 10.590248] The buggy address belongs to the page:
> > [ 10.595195] page:ffbf0367c000 count:1 mapcount:0
> > mapping:ffc0de40f680 index:0x0 compound_mapcount: 0
> > [ 10.605321] flags: 0x40008100(slab|head)
> > [ 10.610100] raw: 40008100 ffbf0369fa08 ffbf0367f008
> > ffc0de40f680
> > [ 10.618077] raw: 00150015 0001
> >
> > [ 10.626049] page dumped because: kasan: bad access detected
> >
> > [ 10.633341] Memory state around the buggy address:
> > [ 10.638282] ffc0d9f06180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 00 00
> > [ 10.645710] ffc0d9f06200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 00 00
> > [ 10.653139] >ffc0d9f06280: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
> > fc fc
> > [ 10.660571] ^
> > [ 10.665774] ffc0d9f06300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > fc fc
> > [ 10.673210] ffc0d9f06380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > fc fc
> > [ 10.680639]
> > ==
> >
> > Fixes: a6ba45afda41 ("drm/msm/dpu: Replace dpu_crtc_reset by atomic helper")
> > Cc: Sean Paul
> > Cc: Bruce Wang
> > Cc: Rob Clark
> > Signed-off-by: Sean Paul
>
> Reviewed-by: Bruce Wang
Thanks for the review, this has been pushed to dpu-staging/for-next
Sean
>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 14 +-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >
Re: [Freedreno] [PATCH] drm/msm: dpu: Allocate proper amount for dpu_crtc_state
On Mon, Dec 3, 2018 at 2:56 PM Sean Paul wrote:
>
> From: Sean Paul
>
> Since dpu_crtc subclasses crtc_state, we need a custom .reset hook in
> order to allocate the right amount of memory to accommodate the
> additional struct members in dpu_crtc_state. So bring it [partially]
> back.
>
> Relevant KASAN splat:
> [ 10.82]
> ==
> [ 10.344288] BUG: KASAN: slab-out-of-bounds in kmemdup+0x50/0x80
> [ 10.350390] Read of size 736 at addr ffc0d9f06080 by task frecon/394
>
> [ 10.358861] CPU: 6 PID: 394 Comm: frecon Tainted: GW
> 4.19.4 #121
> [ 10.366476] Hardware name: Google Cheza (rev2) (DT)
> [ 10.371514] Call trace:
> [ 10.374087] dump_backtrace+0x0/0x194
> [ 10.377878] show_stack+0x20/0x28
> [ 10.381330] dump_stack+0xa0/0xc8
> [ 10.384783] print_address_description+0x78/0x2e0
> [ 10.389639] kasan_report+0x290/0x2d0
> [ 10.393428] check_memory_region+0x20/0x14c
> [ 10.397740] __asan_loadN+0x14/0x1c
> [ 10.401345] kmemdup+0x50/0x80
> [ 10.404524] dpu_crtc_duplicate_state+0x58/0xa0
> [ 10.409228] drm_atomic_get_crtc_state+0xac/0x178
> [ 10.414095] __drm_atomic_helper_set_config+0x54/0x4a4
> [ 10.419393] drm_atomic_helper_set_config+0x60/0xb4
> [ 10.424435] drm_mode_setcrtc+0x720/0x760
> [ 10.428570] drm_ioctl_kernel+0xd8/0x13c
> [ 10.432617] drm_ioctl+0x380/0x4f4
> [ 10.436150] drm_compat_ioctl+0x54/0x13c
> [ 10.440219] __arm64_compat_sys_ioctl+0x1d8/0xef4
> [ 10.445086] el0_svc_common+0xd8/0x138
> [ 10.448961] el0_svc_compat_handler+0x58/0x68
> [ 10.453463] el0_svc_compat+0x8/0x18
>
> [ 10.458712] Allocated by task 56:
> [ 10.462148] kasan_kmalloc.part.4+0x48/0xf4
> [ 10.466465] kasan_kmalloc+0x8c/0xa0
> [ 10.470165] kmem_cache_alloc_trace+0x25c/0x27c
> [ 10.474848] drm_atomic_helper_crtc_reset+0x68/0x98
> [ 10.479877] drm_mode_config_reset+0xc4/0x19c
> [ 10.484383] msm_drm_bind+0x814/0x8dc
> [ 10.488169] try_to_bring_up_master.part.7+0x48/0xac
> [ 10.493282] component_master_add_with_match+0x158/0x198
> [ 10.498758] msm_pdev_probe+0x328/0x348
> [ 10.502736] platform_drv_probe+0x74/0xc8
> [ 10.506877] really_probe+0x1ac/0x35c
> [ 10.510659] driver_probe_device+0xd4/0x118
> [ 10.514975] __device_attach_driver+0xc8/0xf4
> [ 10.519477] bus_for_each_drv+0xb4/0xe4
> [ 10.523439] __device_attach+0xd0/0x158
> [ 10.527394] device_initial_probe+0x24/0x30
> [ 10.531715] bus_probe_device+0x50/0xe4
> [ 10.535681] deferred_probe_work_func+0xac/0xdc
> [ 10.540376] process_one_work+0x3f0/0x6d4
> [ 10.544521] worker_thread+0x3f4/0x520
> [ 10.548399] kthread+0x1b4/0x1c8
> [ 10.551740] ret_from_fork+0x10/0x18
>
> [ 10.556986] Freed by task 0:
> [ 10.559967] (stack is not available)
>
> [ 10.565216] The buggy address belongs to the object at ffc0d9f06080
> which belongs to the cache kmalloc-1024 of size 1024
> [ 10.578268] The buggy address is located 0 bytes inside of
> 1024-byte region [ffc0d9f06080, ffc0d9f06480)
> [ 10.590248] The buggy address belongs to the page:
> [ 10.595195] page:ffbf0367c000 count:1 mapcount:0
> mapping:ffc0de40f680 index:0x0 compound_mapcount: 0
> [ 10.605321] flags: 0x40008100(slab|head)
> [ 10.610100] raw: 40008100 ffbf0369fa08 ffbf0367f008
> ffc0de40f680
> [ 10.618077] raw: 00150015 0001
>
> [ 10.626049] page dumped because: kasan: bad access detected
>
> [ 10.633341] Memory state around the buggy address:
> [ 10.638282] ffc0d9f06180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00
> [ 10.645710] ffc0d9f06200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00
> [ 10.653139] >ffc0d9f06280: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
> fc fc
> [ 10.660571] ^
> [ 10.665774] ffc0d9f06300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> fc fc
> [ 10.673210] ffc0d9f06380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> fc fc
> [ 10.680639]
> ==
>
> Fixes: a6ba45afda41 ("drm/msm/dpu: Replace dpu_crtc_reset by atomic helper")
> Cc: Sean Paul
> Cc: Bruce Wang
> Cc: Rob Clark
> Signed-off-by: Sean Paul
Reviewed-by: Bruce Wang
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 1e3e57817b72b..d27ea2a95f85b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -819,6 +819,18 @@ static void _dpu_crtc_vblank_enable_no_lock(
> }
> }
>
> +static void dpu_crtc_reset(struct drm_crtc *crtc)
> +{
> + struct dpu_crtc_state *cstate;
> +
> +
[Freedreno] [PATCH v4 4/5] ARM: dts: imx5: add gpu nodes
This adds the gpu nodes for the adreno 200 GPU on iMX51 and iMX53, now
supported by the freedreno driver.
The compatible for the iMX51 uses a patchid of 1, which is used by drm/msm
driver to identify the smaller 128KiB GMEM size.
Signed-off-by: Jonathan Marek
---
v4:
added commit message
moved to follow address order
removed frequencies, they are not needed as clocks have fixed rate
arch/arm/boot/dts/imx51.dtsi | 10 ++
arch/arm/boot/dts/imx53.dtsi | 10 ++
2 files changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 67d462715..14dcc5a41 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -123,6 +123,16 @@
reg = <0x1ffe 0x2>;
};
+ gpu: gpu@3000 {
+ compatible = "amd,imageon-200.1", "amd,imageon";
+ reg = <0x3000 0x2>;
+ reg-names = "kgsl_3d0_reg_memory";
+ interrupts = <12>;
+ interrupt-names = "kgsl_3d0_irq";
+ clocks = < IMX5_CLK_GPU3D_GATE>, <
IMX5_CLK_GARB_GATE>;
+ clock-names = "core_clk", "mem_iface_clk";
+ };
+
ipu: ipu@4000 {
#address-cells = <1>;
#size-cells = <0>;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 207eb557c..d7d5fe4c7 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -209,6 +209,16 @@
};
};
+ gpu: gpu@3000 {
+ compatible = "amd,imageon-200.0", "amd,imageon";
+ reg = <0x3000 0x2>;
+ reg-names = "kgsl_3d0_reg_memory";
+ interrupts = <12>;
+ interrupt-names = "kgsl_3d0_irq";
+ clocks = < IMX5_CLK_GPU3D_GATE>, <
IMX5_CLK_GARB_GATE>;
+ clock-names = "core_clk", "mem_iface_clk";
+ };
+
aips@5000 { /* AIPS1 */
compatible = "fsl,aips-bus", "simple-bus";
#address-cells = <1>;
--
2.17.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4 3/5] drm/msm: implement a2xx mmu
A2XX has its own very simple MMU.
Added a msm_use_mmu() function because we can't rely on iommu_present to
decide to use MMU or not.
Signed-off-by: Jonathan Marek
---
v3: rebased on msm-next-staging and moved is_a2xx initialization earlier
drivers/gpu/drm/msm/Makefile | 3 +-
drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 50 -
drivers/gpu/drm/msm/adreno/adreno_device.c | 3 +
drivers/gpu/drm/msm/adreno/adreno_gpu.c| 3 +
drivers/gpu/drm/msm/msm_drv.c | 11 +-
drivers/gpu/drm/msm/msm_drv.h | 8 ++
drivers/gpu/drm/msm/msm_gem.c | 4 +-
drivers/gpu/drm/msm/msm_gem_vma.c | 23
drivers/gpu/drm/msm/msm_gpu.c | 31 --
drivers/gpu/drm/msm/msm_gpummu.c | 122 +
drivers/gpu/drm/msm/msm_mmu.h | 3 +
11 files changed, 241 insertions(+), 20 deletions(-)
create mode 100644 drivers/gpu/drm/msm/msm_gpummu.c
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 61e76f87a..1b26c4105 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -93,7 +93,8 @@ msm-y := \
msm_rd.o \
msm_ringbuffer.o \
msm_submitqueue.o \
- msm_gpu_tracepoints.o
+ msm_gpu_tracepoints.o \
+ msm_gpummu.o
msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \
disp/dpu1/dpu_dbg.o
diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
index 5eddcf14e..1f83bc18d 100644
--- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
@@ -2,6 +2,8 @@
/* Copyright (c) 2018 The Linux Foundation. All rights reserved. */
#include "a2xx_gpu.h"
+#include "msm_gem.h"
+#include "msm_mmu.h"
extern bool hang_debug;
@@ -58,9 +60,12 @@ static bool a2xx_me_init(struct msm_gpu *gpu)
static int a2xx_hw_init(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ dma_addr_t pt_base, tran_error;
uint32_t *ptr, len;
int i, ret;
+ msm_gpummu_params(gpu->aspace->mmu, _base, _error);
+
DBG("%s", gpu->name);
/* halt ME to avoid ucode upload issues on a20x */
@@ -80,9 +85,34 @@ static int a2xx_hw_init(struct msm_gpu *gpu)
/* note: kgsl uses 0x for a20x */
gpu_write(gpu, REG_A2XX_RBBM_CNTL, 0x4442);
- gpu_write(gpu, REG_A2XX_MH_MMU_CONFIG, 0);
- gpu_write(gpu, REG_A2XX_MH_MMU_MPU_BASE, 0);
+ /* MPU: physical range */
+ gpu_write(gpu, REG_A2XX_MH_MMU_MPU_BASE, 0x);
gpu_write(gpu, REG_A2XX_MH_MMU_MPU_END, 0xf000);
+
+ gpu_write(gpu, REG_A2XX_MH_MMU_CONFIG, A2XX_MH_MMU_CONFIG_MMU_ENABLE |
+ A2XX_MH_MMU_CONFIG_RB_W_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_CP_W_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_CP_R0_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_CP_R1_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_CP_R2_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_CP_R3_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_CP_R4_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_VGT_R0_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_VGT_R1_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_TC_R_CLNT_BEHAVIOR(BEH_TRAN_RNG) |
+ A2XX_MH_MMU_CONFIG_PA_W_CLNT_BEHAVIOR(BEH_TRAN_RNG));
+
+ /* same as parameters in adreno_gpu */
+ gpu_write(gpu, REG_A2XX_MH_MMU_VA_RANGE, SZ_16M |
+ A2XX_MH_MMU_VA_RANGE_NUM_64KB_REGIONS(0xfff));
+
+ gpu_write(gpu, REG_A2XX_MH_MMU_PT_BASE, pt_base);
+ gpu_write(gpu, REG_A2XX_MH_MMU_TRAN_ERROR, tran_error);
+
+ gpu_write(gpu, REG_A2XX_MH_MMU_INVALIDATE,
+ A2XX_MH_MMU_INVALIDATE_INVALIDATE_ALL |
+ A2XX_MH_MMU_INVALIDATE_INVALIDATE_TC);
+
gpu_write(gpu, REG_A2XX_MH_ARBITER_CONFIG,
A2XX_MH_ARBITER_CONFIG_SAME_PAGE_LIMIT(16) |
A2XX_MH_ARBITER_CONFIG_L1_ARB_ENABLE |
@@ -109,9 +139,21 @@ static int a2xx_hw_init(struct msm_gpu *gpu)
/* note: gsl doesn't set this */
gpu_write(gpu, REG_A2XX_RBBM_DEBUG, 0x0008);
- gpu_write(gpu, REG_A2XX_RBBM_INT_CNTL, 0);
- gpu_write(gpu, REG_AXXX_CP_INT_CNTL, 0x8000); /* RB INT */
+ gpu_write(gpu, REG_A2XX_RBBM_INT_CNTL,
+ A2XX_RBBM_INT_CNTL_RDERR_INT_MASK);
+ gpu_write(gpu, REG_AXXX_CP_INT_CNTL,
+ AXXX_CP_INT_CNTL_T0_PACKET_IN_IB_MASK |
+ AXXX_CP_INT_CNTL_OPCODE_ERROR_MASK |
+ AXXX_CP_INT_CNTL_PROTECTED_MODE_ERROR_MASK |
+ AXXX_CP_INT_CNTL_RESERVED_BIT_ERROR_MASK |
+ AXXX_CP_INT_CNTL_IB_ERROR_MASK |
+ AXXX_CP_INT_CNTL_IB1_INT_MASK |
+ AXXX_CP_INT_CNTL_RB_INT_MASK);
gpu_write(gpu,
[Freedreno] [PATCH v4 1/5] drm/msm/mdp4: add lcdc-align-lsb flag to control lane alignment
This allows controlling which of the 8 lanes are used for 6 bit color. Signed-off-by: Jonathan Marek --- v3: removed empty line and added documentation .../devicetree/bindings/display/msm/mdp4.txt | 2 ++ .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 21 --- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/mdp4.txt b/Documentation/devicetree/bindings/display/msm/mdp4.txt index 3c341a15c..b07eeb38f 100644 --- a/Documentation/devicetree/bindings/display/msm/mdp4.txt +++ b/Documentation/devicetree/bindings/display/msm/mdp4.txt @@ -38,6 +38,8 @@ Required properties: Optional properties: - clock-names: the following clocks are optional: * "lut_clk" +- qcom,lcdc-align-lsb: Boolean value indicating that LSB alignment should be + used for LCDC. This is only valid for 18bpp panels. Example: diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c index 9e08c2efa..c9e34501a 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c @@ -377,20 +377,25 @@ static void mdp4_lcdc_encoder_enable(struct drm_encoder *encoder) unsigned long pc = mdp4_lcdc_encoder->pixclock; struct mdp4_kms *mdp4_kms = get_kms(encoder); struct drm_panel *panel; + uint32_t config; int i, ret; if (WARN_ON(mdp4_lcdc_encoder->enabled)) return; /* TODO: hard-coded for 18bpp: */ - mdp4_crtc_set_config(encoder->crtc, - MDP4_DMA_CONFIG_R_BPC(BPC6) | - MDP4_DMA_CONFIG_G_BPC(BPC6) | - MDP4_DMA_CONFIG_B_BPC(BPC6) | - MDP4_DMA_CONFIG_PACK_ALIGN_MSB | - MDP4_DMA_CONFIG_PACK(0x21) | - MDP4_DMA_CONFIG_DEFLKR_EN | - MDP4_DMA_CONFIG_DITHER_EN); + config = + MDP4_DMA_CONFIG_R_BPC(BPC6) | + MDP4_DMA_CONFIG_G_BPC(BPC6) | + MDP4_DMA_CONFIG_B_BPC(BPC6) | + MDP4_DMA_CONFIG_PACK(0x21) | + MDP4_DMA_CONFIG_DEFLKR_EN | + MDP4_DMA_CONFIG_DITHER_EN; + + if (!of_property_read_bool(dev->dev->of_node, "qcom,lcdc-align-lsb")) + config |= MDP4_DMA_CONFIG_PACK_ALIGN_MSB; + + mdp4_crtc_set_config(encoder->crtc, config); mdp4_crtc_set_intf(encoder->crtc, INTF_LCDC_DTV, 0); bs_set(mdp4_lcdc_encoder, 1); -- 2.17.1 ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4 5/5] dt-bindings: display: msm/gpu: document amd, imageon compatible
Document the new amd,imageon compatible, used for non-qcom hardware that uses the drm/msm driver (iMX5). Signed-off-by: Jonathan Marek --- Documentation/devicetree/bindings/display/msm/gpu.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt index 43fac0fe0..ac8df3b87 100644 --- a/Documentation/devicetree/bindings/display/msm/gpu.txt +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt @@ -1,11 +1,13 @@ Qualcomm adreno/snapdragon GPU Required properties: -- compatible: "qcom,adreno-XYZ.W", "qcom,adreno" +- compatible: "qcom,adreno-XYZ.W", "qcom,adreno" or + "amd,imageon-XYZ.W", "amd,imageon" for example: "qcom,adreno-306.0", "qcom,adreno" Note that you need to list the less specific "qcom,adreno" (since this is what the device is matched on), in addition to the more specific with the chip-id. + If "amd,imageon" is used, there should be no top level msm device. - reg: Physical base address and length of the controller's registers. - interrupts: The interrupt signal from the gpu. - clocks: device clocks -- 2.17.1 ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v2 00/24] drm/msm: Various dpu locking and legacy cleanups
On Fri, Nov 16, 2018 at 01:42:10PM -0500, Sean Paul wrote: > From: Sean Paul > > This was originally 3 patchsets, but none have gotten full review, so I > figured I'd package the v2's all up into one set so it's easier to track. > > Set 1- > https://lists.freedesktop.org/archives/dri-devel/2018-November/196170.html > Set 2- > https://lists.freedesktop.org/archives/dri-devel/2018-November/196184.html > Set 3- > https://lists.freedesktop.org/archives/dri-devel/2018-November/196276.html > Thanks for the reviews, I've pushed it to dpu-staging/for-next Sean > Thanks, > Sean > > > Sean Paul (24): > drm/msm: dpu: Remove dpu_power_handle_get_dbus_name() > drm/msm: dpu: Remove unused trace_dpu_perf_update_bus() > drm/msm: dpu: Remove dpu_power_client > drm/msm: dpu: Don't use power_event for vbif_init_memtypes > drm/msm: dpu: Handle crtc pm_runtime_resume() directly > drm/msm: dpu: Remove power_handle from core_perf > drm/msm: dpu: Include dpu_io_util.h directly in dpu_kms.h > drm/msm: dpu: Move DPU_POWER_HANDLE_DBUS_ID to core_perf > drm/msm: dpu: Remove dpu_power_handle > drm/msm: dpu: Fix typo in dpu_encoder > drm/msm: dpu: Add ->enabled to dpu_encoder_virt > drm/msm: dpu: Move crtc runtime resume to encoder > drm/msm: dpu: Don't drop locks in crtc_vblank_enable > drm/msm: dpu: Grab the modeset locks in frame_event > drm/msm: dpu: Stop using encoder->crtc pointer > drm/msm: dpu: Add modeset lock checks where applicable > drm/msm: dpu: Move pm_runtime_(get|put) from vblank_enable > drm/msm: dpu: Remove crtc_lock from setup_mixers > drm/msm: dpu: Remove vblank_callback from encoder > drm/msm: dpu: Use atomic_disable for dpu_crtc_disable > drm/msm: dpu: Don't bother checking ->enabled in dpu_crtc_vblank > drm/msm: dpu: Separate crtc assignment from vblank enable > drm/msm: dpu: Remove vblank_requested flag from dpu_crtc > drm/msm: dpu: Remove crtc_lock > > drivers/gpu/drm/msm/Makefile | 1 - > drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 37 ++- > drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 22 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 216 +--- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 15 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 97 --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 24 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 76 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 6 +- > .../gpu/drm/msm/disp/dpu1/dpu_power_handle.c | 240 -- > .../gpu/drm/msm/disp/dpu1/dpu_power_handle.h | 217 > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 43 +--- > 12 files changed, 204 insertions(+), 790 deletions(-) > delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c > delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h > > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- Sean Paul, Software Engineer, Google / Chromium OS ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v3 00/10] drm/msm/dpu: cleanups
On Mon, Dec 03, 2018 at 03:47:13PM -0700, Jordan Crouse wrote:
> This is a rebase of
>
> https://patchwork.freedesktop.org/series/51214/
>
> On top of
>
> https://gitlab.freedesktop.org/seanpaul/dpu-staging/tags/for_jcrouse
Thanks for the resend, I've pushed it to dpu-staging/for-next
Sean
>
> Which should be up to date for the latest and greatest of the DPU. This should
> be mostly unchanged since the last revision with the exception that I dropped
> ("drm/msm/dpu: Use DEFINE_SHOW_ATTRIBUTE") in favor of
>
> https://patchwork.freedesktop.org/patch/265159/
>
> Which does the same thing but is a little bit more tree-wide in its efforts.
>
> Jordan Crouse (10):
> drm/msm/dpu: Remove dpu_dbg
> drm/msm/dpu: Remove dpu_crtc_get_mixer_height
> drm/msm/dpu: Remove dpu_crtc_is_enabled()
> drm/msm/dpu: Remove unused functions
> drm/msm/dpu: Cleanup callers of dpu_hw_blk_init
> drm/msm: Make irq_postinstall optional
> drm/msm/dpu: Remove dpu_irq and unused functions
> drm/msm/dpu: Cleanup the debugfs functions
> drm/msm/dpu: Further cleanups for static inline functions
> drm/msm/dpu: Clean up dpu_media_info.h static inline functions
>
> drivers/gpu/drm/msm/Makefile |4 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 45 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 16 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 122 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |7 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 45 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 32 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 2393 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 103 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35 +-
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h |2 +-
> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 12 +-
> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c |1 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c| 10 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h|2 +-
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h|9 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c| 18 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 18 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 10 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 24 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h |5 -
> .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 18 +-
> .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 10 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 21 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 10 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c| 20 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h| 10 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_vbif.c |1 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c | 66 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h | 59 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 154 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |8 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c |8 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 108 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 24 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h | 15 +-
> .../gpu/drm/msm/disp/dpu1/msm_media_info.h| 164 +-
> drivers/gpu/drm/msm/msm_drv.c |6 +-
> 38 files changed, 221 insertions(+), 3394 deletions(-)
> delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c
> delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h
> delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c
> delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.h
>
> --
> 2.18.0
>
> ___
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] gpu/drm: remove DEFINE_DPU_DEBUGFS_SEQ_FOPS()
On Sat, Dec 01, 2018 at 10:19:11PM -0500, Yangtao Li wrote:
> We already have the DEFINE_SHOW_ATTRIBUTE.There is no need to define
> such a macro separately,so remove DEFINE_DPU_DEBUGFS_SEQ_FOPS.
> Also use DEFINE_SHOW_ATTRIBUTE to simplify some code.
>
> Signed-off-by: Yangtao Li
> ---
> drivers/gpu/drm/armada/armada_debugfs.c | 21
> drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 14 +---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 33 +++---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 17 ++---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 36
For the dpu portion:
Acked-by: Sean Paul
> drivers/gpu/host1x/debug.c | 34 --
> 6 files changed, 29 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_debugfs.c
> b/drivers/gpu/drm/armada/armada_debugfs.c
> index 6758c3a83de2..64dccb7c8be2 100644
> --- a/drivers/gpu/drm/armada/armada_debugfs.c
> +++ b/drivers/gpu/drm/armada/armada_debugfs.c
> @@ -28,7 +28,7 @@ static int armada_debugfs_gem_linear_show(struct seq_file
> *m, void *data)
> return 0;
> }
>
> -static int armada_debugfs_reg_show(struct seq_file *m, void *data)
> +static int reg_show(struct seq_file *m, void *data)
> {
> struct drm_device *dev = m->private;
> struct armada_private *priv = dev->dev_private;
> @@ -50,18 +50,7 @@ static int armada_debugfs_reg_show(struct seq_file *m,
> void *data)
> return 0;
> }
>
> -static int armada_debugfs_reg_r_open(struct inode *inode, struct file *file)
> -{
> - return single_open(file, armada_debugfs_reg_show, inode->i_private);
> -}
> -
> -static const struct file_operations fops_reg_r = {
> - .owner = THIS_MODULE,
> - .open = armada_debugfs_reg_r_open,
> - .read = seq_read,
> - .llseek = seq_lseek,
> - .release = single_release,
> -};
> +DEFINE_SHOW_ATTRIBUTE(reg);
>
> static int armada_debugfs_write(struct file *file, const char __user *ptr,
> size_t len, loff_t *off)
> @@ -119,12 +108,14 @@ int armada_drm_debugfs_init(struct drm_minor *minor)
> return ret;
>
> de = debugfs_create_file("reg", S_IFREG | S_IRUSR,
> - minor->debugfs_root, minor->dev, _reg_r);
> + minor->debugfs_root, minor->dev,
> + _fops);
> if (!de)
> return -ENOMEM;
>
> de = debugfs_create_file("reg_wr", S_IFREG | S_IWUSR,
> - minor->debugfs_root, minor->dev, _reg_w);
> + minor->debugfs_root, minor->dev,
> + _reg_w);
> if (!de)
> return -ENOMEM;
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
> index 879c13fe74e0..7607aac9449c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
> @@ -299,18 +299,6 @@ static void dpu_disable_all_irqs(struct dpu_kms *dpu_kms)
> }
>
> #ifdef CONFIG_DEBUG_FS
> -#define DEFINE_DPU_DEBUGFS_SEQ_FOPS(__prefix)
> \
> -static int __prefix ## _open(struct inode *inode, struct file *file) \
> -{\
> - return single_open(file, __prefix ## _show, inode->i_private); \
> -}\
> -static const struct file_operations __prefix ## _fops = {\
> - .owner = THIS_MODULE, \
> - .open = __prefix ## _open, \
> - .release = single_release, \
> - .read = seq_read, \
> - .llseek = seq_lseek,\
> -}
>
> static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v)
> {
> @@ -341,7 +329,7 @@ static int dpu_debugfs_core_irq_show(struct seq_file *s,
> void *v)
> return 0;
> }
>
> -DEFINE_DPU_DEBUGFS_SEQ_FOPS(dpu_debugfs_core_irq);
> +DEFINE_SHOW_ATTRIBUTE(dpu_debugfs_core_irq);
>
> int dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
> struct dentry *parent)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..e705bad980ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1309,7 +1309,7 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
> }
>
> #ifdef CONFIG_DEBUG_FS
> -static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
> +static int status_show(struct seq_file *s, void *data)
> {
> struct dpu_crtc *dpu_crtc;
> struct dpu_plane_state *pstate = NULL;
> @@ -1428,24 +1428,6 @@ static int _dpu_debugfs_status_show(struct
Re: [Freedreno] [PATCH v2] drm/msm/dpu: add display port support in DPU
On Mon, Dec 03, 2018 at 07:02:07PM -0800, Jeykumar Sankaran wrote:
> On 2018-12-03 06:47, Sean Paul wrote:
> > On Tue, Nov 27, 2018 at 02:28:30PM -0800, Jeykumar Sankaran wrote:
> > > Add display port support in DPU by creating hooks
> > > for DP encoder enumeration and encoder mode
> > > initialization.
> > >
> > > This change is based on the SDM845 Display port
> > > driver changes[1].
> > >
> > > changes in v2:
> > > - rebase on [2] (Sean Paul)
> > > - remove unwanted error checks and
> > > switch cases (Jordan Crouse)
> > >
> > > [1] https://lwn.net/Articles/768265/
> > > [2] https://lkml.org/lkml/2018/11/17/87
> > >
> > > Signed-off-by: Jeykumar Sankaran
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 ++---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 47
> > +
> > > 2 files changed, 45 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > index d3f4501..1f6b4b1 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > @@ -2015,7 +2015,7 @@ static int dpu_encoder_setup_display(struct
> > dpu_encoder_virt *dpu_enc,
> > > {
> > > int ret = 0;
> > > int i = 0;
> > > - enum dpu_intf_type intf_type;
> > > + enum dpu_intf_type intf_type = INTF_NONE;
> >
> > dpu_intf_type seems unnecessary, you could just use the
> > DRM_MODE_ENCODER_*
> > value
> > directly?
> >
> > > struct dpu_enc_phys_init_params phys_params;
> > >
> > > if (!dpu_enc || !dpu_kms) {
> > > @@ -2038,9 +2038,9 @@ static int dpu_encoder_setup_display(struct
> > dpu_encoder_virt *dpu_enc,
> > > case DRM_MODE_ENCODER_DSI:
> > > intf_type = INTF_DSI;
> > > break;
> > > - default:
> > > - DPU_ERROR_ENC(dpu_enc, "unsupported display interface
> > type\n");
> > > - return -EINVAL;
> > > + case DRM_MODE_ENCODER_TMDS:
> > > + intf_type = INTF_DP;
> > > + break;
> > > }
> > >
> > > WARN_ON(disp_info->num_of_h_tiles < 1);
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > index 985c855..7d931ae 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > @@ -473,6 +473,32 @@ static void _dpu_kms_initialize_dsi(struct
> > drm_device *dev,
> > > }
> > > }
> > >
> > > +static void _dpu_kms_initialize_displayport(struct drm_device *dev,
> > > + struct msm_drm_private *priv,
> > > + struct dpu_kms *dpu_kms)
> > > +{
> > > + struct drm_encoder *encoder = NULL;
> > > + int rc;
> > > +
> > > + if (!priv->dp)
> > > + return;
> > > +
> > > + encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
> > > + if (IS_ERR(encoder)) {
> > > + DPU_ERROR("encoder init failed for dsi display\n");
> > > + return;
> > > + }
> > > +
> > > + rc = msm_dp_modeset_init(priv->dp, dev, encoder);
> > > + if (rc) {
> > > + DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
> > > + drm_encoder_cleanup(encoder);
> > > + return;
> > > + }
> > > +
> > > + priv->encoders[priv->num_encoders++] = encoder;
> >
> > No need to keep track of drm resources at the driver level, the core
> > will
> > do
> > this for you. So can you please add a patch preceding this one to remove
> > the
> > priv->encoders/crtc/planes/connectors arrays?
> >
> priv arrays for tracking drm components and priv->num_** counters are
> introduced
> by mdp4/5. DPU just adapted the implementation.
>
> De-coupling DPU from these arrays is much easier than fixing them in mdp4/5.
> I see
> mdp5 is using it to track the max resources and also in ./msm_fbdev.c.
Ok, I'll take a look at it
Sean
>
> Thanks and Regards,
> Jeykumar S.
> > > +}
> > > +
> > > /**
> > > * _dpu_kms_setup_displays - create encoders, bridges and connectors
> > > * for underlying displays
> > > @@ -487,6 +513,8 @@ static void _dpu_kms_setup_displays(struct
> > drm_device *dev,
> >
> > Why are these functions voids? Seems like there are plenty of places for
> > them to
> > fail :)
> >
> > Let's add a patch to the beginning of this series to properly handle
> > failures in
> > setup_displays and initialize_dsi
> >
> > > {
> > > _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
> > >
> > > + _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
> > > +
> > > /**
> > >* Extend this function to initialize other
> > >* types of displays
> > > @@ -723,13 +751,20 @@ static void _dpu_kms_set_encoder_mode(struct
> > msm_kms *kms,
> > > info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
> > > MSM_DISPLAY_CAP_VID_MODE;
> > >
> > > - /* TODO: No support for DSI swap */
> > > - for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> > > - if
