[RFC,11/13] drm/msm/disp/dpu1: Add support for DSC in topology

Message ID 20210521124946.3617862-16-vkoul@kernel.org
State New
Headers show
Series
  • drm/msm: Add Display Stream Compression Support
Related show

Commit Message

Vinod Koul May 21, 2021, 12:49 p.m.
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.

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(+)

-- 
2.26.3

Comments

Abhinav Kumar May 28, 2021, 10:23 p.m. | #1
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.

>> 

>> 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,

>>
Dmitry Baryshkov May 28, 2021, 10:29 p.m. | #2
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

Patch

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,