Messages in this thread | | | Subject | Re: [RFC PATCH 11/13] drm/msm/disp/dpu1: Add support for DSC in topology | From | Dmitry Baryshkov <> | Date | Sat, 29 May 2021 01:29:57 +0300 |
| |
On 29/05/2021 01:23, abhinavk@codeaurora.org wrote: > On 2021-05-28 03:39, Dmitry Baryshkov wrote: >> On 21/05/2021 15:49, Vinod Koul wrote: >>> For DSC to work we typically need a 2,2,1 configuration. This should >>> suffice for resolutions upto 4k. For more resolutions like 8k this won't >>> work. >>> >>> Furthermore, we can use 1 DSC encoder in lesser resulutions, but that is >>> not power efficient according to Abhinav, so it is recommended to always >>> use 2 encoders. >> >> Not power efficient because the second DSC would also be powered on or >> because single DSC enc would consume more power than two DSCs? > > I havent got through the series yet but just thought of answering this, > > So before coming to the power aspects of this, hard-coding was done for > the foll reasons: > > -> We do not have a topology DTSI property in upstream and will probably > not have as well till > other features are added which support all the topologies > -> The DSC panel which is being upstreamed as part of this series is > working with this 2,2,1 topology > downstream ( dual lm, dual DSC encoders, single DSI ). Other topologies > have not been tried on it yet > -> There needs to be a better approach to handle all topologies once we > have added support for them. > It can be either a DTSI property if others agree OR some helper API > which will determine the best topology > based on various factors. Till then, since this will be the only DSC > panel we are adding support for > I thought we can start with a fixed topology for now. > > Coming to the power aspect, I only recommended 2-2-1 here because using > two mixers is better power wise > as it will split the width/2. We can also do 2-1-1 by enabling 3D mux > but this panel has not been validated > with a single DSC. So to keep things simple with what has been > validated, I thought we can go ahead with > 2-2-1 for now. > > So rather than giving too much importance to the power aspect of it, the > other reasons should also > be highlighted here as the main reason and the commit text should give > these details as well.
Sounds reasonable now, thank you!
> >>> >>> So for now we blindly create 2,2,1 topology when DSC is enabled >>> >>> Co-developed-by: Abhinav Kumar <abhinavk@codeaurora.org> >>> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> >>> Signed-off-by: Vinod Koul <vkoul@kernel.org> >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> index 18cb1274a8bb..bffb40085c67 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> @@ -609,8 +609,22 @@ static struct msm_display_topology >>> dpu_encoder_get_topology( >>> topology.num_enc = 0; >>> topology.num_intf = intf_count; >>> + drm_enc = &dpu_enc->base; >>> + priv = drm_enc->dev->dev_private; >>> + if (priv && priv->dsc) { >>> + /* In case of Display Stream Compression DSC, we would use >>> + * 2 encoders, 2 line mixers and 1 interface >>> + * this is power optimal and can drive upto (including) 4k >>> + * screens >>> + */ >>> + topology.num_enc = 2; >>> + topology.num_intf = 1; >>> + topology.num_lm = 2; >>> + } >>> + >>> return topology; >>> } >>> + >>> static int dpu_encoder_virt_atomic_check( >>> struct drm_encoder *drm_enc, >>> struct drm_crtc_state *crtc_state, >>>
-- With best wishes Dmitry
| |