* [PATCH 0/3] Reserve dspps based on user request
@ 2023-01-30 15:21 Kalyan Thota
2023-01-30 15:21 ` [v1 1/3] drm/msm/disp/dpu1: clear dspp reservations in rm release Kalyan Thota
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Kalyan Thota @ 2023-01-30 15:21 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: Kalyan Thota, robdclark, dianders, quic_abhinavk, linux-kernel,
swboyd, dmitry.baryshkov, marijn.suijten, quic_vpolimer
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 (3):
drm/msm/disp/dpu1: clear dspp reservations in rm release
drm/msm/disp/dpu1: add dspps into reservation if there is a ctm
request
drm/msm/disp/dpu1: reserve the resources on topology change
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 54 +++++++++++++++++++++++------
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 6 ++--
5 files changed, 50 insertions(+), 17 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 18+ messages in thread* [v1 1/3] drm/msm/disp/dpu1: clear dspp reservations in rm release 2023-01-30 15:21 [PATCH 0/3] Reserve dspps based on user request Kalyan Thota @ 2023-01-30 15:21 ` Kalyan Thota 2023-02-01 11:10 ` Marijn Suijten 2023-01-30 15:21 ` [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request Kalyan Thota ` (2 subsequent siblings) 3 siblings, 1 reply; 18+ messages in thread From: Kalyan Thota @ 2023-01-30 15:21 UTC (permalink / raw) To: dri-devel, linux-arm-msm, freedreno, devicetree Cc: Kalyan Thota, robdclark, dianders, quic_abhinavk, linux-kernel, swboyd, dmitry.baryshkov, marijn.suijten, quic_vpolimer Clear dspp reservations from the global state during rm release Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [v1 1/3] drm/msm/disp/dpu1: clear dspp reservations in rm release 2023-01-30 15:21 ` [v1 1/3] drm/msm/disp/dpu1: clear dspp reservations in rm release Kalyan Thota @ 2023-02-01 11:10 ` Marijn Suijten 2023-02-01 11:21 ` Marijn Suijten 0 siblings, 1 reply; 18+ messages in thread From: Marijn Suijten @ 2023-02-01 11:10 UTC (permalink / raw) To: Kalyan Thota Cc: robdclark, devicetree, quic_abhinavk, linux-arm-msm, linux-kernel, dri-devel, dianders, dmitry.baryshkov, swboyd, freedreno, quic_vpolimer On 2023-01-30 07:21:30, Kalyan Thota wrote: > Clear dspp reservations from the global state during > rm release DSPP, and a period at the end of a sentence. Also noticing inconsistent linebreaks across these patches, stick to 72 chars. > Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> Should this be considered a fix to be backported, or is it harmless? If so: Fixes: e47616df008b ("drm/msm/dpu: add support for color processing blocks in dpu driver") > --- > 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 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [v1 1/3] drm/msm/disp/dpu1: clear dspp reservations in rm release 2023-02-01 11:10 ` Marijn Suijten @ 2023-02-01 11:21 ` Marijn Suijten 0 siblings, 0 replies; 18+ messages in thread From: Marijn Suijten @ 2023-02-01 11:21 UTC (permalink / raw) To: Kalyan Thota Cc: robdclark, devicetree, quic_abhinavk, linux-arm-msm, linux-kernel, dri-devel, dianders, dmitry.baryshkov, swboyd, freedreno, quic_vpolimer On 2023-02-01 12:10:33, Marijn Suijten wrote: > On 2023-01-30 07:21:30, Kalyan Thota wrote: > > Clear dspp reservations from the global state during > > rm release > > DSPP, and a period at the end of a sentence. Also noticing inconsistent > linebreaks across these patches, stick to 72 chars. > > > Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > > Should this be considered a fix to be backported, or is it harmless? If > so: > > Fixes: e47616df008b ("drm/msm/dpu: add support for color processing blocks in dpu driver") Right, it should, Dmitry also requested this in v1. - Marijn ^ permalink raw reply [flat|nested] 18+ messages in thread
* [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request 2023-01-30 15:21 [PATCH 0/3] Reserve dspps based on user request Kalyan Thota 2023-01-30 15:21 ` [v1 1/3] drm/msm/disp/dpu1: clear dspp reservations in rm release Kalyan Thota @ 2023-01-30 15:21 ` Kalyan Thota 2023-01-30 18:36 ` Dmitry Baryshkov 2023-02-01 11:16 ` Marijn Suijten 2023-01-30 15:21 ` [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change Kalyan Thota 2023-01-30 19:18 ` [PATCH 0/3] Reserve dspps based on user request Dmitry Baryshkov 3 siblings, 2 replies; 18+ messages in thread From: Kalyan Thota @ 2023-01-30 15:21 UTC (permalink / raw) To: dri-devel, linux-arm-msm, freedreno, devicetree Cc: Kalyan Thota, robdclark, dianders, quic_abhinavk, linux-kernel, swboyd, dmitry.baryshkov, marijn.suijten, quic_vpolimer Add dspp blocks into the topology for reservation, if there is a ctm request for that composition. Changes in v1: - Minor nits (Dmitry) Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 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..3bd46b4 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; @@ -573,11 +574,9 @@ 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 (dpu_kms->catalog->dspp && + crtc_state->ctm && (dpu_kms->catalog->dspp_count >= topology.num_lm)) + topology.num_dspp = topology.num_lm; topology.num_enc = 0; topology.num_intf = intf_count; @@ -643,7 +642,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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request 2023-01-30 15:21 ` [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request Kalyan Thota @ 2023-01-30 18:36 ` Dmitry Baryshkov 2023-02-01 11:16 ` Marijn Suijten 1 sibling, 0 replies; 18+ messages in thread From: Dmitry Baryshkov @ 2023-01-30 18:36 UTC (permalink / raw) To: Kalyan Thota, dri-devel, linux-arm-msm, freedreno, devicetree Cc: robdclark, dianders, quic_abhinavk, linux-kernel, quic_vpolimer, marijn.suijten, swboyd On 30/01/2023 17:21, Kalyan Thota wrote: > Add dspp blocks into the topology for reservation, if there is a ctm > request for that composition. > > Changes in v1: > - Minor nits (Dmitry) > > Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 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..3bd46b4 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; > @@ -573,11 +574,9 @@ 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 (dpu_kms->catalog->dspp && > + crtc_state->ctm && (dpu_kms->catalog->dspp_count >= topology.num_lm)) This condition doesn't look correct anymore for the following reasons: - if there are no DSPPs we will completely ignore the ctm property - if there are not enough DSPPs, the CTM property will be ignore I think, this should be just: if (crtc_state->ctm) topology.num_dspp = topology.num_lm; > + topology.num_dspp = topology.num_lm; > > topology.num_enc = 0; > topology.num_intf = intf_count; > @@ -643,7 +642,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) { -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request 2023-01-30 15:21 ` [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request Kalyan Thota 2023-01-30 18:36 ` Dmitry Baryshkov @ 2023-02-01 11:16 ` Marijn Suijten 2023-02-01 11:26 ` Marijn Suijten 2023-02-01 13:48 ` Dmitry Baryshkov 1 sibling, 2 replies; 18+ messages in thread From: Marijn Suijten @ 2023-02-01 11:16 UTC (permalink / raw) To: Kalyan Thota Cc: robdclark, devicetree, quic_abhinavk, linux-arm-msm, linux-kernel, dri-devel, dianders, dmitry.baryshkov, swboyd, freedreno, quic_vpolimer On 2023-01-30 07:21:31, Kalyan Thota wrote: > Add dspp blocks into the topology for reservation, if there is a ctm > request for that composition. DSPP > Changes in v1: > - Minor nits (Dmitry) This should go below the triple dashes, so that it /does not/ become part of the patch/commit that is applied to the tree (where review history is irrelevant as it can be searched for separately). > Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 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..3bd46b4 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; > @@ -573,11 +574,9 @@ 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 (dpu_kms->catalog->dspp && > + crtc_state->ctm && (dpu_kms->catalog->dspp_count >= topology.num_lm)) Multiline-if-clause is typically indented with two tabs, not a half tab (4 spaces). Nit: swap the && here? dspp and dspp_count are related, so check ctm first or last but not in the middle - makes reading easier. > + topology.num_dspp = topology.num_lm; > > topology.num_enc = 0; > topology.num_intf = intf_count; > @@ -643,7 +642,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 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request 2023-02-01 11:16 ` Marijn Suijten @ 2023-02-01 11:26 ` Marijn Suijten 2023-02-01 13:49 ` Dmitry Baryshkov 2023-02-01 13:48 ` Dmitry Baryshkov 1 sibling, 1 reply; 18+ messages in thread From: Marijn Suijten @ 2023-02-01 11:26 UTC (permalink / raw) To: Kalyan Thota Cc: robdclark, devicetree, quic_abhinavk, linux-arm-msm, linux-kernel, dri-devel, dianders, dmitry.baryshkov, swboyd, freedreno, quic_vpolimer On 2023-02-01 12:16:05, Marijn Suijten wrote: <snip> > > + if (dpu_kms->catalog->dspp && > > + crtc_state->ctm && (dpu_kms->catalog->dspp_count >= topology.num_lm)) > > Multiline-if-clause is typically indented with two tabs, not a half tab > (4 spaces). Hmm, Dmitry requested indent-to-opening-parenthesis in v1 instead; and the majority of dpu1 uses the worst version of all: indent with a single tab so that the contents line up with the code block below. Dmitry, I'll leave final say to you (and fix it up in my own DPU series accordingly too). - Marijn ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request 2023-02-01 11:26 ` Marijn Suijten @ 2023-02-01 13:49 ` Dmitry Baryshkov 0 siblings, 0 replies; 18+ messages in thread From: Dmitry Baryshkov @ 2023-02-01 13:49 UTC (permalink / raw) To: Marijn Suijten, Kalyan Thota Cc: robdclark, devicetree, quic_abhinavk, linux-arm-msm, linux-kernel, dri-devel, dianders, swboyd, freedreno, quic_vpolimer On 01/02/2023 13:26, Marijn Suijten wrote: > On 2023-02-01 12:16:05, Marijn Suijten wrote: > <snip> >>> + if (dpu_kms->catalog->dspp && >>> + crtc_state->ctm && (dpu_kms->catalog->dspp_count >= topology.num_lm)) >> >> Multiline-if-clause is typically indented with two tabs, not a half tab >> (4 spaces). > > Hmm, Dmitry requested indent-to-opening-parenthesis in v1 instead; and > the majority of dpu1 uses the worst version of all: indent with a single > tab so that the contents line up with the code block below. Dmitry, > I'll leave final say to you (and fix it up in my own DPU series > accordingly too). Well, :set cino=(0 > > - Marijn -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request 2023-02-01 11:16 ` Marijn Suijten 2023-02-01 11:26 ` Marijn Suijten @ 2023-02-01 13:48 ` Dmitry Baryshkov 2023-02-01 15:10 ` Marijn Suijten 1 sibling, 1 reply; 18+ messages in thread From: Dmitry Baryshkov @ 2023-02-01 13:48 UTC (permalink / raw) To: Marijn Suijten, Kalyan Thota Cc: robdclark, devicetree, quic_abhinavk, linux-arm-msm, linux-kernel, dri-devel, dianders, swboyd, freedreno, quic_vpolimer On 01/02/2023 13:16, Marijn Suijten wrote: > On 2023-01-30 07:21:31, Kalyan Thota wrote: >> Add dspp blocks into the topology for reservation, if there is a ctm >> request for that composition. > > DSPP > >> Changes in v1: >> - Minor nits (Dmitry) > > This should go below the triple dashes, so that it /does not/ become > part of the patch/commit that is applied to the tree (where review > history is irrelevant as it can be searched for separately). This is one of DRM peculiarities which we have to live with. > >> Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++------- >> 1 file changed, 6 insertions(+), 7 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..3bd46b4 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; >> @@ -573,11 +574,9 @@ 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 (dpu_kms->catalog->dspp && >> + crtc_state->ctm && (dpu_kms->catalog->dspp_count >= topology.num_lm)) > > Multiline-if-clause is typically indented with two tabs, not a half tab > (4 spaces). I tend to disagree here. Lately I have mostly seen it being indented to the opening parenthesis, so that nested statements also indent nicely. > Nit: swap the && here? dspp and dspp_count are related, so check ctm > first or last but not in the middle - makes reading easier. I think we can ignore dpu_kms->catalog->dspp completely. checking dspp_count should be enough for the purpose of the check (and note, the check for dspp/dspp_count is misleading and should be omitted). > >> + topology.num_dspp = topology.num_lm; >> >> topology.num_enc = 0; >> topology.num_intf = intf_count; >> @@ -643,7 +642,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 >> -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request 2023-02-01 13:48 ` Dmitry Baryshkov @ 2023-02-01 15:10 ` Marijn Suijten 0 siblings, 0 replies; 18+ messages in thread From: Marijn Suijten @ 2023-02-01 15:10 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Kalyan Thota, devicetree, quic_abhinavk, linux-arm-msm, swboyd, linux-kernel, dri-devel, dianders, robdclark, freedreno, quic_vpolimer On 2023-02-01 15:48:02, Dmitry Baryshkov wrote: > On 01/02/2023 13:16, Marijn Suijten wrote: > > On 2023-01-30 07:21:31, Kalyan Thota wrote: > >> Add dspp blocks into the topology for reservation, if there is a ctm > >> request for that composition. > > > > DSPP > > > >> Changes in v1: > >> - Minor nits (Dmitry) > > > > This should go below the triple dashes, so that it /does not/ become > > part of the patch/commit that is applied to the tree (where review > > history is irrelevant as it can be searched for separately). > > This is one of DRM peculiarities which we have to live with. Not sure I follow. Keeping "changes since vXX" out of commit messages seems to be a kernel-wide convention, after all the title doesn't include which revision of the patch ended up being applied to the tree either. Having the changelog checked in to the tree has no relevance. > >> Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> > >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 ++++++------- > >> 1 file changed, 6 insertions(+), 7 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..3bd46b4 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; > >> @@ -573,11 +574,9 @@ 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 (dpu_kms->catalog->dspp && > >> + crtc_state->ctm && (dpu_kms->catalog->dspp_count >= topology.num_lm)) > > > > Multiline-if-clause is typically indented with two tabs, not a half tab > > (4 spaces). > > I tend to disagree here. Lately I have mostly seen it being indented to > the opening parenthesis, so that nested statements also indent nicely. Ack, hence double-checked in a followup message; there's no concistency in dpu1 now but I agree that for ts=8 a 4-space-indented wraparound neatly aligns with the expression on the first line /and/ prevents inadvertently aligning with the conditional body on the next line. Will fix up in my own series too, thanks! > > Nit: swap the && here? dspp and dspp_count are related, so check ctm > > first or last but not in the middle - makes reading easier. > > I think we can ignore dpu_kms->catalog->dspp completely. checking > dspp_count should be enough for the purpose of the check (and note, the > check for dspp/dspp_count is misleading and should be omitted). Ack, thanks! - Marijn ^ permalink raw reply [flat|nested] 18+ messages in thread
* [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change 2023-01-30 15:21 [PATCH 0/3] Reserve dspps based on user request Kalyan Thota 2023-01-30 15:21 ` [v1 1/3] drm/msm/disp/dpu1: clear dspp reservations in rm release Kalyan Thota 2023-01-30 15:21 ` [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request Kalyan Thota @ 2023-01-30 15:21 ` Kalyan Thota 2023-01-30 19:17 ` Dmitry Baryshkov ` (2 more replies) 2023-01-30 19:18 ` [PATCH 0/3] Reserve dspps based on user request Dmitry Baryshkov 3 siblings, 3 replies; 18+ messages in thread From: Kalyan Thota @ 2023-01-30 15:21 UTC (permalink / raw) To: dri-devel, linux-arm-msm, freedreno, devicetree Cc: Kalyan Thota, robdclark, dianders, quic_abhinavk, linux-kernel, swboyd, dmitry.baryshkov, marijn.suijten, quic_vpolimer 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. Changes in v1: - Avoid mode_set call directly instead change the mode_changed (Dmitry) Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 42 ++++++++++++++++++----------- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++- 3 files changed, 32 insertions(+), 16 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..58e8c72 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 ctm reservation 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 3bd46b4..0ddf2c9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -217,6 +217,22 @@ 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_dspps_changed(struct drm_crtc_state *crtc_state, + struct msm_display_topology topology) +{ + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state); + + if (drm_atomic_crtc_needs_modeset(crtc_state)) + return true; + + 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) { @@ -638,25 +654,21 @@ 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); + _dpu_enc_is_dspps_changed(crtc_state, topology); + /* 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); + 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(&dpu_kms->rm, global_state, - drm_enc, crtc_state, topology); - } + if (crtc_state->enable) + ret = dpu_rm_reserve(&dpu_kms->rm, global_state, + drm_enc, crtc_state, topology); } trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags); @@ -1027,7 +1039,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; @@ -1058,7 +1070,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(&dpu_kms->rm, global_state, drm_enc->base.id, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm)); - dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, + num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp, ARRAY_SIZE(hw_dspp)); @@ -1089,7 +1101,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++) { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index 9e7236e..4cbe20c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -150,8 +150,10 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, * dpu_encoder_prepare_commit - prepare encoder at the very beginning of an * atomic commit, before any registers are written * @drm_enc: Pointer to previously created drm encoder structure + * @crtc_state: Pointer to drm crtc state */ -void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc); +void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc, + struct drm_crtc_state *crtc_state); /** * dpu_encoder_set_idle_timeout - set the idle timeout for video -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change 2023-01-30 15:21 ` [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change Kalyan Thota @ 2023-01-30 19:17 ` Dmitry Baryshkov 2023-02-02 4:22 ` kernel test robot 2023-02-02 14:08 ` kernel test robot 2 siblings, 0 replies; 18+ messages in thread From: Dmitry Baryshkov @ 2023-01-30 19:17 UTC (permalink / raw) To: Kalyan Thota, dri-devel, linux-arm-msm, freedreno, devicetree Cc: robdclark, dianders, quic_abhinavk, linux-kernel, quic_vpolimer, marijn.suijten, swboyd On 30/01/2023 17:21, 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. > > Changes in v1: > - Avoid mode_set call directly instead change the mode_changed (Dmitry) Thanks, I like the overall idea. Minor nits below. Also, could you please fix your scripts to include the PATCH keyword into the subject? If you do `git format-patches -# -v#`, where # is the number of commits to include and the version of the patchset, it will do the trick on its own. > > Signed-off-by: Kalyan Thota <quic_kalyant@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 42 ++++++++++++++++++----------- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++- > 3 files changed, 32 insertions(+), 16 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..58e8c72 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 ctm reservation state Nit: we do not reserve CTMs. We reserve DSPPs. So, ctm_enabled is '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 3bd46b4..0ddf2c9 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -217,6 +217,22 @@ 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_dspps_changed(struct drm_crtc_state *crtc_state, > + struct msm_display_topology topology) > +{ > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state); > + > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > + return true; > + > + 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) > { > @@ -638,25 +654,21 @@ static int dpu_encoder_virt_atomic_check( > if (ret) { > DPU_ERROR_ENC(dpu_enc, > "mode unsupported, phys idx %d\n", i); > - break; > + return ret; This deserves a separate commit with the proper Fixes: tag. > } > } > > topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state); > > + _dpu_enc_is_dspps_changed(crtc_state, topology); > + > /* 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); > + 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(&dpu_kms->rm, global_state, > - drm_enc, crtc_state, topology); > - } > + if (crtc_state->enable) > + ret = dpu_rm_reserve(&dpu_kms->rm, global_state, > + drm_enc, crtc_state, topology); > } > > trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags); > @@ -1027,7 +1039,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; > > @@ -1058,7 +1070,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(&dpu_kms->rm, global_state, > drm_enc->base.id, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm)); > - dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, > + num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state, > drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp, > ARRAY_SIZE(hw_dspp)); > > @@ -1089,7 +1101,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++) { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > index 9e7236e..4cbe20c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > @@ -150,8 +150,10 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, > * dpu_encoder_prepare_commit - prepare encoder at the very beginning of an > * atomic commit, before any registers are written > * @drm_enc: Pointer to previously created drm encoder structure > + * @crtc_state: Pointer to drm crtc state > */ > -void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc); > +void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc, > + struct drm_crtc_state *crtc_state); I think this change on it's own breaks compilation. Please drop it. > > /** > * dpu_encoder_set_idle_timeout - set the idle timeout for video -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change 2023-01-30 15:21 ` [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change Kalyan Thota 2023-01-30 19:17 ` Dmitry Baryshkov @ 2023-02-02 4:22 ` kernel test robot 2023-02-02 14:08 ` kernel test robot 2 siblings, 0 replies; 18+ messages in thread From: kernel test robot @ 2023-02-02 4:22 UTC (permalink / raw) To: Kalyan Thota, dri-devel, linux-arm-msm, freedreno, devicetree Cc: Kalyan Thota, robdclark, quic_abhinavk, llvm, linux-kernel, dianders, oe-kbuild-all, dmitry.baryshkov, marijn.suijten, swboyd, quic_vpolimer Hi Kalyan, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.2-rc6 next-20230201] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Kalyan-Thota/drm-msm-disp-dpu1-clear-dspp-reservations-in-rm-release/20230130-232224 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/1675092092-26412-4-git-send-email-quic_kalyant%40quicinc.com patch subject: [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change config: riscv-randconfig-r042-20230130 (https://download.01.org/0day-ci/archive/20230202/202302021238.o9yx7MKs-lkp@intel.com/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install riscv cross compiling tool for clang build # apt-get install binutils-riscv64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/4c49c3233fc18f3b746a96b5ff4ce5008da3bfec git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Kalyan-Thota/drm-msm-disp-dpu1-clear-dspp-reservations-in-rm-release/20230130-232224 git checkout 4c49c3233fc18f3b746a96b5ff4ce5008da3bfec # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpu/drm/msm/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:2091:6: error: conflicting types for 'dpu_encoder_prepare_commit' void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc) ^ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h:155:6: note: previous declaration is here void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc, ^ 1 error generated. -- >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c:443:38: error: too few arguments to function call, expected 2, have 1 dpu_encoder_prepare_commit(encoder); ~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h:155:6: note: 'dpu_encoder_prepare_commit' declared here void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc, ^ 1 error generated. vim +/dpu_encoder_prepare_commit +2091 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c ae4d721ce10057a Abhinav Kumar 2022-04-26 2090 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 @2091 void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc) 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2092 { 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2093 struct dpu_encoder_virt *dpu_enc; 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2094 struct dpu_encoder_phys *phys; 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2095 int i; 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2096 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2097 if (!drm_enc) { 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2098 DPU_ERROR("invalid encoder\n"); 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2099 return; 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2100 } 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2101 dpu_enc = to_dpu_encoder_virt(drm_enc); 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2102 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2103 for (i = 0; i < dpu_enc->num_phys_encs; i++) { 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2104 phys = dpu_enc->phys_encs[i]; b6fadcade627040 Drew Davenport 2019-12-06 2105 if (phys->ops.prepare_commit) 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2106 phys->ops.prepare_commit(phys); 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2107 } 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2108 } 25fdd5933e4c0f5 Jeykumar Sankaran 2018-06-27 2109 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change 2023-01-30 15:21 ` [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change Kalyan Thota 2023-01-30 19:17 ` Dmitry Baryshkov 2023-02-02 4:22 ` kernel test robot @ 2023-02-02 14:08 ` kernel test robot 2 siblings, 0 replies; 18+ messages in thread From: kernel test robot @ 2023-02-02 14:08 UTC (permalink / raw) To: Kalyan Thota, dri-devel, linux-arm-msm, freedreno, devicetree Cc: Kalyan Thota, robdclark, quic_abhinavk, linux-kernel, dianders, swboyd, oe-kbuild-all, dmitry.baryshkov, marijn.suijten, quic_vpolimer Hi Kalyan, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.2-rc6] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Kalyan-Thota/drm-msm-disp-dpu1-clear-dspp-reservations-in-rm-release/20230130-232224 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/1675092092-26412-4-git-send-email-quic_kalyant%40quicinc.com patch subject: [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change config: arm64-randconfig-r034-20230129 (https://download.01.org/0day-ci/archive/20230202/202302022254.37XyfGnR-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/4c49c3233fc18f3b746a96b5ff4ce5008da3bfec git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Kalyan-Thota/drm-msm-disp-dpu1-clear-dspp-reservations-in-rm-release/20230130-232224 git checkout 4c49c3233fc18f3b746a96b5ff4ce5008da3bfec # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/gpu/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:2091:6: error: conflicting types for 'dpu_encoder_prepare_commit'; have 'void(struct drm_encoder *)' 2091 | void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h:19, from drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:29: drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h:155:6: note: previous declaration of 'dpu_encoder_prepare_commit' with type 'void(struct drm_encoder *, struct drm_crtc_state *)' 155 | void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc, | ^~~~~~~~~~~~~~~~~~~~~~~~~~ -- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c: In function 'dpu_kms_prepare_commit': >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c:443:25: error: too few arguments to function 'dpu_encoder_prepare_commit' 443 | dpu_encoder_prepare_commit(encoder); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c:30: drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h:155:6: note: declared here 155 | void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc, | ^~~~~~~~~~~~~~~~~~~~~~~~~~ vim +2091 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c ae4d721ce10057 Abhinav Kumar 2022-04-26 2090 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 @2091 void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc) 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2092 { 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2093 struct dpu_encoder_virt *dpu_enc; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2094 struct dpu_encoder_phys *phys; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2095 int i; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2096 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2097 if (!drm_enc) { 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2098 DPU_ERROR("invalid encoder\n"); 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2099 return; 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2100 } 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2101 dpu_enc = to_dpu_encoder_virt(drm_enc); 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2102 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2103 for (i = 0; i < dpu_enc->num_phys_encs; i++) { 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2104 phys = dpu_enc->phys_encs[i]; b6fadcade62704 Drew Davenport 2019-12-06 2105 if (phys->ops.prepare_commit) 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2106 phys->ops.prepare_commit(phys); 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2107 } 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2108 } 25fdd5933e4c0f Jeykumar Sankaran 2018-06-27 2109 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] Reserve dspps based on user request 2023-01-30 15:21 [PATCH 0/3] Reserve dspps based on user request Kalyan Thota ` (2 preceding siblings ...) 2023-01-30 15:21 ` [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change Kalyan Thota @ 2023-01-30 19:18 ` Dmitry Baryshkov 2023-02-01 11:01 ` Marijn Suijten 3 siblings, 1 reply; 18+ messages in thread From: Dmitry Baryshkov @ 2023-01-30 19:18 UTC (permalink / raw) To: Kalyan Thota, dri-devel, linux-arm-msm, freedreno, devicetree Cc: robdclark, dianders, quic_abhinavk, linux-kernel, quic_vpolimer, marijn.suijten, swboyd On 30/01/2023 17:21, 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. Nit: the subject of the cover letter should include the version, if you are including one into the individual patches Subject. > > Kalyan Thota (3): > drm/msm/disp/dpu1: clear dspp reservations in rm release > drm/msm/disp/dpu1: add dspps into reservation if there is a ctm > request > drm/msm/disp/dpu1: reserve the resources on topology change > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 54 +++++++++++++++++++++++------ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 6 ++-- > 5 files changed, 50 insertions(+), 17 deletions(-) > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] Reserve dspps based on user request 2023-01-30 19:18 ` [PATCH 0/3] Reserve dspps based on user request Dmitry Baryshkov @ 2023-02-01 11:01 ` Marijn Suijten 0 siblings, 0 replies; 18+ messages in thread From: Marijn Suijten @ 2023-02-01 11:01 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Kalyan Thota, devicetree, quic_abhinavk, linux-arm-msm, swboyd, linux-kernel, dri-devel, dianders, robdclark, freedreno, quic_vpolimer On 2023-01-30 21:18:30, Dmitry Baryshkov wrote: > On 30/01/2023 17:21, 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. > > Nit: the subject of the cover letter should include the version, if you > are including one into the individual patches Subject. Indeed this makes it hard to tell the versions apart, and lore also confusingly bundles both series in "loose matches on Subject: below". Nit ^2: and individual patches should still have the PATCH moniker, i.e. [PATCH v2 1/3]. git format-patch -v2 --cover-letter ... takes care of /all this/ this for you. And one more: as DSPP is an abbreviation, can we capitalize it? So DSPP / DSPPs in these titles? > > > > Kalyan Thota (3): > > drm/msm/disp/dpu1: clear dspp reservations in rm release > > drm/msm/disp/dpu1: add dspps into reservation if there is a ctm > > request > > drm/msm/disp/dpu1: reserve the resources on topology change We just discussed in the DSC series that the subsystem prefix is drm/msm/dpu. - Marijn > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 1 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 54 +++++++++++++++++++++++------ > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++- > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 6 ++-- > > 5 files changed, 50 insertions(+), 17 deletions(-) > > > > -- > With best wishes > Dmitry > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/3] Reserve dspps based on user request
@ 2023-01-27 10:15 Kalyan Thota
0 siblings, 0 replies; 18+ messages in thread
From: Kalyan Thota @ 2023-01-27 10:15 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: Kalyan Thota, robdclark, dianders, quic_abhinavk, linux-kernel,
swboyd, dmitry.baryshkov, marijn.suijten, quic_vpolimer
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 (3):
drm/msm/disp/dpu1: clear dspp reservations in rm release
drm/msm/disp/dpu1: add dspps into reservation if there is a ctm
request
drm/msm/disp/dpu1: reserve the resources on topology change
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 54 +++++++++++++++++++++++------
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 6 ++--
5 files changed, 50 insertions(+), 17 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 18+ messages in threadend of thread, other threads:[~2023-02-02 14:09 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-30 15:21 [PATCH 0/3] Reserve dspps based on user request Kalyan Thota 2023-01-30 15:21 ` [v1 1/3] drm/msm/disp/dpu1: clear dspp reservations in rm release Kalyan Thota 2023-02-01 11:10 ` Marijn Suijten 2023-02-01 11:21 ` Marijn Suijten 2023-01-30 15:21 ` [v1 2/3] drm/msm/disp/dpu1: add dspps into reservation if there is a ctm request Kalyan Thota 2023-01-30 18:36 ` Dmitry Baryshkov 2023-02-01 11:16 ` Marijn Suijten 2023-02-01 11:26 ` Marijn Suijten 2023-02-01 13:49 ` Dmitry Baryshkov 2023-02-01 13:48 ` Dmitry Baryshkov 2023-02-01 15:10 ` Marijn Suijten 2023-01-30 15:21 ` [v1 3/3] drm/msm/disp/dpu1: reserve the resources on topology change Kalyan Thota 2023-01-30 19:17 ` Dmitry Baryshkov 2023-02-02 4:22 ` kernel test robot 2023-02-02 14:08 ` kernel test robot 2023-01-30 19:18 ` [PATCH 0/3] Reserve dspps based on user request Dmitry Baryshkov 2023-02-01 11:01 ` Marijn Suijten -- strict thread matches above, loose matches on Subject: below -- 2023-01-27 10:15 Kalyan Thota
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).