Re: [Freedreno] [PATCH v3 0/4] Reserve DSPPs based on user request
Hi, On Wed, Feb 8, 2023 at 8:16 PM Kalyan Thota wrote: > > Hi Doug, > > Have you picked the core change to program dspp's (below) ? the current > series will go on top of it. > https://patchwork.kernel.org/project/linux-arm-msm/patch/1671542719-12655-1-git-send-email-quic_kaly...@quicinc.com/ I didn't pick v11 of it like you link, but I did pick v12 of the same patch. In my response I said that I picked 5 patches, this series plus [1] where [1] is: [1] https://lore.kernel.org/all/1674814487-2112-1-git-send-email-quic_kaly...@quicinc.com/ > Thanks, > Kalyan > > >-Original Message- > >From: Doug Anderson > >Sent: Wednesday, February 8, 2023 10:44 PM > >To: Kalyan Thota (QUIC) > >Cc: dri-de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org; > >freedreno@lists.freedesktop.org; devicet...@vger.kernel.org; linux- > >ker...@vger.kernel.org; robdcl...@chromium.org; swb...@chromium.org; > >Vinod Polimera (QUIC) ; > >dmitry.barysh...@linaro.org; Abhinav Kumar (QUIC) > >; marijn.suij...@somainline.org > >Subject: Re: [PATCH v3 0/4] Reserve DSPPs based on user request > > > >WARNING: This email originated from outside of Qualcomm. Please be wary of > >any links or attachments, and do not enable macros. > > > >Hi, > > > >On Wed, Feb 8, 2023 at 5:42 AM Kalyan Thota > >wrote: > >> > >> This series will enable color features on sc7280 target which has > >> primary panel as eDP > >> > >> The series removes DSPP allocation based on encoder type and allows > >> the DSPP reservation based on user request via CTM. > >> > >> The series will release/reserve the dpu resources when ever there is a > >> topology change to suit the new requirements. > >> > >> Kalyan Thota (4): > >> drm/msm/dpu: clear DSPP reservations in rm release > >> drm/msm/dpu: add DSPPs into reservation upon a CTM request > >> drm/msm/dpu: avoid unnecessary check in DPU reservations > >> drm/msm/dpu: reserve the resources on topology change > >> > >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 + > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 58 -- > >--- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 + > >> 3 files changed, 37 insertions(+), 25 deletions(-) > > > >I tried out your changes, but unfortunately it seems like there's something > >wrong. > >:( I did this: > > > >1. Picked your 5 patches to the chromeos-5.15 tree (this series plus [1]) > > > >2. Put them on herobrine villager. > > > >3. Booted up with no external display plugged in. > > > >4. Tried to enable night light in the ChromeOS UI. > > > >5. Night light didn't turn on for the internal display. > > > > > >I also tried applying them to the top of msm-next (had to resolve some small > >conflicts). Same thing, night light didn't work. > > > > > >I thought maybe this was because the Chrome browser hasn't been updated to > >properly use atomic_check for testing for night light, so I hacked my > >herobrine > >device tree to not mark "mdss_dp" as "okay". Now there's _only_ an eDP > >display. > >Same thing, night light didn't work. > > > > > >I could only get night light to work for the internal display if I plugged > >and > >unplugged an external display in. > > > > > >Is the above the behavior that's expected right now? > > > > > >[1] https://lore.kernel.org/all/1674814487-2112-1-git-send-email- > >quic_kaly...@quicinc.com/
Re: [Freedreno] [PATCH] firmware: qcom_scm: Move qcom_scm.h to include/linux/firmware/qcom/
On Fri, 3 Feb 2023 13:09:52 -0800, Elliot Berman wrote: > Move include/linux/qcom_scm.h to include/linux/firmware/qcom/qcom_scm.h. > This removes 1 of a few remaining Qualcomm-specific headers into a more > approciate subdirectory under include/. > > Applied, thanks! [1/1] firmware: qcom_scm: Move qcom_scm.h to include/linux/firmware/qcom/ commit: 3bf90eca76c98c55c975fa817799789b9176f9f3 Best regards, -- Bjorn Andersson
Re: [Freedreno] (subset) [PATCH v2 0/8] arm64: dts: qcom: sm8350: enable GPU on the HDK board
On Mon, 6 Feb 2023 16:56:59 +0200, Dmitry Baryshkov wrote: > Add A660 device to the Qualcomm SM8350 platform and enable it for the > sm8350-hdk board. Unfortunately while adding the GPU & related devices I > noticed that DT nodes on SM8350 are greatly out of the preagreed order, > so patches 4-6 reorder DT nodes to follow the agreement. > > Changes since v1: > - Fixed the subject and commit message for patch 1 > - Fixed GMU's clocks to follow the vendor kernel > - Marked Adreno SMMU as dma-coherent > - Dropped comments targeting sm8350 v1, we do not support that chip > revision. > > [...] Applied, thanks! [2/8] dt-bindings: power: qcom,rpmpd: add RPMH_REGULATOR_LEVEL_LOW_SVS_L1 commit: bdd133c2eeffad142e7c8a48ab7e86e1ca37e67d Best regards, -- Bjorn Andersson
Re: [Freedreno] (subset) [PATCH 0/6] drm/msm/hdmi: integrate msm8960 HDMI PHY with DT clocks infrastructure
On Thu, 19 Jan 2023 15:22:13 +0200, Dmitry Baryshkov wrote: > Make msm8960's HDMI PHY accept clocks from DT and also register it as a > DT clock provider. > > Dmitry Baryshkov (6): > dt-bindings: phy: qcom,hdmi-phy-other: use pxo clock > dt-bindings: phy: qcom,hdmi-phy-other: mark it as clock provider > drm/msm/hdmi: switch hdmi_pll_8960 to use parent_data > drm/msm/hdmi: make hdmi_phy_8960 OF clk provider > ARM: dts: qcom: apq8064: add #clock-cells to the HDMI PHY node > ARM: dts: qcom: apq8064: use hdmi_phy for the MMCC's hdmipll clock > > [...] Applied, thanks! [5/6] ARM: dts: qcom: apq8064: add #clock-cells to the HDMI PHY node commit: c9f678afe0bbdfb3748c1db5ac94d1c02a6eb64b [6/6] ARM: dts: qcom: apq8064: use hdmi_phy for the MMCC's hdmipll clock commit: f1a359db6d9d198b84877e2340dd7c37809a4882 Best regards, -- Bjorn Andersson
Re: [Freedreno] [PATCH v3 0/4] Reserve DSPPs based on user request
Hi Doug, Have you picked the core change to program dspp's (below) ? the current series will go on top of it. https://patchwork.kernel.org/project/linux-arm-msm/patch/1671542719-12655-1-git-send-email-quic_kaly...@quicinc.com/ Thanks, Kalyan >-Original Message- >From: Doug Anderson >Sent: Wednesday, February 8, 2023 10:44 PM >To: Kalyan Thota (QUIC) >Cc: dri-de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org; >freedreno@lists.freedesktop.org; devicet...@vger.kernel.org; linux- >ker...@vger.kernel.org; robdcl...@chromium.org; swb...@chromium.org; >Vinod Polimera (QUIC) ; >dmitry.barysh...@linaro.org; Abhinav Kumar (QUIC) >; marijn.suij...@somainline.org >Subject: Re: [PATCH v3 0/4] Reserve DSPPs based on user request > >WARNING: This email originated from outside of Qualcomm. Please be wary of >any links or attachments, and do not enable macros. > >Hi, > >On Wed, Feb 8, 2023 at 5:42 AM Kalyan Thota >wrote: >> >> This series will enable color features on sc7280 target which has >> primary panel as eDP >> >> The series removes DSPP allocation based on encoder type and allows >> the DSPP reservation based on user request via CTM. >> >> The series will release/reserve the dpu resources when ever there is a >> topology change to suit the new requirements. >> >> Kalyan Thota (4): >> drm/msm/dpu: clear DSPP reservations in rm release >> drm/msm/dpu: add DSPPs into reservation upon a CTM request >> drm/msm/dpu: avoid unnecessary check in DPU reservations >> drm/msm/dpu: reserve the resources on topology change >> >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 58 -- >--- >> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 + >> 3 files changed, 37 insertions(+), 25 deletions(-) > >I tried out your changes, but unfortunately it seems like there's something >wrong. >:( I did this: > >1. Picked your 5 patches to the chromeos-5.15 tree (this series plus [1]) > >2. Put them on herobrine villager. > >3. Booted up with no external display plugged in. > >4. Tried to enable night light in the ChromeOS UI. > >5. Night light didn't turn on for the internal display. > > >I also tried applying them to the top of msm-next (had to resolve some small >conflicts). Same thing, night light didn't work. > > >I thought maybe this was because the Chrome browser hasn't been updated to >properly use atomic_check for testing for night light, so I hacked my herobrine >device tree to not mark "mdss_dp" as "okay". Now there's _only_ an eDP display. >Same thing, night light didn't work. > > >I could only get night light to work for the internal display if I plugged and >unplugged an external display in. > > >Is the above the behavior that's expected right now? > > >[1] https://lore.kernel.org/all/1674814487-2112-1-git-send-email- >quic_kaly...@quicinc.com/
Re: [Freedreno] [PATCH v2 4/8] arm64: dts: qcom: sm8350: reorder device nodes
On Mon, Feb 06, 2023 at 04:57:03PM +0200, Dmitry Baryshkov wrote:
> Start ordering DT nodes according to agreed order. Move apps SMMU, GIC,
> timer, apps RSC, cpufreq ADSP and cDSP nodes to the end to the proper
> position at the end of /soc/.
>
I think "according to agreed order" means "sorted by address", but it
would be nice to have that expressed in the message. If nothing else for
others to know what such agreed order might be.
Unfortunately this doesn't apply to my tree, and it's not clear where it
failed. Could you please rebase this?
Thanks,
Bjorn
> Signed-off-by: Dmitry Baryshkov
> ---
> arch/arm64/boot/dts/qcom/sm8350.dtsi | 1228 +-
> 1 file changed, 614 insertions(+), 614 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index 0de42a333d32..061aa3fec1c4 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -1423,111 +1423,6 @@ spi13: spi@a94000 {
> };
> };
>
> - apps_smmu: iommu@1500 {
> - compatible = "qcom,sm8350-smmu-500", "arm,mmu-500";
> - reg = <0 0x1500 0 0x10>;
> - #iommu-cells = <2>;
> - #global-interrupts = <2>;
> - interrupts =,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> -
Re: [Freedreno] [PATCH v3 27/27] drm/msm/dpu: add support for wide planes
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Typically SSPP can support rectangle with width up to 2560. However it's
Not always 2560. Depends on the chipset.
possible to use multirect feature and split source to use the SSPP to
output two consecutive rectangles. This commit brings in this capability
to support wider screen resolutions.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 116 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 +
3 files changed, 114 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0ca3bc38ff7e..867832a752b2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -485,6 +485,12 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
fetch_active,
>pipe);
+ _dpu_crtc_blend_setup_pipe(crtc, plane,
+ mixer, cstate->num_mixers,
+ stage_cfg, pstate->stage, 1,
+ fetch_active,
+ >r_pipe);
+
/* blend config update */
for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
_dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate,
format);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index e2e85688ed3c..401ead64c6bd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -365,6 +365,9 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_hw_pipe_qos_cfg pipe_qos_cfg;
+ if (!pipe->sspp)
+ return;
+
memset(_qos_cfg, 0, sizeof(pipe_qos_cfg));
if (flags & DPU_PLANE_QOS_VBLANK_CTRL) {
@@ -647,6 +650,9 @@ static int _dpu_plane_color_fill_pipe(struct
dpu_plane_state *pstate,
{
struct dpu_hw_sspp_cfg pipe_cfg;
+ if (!pipe->sspp)
+ return 0;
instead of checking if sspp was present, is it not better for the caller
to check if the rpipe is valid before calling this?
+
/* update sspp */
if (!pipe->sspp->ops.setup_solidfill)
return 0;
@@ -701,6 +707,8 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
/* update sspp */
_dpu_plane_color_fill_pipe(pstate, >pipe, >pipe_cfg,
fill_color, fmt);
+
+ _dpu_plane_color_fill_pipe(pstate, >r_pipe,
>r_pipe_cfg, fill_color, fmt);
}
So cant we do
if (pstate->r_pipe.sspp)
_dpu_plane_color_fill_pipe(pstate, >r_pipe,
>r_pipe_cfg, fill_color, fmt);
It just seems better to me as the caller would already know if the sspp
was assigned.
int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
@@ -911,6 +919,9 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane
*pdpu,
{
uint32_t min_src_size;
+ if (!pipe->sspp)
+ return 0;
+
min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
if (DPU_FORMAT_IS_YUV(fmt) &&
@@ -957,9 +968,12 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
int ret = 0, min_scale;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
+ struct dpu_sw_pipe *pipe = >pipe;
+ struct dpu_sw_pipe *r_pipe = >r_pipe;
const struct drm_crtc_state *crtc_state = NULL;
const struct dpu_format *fmt;
struct dpu_hw_sspp_cfg *pipe_cfg = >pipe_cfg;
+ struct dpu_hw_sspp_cfg *r_pipe_cfg = >r_pipe_cfg;
struct drm_rect fb_rect = { 0 };
uint32_t max_linewidth;
unsigned int rotation;
@@ -983,8 +997,11 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->visible)
return 0;
- pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
- pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+ pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+ r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ r_pipe->sspp = NULL;
pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
@@ -1016,16 +1033,53 @@ static int dpu_plane_atomic_check(struct drm_plane
*plane,
max_linewidth = pdpu->catalog->caps->max_linewidth;
- /* check decimated source width */
if (drm_rect_width(_cfg->src_rect) > max_linewidth) {
- DPU_DEBUG_PLANE(pdpu, "invalid src "
Re: [Freedreno] [PATCH v3 25/27] drm/msm/dpu: rework static color fill code
On 09/02/2023 00:34, Abhinav Kumar wrote:
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Rework static color fill code to separate the pipe / pipe_cfg handling.
This is a preparation for the r_pipe support.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 70 +--
1 file changed, 41 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 05047192cb37..e2e85688ed3c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -639,20 +639,54 @@ static void _dpu_plane_setup_scaler(struct
dpu_sw_pipe *pipe,
fmt);
}
+static int _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate,
+ struct dpu_sw_pipe *pipe,
+ struct dpu_hw_sspp_cfg *old_pipe_cfg,
Why is this called old_pipe_cfg instead of just pipe_cfg?
Ack. Probably got that wrong during mass-renaming and then missed to fix it.
+ u32 fill_color,
+ const struct dpu_format *fmt)
+{
+ struct dpu_hw_sspp_cfg pipe_cfg;
+
+ /* update sspp */
+ if (!pipe->sspp->ops.setup_solidfill)
+ return 0;
You can just return from here and make this function void?
Of course.
+
+ pipe->sspp->ops.setup_solidfill(pipe, fill_color);
+
+ /* override scaler/decimation if solid fill */
+ pipe_cfg.dst_rect = old_pipe_cfg->dst_rect;
+
+ pipe_cfg.src_rect.x1 = 0;
+ pipe_cfg.src_rect.y1 = 0;
+ pipe_cfg.src_rect.x2 =
+ drm_rect_width(_cfg.dst_rect);
+ pipe_cfg.src_rect.y2 =
+ drm_rect_height(_cfg.dst_rect);
+
+ if (pipe->sspp->ops.setup_format)
+ pipe->sspp->ops.setup_format(pipe, fmt, DPU_SSPP_SOLID_FILL);
+
+ if (pipe->sspp->ops.setup_rects)
+ pipe->sspp->ops.setup_rects(pipe, _cfg);
+
+ _dpu_plane_setup_scaler(pipe, fmt, true, _cfg,
pstate->rotation);
+
+ return 0;
+}
+
/**
* _dpu_plane_color_fill - enables color fill on plane
* @pdpu: Pointer to DPU plane object
* @color: RGB fill color value, [23..16] Blue, [15..8] Green,
[7..0] Red
* @alpha: 8-bit fill alpha value, 255 selects 100% alpha
- * Returns: 0 on success
*/
-static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
+static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
uint32_t color, uint32_t alpha)
{
const struct dpu_format *fmt;
const struct drm_plane *plane = >base;
struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
- struct dpu_hw_sspp_cfg pipe_cfg;
+ u32 fill_color = (color & 0xFF) | ((alpha & 0xFF) << 24);
DPU_DEBUG_PLANE(pdpu, "\n");
@@ -661,34 +695,12 @@ static int _dpu_plane_color_fill(struct
dpu_plane *pdpu,
* h/w only supports RGB variants
*/
fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+ /* should not happen ever */
+ if (!fmt)
+ return;
/* update sspp */
- if (fmt && pstate->pipe.sspp->ops.setup_solidfill) {
- pstate->pipe.sspp->ops.setup_solidfill(>pipe,
- (color & 0xFF) | ((alpha & 0xFF) << 24));
-
- /* override scaler/decimation if solid fill */
- pipe_cfg.dst_rect = pstate->base.dst;
-
- pipe_cfg.src_rect.x1 = 0;
- pipe_cfg.src_rect.y1 = 0;
- pipe_cfg.src_rect.x2 =
- drm_rect_width(_cfg.dst_rect);
- pipe_cfg.src_rect.y2 =
- drm_rect_height(_cfg.dst_rect);
-
- if (pstate->pipe.sspp->ops.setup_format)
- pstate->pipe.sspp->ops.setup_format(>pipe,
- fmt, DPU_SSPP_SOLID_FILL);
-
- if (pstate->pipe.sspp->ops.setup_rects)
- pstate->pipe.sspp->ops.setup_rects(>pipe,
- _cfg);
-
- _dpu_plane_setup_scaler(>pipe, fmt, true, _cfg,
pstate->rotation);
- }
-
- return 0;
+ _dpu_plane_color_fill_pipe(pstate, >pipe,
>pipe_cfg, fill_color, fmt);
}
int dpu_plane_validate_multirect_v2(struct
dpu_multirect_plane_states *plane)
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
On 07/02/2023 02:22, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the sspp-dependent? No, this is really pipe-dependent. It takes dpu_sw_pipe and dpu_sw_pipe_cfg arguments. separate function dpu_plane_sspp_update_pipe(). This is one of preparational steps to add r_pipe support. Signed-off-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [Freedreno] [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
On 07/02/2023 02:22, Abhinav Kumar wrote:
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the
sspp-dependent?
separate function dpu_plane_sspp_update_pipe(). This is one of
preparational steps to add r_pipe support.
Signed-off-by: Dmitry Baryshkov
Just a couple of minor comments below but otherwise this split up lgtm
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 113 --
1 file changed, 63 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 0986e740b978..f94e132733f3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -404,12 +404,13 @@ static void _dpu_plane_set_qos_ctrl(struct
drm_plane *plane,
* _dpu_plane_set_ot_limit - set OT limit for the given plane
* @plane: Pointer to drm plane
* @pipe: Pointer to software pipe
- * @crtc: Pointer to drm crtc
* @pipe_cfg: Pointer to pipe configuration
+ * @frame_rate: CRTC's frame rate
Can you please check the spacing here. There seems to be an extra tab
before the CRTC's frame rate
*/
static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
struct dpu_sw_pipe *pipe,
- struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg)
+ struct dpu_hw_sspp_cfg *pipe_cfg,
+ int frame_rate)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_vbif_set_ot_params ot_params;
@@ -421,7 +422,7 @@ static void _dpu_plane_set_ot_limit(struct
drm_plane *plane,
ot_params.width = drm_rect_width(_cfg->src_rect);
ot_params.height = drm_rect_height(_cfg->src_rect);
ot_params.is_wfd = !pdpu->is_rt_pipe;
- ot_params.frame_rate = drm_mode_vrefresh(>mode);
+ ot_params.frame_rate = frame_rate;
ot_params.vbif_idx = VBIF_RT;
ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
ot_params.rd = true;
@@ -457,26 +458,6 @@ static void _dpu_plane_set_qos_remap(struct
drm_plane *plane,
dpu_vbif_set_qos_remap(dpu_kms, _params);
}
-static void _dpu_plane_set_scanout(struct drm_plane *plane,
- struct dpu_plane_state *pstate,
- struct drm_framebuffer *fb)
-{
- struct dpu_plane *pdpu = to_dpu_plane(plane);
- struct dpu_kms *kms = _dpu_plane_get_kms(>base);
- struct msm_gem_address_space *aspace = kms->base.aspace;
- struct dpu_hw_fmt_layout layout;
- int ret;
-
- ret = dpu_format_populate_layout(aspace, fb, );
- if (ret)
- DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
- else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
- trace_dpu_plane_set_scanout(>pipe,
- );
- pstate->pipe.sspp->ops.setup_sourceaddress(>pipe,
);
- }
-}
-
static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
struct dpu_hw_scaler3_cfg *scale_cfg,
@@ -1102,35 +1083,25 @@ void dpu_plane_set_error(struct drm_plane
*plane, bool error)
pdpu->is_error = error;
}
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
+static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
+ struct dpu_sw_pipe *pipe,
+ struct dpu_hw_sspp_cfg *pipe_cfg,
You can call this parameter sspp_cfg instead of pipe_cfg?
I think, I'll add a commit renaming dpu_hw_sspp_cfg to dpu_sw_pipe_cfg,
it would be simmetrical to dpu_sw_pipe then.
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v3 18/27] drm/msm/dpu: populate SmartDMA features in hw catalog
On 05/02/2023 02:36, Abhinav Kumar wrote: On 2/4/2023 4:29 PM, Dmitry Baryshkov wrote: On Sun, 5 Feb 2023 at 01:20, Abhinav Kumar wrote: On 2/4/2023 1:08 PM, Dmitry Baryshkov wrote: On 04/02/2023 20:35, Abhinav Kumar wrote: On 2/4/2023 2:43 AM, Dmitry Baryshkov wrote: On 04/02/2023 07:10, Abhinav Kumar wrote: On 2/3/2023 8:10 PM, Dmitry Baryshkov wrote: On 04/02/2023 04:43, Abhinav Kumar wrote: On 2/3/2023 6:29 PM, Dmitry Baryshkov wrote: On 04/02/2023 01:35, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Downstream driver uses dpu->caps->smart_dma_rev to update sspp->cap->features with the bit corresponding to the supported SmartDMA version. Upstream driver does not do this, resulting in SSPP subdriver not enbaling setup_multirect callback. Add corresponding SmartDMA SSPP feature bits to dpu hw catalog. While reviewing this patch, I had a first hand experience of how we are reusing SSPP bitmasks for so many chipsets but I think overall you got them right here :) Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 cf053e8f081e..fc818b0273e7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -21,13 +21,16 @@ (VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3)) #define VIG_SDM845_MASK \ - (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3)) + (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3) |\ + BIT(DPU_SSPP_SMART_DMA_V2)) #define VIG_SC7180_MASK \ - (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4)) + (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4) |\ + BIT(DPU_SSPP_SMART_DMA_V2)) #define VIG_SM8250_MASK \ - (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE)) + (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE) |\ + BIT(DPU_SSPP_SMART_DMA_V2)) #define VIG_QCM2290_MASK (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL)) @@ -42,6 +45,7 @@ #define DMA_SDM845_MASK \ (BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) | BIT(DPU_SSPP_QOS_8LVL) |\ BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\ + BIT(DPU_SSPP_SMART_DMA_V2) |\ BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_EXCL_RECT)) #define DMA_CURSOR_SDM845_MASK \ VIG_SDM845_MASK and DMA_SDM845_MASK are used for many other chipsets like 8250, 8450, 8550. At the moment, for visual validation of this series, I only have sc7180/sc7280. We are leaving the rest for CI. Was that an intentional approach? If so, we will need tested-by tags from folks having 8350/8450/8550/sc8280x,qcm2290? I am only owning the visual validation on sc7280 atm. I'm not quite sure what is your intent here. Are there any SoCs after 845 that do not have SmartDMA 2.5? Or do you propose to enable SmartDMA only for the chipsets that we can visually test? That sounds strange. Yes I was thinking to enable smartDMA at the moment on chipsets which we can validate visually that display comes up. But I am not sure if thats entirely practical. But the intent was I just want to make sure basic display does come up with smartDMA enabled if we are enabling it for all chipsets. I don't think it is practical or logical. We don't require validating other changes on all possible chipsets, so what is so different with this one? Thats because with smartDMA if the programming of stages goes wrong we could potentially just see a blank screen. Its not about other changes, this change in particular controls enabling a feature. But thats just my thought. I am not going to request to ensure this or block this for this. You can still have my Reviewed-by: Abhinav Kumar But think of the validations that have to be done before we merge it. The usual way: verify as much as feasible and let anybody else complain during the development cycle. Well, our perspective is to enable the feature on devices on which you are able to test and not enable then wait for others to complain. This would not be really practical. There are plenty of people who can test things on obscure platforms, but unfortunately far less amount of people who tightly follow the development and can track which new feature applies to a particular platform. I hope to be able to fix that slightly with the hw catalog rework. However enabling features on other platforms definitely requires more knowledge than simply testing the kernel. I did not say test all devices. My point was to enable smartDMA on devices which we are able to test. There are other examples of this, like inline rotation, writeback etc. which are at the moment enabled only on devices which QC or others have tested on. But at the time it was added, inline rotation 2.0 could only be supported on
Re: [Freedreno] [PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer
On 09/02/2023 01:44, Abhinav Kumar wrote:
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a
separate functon. This is a preparation for the r_pipe support.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 86 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++-
2 files changed, 63 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 73e1a8c69ef0..0ca3bc38ff7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -400,6 +400,47 @@ static void
_dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
}
}
+static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
+ struct drm_plane *plane,
+ struct dpu_crtc_mixer *mixer,
+ u32 num_mixers,
+ struct dpu_hw_stage_cfg *stage_cfg,
+ enum dpu_stage stage,
+ unsigned int stage_idx,
+ unsigned long *fetch_active,
+ struct dpu_sw_pipe *pipe
+ )
+{
+ uint32_t lm_idx;
+ enum dpu_sspp sspp_idx;
+ struct drm_plane_state *state;
+
+ if (!pipe->sspp)
+ return;
+
+ sspp_idx = pipe->sspp->idx;
+
+ state = plane->state;
+
+ DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
+ crtc->base.id,
+ stage,
+ plane->base.id,
+ sspp_idx - SSPP_NONE,
+ state->fb ? state->fb->base.id : -1);
+
+ set_bit(sspp_idx, fetch_active);
+
+ stage_cfg->stage[stage][stage_idx] = sspp_idx;
+ stage_cfg->multirect_index[stage][stage_idx] =
+ pipe->multirect_index;
+
+ /* blend config update */
+ for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
+
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,
+ sspp_idx);
If you just pass the format to this function you can move rest of the
for loop also to this function.
Also, you will be able to add the trace_dpu_crtc_setup_mixer() with
complete information.
trace_dpu_crtc_setup_mixer is currently missing te stage_idx which is
important to debug blend issues.
Ack, I'll add it back, thanks for the note.
+}
+
static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer,
struct dpu_hw_stage_cfg *stage_cfg)
@@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct
drm_crtc *crtc,
struct dpu_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
- uint32_t stage_idx, lm_idx;
- int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
+ uint32_t lm_idx;
bool bg_alpha_enable = false;
DECLARE_BITMAP(fetch_active, SSPP_MAX);
memset(fetch_active, 0, sizeof(fetch_active));
drm_atomic_crtc_for_each_plane(plane, crtc) {
- enum dpu_sspp sspp_idx;
-
state = plane->state;
if (!state)
continue;
@@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct
drm_crtc *crtc,
pstate = to_dpu_plane_state(state);
fb = state->fb;
- sspp_idx = pstate->pipe.sspp->idx;
- set_bit(sspp_idx, fetch_active);
-
- DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
- crtc->base.id,
- pstate->stage,
- plane->base.id,
- sspp_idx - SSPP_VIG0,
- state->fb ? state->fb->base.id : -1);
-
format =
to_dpu_format(msm_framebuffer_format(pstate->base.fb));
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
- stage_idx = zpos_cnt[pstate->stage]++;
- stage_cfg->stage[pstate->stage][stage_idx] =
- sspp_idx;
- stage_cfg->multirect_index[pstate->stage][stage_idx] =
- pstate->pipe.multirect_index;
-
trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
- state, pstate, stage_idx,
+ state, pstate,
format->base.pixel_format,
fb ? fb->modifier : 0);
+ _dpu_crtc_blend_setup_pipe(crtc, plane,
+ mixer, cstate->num_mixers,
+ stage_cfg, pstate->stage, 0,
+ fetch_active,
+ >pipe);
+
/* blend config update */
for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
- _dpu_crtc_setup_blend_cfg(mixer + lm_idx,
- pstate, format);
-
-
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,
- sspp_idx);
+
Re: [Freedreno] [PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a
separate functon. This is a preparation for the r_pipe support.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 86 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++-
2 files changed, 63 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 73e1a8c69ef0..0ca3bc38ff7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -400,6 +400,47 @@ static void _dpu_crtc_program_lm_output_roi(struct
drm_crtc *crtc)
}
}
+static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
+ struct drm_plane *plane,
+ struct dpu_crtc_mixer *mixer,
+ u32 num_mixers,
+ struct dpu_hw_stage_cfg *stage_cfg,
+ enum dpu_stage stage,
+ unsigned int stage_idx,
+ unsigned long *fetch_active,
+ struct dpu_sw_pipe *pipe
+ )
+{
+ uint32_t lm_idx;
+ enum dpu_sspp sspp_idx;
+ struct drm_plane_state *state;
+
+ if (!pipe->sspp)
+ return;
+
+ sspp_idx = pipe->sspp->idx;
+
+ state = plane->state;
+
+ DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
+crtc->base.id,
+stage,
+plane->base.id,
+sspp_idx - SSPP_NONE,
+state->fb ? state->fb->base.id : -1);
+
+ set_bit(sspp_idx, fetch_active);
+
+ stage_cfg->stage[stage][stage_idx] = sspp_idx;
+ stage_cfg->multirect_index[stage][stage_idx] =
+ pipe->multirect_index;
+
+ /* blend config update */
+ for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
+
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,
+ sspp_idx);
If you just pass the format to this function you can move rest of the
for loop also to this function.
Also, you will be able to add the trace_dpu_crtc_setup_mixer() with
complete information.
trace_dpu_crtc_setup_mixer is currently missing te stage_idx which is
important to debug blend issues.
+}
+
static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer,
struct dpu_hw_stage_cfg *stage_cfg)
@@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
struct dpu_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
- uint32_t stage_idx, lm_idx;
- int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
+ uint32_t lm_idx;
bool bg_alpha_enable = false;
DECLARE_BITMAP(fetch_active, SSPP_MAX);
memset(fetch_active, 0, sizeof(fetch_active));
drm_atomic_crtc_for_each_plane(plane, crtc) {
- enum dpu_sspp sspp_idx;
-
state = plane->state;
if (!state)
continue;
@@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
pstate = to_dpu_plane_state(state);
fb = state->fb;
- sspp_idx = pstate->pipe.sspp->idx;
- set_bit(sspp_idx, fetch_active);
-
- DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
- crtc->base.id,
- pstate->stage,
- plane->base.id,
- sspp_idx - SSPP_VIG0,
- state->fb ? state->fb->base.id : -1);
-
format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
- stage_idx = zpos_cnt[pstate->stage]++;
- stage_cfg->stage[pstate->stage][stage_idx] =
- sspp_idx;
- stage_cfg->multirect_index[pstate->stage][stage_idx] =
- pstate->pipe.multirect_index;
-
trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
- state, pstate, stage_idx,
+ state, pstate,
format->base.pixel_format,
fb ? fb->modifier : 0);
+ _dpu_crtc_blend_setup_pipe(crtc, plane,
+ mixer,
Re: [Freedreno] [RFC PATCH] drm/msm/dpu: Move TE setup to prepare_for_kickoff()
On 09/02/2023 01:24, Jessica Zhang wrote:
On 2/8/2023 2:18 PM, Dmitry Baryshkov wrote:
On 08/02/2023 23:37, Jessica Zhang wrote:
Currently, DPU will enable TE during prepare_commit(). However, this
will cause issues when trying to read/write to register in
get_autorefresh_config(), because the core clock rates aren't set at
that time.
This used to work because phys_enc->hw_pp is only initialized in mode
set [1], so the first prepare_commit() will return before any register
read/write as hw_pp would be NULL.
However, when we try to implement support for INTF TE, we will run into
the clock issue described above as hw_intf will *not* be NULL on the
first prepare_commit(). This is because the initialization of
dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2].
To avoid this issue, let's enable TE during prepare_for_kickoff()
instead as the core clock rates are guaranteed to be set then.
Depends on: "Implement tearcheck support on INTF block" [3]
[1]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109
[2]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339
[3] https://patchwork.freedesktop.org/series/112332/
Signed-off-by: Jessica Zhang
---
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++-
1 file changed, 43 insertions(+), 35 deletions(-)
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 279a0b7015ce..746250bce3d1 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
@@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct
dpu_encoder_phys *phys_enc)
kfree(cmd_enc);
}
-static void dpu_encoder_phys_cmd_prepare_for_kickoff(
- struct dpu_encoder_phys *phys_enc)
-{
- struct dpu_encoder_phys_cmd *cmd_enc =
- to_dpu_encoder_phys_cmd(phys_enc);
- int ret;
-
- if (!phys_enc->hw_pp) {
- DPU_ERROR("invalid encoder\n");
- return;
- }
- DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n",
DRMID(phys_enc->parent),
- phys_enc->hw_pp->idx - PINGPONG_0,
- atomic_read(_enc->pending_kickoff_cnt));
-
- /*
- * Mark kickoff request as outstanding. If there are more than one,
- * outstanding, then we have to wait for the previous one to
complete
- */
- ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc);
- if (ret) {
- /* force pending_kickoff_cnt 0 to discard failed kickoff */
- atomic_set(_enc->pending_kickoff_cnt, 0);
- DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n",
- DRMID(phys_enc->parent), ret,
- phys_enc->hw_pp->idx - PINGPONG_0);
- }
-
- DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n",
- phys_enc->hw_pp->idx - PINGPONG_0,
- atomic_read(_enc->pending_kickoff_cnt));
-}
-
static bool dpu_encoder_phys_cmd_is_ongoing_pptx(
struct dpu_encoder_phys *phys_enc)
{
@@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx(
return false;
}
-static void dpu_encoder_phys_cmd_prepare_commit(
- struct dpu_encoder_phys *phys_enc)
+static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys
*phys_enc)
{
struct dpu_encoder_phys_cmd *cmd_enc =
to_dpu_encoder_phys_cmd(phys_enc);
@@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit(
"disabled autorefresh\n");
}
+static void dpu_encoder_phys_cmd_prepare_for_kickoff(
+ struct dpu_encoder_phys *phys_enc)
Could you please move the function back to the place, so that we can
see the actual difference?
Hi Dmitry,
This function was moved because prepare_commit() and is_ongoing_pptx()
(which is called in prepare_commit()) were originally defined later in
the file.
+{
+ struct dpu_encoder_phys_cmd *cmd_enc =
+ to_dpu_encoder_phys_cmd(phys_enc);
+ int ret;
+
+ if (!phys_enc->hw_pp) {
+ DPU_ERROR("invalid encoder\n");
+ return;
+ }
+
+
+ DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n",
DRMID(phys_enc->parent),
+ phys_enc->hw_pp->idx - PINGPONG_0,
+ atomic_read(_enc->pending_kickoff_cnt));
+
+ /*
+ * Mark kickoff request as outstanding. If there are more than one,
+ * outstanding, then we have to wait for the previous one to
complete
+ */
+ ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc);
+ if (ret) {
+ /* force pending_kickoff_cnt 0 to discard failed kickoff */
+ atomic_set(_enc->pending_kickoff_cnt, 0);
+ DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n",
+ DRMID(phys_enc->parent), ret,
+ phys_enc->hw_pp->idx - PINGPONG_0);
+ }
+
+ dpu_encoder_phys_cmd_enable_te(phys_enc);
+
+ DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n",
+
Re: [Freedreno] [RFC PATCH] drm/msm/dpu: Move TE setup to prepare_for_kickoff()
On 2/8/2023 2:18 PM, Dmitry Baryshkov wrote:
On 08/02/2023 23:37, Jessica Zhang wrote:
Currently, DPU will enable TE during prepare_commit(). However, this
will cause issues when trying to read/write to register in
get_autorefresh_config(), because the core clock rates aren't set at
that time.
This used to work because phys_enc->hw_pp is only initialized in mode
set [1], so the first prepare_commit() will return before any register
read/write as hw_pp would be NULL.
However, when we try to implement support for INTF TE, we will run into
the clock issue described above as hw_intf will *not* be NULL on the
first prepare_commit(). This is because the initialization of
dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2].
To avoid this issue, let's enable TE during prepare_for_kickoff()
instead as the core clock rates are guaranteed to be set then.
Depends on: "Implement tearcheck support on INTF block" [3]
[1]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109
[2]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339
[3] https://patchwork.freedesktop.org/series/112332/
Signed-off-by: Jessica Zhang
---
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++-
1 file changed, 43 insertions(+), 35 deletions(-)
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 279a0b7015ce..746250bce3d1 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
@@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct
dpu_encoder_phys *phys_enc)
kfree(cmd_enc);
}
-static void dpu_encoder_phys_cmd_prepare_for_kickoff(
- struct dpu_encoder_phys *phys_enc)
-{
- struct dpu_encoder_phys_cmd *cmd_enc =
- to_dpu_encoder_phys_cmd(phys_enc);
- int ret;
-
- if (!phys_enc->hw_pp) {
- DPU_ERROR("invalid encoder\n");
- return;
- }
- DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n",
DRMID(phys_enc->parent),
- phys_enc->hw_pp->idx - PINGPONG_0,
- atomic_read(_enc->pending_kickoff_cnt));
-
- /*
- * Mark kickoff request as outstanding. If there are more than one,
- * outstanding, then we have to wait for the previous one to
complete
- */
- ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc);
- if (ret) {
- /* force pending_kickoff_cnt 0 to discard failed kickoff */
- atomic_set(_enc->pending_kickoff_cnt, 0);
- DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n",
- DRMID(phys_enc->parent), ret,
- phys_enc->hw_pp->idx - PINGPONG_0);
- }
-
- DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n",
- phys_enc->hw_pp->idx - PINGPONG_0,
- atomic_read(_enc->pending_kickoff_cnt));
-}
-
static bool dpu_encoder_phys_cmd_is_ongoing_pptx(
struct dpu_encoder_phys *phys_enc)
{
@@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx(
return false;
}
-static void dpu_encoder_phys_cmd_prepare_commit(
- struct dpu_encoder_phys *phys_enc)
+static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys
*phys_enc)
{
struct dpu_encoder_phys_cmd *cmd_enc =
to_dpu_encoder_phys_cmd(phys_enc);
@@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit(
"disabled autorefresh\n");
}
+static void dpu_encoder_phys_cmd_prepare_for_kickoff(
+ struct dpu_encoder_phys *phys_enc)
Could you please move the function back to the place, so that we can see
the actual difference?
Hi Dmitry,
This function was moved because prepare_commit() and is_ongoing_pptx()
(which is called in prepare_commit()) were originally defined later in
the file.
+{
+ struct dpu_encoder_phys_cmd *cmd_enc =
+ to_dpu_encoder_phys_cmd(phys_enc);
+ int ret;
+
+ if (!phys_enc->hw_pp) {
+ DPU_ERROR("invalid encoder\n");
+ return;
+ }
+
+
+ DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n",
DRMID(phys_enc->parent),
+ phys_enc->hw_pp->idx - PINGPONG_0,
+ atomic_read(_enc->pending_kickoff_cnt));
+
+ /*
+ * Mark kickoff request as outstanding. If there are more than one,
+ * outstanding, then we have to wait for the previous one to
complete
+ */
+ ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc);
+ if (ret) {
+ /* force pending_kickoff_cnt 0 to discard failed kickoff */
+ atomic_set(_enc->pending_kickoff_cnt, 0);
+ DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n",
+ DRMID(phys_enc->parent), ret,
+ phys_enc->hw_pp->idx - PINGPONG_0);
+ }
+
+ dpu_encoder_phys_cmd_enable_te(phys_enc);
+
+ DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n",
+ phys_enc->hw_pp->idx - PINGPONG_0,
+
Re: [Freedreno] [PATCH v3 17/27] drm/msm/dpu: rewrite plane's QoS-related functions to take dpu_sw_pipe and dpu_format
On 2/3/2023 3:20 PM, Dmitry Baryshkov wrote: On 04/02/2023 01:07, Abhinav Kumar wrote: On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote: Rewrite dpu_plane's QoS related functions to take struct dpu_sw_pipe and struct dpu_format as arguments rather than fetching them from the pstate or drm_framebuffer. Signed-off-by: Dmitry Baryshkov Nothing wrong with the change as such but why is this needed? I looked through tne next patches in the series briefly and unless I am missing something, I am not able to see how this rewrite is helping or needed for the remaining patches. Having a separate pipe argument eases adding support for r_pipe. After all these changes only upper level functions access pstate->pipe. Then it becomes natural to do: dpu_plane_do_something(plane->pipe); if (plane->r_pipe) dpu_plane_do_something(plane->r_pipe); Understood, Reviewed-by: Abhinav Kumar
Re: [Freedreno] [PATCH v3 25/27] drm/msm/dpu: rework static color fill code
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Rework static color fill code to separate the pipe / pipe_cfg handling.
This is a preparation for the r_pipe support.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 70 +--
1 file changed, 41 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 05047192cb37..e2e85688ed3c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -639,20 +639,54 @@ static void _dpu_plane_setup_scaler(struct dpu_sw_pipe
*pipe,
fmt);
}
+static int _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate,
+ struct dpu_sw_pipe *pipe,
+ struct dpu_hw_sspp_cfg *old_pipe_cfg,
Why is this called old_pipe_cfg instead of just pipe_cfg?
+ u32 fill_color,
+ const struct dpu_format *fmt)
+{
+ struct dpu_hw_sspp_cfg pipe_cfg;
+
+ /* update sspp */
+ if (!pipe->sspp->ops.setup_solidfill)
+ return 0;
You can just return from here and make this function void?
+
+ pipe->sspp->ops.setup_solidfill(pipe, fill_color);
+
+ /* override scaler/decimation if solid fill */
+ pipe_cfg.dst_rect = old_pipe_cfg->dst_rect;
+
+ pipe_cfg.src_rect.x1 = 0;
+ pipe_cfg.src_rect.y1 = 0;
+ pipe_cfg.src_rect.x2 =
+ drm_rect_width(_cfg.dst_rect);
+ pipe_cfg.src_rect.y2 =
+ drm_rect_height(_cfg.dst_rect);
+
+ if (pipe->sspp->ops.setup_format)
+ pipe->sspp->ops.setup_format(pipe, fmt, DPU_SSPP_SOLID_FILL);
+
+ if (pipe->sspp->ops.setup_rects)
+ pipe->sspp->ops.setup_rects(pipe, _cfg);
+
+ _dpu_plane_setup_scaler(pipe, fmt, true, _cfg, pstate->rotation);
+
+ return 0;
+}
+
/**
* _dpu_plane_color_fill - enables color fill on plane
* @pdpu: Pointer to DPU plane object
* @color: RGB fill color value, [23..16] Blue, [15..8] Green, [7..0] Red
* @alpha: 8-bit fill alpha value, 255 selects 100% alpha
- * Returns: 0 on success
*/
-static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
+static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
uint32_t color, uint32_t alpha)
{
const struct dpu_format *fmt;
const struct drm_plane *plane = >base;
struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
- struct dpu_hw_sspp_cfg pipe_cfg;
+ u32 fill_color = (color & 0xFF) | ((alpha & 0xFF) << 24);
DPU_DEBUG_PLANE(pdpu, "\n");
@@ -661,34 +695,12 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
* h/w only supports RGB variants
*/
fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+ /* should not happen ever */
+ if (!fmt)
+ return;
/* update sspp */
- if (fmt && pstate->pipe.sspp->ops.setup_solidfill) {
- pstate->pipe.sspp->ops.setup_solidfill(>pipe,
- (color & 0xFF) | ((alpha & 0xFF) << 24));
-
- /* override scaler/decimation if solid fill */
- pipe_cfg.dst_rect = pstate->base.dst;
-
- pipe_cfg.src_rect.x1 = 0;
- pipe_cfg.src_rect.y1 = 0;
- pipe_cfg.src_rect.x2 =
- drm_rect_width(_cfg.dst_rect);
- pipe_cfg.src_rect.y2 =
- drm_rect_height(_cfg.dst_rect);
-
- if (pstate->pipe.sspp->ops.setup_format)
- pstate->pipe.sspp->ops.setup_format(>pipe,
- fmt, DPU_SSPP_SOLID_FILL);
-
- if (pstate->pipe.sspp->ops.setup_rects)
- pstate->pipe.sspp->ops.setup_rects(>pipe,
- _cfg);
-
- _dpu_plane_setup_scaler(>pipe, fmt, true, _cfg,
pstate->rotation);
- }
-
- return 0;
+ _dpu_plane_color_fill_pipe(pstate, >pipe, >pipe_cfg,
fill_color, fmt);
}
int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
Re: [Freedreno] [RFC PATCH] drm/msm/dpu: Move TE setup to prepare_for_kickoff()
On 08/02/2023 23:37, Jessica Zhang wrote:
Currently, DPU will enable TE during prepare_commit(). However, this
will cause issues when trying to read/write to register in
get_autorefresh_config(), because the core clock rates aren't set at
that time.
This used to work because phys_enc->hw_pp is only initialized in mode
set [1], so the first prepare_commit() will return before any register
read/write as hw_pp would be NULL.
However, when we try to implement support for INTF TE, we will run into
the clock issue described above as hw_intf will *not* be NULL on the
first prepare_commit(). This is because the initialization of
dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2].
To avoid this issue, let's enable TE during prepare_for_kickoff()
instead as the core clock rates are guaranteed to be set then.
Depends on: "Implement tearcheck support on INTF block" [3]
[1]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109
[2]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339
[3] https://patchwork.freedesktop.org/series/112332/
Signed-off-by: Jessica Zhang
---
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++-
1 file changed, 43 insertions(+), 35 deletions(-)
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 279a0b7015ce..746250bce3d1 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
@@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct
dpu_encoder_phys *phys_enc)
kfree(cmd_enc);
}
-static void dpu_encoder_phys_cmd_prepare_for_kickoff(
- struct dpu_encoder_phys *phys_enc)
-{
- struct dpu_encoder_phys_cmd *cmd_enc =
- to_dpu_encoder_phys_cmd(phys_enc);
- int ret;
-
- if (!phys_enc->hw_pp) {
- DPU_ERROR("invalid encoder\n");
- return;
- }
- DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent),
- phys_enc->hw_pp->idx - PINGPONG_0,
- atomic_read(_enc->pending_kickoff_cnt));
-
- /*
-* Mark kickoff request as outstanding. If there are more than one,
-* outstanding, then we have to wait for the previous one to complete
-*/
- ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc);
- if (ret) {
- /* force pending_kickoff_cnt 0 to discard failed kickoff */
- atomic_set(_enc->pending_kickoff_cnt, 0);
- DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n",
- DRMID(phys_enc->parent), ret,
- phys_enc->hw_pp->idx - PINGPONG_0);
- }
-
- DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n",
- phys_enc->hw_pp->idx - PINGPONG_0,
- atomic_read(_enc->pending_kickoff_cnt));
-}
-
static bool dpu_encoder_phys_cmd_is_ongoing_pptx(
struct dpu_encoder_phys *phys_enc)
{
@@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx(
return false;
}
-static void dpu_encoder_phys_cmd_prepare_commit(
- struct dpu_encoder_phys *phys_enc)
+static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc)
{
struct dpu_encoder_phys_cmd *cmd_enc =
to_dpu_encoder_phys_cmd(phys_enc);
@@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit(
"disabled autorefresh\n");
}
+static void dpu_encoder_phys_cmd_prepare_for_kickoff(
+ struct dpu_encoder_phys *phys_enc)
Could you please move the function back to the place, so that we can see
the actual difference?
+{
+ struct dpu_encoder_phys_cmd *cmd_enc =
+ to_dpu_encoder_phys_cmd(phys_enc);
+ int ret;
+
+ if (!phys_enc->hw_pp) {
+ DPU_ERROR("invalid encoder\n");
+ return;
+ }
+
+
+ DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent),
+ phys_enc->hw_pp->idx - PINGPONG_0,
+ atomic_read(_enc->pending_kickoff_cnt));
+
+ /*
+* Mark kickoff request as outstanding. If there are more than one,
+* outstanding, then we have to wait for the previous one to complete
+*/
+ ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc);
+ if (ret) {
+ /* force pending_kickoff_cnt 0 to discard failed kickoff */
+ atomic_set(_enc->pending_kickoff_cnt, 0);
+ DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n",
+ DRMID(phys_enc->parent), ret,
+ phys_enc->hw_pp->idx - PINGPONG_0);
+ }
+
+ dpu_encoder_phys_cmd_enable_te(phys_enc);
+
+
Re: [Freedreno] [PATCH v12 13/14] drm/msm/disp/dpu: add PSR support for eDP interface in dpu driver
On 07/02/2023 17:25, Dmitry Baryshkov wrote:
On 07/02/2023 16:26, Vinod Polimera wrote:
-Original Message-
From: Dmitry Baryshkov
Sent: Tuesday, January 31, 2023 6:29 PM
To: Vinod Polimera (QUIC) ; dri-
de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org;
freedreno@lists.freedesktop.org; devicet...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org; robdcl...@gmail.com;
diand...@chromium.org; swb...@chromium.org; Kalyan Thota (QUIC)
; Kuogee Hsieh (QUIC)
; Vishnuvardhan Prodduturi (QUIC)
; Bjorn Andersson (QUIC)
; Abhinav Kumar (QUIC)
; Sankeerth Billakanti (QUIC)
Subject: Re: [PATCH v12 13/14] drm/msm/disp/dpu: add PSR support for eDP
interface in dpu driver
On 30/01/2023 17:11, Vinod Polimera wrote:
Enable PSR on eDP interface using drm self-refresh librabry.
This patch uses a trigger from self-refresh library to enter/exit
into PSR, when there are no updates from framework.
Signed-off-by: Kalyan Thota
Signed-off-by: Vinod Polimera
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 -
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +-
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index f29a339..60e5984 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include "dpu_kms.h"
#include "dpu_hw_lm.h"
@@ -1021,6 +1022,9 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc,
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
+ if (old_crtc_state->self_refresh_active)
+ return;
+
I have been looking at the crtc_needs_disable(). It explicitly mentions
that 'We also need to run through the crtc_funcs->disable() function
[..] if it's transitioning to self refresh mode...'. Don't we need to
perform some cleanup here (like disabling the vblank irq handling,
freeing the bandwidth, etc)?
When self refresh active is enabled, then we will clean up irq
handling and bandwidth etc.
The above case is to handle display off commit triggered when we are
in psr as all
the resources are already cleaned up . we just need to do an early
return.
/* Disable/save vblank irq handling */
drm_crtc_vblank_off(crtc);
@@ -1577,7 +1581,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device
*dev, struct drm_plane *plane,
{
struct drm_crtc *crtc = NULL;
struct dpu_crtc *dpu_crtc = NULL;
- int i;
+ int i, ret;
dpu_crtc = kzalloc(sizeof(*dpu_crtc), GFP_KERNEL);
if (!dpu_crtc)
@@ -1614,6 +1618,13 @@ struct drm_crtc *dpu_crtc_init(struct
drm_device *dev, struct drm_plane *plane,
/* initialize event handling */
spin_lock_init(_crtc->event_lock);
+ ret = drm_self_refresh_helper_init(crtc);
+ if (ret) {
+ DPU_ERROR("Failed to initialize %s with self-refresh
helpers %d\n",
+ crtc->name, ret);
+ return ERR_PTR(ret);
+ }
+
DRM_DEBUG_KMS("%s: successfully initialized crtc\n", dpu_crtc-
name);
return crtc;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 01b7509..450abb1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -12,6 +12,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1212,11 +1213,24 @@ static void
dpu_encoder_virt_atomic_disable(struct drm_encoder *drm_enc,
struct drm_atomic_state *state)
{
struct dpu_encoder_virt *dpu_enc = NULL;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *old_state = NULL;
int i = 0;
dpu_enc = to_dpu_encoder_virt(drm_enc);
DPU_DEBUG_ENC(dpu_enc, "\n");
+ crtc = drm_atomic_get_old_crtc_for_encoder(state, drm_enc);
+ if (crtc)
+ old_state = drm_atomic_get_old_crtc_state(state, crtc);
+
+ /*
+ * The encoder is already disabled if self refresh mode was
set earlier,
+ * in the old_state for the corresponding crtc.
+ */
+ if (old_state && old_state->self_refresh_active)
+ return;
+
Again the same question here, doesn't crtc_needs_disable() take care of
this clause? I might be missing something in the PSR state transitions.
Could you please add some explanation here?
Same usecase as above, applies to encoder disable also when triggered
via disable commit
When driver is in psr state.
Ack, thank you for the explanations. I'd like to take another glance
later today, but generally it look good to me.
After another glance it still looks good to me. Please send the last
iteration of the series:
- moving all core patches to the first place, as it was asked
previously. This will help
[Freedreno] [RFC PATCH] drm/msm/dpu: Move TE setup to prepare_for_kickoff()
Currently, DPU will enable TE during prepare_commit(). However, this
will cause issues when trying to read/write to register in
get_autorefresh_config(), because the core clock rates aren't set at
that time.
This used to work because phys_enc->hw_pp is only initialized in mode
set [1], so the first prepare_commit() will return before any register
read/write as hw_pp would be NULL.
However, when we try to implement support for INTF TE, we will run into
the clock issue described above as hw_intf will *not* be NULL on the
first prepare_commit(). This is because the initialization of
dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2].
To avoid this issue, let's enable TE during prepare_for_kickoff()
instead as the core clock rates are guaranteed to be set then.
Depends on: "Implement tearcheck support on INTF block" [3]
[1]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109
[2]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339
[3] https://patchwork.freedesktop.org/series/112332/
Signed-off-by: Jessica Zhang
---
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++-
1 file changed, 43 insertions(+), 35 deletions(-)
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 279a0b7015ce..746250bce3d1 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
@@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct
dpu_encoder_phys *phys_enc)
kfree(cmd_enc);
}
-static void dpu_encoder_phys_cmd_prepare_for_kickoff(
- struct dpu_encoder_phys *phys_enc)
-{
- struct dpu_encoder_phys_cmd *cmd_enc =
- to_dpu_encoder_phys_cmd(phys_enc);
- int ret;
-
- if (!phys_enc->hw_pp) {
- DPU_ERROR("invalid encoder\n");
- return;
- }
- DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent),
- phys_enc->hw_pp->idx - PINGPONG_0,
- atomic_read(_enc->pending_kickoff_cnt));
-
- /*
-* Mark kickoff request as outstanding. If there are more than one,
-* outstanding, then we have to wait for the previous one to complete
-*/
- ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc);
- if (ret) {
- /* force pending_kickoff_cnt 0 to discard failed kickoff */
- atomic_set(_enc->pending_kickoff_cnt, 0);
- DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n",
- DRMID(phys_enc->parent), ret,
- phys_enc->hw_pp->idx - PINGPONG_0);
- }
-
- DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n",
- phys_enc->hw_pp->idx - PINGPONG_0,
- atomic_read(_enc->pending_kickoff_cnt));
-}
-
static bool dpu_encoder_phys_cmd_is_ongoing_pptx(
struct dpu_encoder_phys *phys_enc)
{
@@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx(
return false;
}
-static void dpu_encoder_phys_cmd_prepare_commit(
- struct dpu_encoder_phys *phys_enc)
+static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc)
{
struct dpu_encoder_phys_cmd *cmd_enc =
to_dpu_encoder_phys_cmd(phys_enc);
@@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit(
"disabled autorefresh\n");
}
+static void dpu_encoder_phys_cmd_prepare_for_kickoff(
+ struct dpu_encoder_phys *phys_enc)
+{
+ struct dpu_encoder_phys_cmd *cmd_enc =
+ to_dpu_encoder_phys_cmd(phys_enc);
+ int ret;
+
+ if (!phys_enc->hw_pp) {
+ DPU_ERROR("invalid encoder\n");
+ return;
+ }
+
+
+ DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent),
+ phys_enc->hw_pp->idx - PINGPONG_0,
+ atomic_read(_enc->pending_kickoff_cnt));
+
+ /*
+* Mark kickoff request as outstanding. If there are more than one,
+* outstanding, then we have to wait for the previous one to complete
+*/
+ ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc);
+ if (ret) {
+ /* force pending_kickoff_cnt 0 to discard failed kickoff */
+ atomic_set(_enc->pending_kickoff_cnt, 0);
+ DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n",
+ DRMID(phys_enc->parent), ret,
+ phys_enc->hw_pp->idx - PINGPONG_0);
+ }
+
+ dpu_encoder_phys_cmd_enable_te(phys_enc);
+
+ DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n",
+ phys_enc->hw_pp->idx - PINGPONG_0,
+
Re: [Freedreno] [PATCH 09/14] drm/msm/a6xx: Fix some A619 tunables
On Thu, Jan 26, 2023 at 04:16:13PM +0100, Konrad Dybcio wrote:
> Adreno 619 expects some tunables to be set differently. Make up for it.
>
> Fixes: b7616b5c69e6 ("drm/msm/adreno: Add A619 support")
> 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 7a480705f407..f34ab3f39f09 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1171,6 +1171,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
> @@ -1188,7 +1190,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);
The number appended to the register is the number of clock ticks to wait
before declaring a hang. 0x3f happens to be the largest value that
can be set for the a6xx family (excepting the 610 which, IIRC, used older
hardware that had a smaller field for the counter). Downstream the
number would creep up over time as unexplained hangs were discovered and
diagnosed or covered up as "just wait longer".
So in theory you could leave this with the "default value" or even bump
up the default value to 0x3f for all targets if you wanted to. An
alternate solution (that downstream does) is to put this as a
pre-defined configuration in gpulist[].
Jordan
Re: [Freedreno] [PATCH v3 0/4] Reserve DSPPs based on user request
Hi, On Wed, Feb 8, 2023 at 5:42 AM Kalyan Thota wrote: > > This series will enable color features on sc7280 target which has > primary panel as eDP > > The series removes DSPP allocation based on encoder type and allows > the DSPP reservation based on user request via CTM. > > The series will release/reserve the dpu resources when ever there is > a topology change to suit the new requirements. > > Kalyan Thota (4): > drm/msm/dpu: clear DSPP reservations in rm release > drm/msm/dpu: add DSPPs into reservation upon a CTM request > drm/msm/dpu: avoid unnecessary check in DPU reservations > drm/msm/dpu: reserve the resources on topology change > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 + > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 58 > - > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 + > 3 files changed, 37 insertions(+), 25 deletions(-) I tried out your changes, but unfortunately it seems like there's something wrong. :( I did this: 1. Picked your 5 patches to the chromeos-5.15 tree (this series plus [1]) 2. Put them on herobrine villager. 3. Booted up with no external display plugged in. 4. Tried to enable night light in the ChromeOS UI. 5. Night light didn't turn on for the internal display. I also tried applying them to the top of msm-next (had to resolve some small conflicts). Same thing, night light didn't work. I thought maybe this was because the Chrome browser hasn't been updated to properly use atomic_check for testing for night light, so I hacked my herobrine device tree to not mark "mdss_dp" as "okay". Now there's _only_ an eDP display. Same thing, night light didn't work. I could only get night light to work for the internal display if I plugged and unplugged an external display in. Is the above the behavior that's expected right now? [1] https://lore.kernel.org/all/1674814487-2112-1-git-send-email-quic_kaly...@quicinc.com/
Re: [Freedreno] [PATCH v2 8/8] arm64: dts: qcom: sm8350-hdk: enable GPU
On 6.02.2023 15:57, Dmitry Baryshkov wrote:
> Enable the GPU on the SM8350-HDK device. The ZAP shader is required for
> the GPU to function properly.
>
> Signed-off-by: Dmitry Baryshkov
> ---
Reviewed-by: Konrad Dybcio
Konrad
> arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts
> b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts
> index df841230d1b7..5e744423a673 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts
> @@ -284,6 +284,14 @@ _dma1 {
> status = "okay";
> };
>
> + {
> + status = "okay";
> +
> + zap-shader {
> + firmware-name = "qcom/sm8350/a660_zap.mbn";
> + };
> +};
> +
> {
> clock-frequency = <40>;
> status = "okay";
Re: [Freedreno] [PATCH v3 4/4] drm/msm/dpu: reserve the resources on topology change
On 08/02/2023 15:42, Kalyan Thota wrote: Some features like CTM can be enabled dynamically. Release and reserve the DPU resources whenever a topology change occurs such that required hw blocks are allocated appropriately. Signed-off-by: Kalyan Thota --- Changes in v1: - Avoid mode_set call directly (Dmitry) Changes in v2: - Minor nits (Dmitry) Changes in v3: - avoid unnecessary modeset check call (Dmitry) --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +++- 2 files changed, 21 insertions(+), 5 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[Freedreno] [PATCH v3 2/4] drm/msm/dpu: add DSPPs into reservation upon a CTM request
Add DSPP blocks into the topology for reservation, if there
is a CTM request for that composition.
Signed-off-by: Kalyan Thota
Reviewed-by: Dmitry Baryshkov
---
Changes in v1:
- Minor nits (Dmitry)
Changes in v2:
- Populate DSPPs into the reservation only if CTM is requested (Dmitry)
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 9c6817b..46d2a5c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -545,7 +545,8 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
static struct msm_display_topology dpu_encoder_get_topology(
struct dpu_encoder_virt *dpu_enc,
struct dpu_kms *dpu_kms,
- struct drm_display_mode *mode)
+ struct drm_display_mode *mode,
+ struct drm_crtc_state *crtc_state)
{
struct msm_display_topology topology = {0};
int i, intf_count = 0;
@@ -563,8 +564,7 @@ static struct msm_display_topology dpu_encoder_get_topology(
* 1 LM, 1 INTF
* 2 LM, 1 INTF (stream merge to support high resolution interfaces)
*
-* Adding color blocks only to primary interface if available in
-* sufficient number
+* Add dspps to the reservation requirements if ctm is requested
*/
if (intf_count == 2)
topology.num_lm = 2;
@@ -573,11 +573,8 @@ static struct msm_display_topology
dpu_encoder_get_topology(
else
topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
- if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
- if (dpu_kms->catalog->dspp &&
- (dpu_kms->catalog->dspp_count >= topology.num_lm))
- topology.num_dspp = topology.num_lm;
- }
+ if (crtc_state->ctm)
+ topology.num_dspp = topology.num_lm;
topology.num_enc = 0;
topology.num_intf = intf_count;
@@ -643,7 +640,7 @@ static int dpu_encoder_virt_atomic_check(
}
}
- topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
+ topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode,
crtc_state);
/* Reserve dynamic resources now. */
if (!ret) {
--
2.7.4
[Freedreno] [PATCH v3 1/4] drm/msm/dpu: clear DSPP reservations in rm release
Clear DSPP reservations from the global state during
rm release
Fixes: e47616df008b ("drm/msm/dpu: add support for color processing blocks in
dpu driver")
Signed-off-by: Kalyan Thota
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Marijn Suijten
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 73b3442..718ea0a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -572,6 +572,8 @@ void dpu_rm_release(struct dpu_global_state *global_state,
ARRAY_SIZE(global_state->ctl_to_enc_id), enc->base.id);
_dpu_rm_clear_mapping(global_state->dsc_to_enc_id,
ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id);
+ _dpu_rm_clear_mapping(global_state->dspp_to_enc_id,
+ ARRAY_SIZE(global_state->dspp_to_enc_id), enc->base.id);
}
int dpu_rm_reserve(
--
2.7.4
[Freedreno] [PATCH v3 0/4] Reserve DSPPs based on user request
This series will enable color features on sc7280 target which has primary panel as eDP The series removes DSPP allocation based on encoder type and allows the DSPP reservation based on user request via CTM. The series will release/reserve the dpu resources when ever there is a topology change to suit the new requirements. Kalyan Thota (4): drm/msm/dpu: clear DSPP reservations in rm release drm/msm/dpu: add DSPPs into reservation upon a CTM request drm/msm/dpu: avoid unnecessary check in DPU reservations drm/msm/dpu: reserve the resources on topology change drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 58 - drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 + 3 files changed, 37 insertions(+), 25 deletions(-) -- 2.7.4
[Freedreno] [PATCH v3 3/4] drm/msm/dpu: avoid unnecessary check in DPU reservations
Return immediately on failure, this will make dpu reservations
part look cleaner.
Signed-off-by: Kalyan Thota
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 46d2a5c..3920efd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -636,25 +636,22 @@ static int dpu_encoder_virt_atomic_check(
if (ret) {
DPU_ERROR_ENC(dpu_enc,
"mode unsupported, phys idx %d\n", i);
- break;
+ return ret;
}
}
topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode,
crtc_state);
- /* Reserve dynamic resources now. */
- if (!ret) {
- /*
-* Release and Allocate resources on every modeset
-* Dont allocate when active is false.
-*/
- if (drm_atomic_crtc_needs_modeset(crtc_state)) {
- dpu_rm_release(global_state, drm_enc);
+ /*
+* Release and Allocate resources on every modeset
+* Dont allocate when active is false.
+*/
+ if (drm_atomic_crtc_needs_modeset(crtc_state)) {
+ dpu_rm_release(global_state, drm_enc);
- if (!crtc_state->active_changed || crtc_state->active)
- ret = dpu_rm_reserve(_kms->rm, global_state,
- drm_enc, crtc_state, topology);
- }
+ if (!crtc_state->active_changed || crtc_state->active)
+ ret = dpu_rm_reserve(_kms->rm, global_state,
+ drm_enc, crtc_state, topology);
}
trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
--
2.7.4
[Freedreno] [PATCH v3 4/4] drm/msm/dpu: reserve the resources on topology change
Some features like CTM can be enabled dynamically. Release
and reserve the DPU resources whenever a topology change
occurs such that required hw blocks are allocated appropriately.
Signed-off-by: Kalyan Thota
---
Changes in v1:
- Avoid mode_set call directly (Dmitry)
Changes in v2:
- Minor nits (Dmitry)
Changes in v3:
- avoid unnecessary modeset check call (Dmitry)
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +++-
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 539b68b..85bd5645 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -204,6 +204,7 @@ struct dpu_crtc {
* @hw_ctls : List of active ctl paths
* @crc_source: CRC source
* @crc_frame_skip_count: Number of frames skipped before getting CRC
+ * @ctm_enabled : Cached color management enablement state
*/
struct dpu_crtc_state {
struct drm_crtc_state base;
@@ -225,6 +226,7 @@ struct dpu_crtc_state {
enum dpu_crtc_crc_source crc_source;
int crc_frame_skip_count;
+ bool ctm_enabled;
};
#define to_dpu_crtc_state(x) \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3920efd..038e077 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -217,6 +217,19 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+static bool _dpu_enc_is_dspp_changed(struct drm_crtc_state *crtc_state,
+ struct msm_display_topology topology)
+{
+ struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
+
+ if ((cstate->ctm_enabled && !topology.num_dspp) ||
+ (!cstate->ctm_enabled && topology.num_dspp)) {
+ crtc_state->mode_changed = true;
+ return true;
+ }
+
+ return false;
+}
bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
{
@@ -642,14 +655,15 @@ static int dpu_encoder_virt_atomic_check(
topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode,
crtc_state);
+ _dpu_enc_is_dspp_changed(crtc_state, topology);
+
/*
* Release and Allocate resources on every modeset
-* Dont allocate when active is false.
*/
if (drm_atomic_crtc_needs_modeset(crtc_state)) {
dpu_rm_release(global_state, drm_enc);
- if (!crtc_state->active_changed || crtc_state->active)
+ if (crtc_state->enable)
ret = dpu_rm_reserve(_kms->rm, global_state,
drm_enc, crtc_state, topology);
}
@@ -1022,7 +1036,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
- int num_lm, num_ctl, num_pp, num_dsc;
+ int num_lm, num_ctl, num_pp, num_dsc, num_dspp;
unsigned int dsc_mask = 0;
int i;
@@ -1053,7 +1067,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
drm_enc->base.id, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
num_lm = dpu_rm_get_assigned_resources(_kms->rm, global_state,
drm_enc->base.id, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
- dpu_rm_get_assigned_resources(_kms->rm, global_state,
+ num_dspp = dpu_rm_get_assigned_resources(_kms->rm, global_state,
drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
ARRAY_SIZE(hw_dspp));
@@ -1084,7 +1098,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
}
cstate->num_mixers = num_lm;
-
+ cstate->ctm_enabled = !!num_dspp;
dpu_enc->connector = conn_state->connector;
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
--
2.7.4
Re: [Freedreno] [PATCH v2 4/8] arm64: dts: qcom: sm8350: reorder device nodes
On 6.02.2023 15:57, Dmitry Baryshkov wrote:
> Start ordering DT nodes according to agreed order. Move apps SMMU, GIC,
> timer, apps RSC, cpufreq ADSP and cDSP nodes to the end to the proper
> position at the end of /soc/.
>
> Signed-off-by: Dmitry Baryshkov
> ---
Moving adjacent nodes is pretty unreviewable without applying
the patch and just checking out the result :/
Konrad
> arch/arm64/boot/dts/qcom/sm8350.dtsi | 1228 +-
> 1 file changed, 614 insertions(+), 614 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index 0de42a333d32..061aa3fec1c4 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -1423,111 +1423,6 @@ spi13: spi@a94000 {
> };
> };
>
> - apps_smmu: iommu@1500 {
> - compatible = "qcom,sm8350-smmu-500", "arm,mmu-500";
> - reg = <0 0x1500 0 0x10>;
> - #iommu-cells = <2>;
> - #global-interrupts = <2>;
> - interrupts =,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> -
Re: [Freedreno] [PATCH 2/3] arm64: dts: qcom: sc8280xp: Add GPU related nodes
On 8.02.2023 04:40, Bjorn Andersson wrote:
> From: Bjorn Andersson
>
> Add Adreno SMMU, GPU clock controller, GMU and GPU nodes for the
> SC8280XP.
>
> Signed-off-by: Bjorn Andersson
> Signed-off-by: Bjorn Andersson
> ---
> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 171 +
> 1 file changed, 171 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index fcd393444f47..94e8d0da9d7b 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
> @@ -2275,6 +2276,176 @@ tcsr: syscon@1fc {
> reg = <0x0 0x01fc 0x0 0x3>;
> };
>
> + gpu: gpu@3d0 {
> + compatible = "qcom,adreno-690.0", "qcom,adreno";
Did Qualcomm really pull off a chip this big with
patchlevel=0/first try? Nice.
> + 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-69000 {
> + opp-hz = /bits/ 64 <69000>;
> + opp-level =
> ;
> + opp-peak-kBps = <2736000>;
> + };
> + };
> + };
> +
> + gmu: gmu@3d6a000 {
> + compatible="qcom,adreno-gmu-690.0", "qcom,adreno-gmu";
This needs a binding update.
> + reg = <0 0x03d6a000 0 0x34000>,
> + <0 0x03de 0 0x1>,
> + <0 0x0b29 0 0x1>,
> + <0 0x0b49 0 0x1>;
> + reg-names = "gmu", "rscc", "gmu_pdc", "gmu_pdc_seq";
I think this should be a vertical list with so many entries.
> + interrupts = ,
> + ;
> + interrupt-names = "hfi", "gmu";
> + clocks = < GPU_CC_CX_GMU_CLK>,
> + < GPU_CC_CXO_CLK>,
> + < GCC_DDRSS_GPU_AXI_CLK>,
> + < GCC_GPU_MEMNOC_GFX_CLK>,
> + < GPU_CC_AHB_CLK>,
> + < GPU_CC_HUB_CX_INT_CLK>,
> + < GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK>,
> + <_qmp>;
> +
