Re: [Freedreno] [PATCH] drm/msm/adreno: make adreno_is_a690()'s argument const
Hi,
On Mon, Jun 12, 2023 at 11:25 AM Dmitry Baryshkov
wrote:
>
> Change adreno_is_a690() prototype to accept the const struct adreno_gpu
> pointer instead of a non-const one. This fixes the following warning:
>
> In file included from drivers/gpu/drm/msm/msm_drv.c:33:
> drivers/gpu/drm/msm/adreno/adreno_gpu.h: In function ‘adreno_is_a660_family’:
> drivers/gpu/drm/msm/adreno/adreno_gpu.h:303:54: warning: passing argument 1
> of ‘adreno_is_a690’ discards ‘const’ qualifier from pointer target type
> [-Wdiscarded-qualifiers]
> 303 | return adreno_is_a660(gpu) || adreno_is_a690(gpu) ||
> adreno_is_7c3(gpu);
>
> Fixes: 1b90e8f8879c ("drm/msm/adreno: change adreno_is_* functions to accept
> const argument")
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Douglas Anderson
Re: [Freedreno] [PATCH] drm/msm/adreno: make adreno_is_a690()'s argument const
On Mon, Jun 12, 2023 at 11:25 AM Dmitry Baryshkov
wrote:
>
> Change adreno_is_a690() prototype to accept the const struct adreno_gpu
> pointer instead of a non-const one. This fixes the following warning:
>
> In file included from drivers/gpu/drm/msm/msm_drv.c:33:
> drivers/gpu/drm/msm/adreno/adreno_gpu.h: In function ‘adreno_is_a660_family’:
> drivers/gpu/drm/msm/adreno/adreno_gpu.h:303:54: warning: passing argument 1
> of ‘adreno_is_a690’ discards ‘const’ qualifier from pointer target type
> [-Wdiscarded-qualifiers]
> 303 | return adreno_is_a660(gpu) || adreno_is_a690(gpu) ||
> adreno_is_7c3(gpu);
>
> Fixes: 1b90e8f8879c ("drm/msm/adreno: change adreno_is_* functions to accept
> const argument")
> Signed-off-by: Dmitry Baryshkov
Reviewed-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 1283e5fe22d2..9a7626c7ac4d 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -284,7 +284,7 @@ static inline int adreno_is_a660(const struct adreno_gpu
> *gpu)
> return adreno_is_revn(gpu, 660);
> }
>
> -static inline int adreno_is_a690(struct adreno_gpu *gpu)
> +static inline int adreno_is_a690(const struct adreno_gpu *gpu)
> {
> return gpu->revn == 690;
> };
> --
> 2.39.2
>
Re: [Freedreno] [PATCH 2/2] drm/msm/dsi: split dsi_ctrl_config() function
On 2023-06-15 01:44:02, Dmitry Baryshkov wrote:
> It makes no sense to pass NULL parameters to dsi_ctrl_config() in the
> disable case. Split dsi_ctrl_config() into enable and disable parts and
> drop unused params.
>
> Signed-off-by: Dmitry Baryshkov
Indeed, it makes much more sense to split this out instead of faking a
_configure(false) with a return right at the top and "bogus" pointers.
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index eaee621aa6c8..3f6dfb4f9d5a 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -705,7 +705,12 @@ static inline enum dsi_cmd_dst_format dsi_get_cmd_fmt(
> }
> }
>
> -static void dsi_ctrl_config(struct msm_dsi_host *msm_host, bool enable,
> +static void dsi_ctrl_disable(struct msm_dsi_host *msm_host)
> +{
> + dsi_write(msm_host, REG_DSI_CTRL, 0);
> +}
> +
> +static void dsi_ctrl_enable(struct msm_dsi_host *msm_host,
> struct msm_dsi_phy_shared_timings *phy_shared_timings,
> struct msm_dsi_phy *phy)
> {
> u32 flags = msm_host->mode_flags;
> @@ -713,11 +718,6 @@ static void dsi_ctrl_config(struct msm_dsi_host
> *msm_host, bool enable,
> const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
> u32 data = 0, lane_ctrl = 0;
>
> - if (!enable) {
> - dsi_write(msm_host, REG_DSI_CTRL, 0);
> - return;
> - }
> -
> if (flags & MIPI_DSI_MODE_VIDEO) {
> if (flags & MIPI_DSI_MODE_VIDEO_HSE)
> data |= DSI_VID_CFG0_PULSE_MODE_HSA_HE;
> @@ -802,7 +802,7 @@ static void dsi_ctrl_config(struct msm_dsi_host
> *msm_host, bool enable,
> if (!(flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)) {
> lane_ctrl = dsi_read(msm_host, REG_DSI_LANE_CTRL);
>
> - if (msm_dsi_phy_set_continuous_clock(phy, enable))
> + if (msm_dsi_phy_set_continuous_clock(phy, true))
> lane_ctrl &= ~DSI_LANE_CTRL_HS_REQ_SEL_PHY;
>
> dsi_write(msm_host, REG_DSI_LANE_CTRL,
> @@ -2358,7 +2358,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
>
> dsi_timing_setup(msm_host, is_bonded_dsi);
> dsi_sw_reset(msm_host);
> - dsi_ctrl_config(msm_host, true, phy_shared_timings, phy);
> + dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
>
> if (msm_host->disp_en_gpio)
> gpiod_set_value(msm_host->disp_en_gpio, 1);
> @@ -2390,7 +2390,7 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)
> goto unlock_ret;
> }
>
> - dsi_ctrl_config(msm_host, false, NULL, NULL);
> + dsi_ctrl_disable(msm_host);
>
> if (msm_host->disp_en_gpio)
> gpiod_set_value(msm_host->disp_en_gpio, 0);
> --
> 2.39.2
>
Re: [Freedreno] [PATCH 1/2] drm/msm/dsi: dsi_host: drop unused clocks
On 2023-06-15 01:44:01, Dmitry Baryshkov wrote:
> Several source clocks are not used anymore, so stop handling them.
>
> Signed-off-by: Dmitry Baryshkov
Indeed, we were not using these parent clocks for anything.
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 33 --
> 1 file changed, 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index fb1d3a25765f..eaee621aa6c8 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -118,8 +118,6 @@ struct msm_dsi_host {
> struct clk *byte_clk;
> struct clk *esc_clk;
> struct clk *pixel_clk;
> - struct clk *byte_clk_src;
> - struct clk *pixel_clk_src;
> struct clk *byte_intf_clk;
>
> unsigned long byte_clk_rate;
> @@ -129,8 +127,6 @@ struct msm_dsi_host {
>
> /* DSI v2 specific clocks */
> struct clk *src_clk;
> - struct clk *esc_clk_src;
> - struct clk *dsi_clk_src;
>
> unsigned long src_clk_rate;
>
> @@ -267,21 +263,6 @@ int dsi_clk_init_v2(struct msm_dsi_host *msm_host)
> return ret;
> }
>
> - msm_host->esc_clk_src = clk_get_parent(msm_host->esc_clk);
> - if (!msm_host->esc_clk_src) {
> - ret = -ENODEV;
> - pr_err("%s: can't get esc clock parent. ret=%d\n",
> - __func__, ret);
> - return ret;
> - }
> -
> - msm_host->dsi_clk_src = clk_get_parent(msm_host->src_clk);
> - if (!msm_host->dsi_clk_src) {
> - ret = -ENODEV;
> - pr_err("%s: can't get src clock parent. ret=%d\n",
> - __func__, ret);
> - }
> -
> return ret;
> }
>
> @@ -346,20 +327,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
> goto exit;
> }
>
> - msm_host->byte_clk_src = clk_get_parent(msm_host->byte_clk);
> - if (IS_ERR(msm_host->byte_clk_src)) {
> - ret = PTR_ERR(msm_host->byte_clk_src);
> - pr_err("%s: can't find byte_clk clock. ret=%d\n", __func__,
> ret);
> - goto exit;
> - }
> -
> - msm_host->pixel_clk_src = clk_get_parent(msm_host->pixel_clk);
> - if (IS_ERR(msm_host->pixel_clk_src)) {
> - ret = PTR_ERR(msm_host->pixel_clk_src);
> - pr_err("%s: can't find pixel_clk clock. ret=%d\n", __func__,
> ret);
> - goto exit;
> - }
> -
> if (cfg_hnd->ops->clk_init_ver)
> ret = cfg_hnd->ops->clk_init_ver(msm_host);
> exit:
> --
> 2.39.2
>
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 2023-06-14 14:23:38, Marijn Suijten wrote: > Tested this on SM8350 which actually has DSI 2.5, and it is also > corrupted with this series so something else on this series might be > broken. Never mind, this was a bad conflict-resolve. Jessica's original BURST_MODE patch was RMW'ing MDP_CTRL2, but the upstreamed patch was only writing, and the way I conflict-resolved that caused the write of BURST_MODE to overwrite the RMW DATABUS_WIDEN. If both are moved to dsi_ctrl_config(), we could do a read, add both flags in conditionally, and write. - Marijn
Re: [Freedreno] [PATCH] drm/msm/dsi: Enable BURST_MODE for command mode for DSI 6G v1.3+
On 13/06/2023 02:37, Jessica Zhang wrote: During a frame transfer in command mode, there could be frequent LP11 <-> HS transitions when multiple DCS commands are sent mid-frame or if the DSI controller is running on slow clock and is throttled. To minimize frame latency due to these transitions, it is recommended to send the frame in a single burst. This feature is supported for DSI 6G 1.3 and above, thus enable burst mode if supported. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/dsi/dsi_host.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 744f2398a6d6..8254b06dca85 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -994,6 +994,11 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL, DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) | DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay)); + + if (msm_host->cfg_hnd->major == MSM_DSI_VER_MAJOR_6G && + msm_host->cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V1_3) + dsi_write(msm_host, REG_DSI_CMD_MODE_MDP_CTRL2, + DSI_CMD_MODE_MDP_CTRL2_BURST_MODE); Please move this to dsi_ctrl_config(), the place where we set all the configs. Also please change this to RMW cycle. } } --- base-commit: dd969f852ba4c66938c71889e826aa8e5300d2f2 change-id: 20230608-b4-add-burst-mode-a5bb144069fa Best regards, -- With best wishes Dmitry
[Freedreno] [PATCH 2/2] drm/msm/dsi: split dsi_ctrl_config() function
It makes no sense to pass NULL parameters to dsi_ctrl_config() in the
disable case. Split dsi_ctrl_config() into enable and disable parts and
drop unused params.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index eaee621aa6c8..3f6dfb4f9d5a 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -705,7 +705,12 @@ static inline enum dsi_cmd_dst_format dsi_get_cmd_fmt(
}
}
-static void dsi_ctrl_config(struct msm_dsi_host *msm_host, bool enable,
+static void dsi_ctrl_disable(struct msm_dsi_host *msm_host)
+{
+ dsi_write(msm_host, REG_DSI_CTRL, 0);
+}
+
+static void dsi_ctrl_enable(struct msm_dsi_host *msm_host,
struct msm_dsi_phy_shared_timings *phy_shared_timings,
struct msm_dsi_phy *phy)
{
u32 flags = msm_host->mode_flags;
@@ -713,11 +718,6 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host,
bool enable,
const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
u32 data = 0, lane_ctrl = 0;
- if (!enable) {
- dsi_write(msm_host, REG_DSI_CTRL, 0);
- return;
- }
-
if (flags & MIPI_DSI_MODE_VIDEO) {
if (flags & MIPI_DSI_MODE_VIDEO_HSE)
data |= DSI_VID_CFG0_PULSE_MODE_HSA_HE;
@@ -802,7 +802,7 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host,
bool enable,
if (!(flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)) {
lane_ctrl = dsi_read(msm_host, REG_DSI_LANE_CTRL);
- if (msm_dsi_phy_set_continuous_clock(phy, enable))
+ if (msm_dsi_phy_set_continuous_clock(phy, true))
lane_ctrl &= ~DSI_LANE_CTRL_HS_REQ_SEL_PHY;
dsi_write(msm_host, REG_DSI_LANE_CTRL,
@@ -2358,7 +2358,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
dsi_timing_setup(msm_host, is_bonded_dsi);
dsi_sw_reset(msm_host);
- dsi_ctrl_config(msm_host, true, phy_shared_timings, phy);
+ dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
if (msm_host->disp_en_gpio)
gpiod_set_value(msm_host->disp_en_gpio, 1);
@@ -2390,7 +2390,7 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)
goto unlock_ret;
}
- dsi_ctrl_config(msm_host, false, NULL, NULL);
+ dsi_ctrl_disable(msm_host);
if (msm_host->disp_en_gpio)
gpiod_set_value(msm_host->disp_en_gpio, 0);
--
2.39.2
[Freedreno] [PATCH 1/2] drm/msm/dsi: dsi_host: drop unused clocks
Several source clocks are not used anymore, so stop handling them.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 33 --
1 file changed, 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index fb1d3a25765f..eaee621aa6c8 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -118,8 +118,6 @@ struct msm_dsi_host {
struct clk *byte_clk;
struct clk *esc_clk;
struct clk *pixel_clk;
- struct clk *byte_clk_src;
- struct clk *pixel_clk_src;
struct clk *byte_intf_clk;
unsigned long byte_clk_rate;
@@ -129,8 +127,6 @@ struct msm_dsi_host {
/* DSI v2 specific clocks */
struct clk *src_clk;
- struct clk *esc_clk_src;
- struct clk *dsi_clk_src;
unsigned long src_clk_rate;
@@ -267,21 +263,6 @@ int dsi_clk_init_v2(struct msm_dsi_host *msm_host)
return ret;
}
- msm_host->esc_clk_src = clk_get_parent(msm_host->esc_clk);
- if (!msm_host->esc_clk_src) {
- ret = -ENODEV;
- pr_err("%s: can't get esc clock parent. ret=%d\n",
- __func__, ret);
- return ret;
- }
-
- msm_host->dsi_clk_src = clk_get_parent(msm_host->src_clk);
- if (!msm_host->dsi_clk_src) {
- ret = -ENODEV;
- pr_err("%s: can't get src clock parent. ret=%d\n",
- __func__, ret);
- }
-
return ret;
}
@@ -346,20 +327,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
goto exit;
}
- msm_host->byte_clk_src = clk_get_parent(msm_host->byte_clk);
- if (IS_ERR(msm_host->byte_clk_src)) {
- ret = PTR_ERR(msm_host->byte_clk_src);
- pr_err("%s: can't find byte_clk clock. ret=%d\n", __func__,
ret);
- goto exit;
- }
-
- msm_host->pixel_clk_src = clk_get_parent(msm_host->pixel_clk);
- if (IS_ERR(msm_host->pixel_clk_src)) {
- ret = PTR_ERR(msm_host->pixel_clk_src);
- pr_err("%s: can't find pixel_clk clock. ret=%d\n", __func__,
ret);
- goto exit;
- }
-
if (cfg_hnd->ops->clk_init_ver)
ret = cfg_hnd->ops->clk_init_ver(msm_host);
exit:
--
2.39.2
Re: [Freedreno] [PATCH v2 0/3] drm/msm/adreno: GPU support on SC8280XP
On Wed, Jun 14, 2023 at 09:03:34AM -0700, Bjorn Andersson wrote: > On Mon, 22 May 2023 18:15:19 -0700, Bjorn Andersson wrote: > > This series introduces support for A690 in the DRM/MSM driver and > > enables it for the two SC8280XP laptops. > > > > Bjorn Andersson (3): > > drm/msm/adreno: Add Adreno A690 support > > arm64: dts: qcom: sc8280xp: Add GPU related nodes > > arm64: dts: qcom: sc8280xp: Enable GPU related nodes > > > > [...] > > Applied, thanks! > > [1/3] drm/msm/adreno: Add Adreno A690 support > (no commit info) > [2/3] arm64: dts: qcom: sc8280xp: Add GPU related nodes > commit: eec51ab2fd6f447a993c502364704d0cb5bc8cae > [3/3] arm64: dts: qcom: sc8280xp: Enable GPU related nodes > commit: 598a06afca5a2ab4850ce9ff8146ec728cca570c > Seems like I managed to confuse b4, only v4 of the DTS patches were merged, while Rob merged the driver change. Regards, Bjorn
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 2023-06-14 13:39:57, Abhinav Kumar wrote: > On 6/14/2023 12:54 PM, Abhinav Kumar wrote: > > On 6/14/2023 12:35 PM, Abhinav Kumar wrote: > >> On 6/14/2023 5:23 AM, Marijn Suijten wrote: > >>> On 2023-06-14 15:01:59, Dmitry Baryshkov wrote: > On 14/06/2023 14:42, Marijn Suijten wrote: > > On 2023-06-13 18:57:11, Jessica Zhang wrote: > >> DPU 5.x+ supports a databus widen mode that allows more data to be > >> sent > >> per pclk. Enable this feature flag on all relevant chipsets. > >> > >> Signed-off-by: Jessica Zhang > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ > >> 2 files changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> index 36ba3f58dcdf..0be7bf0bfc41 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> @@ -103,7 +103,8 @@ > >> (BIT(DPU_INTF_INPUT_CTRL) | \ > >> BIT(DPU_INTF_TE) | \ > >> BIT(DPU_INTF_STATUS_SUPPORTED) | \ > >> - BIT(DPU_DATA_HCTL_EN)) > >> + BIT(DPU_DATA_HCTL_EN) | \ > >> + BIT(DPU_INTF_DATABUS_WIDEN)) > > > > This doesn't work. DPU 5.0.0 is SM8150, which has DSI 6G 2.3. In the > > last patch for DSI you state and enable widebus for DSI 6G 2.5+ only, > > meaning DPU and DSI are now desynced, and the output is completely > > corrupted. > >>> > > I looked at the internal docs and also this change. This change is > incorrect because this will try to enable widebus for DPU >= 5.0 and DSI > >= 2.5 > > That was not the intended right condition as thats not what the docs say. > > We should enable for DPU >= 7.0 and DSI >= 2.5 That makes more sense, DSI 2.5 is SM8350 which has DPU 7.0. > Is there any combination where this compatibility is broken? That would > be the strange thing for me ( not DPU 5.0 and DSI 2.5 as that was incorrect) No clue if there are any interim SoCs... > Part of this confusion is because of catalog macro re-use again. Somewhat agreed. SC7180 is a DPU 6.2 SoC, and for this mask to be used across DPU 5.x and above it should have been renamed to SM8150 and as suggested by Dmitry, have DPU_5_x_` as suffix. As I've asked many times before, we should inline these masks (not just the macros) (disclaimer: haven't reviewed if Dmitry's series actually does so!). > This series is a good candidate and infact I think we should only do > core_revision based check on DPU and DSI to avoid bringing the catalog > mess into this. > > >>> Tested this on SM8350 which actually has DSI 2.5, and it is also > >>> corrupted with this series so something else on this series might be > >>> broken. > >>> > > > > Missed this response. That seems strange. No worries. But don't forget to look at the comments on patch 2/3 either. Some of it is a continuation of pclk scaling factor for DSC which discussion seems to have ceased on. > > This series was tested on SM8350 HDK with a command mode panel. > > > > We will fix the DPU-DSI handshake and post a v2 but your issue needs > > investigation in parallel. > > > > So another bug to track that would be great. Will see if I can set that up for you! > > Is the bound in dsi_host wrong, or do DPU and DSI need to communicate > > when widebus will be enabled, based on DPU && DSI supporting it? > > I'd prefer to follow the second approach, as we did for DP. DPU asks > the > actual video output driver if widebus is to be enabled. > >>> > >> > >> I was afraid of this. This series was made on an assumption that the > >> DPU version of widebus and DSI version of widebus would be compatible > >> but looks like already SM8150 is an outlier. Fwiw SM8250 would have been an outlier as well :) > >> Yes, I think we have to go with second approach. > >> > >> DPU queries DSI if it supports widebus and enables it. > >> > >> Thanks for your responses. We will post a v2. No hurry, btw. As alluded to above, let's also look at the comments on patch 2/3 and discuss how this affects pclk. > >>> Doesn't it seem very strange that DPU 5.x+ comes with a widebus feature, > >>> but the DSI does not until two revisions later? Or is this available on > >>> every interface, but only for a different (probably DP) encoder block? > >>> > >> > >> Yes its strange. > >> > > I have clarified this above. Its not strange but appeared strange > because we were checking wrong conditions. Hehe :) - Marijn
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 14/06/2023 23:39, Abhinav Kumar wrote: On 6/14/2023 12:54 PM, Abhinav Kumar wrote: On 6/14/2023 12:35 PM, Abhinav Kumar wrote: On 6/14/2023 5:23 AM, Marijn Suijten wrote: On 2023-06-14 15:01:59, Dmitry Baryshkov wrote: On 14/06/2023 14:42, Marijn Suijten wrote: On 2023-06-13 18:57:11, Jessica Zhang wrote: DPU 5.x+ supports a databus widen mode that allows more data to be sent per pclk. Enable this feature flag on all relevant chipsets. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 36ba3f58dcdf..0be7bf0bfc41 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -103,7 +103,8 @@ (BIT(DPU_INTF_INPUT_CTRL) | \ BIT(DPU_INTF_TE) | \ BIT(DPU_INTF_STATUS_SUPPORTED) | \ - BIT(DPU_DATA_HCTL_EN)) + BIT(DPU_DATA_HCTL_EN) | \ + BIT(DPU_INTF_DATABUS_WIDEN)) This doesn't work. DPU 5.0.0 is SM8150, which has DSI 6G 2.3. In the last patch for DSI you state and enable widebus for DSI 6G 2.5+ only, meaning DPU and DSI are now desynced, and the output is completely corrupted. I looked at the internal docs and also this change. This change is incorrect because this will try to enable widebus for DPU >= 5.0 and DSI >= 2.5 That was not the intended right condition as thats not what the docs say. We should enable for DPU >= 7.0 and DSI >= 2.5 Is there any combination where this compatibility is broken? That would be the strange thing for me ( not DPU 5.0 and DSI 2.5 as that was incorrect) Part of this confusion is because of catalog macro re-use again. This series is a good candidate and infact I think we should only do core_revision based check on DPU and DSI to avoid bringing the catalog mess into this. I have just a single request here: can we please have the same approach for both DSI and DP? I don't mind changing DP code if it makes it better. If you don't have better reasons, I like the idea of DSI/DP dictating whether wide bus should be used on the particular interface. It allows us to handle possible errata or corner cases there. Another option would be to make DPU tell DSI / DP whether the wide bus is enabled or not, but I'd say, this is slightly worse solution. Tested this on SM8350 which actually has DSI 2.5, and it is also corrupted with this series so something else on this series might be broken. Missed this response. That seems strange. This series was tested on SM8350 HDK with a command mode panel. We will fix the DPU-DSI handshake and post a v2 but your issue needs investigation in parallel. So another bug to track that would be great. Is the bound in dsi_host wrong, or do DPU and DSI need to communicate when widebus will be enabled, based on DPU && DSI supporting it? I'd prefer to follow the second approach, as we did for DP. DPU asks the actual video output driver if widebus is to be enabled. I was afraid of this. This series was made on an assumption that the DPU version of widebus and DSI version of widebus would be compatible but looks like already SM8150 is an outlier. Yes, I think we have to go with second approach. DPU queries DSI if it supports widebus and enables it. Thanks for your responses. We will post a v2. Doesn't it seem very strange that DPU 5.x+ comes with a widebus feature, but the DSI does not until two revisions later? Or is this available on every interface, but only for a different (probably DP) encoder block? Yes its strange. I have clarified this above. Its not strange but appeared strange because we were checking wrong conditions. - Marijn -- With best wishes Dmitry
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 6/14/2023 12:54 PM, Abhinav Kumar wrote: On 6/14/2023 12:35 PM, Abhinav Kumar wrote: On 6/14/2023 5:23 AM, Marijn Suijten wrote: On 2023-06-14 15:01:59, Dmitry Baryshkov wrote: On 14/06/2023 14:42, Marijn Suijten wrote: On 2023-06-13 18:57:11, Jessica Zhang wrote: DPU 5.x+ supports a databus widen mode that allows more data to be sent per pclk. Enable this feature flag on all relevant chipsets. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 36ba3f58dcdf..0be7bf0bfc41 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -103,7 +103,8 @@ (BIT(DPU_INTF_INPUT_CTRL) | \ BIT(DPU_INTF_TE) | \ BIT(DPU_INTF_STATUS_SUPPORTED) | \ - BIT(DPU_DATA_HCTL_EN)) + BIT(DPU_DATA_HCTL_EN) | \ + BIT(DPU_INTF_DATABUS_WIDEN)) This doesn't work. DPU 5.0.0 is SM8150, which has DSI 6G 2.3. In the last patch for DSI you state and enable widebus for DSI 6G 2.5+ only, meaning DPU and DSI are now desynced, and the output is completely corrupted. I looked at the internal docs and also this change. This change is incorrect because this will try to enable widebus for DPU >= 5.0 and DSI >= 2.5 That was not the intended right condition as thats not what the docs say. We should enable for DPU >= 7.0 and DSI >= 2.5 Is there any combination where this compatibility is broken? That would be the strange thing for me ( not DPU 5.0 and DSI 2.5 as that was incorrect) Part of this confusion is because of catalog macro re-use again. This series is a good candidate and infact I think we should only do core_revision based check on DPU and DSI to avoid bringing the catalog mess into this. Tested this on SM8350 which actually has DSI 2.5, and it is also corrupted with this series so something else on this series might be broken. Missed this response. That seems strange. This series was tested on SM8350 HDK with a command mode panel. We will fix the DPU-DSI handshake and post a v2 but your issue needs investigation in parallel. So another bug to track that would be great. Is the bound in dsi_host wrong, or do DPU and DSI need to communicate when widebus will be enabled, based on DPU && DSI supporting it? I'd prefer to follow the second approach, as we did for DP. DPU asks the actual video output driver if widebus is to be enabled. I was afraid of this. This series was made on an assumption that the DPU version of widebus and DSI version of widebus would be compatible but looks like already SM8150 is an outlier. Yes, I think we have to go with second approach. DPU queries DSI if it supports widebus and enables it. Thanks for your responses. We will post a v2. Doesn't it seem very strange that DPU 5.x+ comes with a widebus feature, but the DSI does not until two revisions later? Or is this available on every interface, but only for a different (probably DP) encoder block? Yes its strange. I have clarified this above. Its not strange but appeared strange because we were checking wrong conditions. - Marijn
Re: [Freedreno] [PATCH] drm/msm/dpu: Configure DP INTF/PHY selector
On 6/13/2023 9:07 AM, Bjorn Andersson wrote:
On Tue, Jun 13, 2023 at 01:31:40AM +0300, Dmitry Baryshkov wrote:
On 13/06/2023 01:10, Bjorn Andersson wrote:
From: Bjorn Andersson
Some platforms provides a mechanism for configuring the mapping between
(one or two) DisplayPort intfs and their PHYs.
In particular SC8180X provides this functionality, without a default
configuration, resulting in no connection between its two external
DisplayPort controllers and any PHYs.
The change implements the logic for optionally configuring which phy
each of the intfs should be connected to, provides a new entry in the
DPU catalog for specifying how many intfs to configure and marks the
SC8180X DPU to program 2 entries.
For now the request is simply to program the mapping 1:1, any support
for alternative mappings is left until the use case arrise.
Note that e.g. msm-4.14 unconditionally maps intf 0 to phy 0 on all
rlatforms, so perhaps this is needed in order to get DisplayPort working
on some other platforms as well.
Signed-off-by: Bjorn Andersson
Signed-off-by: Bjorn Andersson
---
.../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 1 +
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c| 23 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h| 8 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10
6 files changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
index 8ed2b263c5ea..9da952692a69 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
@@ -19,6 +19,7 @@ static const struct dpu_caps sc8180x_dpu_caps = {
.pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
.max_hdeci_exp = MAX_HORZ_DECIMATION,
.max_vdeci_exp = MAX_VERT_DECIMATION,
+ .num_dp_intf_sel = 2,
};
static const struct dpu_ubwc_cfg sc8180x_ubwc_cfg = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index ac4a9e73705c..4cb8d096d8ec 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -357,6 +357,7 @@ struct dpu_rotation_cfg {
* @pixel_ram_size size of latency hiding and de-tiling buffer in bytes
* @max_hdeci_exp max horizontal decimation supported (max is 2^value)
* @max_vdeci_exp max vertical decimation supported (max is 2^value)
+ * @num_dp_intf_selnumber of DP intfs to configure PHY selection for
*/
struct dpu_caps {
u32 max_mixer_width;
@@ -371,6 +372,7 @@ struct dpu_caps {
u32 pixel_ram_size;
u32 max_hdeci_exp;
u32 max_vdeci_exp;
+ u32 num_dp_intf_sel;
};
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
index 963bdb5e0252..5afa99cb148c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
@@ -250,6 +250,27 @@ static void dpu_hw_intf_audio_select(struct dpu_hw_mdp
*mdp)
DPU_REG_WRITE(c, HDMI_DP_CORE_SELECT, 0x1);
}
+static void dpu_hw_dp_phy_intf_sel(struct dpu_hw_mdp *mdp, unsigned int *phys,
+ unsigned int num_intfs)
+{
+ struct dpu_hw_blk_reg_map *c = >hw;
+ unsigned int intf;
+ u32 sel = 0;
+
+ if (!num_intfs)
+ return;
+
+ for (intf = 0; intf < num_intfs; intf++) {
+ /* Specify the PHY (1-indexed) for @intf */
+ sel |= (phys[intf] + 1) << (intf * 3);
+
+ /* Specify the @intf (1-indexed) of targeted PHY */
+ sel |= (intf + 1) << (6 + phys[intf] * 3);
From what I can see, phys[intf] is const. What about defining indexed masks
instead?
intf is the loop variable. What am I missing?
+ }
+
+ DPU_REG_WRITE(c, DP_PHY_INTF_SEL, sel);
+}
+
static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops,
unsigned long cap)
{
@@ -264,6 +285,8 @@ static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops,
ops->get_safe_status = dpu_hw_get_safe_status;
+ ops->dp_phy_intf_sel = dpu_hw_dp_phy_intf_sel;
Should this be gated for DPU < 4.0? Or 5.0?
+
if (cap & BIT(DPU_MDP_AUDIO_SELECT))
ops->intf_audio_select = dpu_hw_intf_audio_select;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
index a1a9e44bed36..8446d74d59b0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
@@ -125,6 +125,14 @@ struct dpu_hw_mdp_ops {
void (*get_safe_status)(struct dpu_hw_mdp *mdp,
struct dpu_danger_safe_status *status);
+ /**
+* dp_phy_intf_sel - configure
Re: [Freedreno] [PATCH v8 18/18] drm/msm/a6xx: Add A610 speedbin support
On Mon, May 29, 2023 at 03:52:37PM +0200, Konrad Dybcio wrote:
>
> A610 is implemented on at least three SoCs: SM6115 (bengal), SM6125
> (trinket) and SM6225 (khaje). Trinket does not support speed binning
> (only a single SKU exists) and we don't yet support khaje upstream.
> Hence, add a fuse mapping table for bengal to allow for per-chip
> frequency limiting.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index d046af5f6de2..c304fa118cff 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2098,6 +2098,30 @@ static bool a6xx_progress(struct msm_gpu *gpu, struct
> msm_ringbuffer *ring)
> return progress;
> }
>
> +static u32 a610_get_speed_bin(u32 fuse)
> +{
> + /*
> + * There are (at least) three SoCs implementing A610: SM6125 (trinket),
> + * SM6115 (bengal) and SM6225 (khaje). Trinket does not have
> speedbinning,
> + * as only a single SKU exists and we don't support khaje upstream yet.
> + * Hence, this matching table is only valid for bengal and can be easily
> + * expanded if need be.
> + */
> +
> + if (fuse == 0)
> + return 0;
> + else if (fuse == 206)
> + return 1;
> + else if (fuse == 200)
> + return 2;
> + else if (fuse == 157)
> + return 3;
> + else if (fuse == 127)
> + return 4;
> +
> + return UINT_MAX;
> +}
> +
> static u32 a618_get_speed_bin(u32 fuse)
> {
> if (fuse == 0)
> @@ -2195,6 +2219,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
> adreno_gpu *adreno_gpu, u3
> {
> u32 val = UINT_MAX;
>
> + if (adreno_is_a610(adreno_gpu))
> + val = a610_get_speed_bin(fuse);
> +
Didn't you update here to convert to 'else if' in one of the earlier
patches??
Reviewed-by: Akhil P Oommen
-Akhil.
> if (adreno_is_a618(adreno_gpu))
> val = a618_get_speed_bin(fuse);
>
>
> --
> 2.40.1
>
Re: [Freedreno] [PATCH v8 17/18] drm/msm/a6xx: Add A619_holi speedbin support
On Mon, May 29, 2023 at 03:52:36PM +0200, Konrad Dybcio wrote:
>
> A619_holi is implemented on at least two SoCs: SM4350 (holi) and SM6375
> (blair). This is what seems to be a first occurrence of this happening,
> but it's easy to overcome by guarding the SoC-specific fuse values with
> of_machine_is_compatible(). Do just that to enable frequency limiting
> on these SoCs.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
Reviewed-by: Akhil P Oommen
-Akhil
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 31 +++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index ca4ffa44097e..d046af5f6de2 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2110,6 +2110,34 @@ static u32 a618_get_speed_bin(u32 fuse)
> return UINT_MAX;
> }
>
> +static u32 a619_holi_get_speed_bin(u32 fuse)
> +{
> + /*
> + * There are (at least) two SoCs implementing A619_holi: SM4350 (holi)
> + * and SM6375 (blair). Limit the fuse matching to the corresponding
> + * SoC to prevent bogus frequency setting (as improbable as it may be,
> + * given unexpected fuse values are.. unexpected! But still possible.)
> + */
> +
> + if (fuse == 0)
> + return 0;
> +
> + if (of_machine_is_compatible("qcom,sm4350")) {
> + if (fuse == 138)
> + return 1;
> + else if (fuse == 92)
> + return 2;
> + } else if (of_machine_is_compatible("qcom,sm6375")) {
> + if (fuse == 190)
> + return 1;
> + else if (fuse == 177)
> + return 2;
> + } else
> + pr_warn("Unknown SoC implementing A619_holi!\n");
> +
> + return UINT_MAX;
> +}
> +
> static u32 a619_get_speed_bin(u32 fuse)
> {
> if (fuse == 0)
> @@ -2170,6 +2198,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
> adreno_gpu *adreno_gpu, u3
> if (adreno_is_a618(adreno_gpu))
> val = a618_get_speed_bin(fuse);
>
> + else if (adreno_is_a619_holi(adreno_gpu))
> + val = a619_holi_get_speed_bin(fuse);
> +
> else if (adreno_is_a619(adreno_gpu))
> val = a619_get_speed_bin(fuse);
>
>
> --
> 2.40.1
>
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 6/14/2023 12:35 PM, Abhinav Kumar wrote: On 6/14/2023 5:23 AM, Marijn Suijten wrote: On 2023-06-14 15:01:59, Dmitry Baryshkov wrote: On 14/06/2023 14:42, Marijn Suijten wrote: On 2023-06-13 18:57:11, Jessica Zhang wrote: DPU 5.x+ supports a databus widen mode that allows more data to be sent per pclk. Enable this feature flag on all relevant chipsets. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 36ba3f58dcdf..0be7bf0bfc41 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -103,7 +103,8 @@ (BIT(DPU_INTF_INPUT_CTRL) | \ BIT(DPU_INTF_TE) | \ BIT(DPU_INTF_STATUS_SUPPORTED) | \ - BIT(DPU_DATA_HCTL_EN)) + BIT(DPU_DATA_HCTL_EN) | \ + BIT(DPU_INTF_DATABUS_WIDEN)) This doesn't work. DPU 5.0.0 is SM8150, which has DSI 6G 2.3. In the last patch for DSI you state and enable widebus for DSI 6G 2.5+ only, meaning DPU and DSI are now desynced, and the output is completely corrupted. Tested this on SM8350 which actually has DSI 2.5, and it is also corrupted with this series so something else on this series might be broken. Missed this response. That seems strange. This series was tested on SM8350 HDK with a command mode panel. We will fix the DPU-DSI handshake and post a v2 but your issue needs investigation in parallel. So another bug to track that would be great. Is the bound in dsi_host wrong, or do DPU and DSI need to communicate when widebus will be enabled, based on DPU && DSI supporting it? I'd prefer to follow the second approach, as we did for DP. DPU asks the actual video output driver if widebus is to be enabled. I was afraid of this. This series was made on an assumption that the DPU version of widebus and DSI version of widebus would be compatible but looks like already SM8150 is an outlier. Yes, I think we have to go with second approach. DPU queries DSI if it supports widebus and enables it. Thanks for your responses. We will post a v2. Doesn't it seem very strange that DPU 5.x+ comes with a widebus feature, but the DSI does not until two revisions later? Or is this available on every interface, but only for a different (probably DP) encoder block? Yes its strange. - Marijn
Re: [Freedreno] [PATCH v8 16/18] drm/msm/a6xx: Use adreno_is_aXYZ macros in speedbin matching
On Mon, May 29, 2023 at 03:52:35PM +0200, Konrad Dybcio wrote:
>
> Before transitioning to using per-SoC and not per-Adreno speedbin
> fuse values (need another patchset to land elsewhere), a good
> improvement/stopgap solution is to use adreno_is_aXYZ macros in
> place of explicit revision matching. Do so to allow differentiating
> between A619 and A619_holi.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
Reviewed-by: Akhil P Oommen
-Akhil
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 18 +-
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 14 --
> 2 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 5faa85543428..ca4ffa44097e 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2163,23 +2163,23 @@ static u32 adreno_7c3_get_speed_bin(u32 fuse)
> return UINT_MAX;
> }
>
> -static u32 fuse_to_supp_hw(struct device *dev, struct adreno_rev rev, u32
> fuse)
> +static u32 fuse_to_supp_hw(struct device *dev, struct adreno_gpu
> *adreno_gpu, u32 fuse)
> {
> u32 val = UINT_MAX;
>
> - if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev))
> + if (adreno_is_a618(adreno_gpu))
> val = a618_get_speed_bin(fuse);
>
> - else if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
> + else if (adreno_is_a619(adreno_gpu))
> val = a619_get_speed_bin(fuse);
>
> - else if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
> + else if (adreno_is_7c3(adreno_gpu))
> val = adreno_7c3_get_speed_bin(fuse);
>
> - else if (adreno_cmp_rev(ADRENO_REV(6, 4, 0, ANY_ID), rev))
> + else if (adreno_is_a640(adreno_gpu))
> val = a640_get_speed_bin(fuse);
>
> - else if (adreno_cmp_rev(ADRENO_REV(6, 5, 0, ANY_ID), rev))
> + else if (adreno_is_a650(adreno_gpu))
> val = a650_get_speed_bin(fuse);
>
> if (val == UINT_MAX) {
> @@ -2192,7 +2192,7 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
> adreno_rev rev, u32 fuse)
> return (1 << val);
> }
>
> -static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev)
> +static int a6xx_set_supported_hw(struct device *dev, struct adreno_gpu
> *adreno_gpu)
> {
> u32 supp_hw;
> u32 speedbin;
> @@ -2211,7 +2211,7 @@ static int a6xx_set_supported_hw(struct device *dev,
> struct adreno_rev rev)
> return ret;
> }
>
> - supp_hw = fuse_to_supp_hw(dev, rev, speedbin);
> + supp_hw = fuse_to_supp_hw(dev, adreno_gpu, speedbin);
>
> ret = devm_pm_opp_set_supported_hw(dev, _hw, 1);
> if (ret)
> @@ -2330,7 +2330,7 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
>
> a6xx_llc_slices_init(pdev, a6xx_gpu);
>
> - ret = a6xx_set_supported_hw(>dev, config->rev);
> + ret = a6xx_set_supported_hw(>dev, adreno_gpu);
> if (ret) {
> a6xx_destroy(&(a6xx_gpu->base.base));
> return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 7a5d595d4b99..21513cec038f 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -268,9 +268,9 @@ static inline int adreno_is_a630(struct adreno_gpu *gpu)
> return gpu->revn == 630;
> }
>
> -static inline int adreno_is_a640_family(struct adreno_gpu *gpu)
> +static inline int adreno_is_a640(struct adreno_gpu *gpu)
> {
> - return (gpu->revn == 640) || (gpu->revn == 680);
> + return gpu->revn == 640;
> }
>
> static inline int adreno_is_a650(struct adreno_gpu *gpu)
> @@ -289,6 +289,11 @@ static inline int adreno_is_a660(struct adreno_gpu *gpu)
> return gpu->revn == 660;
> }
>
> +static inline int adreno_is_a680(struct adreno_gpu *gpu)
> +{
> + return gpu->revn == 680;
> +}
> +
> /* check for a615, a616, a618, a619 or any derivatives */
> static inline int adreno_is_a615_family(struct adreno_gpu *gpu)
> {
> @@ -306,6 +311,11 @@ static inline int adreno_is_a650_family(struct
> adreno_gpu *gpu)
> return gpu->revn == 650 || gpu->revn == 620 ||
> adreno_is_a660_family(gpu);
> }
>
> +static inline int adreno_is_a640_family(struct adreno_gpu *gpu)
> +{
> + return adreno_is_a640(gpu) || adreno_is_a680(gpu);
> +}
> +
> u64 adreno_private_address_space_size(struct msm_gpu *gpu);
> int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
>uint32_t param, uint64_t *value, uint32_t *len);
>
> --
> 2.40.1
>
Re: [Freedreno] [PATCH v8 15/18] drm/msm/a6xx: Use "else if" in GPU speedbin rev matching
On Mon, May 29, 2023 at 03:52:34PM +0200, Konrad Dybcio wrote:
>
> The GPU can only be one at a time. Turn a series of ifs into if +
> elseifs to save some CPU cycles.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
Reviewed-by: Akhil P Oommen
-Akhil
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 1a29e7dd9975..5faa85543428 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2170,16 +2170,16 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
> adreno_rev rev, u32 fuse)
> if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev))
> val = a618_get_speed_bin(fuse);
>
> - if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
> + else if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
> val = a619_get_speed_bin(fuse);
>
> - if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
> + else if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
> val = adreno_7c3_get_speed_bin(fuse);
>
> - if (adreno_cmp_rev(ADRENO_REV(6, 4, 0, ANY_ID), rev))
> + else if (adreno_cmp_rev(ADRENO_REV(6, 4, 0, ANY_ID), rev))
> val = a640_get_speed_bin(fuse);
>
> - if (adreno_cmp_rev(ADRENO_REV(6, 5, 0, ANY_ID), rev))
> + else if (adreno_cmp_rev(ADRENO_REV(6, 5, 0, ANY_ID), rev))
> val = a650_get_speed_bin(fuse);
>
> if (val == UINT_MAX) {
>
> --
> 2.40.1
>
Re: [Freedreno] [PATCH v8 14/18] drm/msm/a6xx: Fix some A619 tunables
On Mon, May 29, 2023 at 03:52:33PM +0200, Konrad Dybcio wrote:
>
> Adreno 619 expects some tunables to be set differently. Make up for it.
>
> Fixes: b7616b5c69e6 ("drm/msm/adreno: Add A619 support")
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index c0d5973320d9..1a29e7dd9975 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1198,6 +1198,8 @@ static int hw_init(struct msm_gpu *gpu)
> gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00200200);
> else if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu))
> gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00300200);
> + else if (adreno_is_a619(adreno_gpu))
> + gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00018000);
> else if (adreno_is_a610(adreno_gpu))
> gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x0008);
> else
> @@ -1215,7 +1217,9 @@ static int hw_init(struct msm_gpu *gpu)
> a6xx_set_ubwc_config(gpu);
>
> /* Enable fault detection */
> - if (adreno_is_a610(adreno_gpu))
> + if (adreno_is_a619(adreno_gpu))
> + gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30)
> | 0x3f);
> + else if (adreno_is_a610(adreno_gpu))
> gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30)
> | 0x3);
> else
> gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30)
> | 0x1f);
Reviewed-by: Akhil P Oommen
-Akhil
>
> --
> 2.40.1
>
Re: [Freedreno] [PATCH v8 13/18] drm/msm/a6xx: Add A610 support
On Mon, May 29, 2023 at 03:52:32PM +0200, Konrad Dybcio wrote:
>
> A610 is one of (if not the) lowest-tier SKUs in the A6XX family. It
> features no GMU, as it's implemented solely on SoCs with SMD_RPM.
> What's more interesting is that it does not feature a VDDGX line
> either, being powered solely by VDDCX and has an unfortunate hardware
> quirk that makes its reset line broken - after a couple of assert/
> deassert cycles, it will hang for good and will not wake up again.
>
> This GPU requires mesa changes for proper rendering, and lots of them
> at that. The command streams are quite far away from any other A6XX
> GPU and hence it needs special care. This patch was validated both
> by running an (incomplete) downstream mesa with some hacks (frames
> rendered correctly, though some instructions made the GPU hangcheck
> which is expected - garbage in, garbage out) and by replaying RD
> traces captured with the downstream KGSL driver - no crashes there,
> ever.
>
> Add support for this GPU on the kernel side, which comes down to
> pretty simply adding A612 HWCG tables, altering a few values and
> adding a special case for handling the reset line.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 101
> +
> drivers/gpu/drm/msm/adreno/adreno_device.c | 12
> drivers/gpu/drm/msm/adreno/adreno_gpu.h| 8 ++-
> 3 files changed, 108 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index bb04f65e6f68..c0d5973320d9 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -252,6 +252,56 @@ static void a6xx_submit(struct msm_gpu *gpu, struct
> msm_gem_submit *submit)
> a6xx_flush(gpu, ring);
> }
>
> +const struct adreno_reglist a612_hwcg[] = {
> + {REG_A6XX_RBBM_CLOCK_CNTL_SP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_CNTL2_SP0, 0x0220},
> + {REG_A6XX_RBBM_CLOCK_DELAY_SP0, 0x0081},
> + {REG_A6XX_RBBM_CLOCK_HYST_SP0, 0xf3cf},
> + {REG_A6XX_RBBM_CLOCK_CNTL_TP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_CNTL2_TP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_CNTL3_TP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_CNTL4_TP0, 0x0002},
> + {REG_A6XX_RBBM_CLOCK_DELAY_TP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_DELAY2_TP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_DELAY3_TP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_DELAY4_TP0, 0x0001},
> + {REG_A6XX_RBBM_CLOCK_HYST_TP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_HYST2_TP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_HYST3_TP0, 0x},
> + {REG_A6XX_RBBM_CLOCK_HYST4_TP0, 0x0007},
> + {REG_A6XX_RBBM_CLOCK_CNTL_RB0, 0x},
> + {REG_A6XX_RBBM_CLOCK_CNTL2_RB0, 0x0120},
> + {REG_A6XX_RBBM_CLOCK_CNTL_CCU0, 0x2220},
> + {REG_A6XX_RBBM_CLOCK_HYST_RB_CCU0, 0x00040f00},
> + {REG_A6XX_RBBM_CLOCK_CNTL_RAC, 0x05522022},
> + {REG_A6XX_RBBM_CLOCK_CNTL2_RAC, 0x},
> + {REG_A6XX_RBBM_CLOCK_DELAY_RAC, 0x0011},
> + {REG_A6XX_RBBM_CLOCK_HYST_RAC, 0x00445044},
> + {REG_A6XX_RBBM_CLOCK_CNTL_TSE_RAS_RBBM, 0x0422},
> + {REG_A6XX_RBBM_CLOCK_MODE_VFD, 0x},
> + {REG_A6XX_RBBM_CLOCK_MODE_GPC, 0x0222},
> + {REG_A6XX_RBBM_CLOCK_DELAY_HLSQ_2, 0x0002},
> + {REG_A6XX_RBBM_CLOCK_MODE_HLSQ, 0x},
> + {REG_A6XX_RBBM_CLOCK_DELAY_TSE_RAS_RBBM, 0x4000},
> + {REG_A6XX_RBBM_CLOCK_DELAY_VFD, 0x},
> + {REG_A6XX_RBBM_CLOCK_DELAY_GPC, 0x0200},
> + {REG_A6XX_RBBM_CLOCK_DELAY_HLSQ, 0x},
> + {REG_A6XX_RBBM_CLOCK_HYST_TSE_RAS_RBBM, 0x},
> + {REG_A6XX_RBBM_CLOCK_HYST_VFD, 0x},
> + {REG_A6XX_RBBM_CLOCK_HYST_GPC, 0x04104004},
> + {REG_A6XX_RBBM_CLOCK_HYST_HLSQ, 0x},
> + {REG_A6XX_RBBM_CLOCK_CNTL_UCHE, 0x},
> + {REG_A6XX_RBBM_CLOCK_HYST_UCHE, 0x0004},
> + {REG_A6XX_RBBM_CLOCK_DELAY_UCHE, 0x0002},
> + {REG_A6XX_RBBM_ISDB_CNT, 0x0182},
> + {REG_A6XX_RBBM_RAC_THRESHOLD_CNT, 0x},
> + {REG_A6XX_RBBM_SP_HYST_CNT, 0x},
> + {REG_A6XX_RBBM_CLOCK_CNTL_GMU_GX, 0x0222},
> + {REG_A6XX_RBBM_CLOCK_DELAY_GMU_GX, 0x0111},
> + {REG_A6XX_RBBM_CLOCK_HYST_GMU_GX, 0x0555},
> + {},
> +};
> +
> /* For a615 family (a615, a616, a618 and a619) */
> const struct adreno_reglist a615_hwcg[] = {
> {REG_A6XX_RBBM_CLOCK_CNTL_SP0, 0x0222},
> @@ -602,6 +652,8 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
>
> if (adreno_is_a630(adreno_gpu))
> clock_cntl_on = 0x8aa8aa02;
> + else if (adreno_is_a610(adreno_gpu))
> + clock_cntl_on = 0xaaa8aa82;
> else
> clock_cntl_on = 0x8aa8aa82;
>
> @@ -612,13 +664,15 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool
> state)
> return;
>
> /*
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 6/14/2023 5:23 AM, Marijn Suijten wrote: On 2023-06-14 15:01:59, Dmitry Baryshkov wrote: On 14/06/2023 14:42, Marijn Suijten wrote: On 2023-06-13 18:57:11, Jessica Zhang wrote: DPU 5.x+ supports a databus widen mode that allows more data to be sent per pclk. Enable this feature flag on all relevant chipsets. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 36ba3f58dcdf..0be7bf0bfc41 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -103,7 +103,8 @@ (BIT(DPU_INTF_INPUT_CTRL) | \ BIT(DPU_INTF_TE) | \ BIT(DPU_INTF_STATUS_SUPPORTED) | \ -BIT(DPU_DATA_HCTL_EN)) +BIT(DPU_DATA_HCTL_EN) | \ +BIT(DPU_INTF_DATABUS_WIDEN)) This doesn't work. DPU 5.0.0 is SM8150, which has DSI 6G 2.3. In the last patch for DSI you state and enable widebus for DSI 6G 2.5+ only, meaning DPU and DSI are now desynced, and the output is completely corrupted. Tested this on SM8350 which actually has DSI 2.5, and it is also corrupted with this series so something else on this series might be broken. Is the bound in dsi_host wrong, or do DPU and DSI need to communicate when widebus will be enabled, based on DPU && DSI supporting it? I'd prefer to follow the second approach, as we did for DP. DPU asks the actual video output driver if widebus is to be enabled. I was afraid of this. This series was made on an assumption that the DPU version of widebus and DSI version of widebus would be compatible but looks like already SM8150 is an outlier. Yes, I think we have to go with second approach. DPU queries DSI if it supports widebus and enables it. Thanks for your responses. We will post a v2. Doesn't it seem very strange that DPU 5.x+ comes with a widebus feature, but the DSI does not until two revisions later? Or is this available on every interface, but only for a different (probably DP) encoder block? Yes its strange. - Marijn
Re: [Freedreno] [PATCH 0/3] drm/msm/adreno: GPU support on SC8280XP
On Tue, 7 Feb 2023 19:40:49 -0800, Bjorn Andersson wrote: > This series introduces support for A690 in the DRM/MSM driver and > enables it for the two SC8280XP laptops. > > Bjorn Andersson (3): > drm/msm/adreno: Add Adreno A690 support > arm64: dts: qcom: sc8280xp: Add GPU related nodes > arm64: dts: qcom: sc8280xp: Enable GPU related nodes > > [...] Applied, thanks! [1/3] drm/msm/adreno: Add Adreno A690 support (no commit info) [2/3] arm64: dts: qcom: sc8280xp: Add GPU related nodes commit: eec51ab2fd6f447a993c502364704d0cb5bc8cae [3/3] arm64: dts: qcom: sc8280xp: Enable GPU related nodes commit: 598a06afca5a2ab4850ce9ff8146ec728cca570c Best regards, -- Bjorn Andersson
Re: [Freedreno] [PATCH v4 0/2] drm/msm/adreno: GPU support on SC8280XP
On Wed, 14 Jun 2023 07:22:02 -0700, Bjorn Andersson wrote: > With the A690 support merged in the drm/msm driver, this series adds the > DeviceTree pieces to make it go on sc8280xp. > > Note that in order for the GPU driver to probe, the last change > requires (which is now in linux-next): > https://lore.kernel.org/linux-arm-msm/20230410185226.3240336-1-dmitry.barysh...@linaro.org/ > > [...] Applied, thanks! [1/2] arm64: dts: qcom: sc8280xp: Add GPU related nodes commit: eec51ab2fd6f447a993c502364704d0cb5bc8cae [2/2] arm64: dts: qcom: sc8280xp: Enable GPU related nodes commit: 598a06afca5a2ab4850ce9ff8146ec728cca570c Best regards, -- Bjorn Andersson
Re: [Freedreno] [PATCH v2 0/3] drm/msm/adreno: GPU support on SC8280XP
On Mon, 22 May 2023 18:15:19 -0700, Bjorn Andersson wrote: > This series introduces support for A690 in the DRM/MSM driver and > enables it for the two SC8280XP laptops. > > Bjorn Andersson (3): > drm/msm/adreno: Add Adreno A690 support > arm64: dts: qcom: sc8280xp: Add GPU related nodes > arm64: dts: qcom: sc8280xp: Enable GPU related nodes > > [...] Applied, thanks! [1/3] drm/msm/adreno: Add Adreno A690 support (no commit info) [2/3] arm64: dts: qcom: sc8280xp: Add GPU related nodes commit: eec51ab2fd6f447a993c502364704d0cb5bc8cae [3/3] arm64: dts: qcom: sc8280xp: Enable GPU related nodes commit: 598a06afca5a2ab4850ce9ff8146ec728cca570c Best regards, -- Bjorn Andersson
Re: [Freedreno] [PATCH v4 2/2] arm64: dts: qcom: sc8280xp: Enable GPU related nodes
On Wed, Jun 14, 2023 at 05:27:24PM +0200, Konrad Dybcio wrote:
> On 14.06.2023 16:22, Bjorn Andersson wrote:
> > From: Bjorn Andersson
> >
> > Add memory reservation for the zap-shader and enable the Adreno SMMU,
> > GPU clock controller, GMU and the GPU nodes for the SC8280XP CRD and the
> > Lenovo ThinkPad X13s.
> >
> > Tested-by: Steev Klimaszewski
> > Signed-off-by: Bjorn Andersson
> > Tested-by: Johan Hovold
> > Signed-off-by: Bjorn Andersson
> > ---
> Reviewed-by: Konrad Dybcio
>
Thanks.
> one question below
> > arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 14 ++
> > .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 14 ++
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > index cd7e0097d8bc..b566e403d1db 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > @@ -210,6 +210,11 @@ vreg_wwan: regulator-wwan {
> > };
> >
> > reserved-memory {
> > + gpu_mem: gpu-mem@8bf0 {
> Is it ever going to differ on other platforms, including the automotive ones?
>
The memory maps for the two live different lives.
Regards,
Bjorn
> Konrad
> > + reg = <0 0x8bf0 0 0x2000>;
> > + no-map;
> > + };
> > +
> > linux,cma {
> > compatible = "shared-dma-pool";
> > size = <0x0 0x800>;
> > @@ -390,6 +395,15 @@ {
> > status = "okay";
> > };
> >
> > + {
> > + status = "okay";
> > +
> > + zap-shader {
> > + memory-region = <_mem>;
> > + firmware-name = "qcom/sc8280xp/qcdxkmsuc8280.mbn";
> > + };
> > +};
> > +
> > {
> > status = "okay";
> > };
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > index 5ae057ad6438..7cc3028440b6 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > @@ -264,6 +264,11 @@ vreg_wwan: regulator-wwan {
> > };
> >
> > reserved-memory {
> > + gpu_mem: gpu-mem@8bf0 {
> > + reg = <0 0x8bf0 0 0x2000>;
> > + no-map;
> > + };
> > +
> > linux,cma {
> > compatible = "shared-dma-pool";
> > size = <0x0 0x800>;
> > @@ -518,6 +523,15 @@ {
> > status = "okay";
> > };
> >
> > + {
> > + status = "okay";
> > +
> > + zap-shader {
> > + memory-region = <_mem>;
> > + firmware-name = "qcom/sc8280xp/LENOVO/21BX/qcdxkmsuc8280.mbn";
> > + };
> > +};
> > +
> > {
> > status = "okay";
> > };
Re: [Freedreno] [PATCH v4 2/2] arm64: dts: qcom: sc8280xp: Enable GPU related nodes
On 14.06.2023 16:22, Bjorn Andersson wrote:
> From: Bjorn Andersson
>
> Add memory reservation for the zap-shader and enable the Adreno SMMU,
> GPU clock controller, GMU and the GPU nodes for the SC8280XP CRD and the
> Lenovo ThinkPad X13s.
>
> Tested-by: Steev Klimaszewski
> Signed-off-by: Bjorn Andersson
> Tested-by: Johan Hovold
> Signed-off-by: Bjorn Andersson
> ---
Reviewed-by: Konrad Dybcio
one question below
> arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 14 ++
> .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 14 ++
> 2 files changed, 28 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> index cd7e0097d8bc..b566e403d1db 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> @@ -210,6 +210,11 @@ vreg_wwan: regulator-wwan {
> };
>
> reserved-memory {
> + gpu_mem: gpu-mem@8bf0 {
Is it ever going to differ on other platforms, including the automotive ones?
Konrad
> + reg = <0 0x8bf0 0 0x2000>;
> + no-map;
> + };
> +
> linux,cma {
> compatible = "shared-dma-pool";
> size = <0x0 0x800>;
> @@ -390,6 +395,15 @@ {
> status = "okay";
> };
>
> + {
> + status = "okay";
> +
> + zap-shader {
> + memory-region = <_mem>;
> + firmware-name = "qcom/sc8280xp/qcdxkmsuc8280.mbn";
> + };
> +};
> +
> {
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> index 5ae057ad6438..7cc3028440b6 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> @@ -264,6 +264,11 @@ vreg_wwan: regulator-wwan {
> };
>
> reserved-memory {
> + gpu_mem: gpu-mem@8bf0 {
> + reg = <0 0x8bf0 0 0x2000>;
> + no-map;
> + };
> +
> linux,cma {
> compatible = "shared-dma-pool";
> size = <0x0 0x800>;
> @@ -518,6 +523,15 @@ {
> status = "okay";
> };
>
> + {
> + status = "okay";
> +
> + zap-shader {
> + memory-region = <_mem>;
> + firmware-name = "qcom/sc8280xp/LENOVO/21BX/qcdxkmsuc8280.mbn";
> + };
> +};
> +
> {
> status = "okay";
> };
Re: [Freedreno] [PATCH v4 1/2] arm64: dts: qcom: sc8280xp: Add GPU related nodes
On 14.06.2023 16:22, Bjorn Andersson wrote:
> From: Bjorn Andersson
>
> Add Adreno SMMU, GPU clock controller, GMU and GPU nodes for the
> SC8280XP.
>
> Tested-by: Steev Klimaszewski
> Signed-off-by: Bjorn Andersson
> Tested-by: Johan Hovold
> Signed-off-by: Bjorn Andersson
> ---
Reviewed-by: Konrad Dybcio
Konrad
> arch/arm64/boot/dts/qcom/sa8540p.dtsi | 8 ++
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 175 +
> 2 files changed, 183 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> index 4a990fda8fc3..bacbdec56281 100644
> --- a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> @@ -167,6 +167,14 @@ opp-259200 {
> };
> };
>
> + {
> + status = "disabled";
> +};
> +
> +_smmu {
> + status = "disabled";
> +};
> +
> {
> compatible = "qcom,pcie-sa8540p";
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 6b1bb203b1d1..ac0596dfdbc4 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -6,6 +6,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -2341,6 +2342,180 @@ tcsr: syscon@1fc {
> reg = <0x0 0x01fc 0x0 0x3>;
> };
>
> + gpu: gpu@3d0 {
> + compatible = "qcom,adreno-690.0", "qcom,adreno";
> +
> + reg = <0 0x03d0 0 0x4>,
> + <0 0x03d9e000 0 0x1000>,
> + <0 0x03d61000 0 0x800>;
> + reg-names = "kgsl_3d0_reg_memory",
> + "cx_mem",
> + "cx_dbgc";
> + interrupts = ;
> + iommus = <_smmu 0 0xc00>, <_smmu 1 0xc00>;
> + operating-points-v2 = <_opp_table>;
> +
> + qcom,gmu = <>;
> + interconnects = <_noc MASTER_GFX3D 0 _virt
> SLAVE_EBI1 0>;
> + interconnect-names = "gfx-mem";
> + #cooling-cells = <2>;
> +
> + status = "disabled";
> +
> + gpu_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-27000 {
> + opp-hz = /bits/ 64 <27000>;
> + opp-level =
> ;
> + opp-peak-kBps = <451000>;
> + };
> +
> + opp-41000 {
> + opp-hz = /bits/ 64 <41000>;
> + opp-level = ;
> + opp-peak-kBps = <1555000>;
> + };
> +
> + opp-5 {
> + opp-hz = /bits/ 64 <5>;
> + opp-level =
> ;
> + opp-peak-kBps = <1555000>;
> + };
> +
> + opp-54700 {
> + opp-hz = /bits/ 64 <54700>;
> + opp-level =
> ;
> + opp-peak-kBps = <1555000>;
> + };
> +
> + opp-60600 {
> + opp-hz = /bits/ 64 <60600>;
> + opp-level = ;
> + opp-peak-kBps = <2736000>;
> + };
> +
> + opp-64000 {
> + opp-hz = /bits/ 64 <64000>;
> + opp-level =
> ;
> + opp-peak-kBps = <2736000>;
> + };
> +
> + opp-65500 {
> + opp-hz = /bits/ 64 <65500>;
> + opp-level =
> ;
> + opp-peak-kBps = <2736000>;
> + };
> +
> + opp-69000 {
> + opp-hz = /bits/ 64 <69000>;
> + opp-level =
> ;
> + opp-peak-kBps = <2736000>;
> + };
> + };
> + };
> +
> + gmu: gmu@3d6a000 {
> + compatible = "qcom,adreno-gmu-690.0", "qcom,adreno-gmu";
> + reg = <0 0x03d6a000 0 0x34000>,
> + <0 0x03de 0 0x1>,
> + <0 0x0b29 0 0x1>;
> +
[Freedreno] [PATCH v4 2/2] arm64: dts: qcom: sc8280xp: Enable GPU related nodes
From: Bjorn Andersson
Add memory reservation for the zap-shader and enable the Adreno SMMU,
GPU clock controller, GMU and the GPU nodes for the SC8280XP CRD and the
Lenovo ThinkPad X13s.
Tested-by: Steev Klimaszewski
Signed-off-by: Bjorn Andersson
Tested-by: Johan Hovold
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 14 ++
.../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 14 ++
2 files changed, 28 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index cd7e0097d8bc..b566e403d1db 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -210,6 +210,11 @@ vreg_wwan: regulator-wwan {
};
reserved-memory {
+ gpu_mem: gpu-mem@8bf0 {
+ reg = <0 0x8bf0 0 0x2000>;
+ no-map;
+ };
+
linux,cma {
compatible = "shared-dma-pool";
size = <0x0 0x800>;
@@ -390,6 +395,15 @@ {
status = "okay";
};
+ {
+ status = "okay";
+
+ zap-shader {
+ memory-region = <_mem>;
+ firmware-name = "qcom/sc8280xp/qcdxkmsuc8280.mbn";
+ };
+};
+
{
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 5ae057ad6438..7cc3028440b6 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -264,6 +264,11 @@ vreg_wwan: regulator-wwan {
};
reserved-memory {
+ gpu_mem: gpu-mem@8bf0 {
+ reg = <0 0x8bf0 0 0x2000>;
+ no-map;
+ };
+
linux,cma {
compatible = "shared-dma-pool";
size = <0x0 0x800>;
@@ -518,6 +523,15 @@ {
status = "okay";
};
+ {
+ status = "okay";
+
+ zap-shader {
+ memory-region = <_mem>;
+ firmware-name = "qcom/sc8280xp/LENOVO/21BX/qcdxkmsuc8280.mbn";
+ };
+};
+
{
status = "okay";
};
--
2.25.1
[Freedreno] [PATCH v4 1/2] arm64: dts: qcom: sc8280xp: Add GPU related nodes
From: Bjorn Andersson
Add Adreno SMMU, GPU clock controller, GMU and GPU nodes for the
SC8280XP.
Tested-by: Steev Klimaszewski
Signed-off-by: Bjorn Andersson
Tested-by: Johan Hovold
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/sa8540p.dtsi | 8 ++
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 175 +
2 files changed, 183 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
index 4a990fda8fc3..bacbdec56281 100644
--- a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
@@ -167,6 +167,14 @@ opp-259200 {
};
};
+ {
+ status = "disabled";
+};
+
+_smmu {
+ status = "disabled";
+};
+
{
compatible = "qcom,pcie-sa8540p";
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 6b1bb203b1d1..ac0596dfdbc4 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -6,6 +6,7 @@
#include
#include
+#include
#include
#include
#include
@@ -2341,6 +2342,180 @@ tcsr: syscon@1fc {
reg = <0x0 0x01fc 0x0 0x3>;
};
+ gpu: gpu@3d0 {
+ compatible = "qcom,adreno-690.0", "qcom,adreno";
+
+ reg = <0 0x03d0 0 0x4>,
+ <0 0x03d9e000 0 0x1000>,
+ <0 0x03d61000 0 0x800>;
+ reg-names = "kgsl_3d0_reg_memory",
+ "cx_mem",
+ "cx_dbgc";
+ interrupts = ;
+ iommus = <_smmu 0 0xc00>, <_smmu 1 0xc00>;
+ operating-points-v2 = <_opp_table>;
+
+ qcom,gmu = <>;
+ interconnects = <_noc MASTER_GFX3D 0 _virt
SLAVE_EBI1 0>;
+ interconnect-names = "gfx-mem";
+ #cooling-cells = <2>;
+
+ status = "disabled";
+
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-27000 {
+ opp-hz = /bits/ 64 <27000>;
+ opp-level =
;
+ opp-peak-kBps = <451000>;
+ };
+
+ opp-41000 {
+ opp-hz = /bits/ 64 <41000>;
+ opp-level = ;
+ opp-peak-kBps = <1555000>;
+ };
+
+ opp-5 {
+ opp-hz = /bits/ 64 <5>;
+ opp-level =
;
+ opp-peak-kBps = <1555000>;
+ };
+
+ opp-54700 {
+ opp-hz = /bits/ 64 <54700>;
+ opp-level =
;
+ opp-peak-kBps = <1555000>;
+ };
+
+ opp-60600 {
+ opp-hz = /bits/ 64 <60600>;
+ opp-level = ;
+ opp-peak-kBps = <2736000>;
+ };
+
+ opp-64000 {
+ opp-hz = /bits/ 64 <64000>;
+ opp-level =
;
+ opp-peak-kBps = <2736000>;
+ };
+
+ opp-65500 {
+ opp-hz = /bits/ 64 <65500>;
+ opp-level =
;
+ opp-peak-kBps = <2736000>;
+ };
+
+ opp-69000 {
+ opp-hz = /bits/ 64 <69000>;
+ opp-level =
;
+ opp-peak-kBps = <2736000>;
+ };
+ };
+ };
+
+ gmu: gmu@3d6a000 {
+ compatible = "qcom,adreno-gmu-690.0", "qcom,adreno-gmu";
+ reg = <0 0x03d6a000 0 0x34000>,
+ <0 0x03de 0 0x1>,
+ <0 0x0b29 0 0x1>;
+ reg-names = "gmu", "rscc", "gmu_pdc";
+ interrupts = ,
+;
+ interrupt-names = "hfi", "gmu";
+ clocks
[Freedreno] [PATCH v4 0/2] drm/msm/adreno: GPU support on SC8280XP
With the A690 support merged in the drm/msm driver, this series adds the DeviceTree pieces to make it go on sc8280xp. Note that in order for the GPU driver to probe, the last change requires (which is now in linux-next): https://lore.kernel.org/linux-arm-msm/20230410185226.3240336-1-dmitry.barysh...@linaro.org/ Changes since v3: - Dropped DRM patch, as it has been merged - Left status okay on gmu, gpucc and smmu nodes, disabled latter two in sa8540p for now - s/adreno_smmu/gpu_smmu/ to get the GPU nodes adjacent in sa8540p Changes since v2: - Added missing opp level (both gpu and gmu) - Corrected opp-level for highest gpu opp - Added dma-coherent to gpu smmu Changes since v1: - Dropped gmu_pdc_seq region from , as it shouldn't have been used. - Added missing compatible to _smmu. - Dropped aoss_qmp clock in and _smmu. Bjorn Andersson (2): arm64: dts: qcom: sc8280xp: Add GPU related nodes arm64: dts: qcom: sc8280xp: Enable GPU related nodes arch/arm64/boot/dts/qcom/sa8540p.dtsi | 8 + arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 14 ++ .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts| 14 ++ arch/arm64/boot/dts/qcom/sc8280xp.dtsi| 175 ++ 4 files changed, 211 insertions(+) -- 2.25.1
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 2023-06-14 15:01:59, Dmitry Baryshkov wrote: > On 14/06/2023 14:42, Marijn Suijten wrote: > > On 2023-06-13 18:57:11, Jessica Zhang wrote: > >> DPU 5.x+ supports a databus widen mode that allows more data to be sent > >> per pclk. Enable this feature flag on all relevant chipsets. > >> > >> Signed-off-by: Jessica Zhang > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ > >> 2 files changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> index 36ba3f58dcdf..0be7bf0bfc41 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> @@ -103,7 +103,8 @@ > >>(BIT(DPU_INTF_INPUT_CTRL) | \ > >> BIT(DPU_INTF_TE) | \ > >> BIT(DPU_INTF_STATUS_SUPPORTED) | \ > >> - BIT(DPU_DATA_HCTL_EN)) > >> + BIT(DPU_DATA_HCTL_EN) | \ > >> + BIT(DPU_INTF_DATABUS_WIDEN)) > > > > This doesn't work. DPU 5.0.0 is SM8150, which has DSI 6G 2.3. In the > > last patch for DSI you state and enable widebus for DSI 6G 2.5+ only, > > meaning DPU and DSI are now desynced, and the output is completely > > corrupted. Tested this on SM8350 which actually has DSI 2.5, and it is also corrupted with this series so something else on this series might be broken. > > Is the bound in dsi_host wrong, or do DPU and DSI need to communicate > > when widebus will be enabled, based on DPU && DSI supporting it? > > I'd prefer to follow the second approach, as we did for DP. DPU asks the > actual video output driver if widebus is to be enabled. Doesn't it seem very strange that DPU 5.x+ comes with a widebus feature, but the DSI does not until two revisions later? Or is this available on every interface, but only for a different (probably DP) encoder block? - Marijn
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 14/06/2023 14:42, Marijn Suijten wrote:
On 2023-06-13 18:57:11, Jessica Zhang wrote:
DPU 5.x+ supports a databus widen mode that allows more data to be sent
per pclk. Enable this feature flag on all relevant chipsets.
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 36ba3f58dcdf..0be7bf0bfc41 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -103,7 +103,8 @@
(BIT(DPU_INTF_INPUT_CTRL) | \
BIT(DPU_INTF_TE) | \
BIT(DPU_INTF_STATUS_SUPPORTED) | \
-BIT(DPU_DATA_HCTL_EN))
+BIT(DPU_DATA_HCTL_EN) | \
+BIT(DPU_INTF_DATABUS_WIDEN))
This doesn't work. DPU 5.0.0 is SM8150, which has DSI 6G 2.3. In the
last patch for DSI you state and enable widebus for DSI 6G 2.5+ only,
meaning DPU and DSI are now desynced, and the output is completely
corrupted.
Is the bound in dsi_host wrong, or do DPU and DSI need to communicate
when widebus will be enabled, based on DPU && DSI supporting it?
I'd prefer to follow the second approach, as we did for DP. DPU asks the
actual video output driver if widebus is to be enabled.
- Marijn
#define INTF_SC7280_MASK (INTF_SC7180_MASK | BIT(DPU_INTF_DATA_COMPRESS))
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index b860784ade72..b9939e00f5e0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -182,6 +182,7 @@ enum {
* than video timing
* @DPU_INTF_STATUS_SUPPORTED INTF block has INTF_STATUS register
* @DPU_INTF_DATA_COMPRESS INTF block has DATA_COMPRESS register
+ * @DPU_INTF_DATABUS_WIDEN INTF block has DATABUS_WIDEN register
* @DPU_INTF_MAX
*/
enum {
@@ -190,6 +191,7 @@ enum {
DPU_DATA_HCTL_EN,
DPU_INTF_STATUS_SUPPORTED,
DPU_INTF_DATA_COMPRESS,
+ DPU_INTF_DATABUS_WIDEN,
DPU_INTF_MAX
};
--
2.40.1
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v5 02/13] fbdev: Add initializer macros for struct fb_ops
Hi
Am 14.06.23 um 13:29 schrieb Christian König:
Am 30.05.23 um 17:02 schrieb Thomas Zimmermann:
For framebuffers in I/O and system memory, add macros that set
struct fb_ops to the respective callback functions.
For deferred I/O, add macros that generate callback functions with
damage handling. Add initializer macros that set struct fb_ops to
the generated callbacks.
These macros can remove a lot boilerplate code from fbdev drivers.
The drivers are supposed to use the macro that is required for its
framebuffer. Each macro is split into smaller helpers, so that
drivers with non-standard callbacks can pick and customize callbacks
as needed. There are individual helper macros for read/write, mmap
and drawing.
v5:
* fix whitespace errors (Jingfeng)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Sam Ravnborg
---
include/linux/fb.h | 112 +
1 file changed, 112 insertions(+)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 2cf8efcb9e32..ce6823e157e6 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -538,9 +538,31 @@ extern ssize_t fb_io_read(struct fb_info *info,
char __user *buf,
extern ssize_t fb_io_write(struct fb_info *info, const char __user
*buf,
size_t count, loff_t *ppos);
+/*
+ * Initializes struct fb_ops for framebuffers in I/O memory.
+ */
+
+#define __FB_DEFAULT_IO_OPS_RDWR \
+ .fb_read = fb_io_read, \
+ .fb_write = fb_io_write
+
+#define __FB_DEFAULT_IO_OPS_DRAW \
+ .fb_fillrect = cfb_fillrect, \
+ .fb_copyarea = cfb_copyarea, \
+ .fb_imageblit = cfb_imageblit
+
+#define __FB_DEFAULT_IO_OPS_MMAP \
+ .fb_mmap = NULL // default implementation
// style comment in a macro? That's usually a very bad idea.
I think I see it now. Thanks! That should delete any commas at the end
of the line. I'll send out an update. It works so far, as I only used
that macro in the correct way.
Best regards
Thomas
Christian.
+
+#define FB_DEFAULT_IO_OPS \
+ __FB_DEFAULT_IO_OPS_RDWR, \
+ __FB_DEFAULT_IO_OPS_DRAW, \
+ __FB_DEFAULT_IO_OPS_MMAP
+
/*
* Drawing operations where framebuffer is in system RAM
*/
+
extern void sys_fillrect(struct fb_info *info, const struct
fb_fillrect *rect);
extern void sys_copyarea(struct fb_info *info, const struct
fb_copyarea *area);
extern void sys_imageblit(struct fb_info *info, const struct
fb_image *image);
@@ -549,6 +571,27 @@ extern ssize_t fb_sys_read(struct fb_info *info,
char __user *buf,
extern ssize_t fb_sys_write(struct fb_info *info, const char __user
*buf,
size_t count, loff_t *ppos);
+/*
+ * Initializes struct fb_ops for framebuffers in system memory.
+ */
+
+#define __FB_DEFAULT_SYS_OPS_RDWR \
+ .fb_read = fb_sys_read, \
+ .fb_write = fb_sys_write
+
+#define __FB_DEFAULT_SYS_OPS_DRAW \
+ .fb_fillrect = sys_fillrect, \
+ .fb_copyarea = sys_copyarea, \
+ .fb_imageblit = sys_imageblit
+
+#define __FB_DEFAULT_SYS_OPS_MMAP \
+ .fb_mmap = NULL // default implementation
+
+#define FB_DEFAULT_SYS_OPS \
+ __FB_DEFAULT_SYS_OPS_RDWR, \
+ __FB_DEFAULT_SYS_OPS_DRAW, \
+ __FB_DEFAULT_SYS_OPS_MMAP
+
/* drivers/video/fbmem.c */
extern int register_framebuffer(struct fb_info *fb_info);
extern void unregister_framebuffer(struct fb_info *fb_info);
@@ -604,6 +647,75 @@ extern void fb_deferred_io_cleanup(struct fb_info
*info);
extern int fb_deferred_io_fsync(struct file *file, loff_t start,
loff_t end, int datasync);
+/*
+ * Generate callbacks for deferred I/O
+ */
+
+#define __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range,
__mode) \
+ static ssize_t __prefix ## _defio_read(struct fb_info *info, char
__user *buf, \
+ size_t count, loff_t *ppos) \
+ { \
+ return fb_ ## __mode ## _read(info, buf, count, ppos); \
+ } \
+ static ssize_t __prefix ## _defio_write(struct fb_info *info,
const char __user *buf, \
+ size_t count, loff_t *ppos) \
+ { \
+ unsigned long offset = *ppos; \
+ ssize_t ret = fb_ ## __mode ## _write(info, buf, count, ppos); \
+ if (ret > 0) \
+ __damage_range(info, offset, ret); \
+ return ret; \
+ }
+
+#define __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area,
__mode) \
+ static void __prefix ## _defio_fillrect(struct fb_info *info, \
+ const struct fb_fillrect *rect) \
+ { \
+ __mode ## _fillrect(info, rect); \
+ __damage_area(info, rect->dx, rect->dy, rect->width,
rect->height); \
+ } \
+ static void __prefix ## _defio_copyarea(struct fb_info *info, \
+ const struct fb_copyarea *area) \
+ { \
+ __mode ## _copyarea(info, area); \
+ __damage_area(info, area->dx, area->dy, area->width,
area->height); \
+ } \
+ static void __prefix ## _defio_imageblit(struct fb_info *info, \
+
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 2023-06-13 18:57:11, Jessica Zhang wrote:
> DPU 5.x+ supports a databus widen mode that allows more data to be sent
> per pclk. Enable this feature flag on all relevant chipsets.
>
> Signed-off-by: Jessica Zhang
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index 36ba3f58dcdf..0be7bf0bfc41 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -103,7 +103,8 @@
> (BIT(DPU_INTF_INPUT_CTRL) | \
>BIT(DPU_INTF_TE) | \
>BIT(DPU_INTF_STATUS_SUPPORTED) | \
> - BIT(DPU_DATA_HCTL_EN))
> + BIT(DPU_DATA_HCTL_EN) | \
> + BIT(DPU_INTF_DATABUS_WIDEN))
This doesn't work. DPU 5.0.0 is SM8150, which has DSI 6G 2.3. In the
last patch for DSI you state and enable widebus for DSI 6G 2.5+ only,
meaning DPU and DSI are now desynced, and the output is completely
corrupted.
Is the bound in dsi_host wrong, or do DPU and DSI need to communicate
when widebus will be enabled, based on DPU && DSI supporting it?
- Marijn
> #define INTF_SC7280_MASK (INTF_SC7180_MASK | BIT(DPU_INTF_DATA_COMPRESS))
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index b860784ade72..b9939e00f5e0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -182,6 +182,7 @@ enum {
> * than video timing
> * @DPU_INTF_STATUS_SUPPORTED INTF block has INTF_STATUS register
> * @DPU_INTF_DATA_COMPRESS INTF block has DATA_COMPRESS register
> + * @DPU_INTF_DATABUS_WIDEN INTF block has DATABUS_WIDEN register
> * @DPU_INTF_MAX
> */
> enum {
> @@ -190,6 +191,7 @@ enum {
> DPU_DATA_HCTL_EN,
> DPU_INTF_STATUS_SUPPORTED,
> DPU_INTF_DATA_COMPRESS,
> + DPU_INTF_DATABUS_WIDEN,
> DPU_INTF_MAX
> };
>
>
> --
> 2.40.1
>
Re: [Freedreno] [PATCH v5 02/13] fbdev: Add initializer macros for struct fb_ops
Am 30.05.23 um 17:02 schrieb Thomas Zimmermann:
For framebuffers in I/O and system memory, add macros that set
struct fb_ops to the respective callback functions.
For deferred I/O, add macros that generate callback functions with
damage handling. Add initializer macros that set struct fb_ops to
the generated callbacks.
These macros can remove a lot boilerplate code from fbdev drivers.
The drivers are supposed to use the macro that is required for its
framebuffer. Each macro is split into smaller helpers, so that
drivers with non-standard callbacks can pick and customize callbacks
as needed. There are individual helper macros for read/write, mmap
and drawing.
v5:
* fix whitespace errors (Jingfeng)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Sam Ravnborg
---
include/linux/fb.h | 112 +
1 file changed, 112 insertions(+)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 2cf8efcb9e32..ce6823e157e6 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -538,9 +538,31 @@ extern ssize_t fb_io_read(struct fb_info *info, char
__user *buf,
extern ssize_t fb_io_write(struct fb_info *info, const char __user *buf,
size_t count, loff_t *ppos);
+/*
+ * Initializes struct fb_ops for framebuffers in I/O memory.
+ */
+
+#define __FB_DEFAULT_IO_OPS_RDWR \
+ .fb_read= fb_io_read, \
+ .fb_write = fb_io_write
+
+#define __FB_DEFAULT_IO_OPS_DRAW \
+ .fb_fillrect= cfb_fillrect, \
+ .fb_copyarea= cfb_copyarea, \
+ .fb_imageblit = cfb_imageblit
+
+#define __FB_DEFAULT_IO_OPS_MMAP \
+ .fb_mmap= NULL // default implementation
// style comment in a macro? That's usually a very bad idea.
Christian.
+
+#define FB_DEFAULT_IO_OPS \
+ __FB_DEFAULT_IO_OPS_RDWR, \
+ __FB_DEFAULT_IO_OPS_DRAW, \
+ __FB_DEFAULT_IO_OPS_MMAP
+
/*
* Drawing operations where framebuffer is in system RAM
*/
+
extern void sys_fillrect(struct fb_info *info, const struct fb_fillrect
*rect);
extern void sys_copyarea(struct fb_info *info, const struct fb_copyarea
*area);
extern void sys_imageblit(struct fb_info *info, const struct fb_image *image);
@@ -549,6 +571,27 @@ extern ssize_t fb_sys_read(struct fb_info *info, char
__user *buf,
extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
size_t count, loff_t *ppos);
+/*
+ * Initializes struct fb_ops for framebuffers in system memory.
+ */
+
+#define __FB_DEFAULT_SYS_OPS_RDWR \
+ .fb_read= fb_sys_read, \
+ .fb_write = fb_sys_write
+
+#define __FB_DEFAULT_SYS_OPS_DRAW \
+ .fb_fillrect= sys_fillrect, \
+ .fb_copyarea= sys_copyarea, \
+ .fb_imageblit = sys_imageblit
+
+#define __FB_DEFAULT_SYS_OPS_MMAP \
+ .fb_mmap= NULL // default implementation
+
+#define FB_DEFAULT_SYS_OPS \
+ __FB_DEFAULT_SYS_OPS_RDWR, \
+ __FB_DEFAULT_SYS_OPS_DRAW, \
+ __FB_DEFAULT_SYS_OPS_MMAP
+
/* drivers/video/fbmem.c */
extern int register_framebuffer(struct fb_info *fb_info);
extern void unregister_framebuffer(struct fb_info *fb_info);
@@ -604,6 +647,75 @@ extern void fb_deferred_io_cleanup(struct fb_info *info);
extern int fb_deferred_io_fsync(struct file *file, loff_t start,
loff_t end, int datasync);
+/*
+ * Generate callbacks for deferred I/O
+ */
+
+#define __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, __mode) \
+ static ssize_t __prefix ## _defio_read(struct fb_info *info, char
__user *buf, \
+ size_t count, loff_t *ppos) \
+ { \
+ return fb_ ## __mode ## _read(info, buf, count, ppos); \
+ } \
+ static ssize_t __prefix ## _defio_write(struct fb_info *info, const
char __user *buf, \
+ size_t count, loff_t *ppos) \
+ { \
+ unsigned long offset = *ppos; \
+ ssize_t ret = fb_ ## __mode ## _write(info, buf, count, ppos); \
+ if (ret > 0) \
+ __damage_range(info, offset, ret); \
+ return ret; \
+ }
+
+#define __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, __mode) \
+ static void __prefix ## _defio_fillrect(struct fb_info *info, \
+ const struct fb_fillrect *rect)
\
+ { \
+ __mode ## _fillrect(info, rect); \
+ __damage_area(info, rect->dx, rect->dy, rect->width,
rect->height); \
+ } \
+ static void __prefix ## _defio_copyarea(struct fb_info *info, \
+ const struct fb_copyarea *area)
\
+ { \
+ __mode ## _copyarea(info, area); \
+ __damage_area(info, area->dx, area->dy, area->width,
area->height); \
+ } \
+ static
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
On 2023-06-14 12:39:13, Marijn Suijten wrote:
> > > > - /* add register dump support */
> > > > - dpu_debugfs_create_regset32("src_blk", 0400,
> > > > - debugfs_root,
> > > > - sblk->src_blk.base + cfg->base,
> > > > - sblk->src_blk.len,
> >
> > Note that this fails to apply on top of
> > https://lore.kernel.org/linux-arm-msm/20230429012353.2569481-2-dmitry.barysh...@linaro.org/
>
> Also noticing just now that this whole patch makes sblk unused:
... thanks to rebasing on top of [1], which is now applied.
[1]:
https://lore.kernel.org/all/2023051838.3815293-6-dmitry.barysh...@linaro.org/
- Marijn
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c: In function
> '_dpu_hw_sspp_init_debugfs':
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:620:41: warning: unused variable
> 'sblk' [-Wunused-variable]
> 620 | const struct dpu_sspp_sub_blks *sblk = cfg->sblk;
> | ^~~~
>
> - Marijn
Re: [Freedreno] [2/2] drm: Remove struct drm_driver.gem_prime_mmap
Hi
Am 14.06.23 um 10:26 schrieb Sui Jingfeng:
Hi,
On 2023/6/14 13:34, Thomas Zimmermann wrote:
Hi
Am 14.06.23 um 04:06 schrieb Sui Jingfeng:
On 2023/6/14 01:27, Sui Jingfeng wrote:
Wow, so many drivers get nuked!
On 2023/6/13 22:51, Thomas Zimmermann wrote:
All drivers initialize this field with drm_gem_prime_mmap(). Call
the function directly and remove the field. Simplifies the code and
resolves a long-standing TODO item.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Alex Deucher
I have tested this patch briefly with drm/amdgpu(RX560), Running
glmark2, the rendered scene looks OK.
But single driver is self-sharing. I think I should test this more
with multiple video card.
No need to test; it's equivalent to removing a wrapper.
Yes, only msm hardware might be affected.
But new DRM (un-upstreamed) drivers cannot be compiled anymore with
this patch applied.
This makes them all out-of-date or going to be outdated; this is
embarrassing!
What do you mean by embarrassing? Simply rebase your driver onto the
change and that's it. This happens regularly for out-of-tree drivers.
But if such a driver would land before this patchset, I'd have to
update the patchset instead.
Thanks for you told me this then.
I worry about what it will happen if two conflict patch got merged
together.
Yes that occasionaly breaks something, but luckily it rarely results in
a significant problem. Drivers that break can be disabled by the
majority of developers. So that's not an issue for most. Core code is a
bit more important. Usually someone provides a patch or workaround quickly.
Best regards
Thomas
If my driver got merged, then one more driver will be nuked together.
Saving a lot of effort.
Best regards
Thomas
---
Documentation/gpu/todo.rst | 9 -
drivers/accel/ivpu/ivpu_drv.c | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 -
drivers/gpu/drm/drm_fbdev_dma.c | 6 +-
drivers/gpu/drm/drm_prime.c | 14 ++
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 -
drivers/gpu/drm/exynos/exynos_drm_drv.c | 1 -
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 1 -
drivers/gpu/drm/lima/lima_drv.c | 1 -
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 -
drivers/gpu/drm/msm/msm_drv.c | 1 -
drivers/gpu/drm/msm/msm_drv.h | 1 -
drivers/gpu/drm/msm/msm_gem_prime.c | 5 -
drivers/gpu/drm/nouveau/nouveau_drm.c | 1 -
drivers/gpu/drm/panfrost/panfrost_drv.c | 1 -
drivers/gpu/drm/pl111/pl111_drv.c | 1 -
drivers/gpu/drm/radeon/radeon_drv.c | 1 -
drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c | 1 -
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 -
drivers/gpu/drm/v3d/v3d_drv.c | 1 -
drivers/gpu/drm/virtio/virtgpu_drv.c | 1 -
drivers/gpu/drm/xen/xen_drm_front.c | 1 -
include/drm/drm_drv.h | 14 --
include/drm/drm_gem_dma_helper.h | 6 ++
include/drm/drm_gem_shmem_helper.h | 1 -
include/drm/drm_gem_vram_helper.h | 1 -
26 files changed, 5 insertions(+), 69 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 68bdafa0284f5..ca1efad8c89c3 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -319,15 +319,6 @@ Contact: Daniel Vetter, Noralf Tronnes
Level: Advanced
-struct drm_gem_object_funcs
-
-GEM objects can now have a function table instead of having the
callbacks on the
-DRM driver struct. This is now the preferred way. Callbacks in
drivers have been
-converted, except for struct drm_driver.gem_prime_mmap.
-
-Level: Intermediate
-
connector register/unregister fixes
---
diff --git a/drivers/accel/ivpu/ivpu_drv.c
b/drivers/accel/ivpu/ivpu_drv.c
index 2df7643b843d5..9f2b9fdcc5498 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -376,7 +376,6 @@ static const struct drm_driver driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = ivpu_gem_prime_import,
- .gem_prime_mmap = drm_gem_prime_mmap,
.ioctls = ivpu_drm_ioctls,
.num_ioctls = ARRAY_SIZE(ivpu_drm_ioctls),
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index c9a41c997c6c7..7681f79f462eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2809,7 +2809,6 @@ static const struct drm_driver
amdgpu_kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
On 2023-05-21 21:16:39, Marijn Suijten wrote:
> On 2023-05-21 20:12:00, Marijn Suijten wrote:
> > On 2023-05-21 20:21:46, Dmitry Baryshkov wrote:
> > > Drop SSPP-specifig debugfs register dumps in favour of using
> > > debugfs/dri/0/kms or devcoredump.
> > >
> > > Signed-off-by: Dmitry Baryshkov
> >
> > Reviewed-by: Marijn Suijten
> >
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 -
> > > 1 file changed, 25 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > index bfd82c2921af..6c5ebee2f7cd 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp
> > > *hw_pipe, struct dpu_kms *kms,
> > > debugfs_create_xul("features", 0600,
> > > debugfs_root, (unsigned long *)_pipe->cap->features);
> > >
> > > - /* add register dump support */
> > > - dpu_debugfs_create_regset32("src_blk", 0400,
> > > - debugfs_root,
> > > - sblk->src_blk.base + cfg->base,
> > > - sblk->src_blk.len,
>
> Note that this fails to apply on top of
> https://lore.kernel.org/linux-arm-msm/20230429012353.2569481-2-dmitry.barysh...@linaro.org/
Also noticing just now that this whole patch makes sblk unused:
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c: In function
'_dpu_hw_sspp_init_debugfs':
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:620:41: warning: unused variable
'sblk' [-Wunused-variable]
620 | const struct dpu_sspp_sub_blks *sblk = cfg->sblk;
| ^~~~
- Marijn
>
> - Marijn
>
> > > - kms);
> > > -
> > > - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> > > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> > > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> > > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> > > - dpu_debugfs_create_regset32("scaler_blk", 0400,
> > > - debugfs_root,
> > > - sblk->scaler_blk.base + cfg->base,
> > > - sblk->scaler_blk.len,
> > > - kms);
> > > -
> > > - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> > > - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> > > - dpu_debugfs_create_regset32("csc_blk", 0400,
> > > - debugfs_root,
> > > - sblk->csc_blk.base + cfg->base,
> > > - sblk->csc_blk.len,
> > > - kms);
> > > -
> > > debugfs_create_u32("xin_id",
> > > 0400,
> > > debugfs_root,
> > > --
> > > 2.39.2
> > >
Re: [Freedreno] [PATCH 0/3] Add support for databus widen mode
On 2023-06-13 18:57:10, Jessica Zhang wrote: > DPU 5.x+ and DSI 6G 2.5.x+ support a databus-widen mode that allows for > more compressed data to be transferred per pclk. > > This series adds support for enabling this feature for both DPU and DSI > by doing the following: > > 1. Add a DPU_INTF_DATABUS_WIDEN feature flag > 2. Add a DPU INTF op to set the DATABUS_WIDEN register > 3. Set the DATABUS_WIDEN register and do the proper hdisplay >calculations in DSI when applicable > > Note: This series will only enable the databus-widen mode for command > mode as we are currently unable to validate it on video mode. > > Depends on: "Add DSC v1.2 Support for DSI" [1] > > [1] https://patchwork.freedesktop.org/series/117219/ Didn't Dmitry already pick that up for msm-next-lumag? - Marijn > Signed-off-by: Jessica Zhang > --- > Jessica Zhang (3): > drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0 > drm/msm/dpu: Set DATABUS_WIDEN on command mode encoders > drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 3 +++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 12 > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 3 +++ > drivers/gpu/drm/msm/dsi/dsi.xml.h| 1 + > drivers/gpu/drm/msm/dsi/dsi_host.c | 19 ++- > 7 files changed, 41 insertions(+), 2 deletions(-) > --- > base-commit: 1981c2c0c05f5d7fe4d4552d4f352cb46840e51e > change-id: 20230525-add-widebus-support-f785546ee751 > > Best regards, > -- > Jessica Zhang >
Re: [Freedreno] [PATCH 3/3] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 2023-06-14 10:49:31, Dmitry Baryshkov wrote:
> On 14/06/2023 04:57, Jessica Zhang wrote:
> > DSI 6G v2.5.x+ supports a data-bus widen mode that allows DSI to send
> > 48 bits of compressed data per pclk instead of 24.
> >
> > For all chipsets that support this mode, enable it whenever DSC is
> > enabled as recommend by the hardware programming guide.
> >
> > Only enable this for command mode as we are currently unable to validate
> > it for video mode.
> >
> > Signed-off-by: Jessica Zhang
> > ---
> >
> > Note: The dsi.xml.h changes were generated using the headergen2 script in
> > envytools [1], but the changes to the copyright and rules-ng-ng source file
> > paths were dropped.
> >
> > [1] https://github.com/freedreno/envytools/
> >
> > drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 +
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 19 ++-
> > 2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h
> > b/drivers/gpu/drm/msm/dsi/dsi.xml.h
> > index a4a154601114..2a7d980e12c3 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi.xml.h
> > +++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h
> > @@ -664,6 +664,7 @@ static inline uint32_t
> > DSI_CMD_MODE_MDP_CTRL2_INPUT_RGB_SWAP(enum dsi_rgb_swap v
> > return ((val) << DSI_CMD_MODE_MDP_CTRL2_INPUT_RGB_SWAP__SHIFT) &
> > DSI_CMD_MODE_MDP_CTRL2_INPUT_RGB_SWAP__MASK;
> > }
> > #define DSI_CMD_MODE_MDP_CTRL2_BURST_MODE 0x0001
> > +#define DSI_CMD_MODE_MDP_CTRL2_DATABUS_WIDEN
> > 0x0010
> >
> > #define REG_DSI_CMD_MODE_MDP_STREAM2_CTRL 0x01b8
> > #define DSI_CMD_MODE_MDP_STREAM2_CTRL_DATA_TYPE__MASK
> > 0x003f
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index 5d7b4409e4e9..1da5238e7105 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -927,6 +927,9 @@ static void dsi_timing_setup(struct msm_dsi_host
> > *msm_host, bool is_bonded_dsi)
> > u32 hdisplay = mode->hdisplay;
> > u32 wc;
> > int ret;
> > + bool widebus_supported = msm_host->cfg_hnd->major ==
> > MSM_DSI_VER_MAJOR_6G &&
> > + msm_host->cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_5_0;
> > +
> >
> > DBG("");
> >
> > @@ -973,8 +976,15 @@ static void dsi_timing_setup(struct msm_dsi_host
> > *msm_host, bool is_bonded_dsi)
> > *
> > * hdisplay will be divided by 3 here to account for the fact
> > * that DPU sends 3 bytes per pclk cycle to DSI.
> > +*
> > +* If widebus is supported, set DATABUS_WIDEN register and
> > divide hdisplay by 6
> > +* instead of 3
>
> This is useless, it is already obvious from the code below. Instead
> there should be something like "wide bus extends that to 6 bytes per
> pclk cycle"
Yes please. In general, don't paraphrase the code, but explain _why_ it
is doing what it does. Saying that the widebus feature doubles the
bandwidth per pclk tick is much more clear than "divide by 6 instead of
3" - we can read that from the code.
Overall comments have been very good so far (such as the original "to
account for the fact that DPU sends 3 bytes per pclk cycle"), though!
> > */
> > - hdisplay =
> > DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
> > + if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) &&
> > widebus_supported)
> > + hdisplay =
> > DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 6);
> > + else
> > + hdisplay =
> > DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
> > +
> > h_total += hdisplay;
> > ha_end = ha_start + hdisplay;
> > }
> > @@ -1027,6 +1037,13 @@ static void dsi_timing_setup(struct msm_dsi_host
> > *msm_host, bool is_bonded_dsi)
> > dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL,
> > DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
> > DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay));
> > +
> > + if (msm_host->dsc && widebus_supported) {
> > + u32 mdp_ctrl2 = dsi_read(msm_host,
> > REG_DSI_CMD_MODE_MDP_CTRL2);
> > +
> > + mdp_ctrl2 |= DSI_CMD_MODE_MDP_CTRL2_DATABUS_WIDEN;
> > + dsi_write(msm_host, REG_DSI_CMD_MODE_MDP_CTRL2,
> > mdp_ctrl2);
>
> Is widebus applicable only to the CMD mode, or video mode can employ it too?
The patch description states that it was not tested on video-mode yet,
so I assume it will. But this should also be highlighted with a comment
(e.g. /* XXX: Allow for video-mode once tested/fixed */), _especially_
on the check for MIPI_DSI_MODE_VIDEO above.
If I understand this correctly DSC is not working for video mode at all
on these setups, right? Or no-one was able to test it? I'm inclined to
request dropping
Re: [Freedreno] [PATCH 3/3] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 2023-06-13 18:57:13, Jessica Zhang wrote:
> DSI 6G v2.5.x+ supports a data-bus widen mode that allows DSI to send
> 48 bits of compressed data per pclk instead of 24.
>
> For all chipsets that support this mode, enable it whenever DSC is
> enabled as recommend by the hardware programming guide.
>
> Only enable this for command mode as we are currently unable to validate
> it for video mode.
>
> Signed-off-by: Jessica Zhang
> ---
>
> Note: The dsi.xml.h changes were generated using the headergen2 script in
> envytools [1], but the changes to the copyright and rules-ng-ng source file
> paths were dropped.
>
> [1] https://github.com/freedreno/envytools/
More interesting would be a link to the Mesa MR upstreaming this
bitfield to dsi.xml [2] (which I have not found on my own yet).
[2]:
https://gitlab.freedesktop.org/mesa/mesa/-/blame/main/src/freedreno/registers/dsi/dsi.xml
> drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 +
> drivers/gpu/drm/msm/dsi/dsi_host.c | 19 ++-
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h
> b/drivers/gpu/drm/msm/dsi/dsi.xml.h
> index a4a154601114..2a7d980e12c3 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.xml.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h
> @@ -664,6 +664,7 @@ static inline uint32_t
> DSI_CMD_MODE_MDP_CTRL2_INPUT_RGB_SWAP(enum dsi_rgb_swap v
> return ((val) << DSI_CMD_MODE_MDP_CTRL2_INPUT_RGB_SWAP__SHIFT) &
> DSI_CMD_MODE_MDP_CTRL2_INPUT_RGB_SWAP__MASK;
> }
> #define DSI_CMD_MODE_MDP_CTRL2_BURST_MODE0x0001
> +#define DSI_CMD_MODE_MDP_CTRL2_DATABUS_WIDEN 0x0010
>
> #define REG_DSI_CMD_MODE_MDP_STREAM2_CTRL0x01b8
> #define DSI_CMD_MODE_MDP_STREAM2_CTRL_DATA_TYPE__MASK
> 0x003f
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 5d7b4409e4e9..1da5238e7105 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -927,6 +927,9 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
> u32 hdisplay = mode->hdisplay;
> u32 wc;
> int ret;
> + bool widebus_supported = msm_host->cfg_hnd->major ==
> MSM_DSI_VER_MAJOR_6G &&
> + msm_host->cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_5_0;
> +
>
> DBG("");
>
> @@ -973,8 +976,15 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
>*
>* hdisplay will be divided by 3 here to account for the fact
>* that DPU sends 3 bytes per pclk cycle to DSI.
> + *
> + * If widebus is supported, set DATABUS_WIDEN register and
> divide hdisplay by 6
> + * instead of 3
So this should allow us to divide pclk by 2, or have much lower latency?
Otherwise it'll tick enough times to transmit the data twice.
Note that I brought up the exact same concerns when you used the 3:1
ratio from dsi_bpp / dsc_bpp in your pclk reduction patch (instad of the
number of bits/bytes that DPU sends to DSI per pclk), but no-one has
replied to my inquiry yet.
>*/
> - hdisplay =
> DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
> + if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) &&
> widebus_supported)
> + hdisplay =
> DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 6);
> + else
> + hdisplay =
> DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
Nit: I wonder if this is more concise when written as:
u32 bytes_per_pclk;
...
if (!video && widebus)
bytes_per_pclk = 6;
else
bytes_per_pclk = 3;
hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc),
bytes_per_pclk);
That is less duplication, **and** the value becomes self-documenting!
> +
> h_total += hdisplay;
> ha_end = ha_start + hdisplay;
> }
> @@ -1027,6 +1037,13 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
> dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL,
> DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
> DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay));
> +
> + if (msm_host->dsc && widebus_supported) {
Can we also support widebus for uncompressed streams (sending 2 pixels
of bpp=24 per pclk), and if so, is that something you want to add in the
future (a comment would be nice)?
> + u32 mdp_ctrl2 = dsi_read(msm_host,
> REG_DSI_CMD_MODE_MDP_CTRL2);
> +
> + mdp_ctrl2 |= DSI_CMD_MODE_MDP_CTRL2_DATABUS_WIDEN;
> + dsi_write(msm_host, REG_DSI_CMD_MODE_MDP_CTRL2,
> mdp_ctrl2);
> + }
Same comment as on your BURST_MODE patch (which this'll conflict
Re: [Freedreno] [PATCH] drm/msm/dsi: Enable BURST_MODE for command mode for DSI 6G v1.3+
On 2023-06-12 16:37:36, Jessica Zhang wrote: > During a frame transfer in command mode, there could be frequent > LP11 <-> HS transitions when multiple DCS commands are sent mid-frame or > if the DSI controller is running on slow clock and is throttled. To > minimize frame latency due to these transitions, it is recommended to > send the frame in a single burst. > > This feature is supported for DSI 6G 1.3 and above, thus enable burst > mode if supported. > > Signed-off-by: Jessica Zhang > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c > b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 744f2398a6d6..8254b06dca85 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -994,6 +994,11 @@ static void dsi_timing_setup(struct msm_dsi_host > *msm_host, bool is_bonded_dsi) > dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL, > DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) | > DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay)); > + > + if (msm_host->cfg_hnd->major == MSM_DSI_VER_MAJOR_6G && > + msm_host->cfg_hnd->minor >= > MSM_DSI_6G_VER_MINOR_V1_3) > + dsi_write(msm_host, REG_DSI_CMD_MODE_MDP_CTRL2, > + DSI_CMD_MODE_MDP_CTRL2_BURST_MODE); This is not part of the timing setup, and a similar BURST_MODE flag is enabled for video-mode in dsi_ctrl_config() - should it be moved there? (There is a dsi_sw_reset() in between the calls to dsi_timing_setup() and dsi_ctrl_cfg()) Note that that function sets up the CMD_CFG0 and CMD_CFG1 register, with the former having a very similar layout to MDP_CTRL2... is there documentation outlining the difference? - Marijn > } > } > > > --- > base-commit: dd969f852ba4c66938c71889e826aa8e5300d2f2 > change-id: 20230608-b4-add-burst-mode-a5bb144069fa > > Best regards, > -- > Jessica Zhang >
Re: [Freedreno] [PATCH 2/3] drm/msm/dpu: Set DATABUS_WIDEN on command mode encoders
On 14/06/2023 04:57, Jessica Zhang wrote:
Add a DPU INTF op to set the DATABUS_WIDEN register to enable the
databus-widen mode datapath.
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 3 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 12
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 3 +++
3 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index b856c6286c85..124ba96bebda 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -70,6 +70,9 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
if (intf_cfg.dsc != 0 && phys_enc->hw_intf->ops.enable_compression)
phys_enc->hw_intf->ops.enable_compression(phys_enc->hw_intf);
+
+ if (phys_enc->hw_intf->ops.enable_widebus)
+ phys_enc->hw_intf->ops.enable_widebus(phys_enc->hw_intf);
No. Please provide a single function which takes necessary
configuration, including compression and wide_bus_enable.
Also note, that we already have dpu_encoder_is_widebus_enabled() and the
rest of support code. Please stick to it too.
}
static void dpu_encoder_phys_cmd_pp_tx_done_irq(void *arg, int irq_idx)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 5b0f6627e29b..03ba3a1c7a46 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -513,6 +513,15 @@ static void dpu_hw_intf_disable_autorefresh(struct
dpu_hw_intf *intf,
}
+static void dpu_hw_intf_enable_widebus(struct dpu_hw_intf *ctx)
+{
+ u32 intf_cfg2 = DPU_REG_READ(>hw, INTF_CONFIG2);
+
+ intf_cfg2 |= INTF_CFG2_DATABUS_WIDEN;
+
+ DPU_REG_WRITE(>hw, INTF_CONFIG2, intf_cfg2);
+}
+
static void dpu_hw_intf_enable_compression(struct dpu_hw_intf *ctx)
{
u32 intf_cfg2 = DPU_REG_READ(>hw, INTF_CONFIG2);
@@ -545,6 +554,9 @@ static void _setup_intf_ops(struct dpu_hw_intf_ops *ops,
if (cap & BIT(DPU_INTF_DATA_COMPRESS))
ops->enable_compression = dpu_hw_intf_enable_compression;
+
+ if (cap & BIT(DPU_INTF_DATABUS_WIDEN))
+ ops->enable_widebus = dpu_hw_intf_enable_widebus;
}
struct dpu_hw_intf *dpu_hw_intf_init(const struct dpu_intf_cfg *cfg,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
index 99e21c4137f9..64a17b99d3d1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -71,6 +71,7 @@ struct intf_status {
* Return: 0 on success, -ETIMEDOUT on timeout
* @vsync_sel: Select vsync signal for tear-effect
configuration
* @enable_compression: Enable data compression
+ * @enable_widebus: Enable widebus
*/
struct dpu_hw_intf_ops {
void (*setup_timing_gen)(struct dpu_hw_intf *intf,
@@ -109,6 +110,8 @@ struct dpu_hw_intf_ops {
void (*disable_autorefresh)(struct dpu_hw_intf *intf, uint32_t
encoder_id, u16 vdisplay);
void (*enable_compression)(struct dpu_hw_intf *intf);
+
+ void (*enable_widebus)(struct dpu_hw_intf *intf);
};
struct dpu_hw_intf {
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH 2/2] drm/msm/dpu/catalog: define DSPP blocks found on sdm845
On Mon, 12 Jun 2023 at 23:55, Dmitry Baryshkov
wrote:
>
> Add definitions of DSPP blocks present on the sdm845 platform. This
> should enable color-management on sdm845-bassed devices.
>
> Signed-off-by: Dmitry Baryshkov
> ---
> .../msm/disp/dpu1/catalog/dpu_4_0_sdm845.h| 21 +++
> 1 file changed, 17 insertions(+), 4 deletions(-)
Thanks for your patch, Dmitry. LGTM.
Please feel free to add:
Reviewed-by: Sumit Semwal
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index 36ea1af10894..b6098141bb9b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -96,19 +96,30 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>
> static const struct dpu_lm_cfg sdm845_lm[] = {
> LM_BLK("lm_0", LM_0, 0x44000, MIXER_SDM845_MASK,
> - _lm_sblk, PINGPONG_0, LM_1, 0),
> + _lm_sblk, PINGPONG_0, LM_1, DSPP_0),
> LM_BLK("lm_1", LM_1, 0x45000, MIXER_SDM845_MASK,
> - _lm_sblk, PINGPONG_1, LM_0, 0),
> + _lm_sblk, PINGPONG_1, LM_0, DSPP_1),
> LM_BLK("lm_2", LM_2, 0x46000, MIXER_SDM845_MASK,
> - _lm_sblk, PINGPONG_2, LM_5, 0),
> + _lm_sblk, PINGPONG_2, LM_5, DSPP_2),
> LM_BLK("lm_3", LM_3, 0x0, MIXER_SDM845_MASK,
> - _lm_sblk, PINGPONG_NONE, 0, 0),
> + _lm_sblk, PINGPONG_NONE, 0, DSPP_3),
> LM_BLK("lm_4", LM_4, 0x0, MIXER_SDM845_MASK,
> _lm_sblk, PINGPONG_NONE, 0, 0),
> LM_BLK("lm_5", LM_5, 0x49000, MIXER_SDM845_MASK,
> _lm_sblk, PINGPONG_3, LM_2, 0),
> };
>
> +static const struct dpu_dspp_cfg sdm845_dspp[] = {
> + DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
> +_dspp_sblk),
> + DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
> +_dspp_sblk),
> + DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
> +_dspp_sblk),
> + DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
> +_dspp_sblk),
> +};
> +
> static const struct dpu_pingpong_cfg sdm845_pp[] = {
> PP_BLK("pingpong_0", PINGPONG_0, 0x7, PINGPONG_SDM845_TE2_MASK,
> 0, sdm845_pp_sblk_te,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> @@ -193,6 +204,8 @@ const struct dpu_mdss_cfg dpu_sdm845_cfg = {
> .sspp = sdm845_sspp,
> .mixer_count = ARRAY_SIZE(sdm845_lm),
> .mixer = sdm845_lm,
> + .dspp_count = ARRAY_SIZE(sdm845_dspp),
> + .dspp = sdm845_dspp,
> .pingpong_count = ARRAY_SIZE(sdm845_pp),
> .pingpong = sdm845_pp,
> .dsc_count = ARRAY_SIZE(sdm845_dsc),
> --
> 2.39.2
>
--
Thanks and regards,
Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Add DPU_INTF_DATABUS_WIDEN feature flag for DPU >= 5.0
On 14/06/2023 04:57, Jessica Zhang wrote: DPU 5.x+ supports a databus widen mode that allows more data to be sent per pclk. Enable this feature flag on all relevant chipsets. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: do not enable color-management if DSPPs are not available
On Mon, 12 Jun 2023 at 23:55, Dmitry Baryshkov
wrote:
>
> We can not support color management without DSPP blocks being provided
> in the HW catalog. Do not enable color management for CRTCs if num_dspps
> is 0.
>
> Fixes: 4259ff7ae509 ("drm/msm/dpu: add support for pcc color block in dpu
> driver")
> Reported-by: Yongqin Liu
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
Thanks for your patch, Dmitry. LGTM.
Please feel free to add:
Reviewed-by: Sumit Semwal
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 6e684a7b49a1..1edf2b6b0a26 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1463,6 +1463,8 @@ static const struct drm_crtc_helper_funcs
> dpu_crtc_helper_funcs = {
> struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane
> *plane,
> struct drm_plane *cursor)
> {
> + struct msm_drm_private *priv = dev->dev_private;
> + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
> struct drm_crtc *crtc = NULL;
> struct dpu_crtc *dpu_crtc = NULL;
> int i, ret;
> @@ -1494,7 +1496,8 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev,
> struct drm_plane *plane,
>
> drm_crtc_helper_add(crtc, _crtc_helper_funcs);
>
> - drm_crtc_enable_color_mgmt(crtc, 0, true, 0);
> + if (dpu_kms->catalog->dspp_count)
> + drm_crtc_enable_color_mgmt(crtc, 0, true, 0);
>
> /* save user friendly CRTC name for later */
> snprintf(dpu_crtc->name, DPU_CRTC_NAME_SIZE, "crtc%u", crtc->base.id);
> --
> 2.39.2
>
--
Thanks and regards,
Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs
Re: [Freedreno] [PATCH 3/3] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 14/06/2023 04:57, Jessica Zhang wrote:
DSI 6G v2.5.x+ supports a data-bus widen mode that allows DSI to send
48 bits of compressed data per pclk instead of 24.
For all chipsets that support this mode, enable it whenever DSC is
enabled as recommend by the hardware programming guide.
Only enable this for command mode as we are currently unable to validate
it for video mode.
Signed-off-by: Jessica Zhang
---
Note: The dsi.xml.h changes were generated using the headergen2 script in
envytools [1], but the changes to the copyright and rules-ng-ng source file
paths were dropped.
[1] https://github.com/freedreno/envytools/
drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 19 ++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h
b/drivers/gpu/drm/msm/dsi/dsi.xml.h
index a4a154601114..2a7d980e12c3 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.xml.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h
@@ -664,6 +664,7 @@ static inline uint32_t
DSI_CMD_MODE_MDP_CTRL2_INPUT_RGB_SWAP(enum dsi_rgb_swap v
return ((val) << DSI_CMD_MODE_MDP_CTRL2_INPUT_RGB_SWAP__SHIFT) &
DSI_CMD_MODE_MDP_CTRL2_INPUT_RGB_SWAP__MASK;
}
#define DSI_CMD_MODE_MDP_CTRL2_BURST_MODE 0x0001
+#define DSI_CMD_MODE_MDP_CTRL2_DATABUS_WIDEN 0x0010
#define REG_DSI_CMD_MODE_MDP_STREAM2_CTRL 0x01b8
#define DSI_CMD_MODE_MDP_STREAM2_CTRL_DATA_TYPE__MASK 0x003f
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 5d7b4409e4e9..1da5238e7105 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -927,6 +927,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host,
bool is_bonded_dsi)
u32 hdisplay = mode->hdisplay;
u32 wc;
int ret;
+ bool widebus_supported = msm_host->cfg_hnd->major == MSM_DSI_VER_MAJOR_6G
&&
+ msm_host->cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_5_0;
+
DBG("");
@@ -973,8 +976,15 @@ static void dsi_timing_setup(struct msm_dsi_host
*msm_host, bool is_bonded_dsi)
*
* hdisplay will be divided by 3 here to account for the fact
* that DPU sends 3 bytes per pclk cycle to DSI.
+*
+* If widebus is supported, set DATABUS_WIDEN register and
divide hdisplay by 6
+* instead of 3
This is useless, it is already obvious from the code below. Instead
there should be something like "wide bus extends that to 6 bytes per
pclk cycle"
*/
- hdisplay =
DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
+ if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) &&
widebus_supported)
+ hdisplay =
DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 6);
+ else
+ hdisplay =
DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
+
h_total += hdisplay;
ha_end = ha_start + hdisplay;
}
@@ -1027,6 +1037,13 @@ static void dsi_timing_setup(struct msm_dsi_host
*msm_host, bool is_bonded_dsi)
dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL,
DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay));
+
+ if (msm_host->dsc && widebus_supported) {
+ u32 mdp_ctrl2 = dsi_read(msm_host,
REG_DSI_CMD_MODE_MDP_CTRL2);
+
+ mdp_ctrl2 |= DSI_CMD_MODE_MDP_CTRL2_DATABUS_WIDEN;
+ dsi_write(msm_host, REG_DSI_CMD_MODE_MDP_CTRL2,
mdp_ctrl2);
Is widebus applicable only to the CMD mode, or video mode can employ it too?
+ }
}
}
--
2.40.1
--
With best wishes
Dmitry
