[PATCH v2 08/14] drm/msm/dpu: update mixer number info earlier

Jun Nie posted 14 patches 1 month, 2 weeks ago
[PATCH v2 08/14] drm/msm/dpu: update mixer number info earlier
Posted by Jun Nie 1 month, 2 weeks ago
Update mixer number info earlier so that the plane nopipe check
can have the info to clip the plane. Otherwise, the first nonpipe
check will have mixer number as 0 and plane is not checked.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index dfe282c607933..68655c8817bf8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -638,6 +638,7 @@ static int dpu_encoder_virt_atomic_check(
 	struct dpu_global_state *global_state;
 	struct drm_framebuffer *fb;
 	struct drm_dsc_config *dsc;
+	struct dpu_crtc_state *cstate;
 	int ret = 0;
 
 	if (!drm_enc || !crtc_state || !conn_state) {
@@ -662,6 +663,8 @@ static int dpu_encoder_virt_atomic_check(
 	dsc = dpu_encoder_get_dsc_config(drm_enc);
 
 	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, dsc);
+	cstate = to_dpu_crtc_state(crtc_state);
+	cstate->num_mixers = topology.num_lm;
 
 	/*
 	 * Use CDM only for writeback or DP at the moment as other interfaces cannot handle it.
@@ -1170,7 +1173,13 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
 	}
 
 	cstate->num_dscs = num_dsc;
-	cstate->num_mixers = num_lm;
+	if (cstate->num_mixers != num_lm) {
+		if (!cstate->num_mixers)
+			DPU_ERROR_ENC(dpu_enc,
+				      "mixer number %d is not as expected %d\n",
+				      num_lm, cstate->num_mixers);
+		cstate->num_mixers = num_lm;
+	}
 	dpu_enc->connector = conn_state->connector;
 
 	/*

-- 
2.34.1
Re: [PATCH v2 08/14] drm/msm/dpu: update mixer number info earlier
Posted by Dmitry Baryshkov 1 month, 2 weeks ago
On Wed, Oct 09, 2024 at 04:50:21PM GMT, Jun Nie wrote:
> Update mixer number info earlier so that the plane nopipe check
> can have the info to clip the plane. Otherwise, the first nonpipe
> check will have mixer number as 0 and plane is not checked.
> 
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index dfe282c607933..68655c8817bf8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -638,6 +638,7 @@ static int dpu_encoder_virt_atomic_check(
>  	struct dpu_global_state *global_state;
>  	struct drm_framebuffer *fb;
>  	struct drm_dsc_config *dsc;
> +	struct dpu_crtc_state *cstate;
>  	int ret = 0;
>  
>  	if (!drm_enc || !crtc_state || !conn_state) {
> @@ -662,6 +663,8 @@ static int dpu_encoder_virt_atomic_check(
>  	dsc = dpu_encoder_get_dsc_config(drm_enc);
>  
>  	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, dsc);
> +	cstate = to_dpu_crtc_state(crtc_state);
> +	cstate->num_mixers = topology.num_lm;
>  
>  	/*
>  	 * Use CDM only for writeback or DP at the moment as other interfaces cannot handle it.
> @@ -1170,7 +1173,13 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>  	}
>  
>  	cstate->num_dscs = num_dsc;
> -	cstate->num_mixers = num_lm;
> +	if (cstate->num_mixers != num_lm) {
> +		if (!cstate->num_mixers)
> +			DPU_ERROR_ENC(dpu_enc,
> +				      "mixer number %d is not as expected %d\n",
> +				      num_lm, cstate->num_mixers);
> +		cstate->num_mixers = num_lm;
> +	}

Is it a possible case or just defensive coding?

>  	dpu_enc->connector = conn_state->connector;
>  
>  	/*
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry
Re: [PATCH v2 08/14] drm/msm/dpu: update mixer number info earlier
Posted by Jun Nie 1 month, 2 weeks ago
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:12写道:
>
> On Wed, Oct 09, 2024 at 04:50:21PM GMT, Jun Nie wrote:
> > Update mixer number info earlier so that the plane nopipe check
> > can have the info to clip the plane. Otherwise, the first nonpipe
> > check will have mixer number as 0 and plane is not checked.
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index dfe282c607933..68655c8817bf8 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -638,6 +638,7 @@ static int dpu_encoder_virt_atomic_check(
> >       struct dpu_global_state *global_state;
> >       struct drm_framebuffer *fb;
> >       struct drm_dsc_config *dsc;
> > +     struct dpu_crtc_state *cstate;
> >       int ret = 0;
> >
> >       if (!drm_enc || !crtc_state || !conn_state) {
> > @@ -662,6 +663,8 @@ static int dpu_encoder_virt_atomic_check(
> >       dsc = dpu_encoder_get_dsc_config(drm_enc);
> >
> >       topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, dsc);
> > +     cstate = to_dpu_crtc_state(crtc_state);
> > +     cstate->num_mixers = topology.num_lm;
> >
> >       /*
> >        * Use CDM only for writeback or DP at the moment as other interfaces cannot handle it.
> > @@ -1170,7 +1173,13 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> >       }
> >
> >       cstate->num_dscs = num_dsc;
> > -     cstate->num_mixers = num_lm;
> > +     if (cstate->num_mixers != num_lm) {
> > +             if (!cstate->num_mixers)
> > +                     DPU_ERROR_ENC(dpu_enc,
> > +                                   "mixer number %d is not as expected %d\n",
> > +                                   num_lm, cstate->num_mixers);
> > +             cstate->num_mixers = num_lm;
> > +     }
>
> Is it a possible case or just defensive coding?

The value is initialized beforehand only in virtual plane case. So we
still need this
for non virtual plane case.
>
> >       dpu_enc->connector = conn_state->connector;
> >
> >       /*
> >
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry
Re: [PATCH v2 08/14] drm/msm/dpu: update mixer number info earlier
Posted by Dmitry Baryshkov 1 month, 2 weeks ago
On Fri, 11 Oct 2024 at 09:30, Jun Nie <jun.nie@linaro.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:12写道:
> >
> > On Wed, Oct 09, 2024 at 04:50:21PM GMT, Jun Nie wrote:
> > > Update mixer number info earlier so that the plane nopipe check
> > > can have the info to clip the plane. Otherwise, the first nonpipe
> > > check will have mixer number as 0 and plane is not checked.
> > >
> > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > ---
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 11 ++++++++++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > index dfe282c607933..68655c8817bf8 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > @@ -638,6 +638,7 @@ static int dpu_encoder_virt_atomic_check(
> > >       struct dpu_global_state *global_state;
> > >       struct drm_framebuffer *fb;
> > >       struct drm_dsc_config *dsc;
> > > +     struct dpu_crtc_state *cstate;
> > >       int ret = 0;
> > >
> > >       if (!drm_enc || !crtc_state || !conn_state) {
> > > @@ -662,6 +663,8 @@ static int dpu_encoder_virt_atomic_check(
> > >       dsc = dpu_encoder_get_dsc_config(drm_enc);
> > >
> > >       topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, dsc);
> > > +     cstate = to_dpu_crtc_state(crtc_state);
> > > +     cstate->num_mixers = topology.num_lm;
> > >
> > >       /*
> > >        * Use CDM only for writeback or DP at the moment as other interfaces cannot handle it.
> > > @@ -1170,7 +1173,13 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> > >       }
> > >
> > >       cstate->num_dscs = num_dsc;
> > > -     cstate->num_mixers = num_lm;
> > > +     if (cstate->num_mixers != num_lm) {
> > > +             if (!cstate->num_mixers)
> > > +                     DPU_ERROR_ENC(dpu_enc,
> > > +                                   "mixer number %d is not as expected %d\n",
> > > +                                   num_lm, cstate->num_mixers);
> > > +             cstate->num_mixers = num_lm;
> > > +     }
> >
> > Is it a possible case or just defensive coding?
>
> The value is initialized beforehand only in virtual plane case. So we
> still need this
> for non virtual plane case.

It looks better if it's handled in the non-virtual code instead.

-- 
With best wishes
Dmitry