| Message ID | 20230419-dpu-tweaks-v1-2-d1bac46db075@freebox.fr (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm/dpu: tweaks for better hardware resources allocation | expand |
On 19/04/2023 17:41, Arnaud Vrac wrote: > This avoids using two LMs instead of one when the display width is lower > than the maximum supported value. For example on MSM8996/MSM8998, the > actual maxwidth is 2560, so we would use two LMs for 1280x720 or > 1920x1080 resolutions, while one is enough. > > Signed-off-by: Arnaud Vrac <avrac@freebox.fr> While this looks correct (and following what we have in 4.4), later vendor kernels specify the topology explicitly. Probably we should check this with the hw guys, because it might be the following case: even though a single LM can supply the mode, it will spend more power compared to two LMs. > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 1dc5dbe585723..dd2914726c4f6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -53,8 +53,6 @@ > > #define IDLE_SHORT_TIMEOUT 1 > > -#define MAX_HDISPLAY_SPLIT 1080 > - > /* timeout in frames waiting for frame done */ > #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5 > > @@ -568,10 +566,12 @@ static struct msm_display_topology dpu_encoder_get_topology( > */ > if (intf_count == 2) > topology.num_lm = 2; > - else if (!dpu_kms->catalog->caps->has_3d_merge) > - topology.num_lm = 1; > + else if (dpu_kms->catalog->caps->has_3d_merge && > + dpu_kms->catalog->mixer_count > 0 && > + mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth) > + topology.num_lm = 2; > else > - topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1; > + topology.num_lm = 1; > > if (crtc_state->ctm) > topology.num_dspp = topology.num_lm; >
On 4/19/2023 3:23 PM, Dmitry Baryshkov wrote: > On 19/04/2023 17:41, Arnaud Vrac wrote: >> This avoids using two LMs instead of one when the display width is lower >> than the maximum supported value. For example on MSM8996/MSM8998, the >> actual maxwidth is 2560, so we would use two LMs for 1280x720 or >> 1920x1080 resolutions, while one is enough. >> >> Signed-off-by: Arnaud Vrac <avrac@freebox.fr> > > While this looks correct (and following what we have in 4.4), later > vendor kernels specify the topology explicitly. Probably we should check > this with the hw guys, because it might be the following case: even > though a single LM can supply the mode, it will spend more power > compared to two LMs. > > Yes. 2 LM split will allow the HW to run in lower mdp core clock. Can you maintain the split_threshold in the hw catalog until per mode topology is available? Jeykumar S >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> index 1dc5dbe585723..dd2914726c4f6 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -53,8 +53,6 @@ >> #define IDLE_SHORT_TIMEOUT 1 >> -#define MAX_HDISPLAY_SPLIT 1080 >> - >> /* timeout in frames waiting for frame done */ >> #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5 >> @@ -568,10 +566,12 @@ static struct msm_display_topology >> dpu_encoder_get_topology( >> */ >> if (intf_count == 2) >> topology.num_lm = 2; >> - else if (!dpu_kms->catalog->caps->has_3d_merge) >> - topology.num_lm = 1; >> + else if (dpu_kms->catalog->caps->has_3d_merge && >> + dpu_kms->catalog->mixer_count > 0 && >> + mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth) >> + topology.num_lm = 2; >> else >> - topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1; >> + topology.num_lm = 1; >> if (crtc_state->ctm) >> topology.num_dspp = topology.num_lm; >> >
On 20/04/2023 20:47, Jeykumar Sankaran wrote: > > > On 4/19/2023 3:23 PM, Dmitry Baryshkov wrote: >> On 19/04/2023 17:41, Arnaud Vrac wrote: >>> This avoids using two LMs instead of one when the display width is lower >>> than the maximum supported value. For example on MSM8996/MSM8998, the >>> actual maxwidth is 2560, so we would use two LMs for 1280x720 or >>> 1920x1080 resolutions, while one is enough. >>> >>> Signed-off-by: Arnaud Vrac <avrac@freebox.fr> >> >> While this looks correct (and following what we have in 4.4), later >> vendor kernels specify the topology explicitly. Probably we should >> check this with the hw guys, because it might be the following case: >> even though a single LM can supply the mode, it will spend more power >> compared to two LMs. >> >> > Yes. 2 LM split will allow the HW to run in lower mdp core clock. Can > you maintain the split_threshold in the hw catalog until per mode > topology is available? I don't think it warrants the trouble, unless we have a real usecase when the device is short of LMs. Arnaud, I'll mark this patch as Rejected for now, unless it fixes an LM shortage for your platform. > > Jeykumar S >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> index 1dc5dbe585723..dd2914726c4f6 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> @@ -53,8 +53,6 @@ >>> #define IDLE_SHORT_TIMEOUT 1 >>> -#define MAX_HDISPLAY_SPLIT 1080 >>> - >>> /* timeout in frames waiting for frame done */ >>> #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5 >>> @@ -568,10 +566,12 @@ static struct msm_display_topology >>> dpu_encoder_get_topology( >>> */ >>> if (intf_count == 2) >>> topology.num_lm = 2; >>> - else if (!dpu_kms->catalog->caps->has_3d_merge) >>> - topology.num_lm = 1; >>> + else if (dpu_kms->catalog->caps->has_3d_merge && >>> + dpu_kms->catalog->mixer_count > 0 && >>> + mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth) >>> + topology.num_lm = 2; >>> else >>> - topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 >>> : 1; >>> + topology.num_lm = 1; >>> if (crtc_state->ctm) >>> topology.num_dspp = topology.num_lm; >>> >>
Le sam. 20 mai 2023 à 22:49, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> a écrit : > > On 20/04/2023 20:47, Jeykumar Sankaran wrote: > > > > > > On 4/19/2023 3:23 PM, Dmitry Baryshkov wrote: > >> On 19/04/2023 17:41, Arnaud Vrac wrote: > >>> This avoids using two LMs instead of one when the display width is lower > >>> than the maximum supported value. For example on MSM8996/MSM8998, the > >>> actual maxwidth is 2560, so we would use two LMs for 1280x720 or > >>> 1920x1080 resolutions, while one is enough. > >>> > >>> Signed-off-by: Arnaud Vrac <avrac@freebox.fr> > >> > >> While this looks correct (and following what we have in 4.4), later > >> vendor kernels specify the topology explicitly. Probably we should > >> check this with the hw guys, because it might be the following case: > >> even though a single LM can supply the mode, it will spend more power > >> compared to two LMs. > >> > >> > > Yes. 2 LM split will allow the HW to run in lower mdp core clock. Can > > you maintain the split_threshold in the hw catalog until per mode > > topology is available? > > I don't think it warrants the trouble, unless we have a real usecase > when the device is short of LMs. > > Arnaud, I'll mark this patch as Rejected for now, unless it fixes an LM > shortage for your platform. It's fine, if I remember correctly I wrote this patch because display wouldn't work before I fixed the LM pairings on msm8998, but now it's not a requirement anymore. > > > > > Jeykumar S > >>> --- > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++----- > >>> 1 file changed, 5 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >>> index 1dc5dbe585723..dd2914726c4f6 100644 > >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >>> @@ -53,8 +53,6 @@ > >>> #define IDLE_SHORT_TIMEOUT 1 > >>> -#define MAX_HDISPLAY_SPLIT 1080 > >>> - > >>> /* timeout in frames waiting for frame done */ > >>> #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5 > >>> @@ -568,10 +566,12 @@ static struct msm_display_topology > >>> dpu_encoder_get_topology( > >>> */ > >>> if (intf_count == 2) > >>> topology.num_lm = 2; > >>> - else if (!dpu_kms->catalog->caps->has_3d_merge) > >>> - topology.num_lm = 1; > >>> + else if (dpu_kms->catalog->caps->has_3d_merge && > >>> + dpu_kms->catalog->mixer_count > 0 && > >>> + mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth) > >>> + topology.num_lm = 2; > >>> else > >>> - topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 > >>> : 1; > >>> + topology.num_lm = 1; > >>> if (crtc_state->ctm) > >>> topology.num_dspp = topology.num_lm; > >>> > >> > > -- > With best wishes > Dmitry >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1dc5dbe585723..dd2914726c4f6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -53,8 +53,6 @@ #define IDLE_SHORT_TIMEOUT 1 -#define MAX_HDISPLAY_SPLIT 1080 - /* timeout in frames waiting for frame done */ #define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5 @@ -568,10 +566,12 @@ static struct msm_display_topology dpu_encoder_get_topology( */ if (intf_count == 2) topology.num_lm = 2; - else if (!dpu_kms->catalog->caps->has_3d_merge) - topology.num_lm = 1; + else if (dpu_kms->catalog->caps->has_3d_merge && + dpu_kms->catalog->mixer_count > 0 && + mode->hdisplay > dpu_kms->catalog->mixer[0].sblk->maxwidth) + topology.num_lm = 2; else - topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1; + topology.num_lm = 1; if (crtc_state->ctm) topology.num_dspp = topology.num_lm;
This avoids using two LMs instead of one when the display width is lower than the maximum supported value. For example on MSM8996/MSM8998, the actual maxwidth is 2560, so we would use two LMs for 1280x720 or 1920x1080 resolutions, while one is enough. Signed-off-by: Arnaud Vrac <avrac@freebox.fr> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)