| Message ID | 20210521124946.3617862-3-vkoul@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm: Add Display Stream Compression Support | expand |
On 21-05-21, 08:18, Rob Herring wrote: > On Fri, May 21, 2021 at 7:50 AM Vinod Koul <vkoul@kernel.org> wrote: > > > > DSC enables streams to be compressed before we send to panel. This > > requires DSC enabled encoder and a panel to be present. So we add this > > information in board DTS and find if DSC can be enabled and the > > parameters required to configure DSC are added to binding document along > > with example > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > > --- > > .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > This is getting converted to schema. Hopefully, v17 will be it. Sigh. I will update these on top, whenever that one gets merged... Any comments on the parameters added here? -- ~Vinod
On Fri 21 May 07:49 CDT 2021, Vinod Koul wrote: > DSC enables streams to be compressed before we send to panel. This > requires DSC enabled encoder and a panel to be present. So we add this > information in board DTS and find if DSC can be enabled and the > parameters required to configure DSC are added to binding document along > with example > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > --- > .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt > index b9a64d3ff184..83d2fb92267e 100644 > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt > @@ -48,6 +48,13 @@ Optional properties: > - pinctrl-n: the "sleep" pinctrl state > - ports: contains DSI controller input and output ports as children, each > containing one endpoint subnode. > +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled > +- qcom,mdss-slice-height: DSC slice height in pixels > +- qcom,mdss-slice-width: DSC slice width in pixels > +- qcom,mdss-slice-per-pkt: DSC slices per packet > +- qcom,mdss-bit-per-component: DSC bits per component > +- qcom,mdss-bit-per-pixel: DSC bits per pixel > +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled > > DSI Endpoint properties: > - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's > @@ -188,6 +195,14 @@ Example: > qcom,master-dsi; > qcom,sync-dual-dsi; > > + qcom,mdss-dsc-enabled; To me the activation of DSC seems to be a property of the panel. > + qcom,mdss-slice-height = <16>; > + qcom,mdss-slice-width = <540>; > + qcom,mdss-slice-per-pkt = <1>; > + qcom,mdss-bit-per-component = <8>; > + qcom,mdss-bit-per-pixel = <8>; > + qcom,mdss-block-prediction-enable; Which of these properties relates to the DSC encoder and what needs to be agreed with the sink? Can't we derive e.g. bpp from the information we have from the attached panel already? Regards, Bjorn > + > qcom,mdss-mdp-transfer-time-us = <12000>; > > pinctrl-names = "default", "sleep"; > -- > 2.26.3 >
On 21-05-21, 09:42, Bjorn Andersson wrote: > On Fri 21 May 07:49 CDT 2021, Vinod Koul wrote: > > > DSC enables streams to be compressed before we send to panel. This > > requires DSC enabled encoder and a panel to be present. So we add this > > information in board DTS and find if DSC can be enabled and the > > parameters required to configure DSC are added to binding document along > > with example > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > > --- > > .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt > > index b9a64d3ff184..83d2fb92267e 100644 > > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt > > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt > > @@ -48,6 +48,13 @@ Optional properties: > > - pinctrl-n: the "sleep" pinctrl state > > - ports: contains DSI controller input and output ports as children, each > > containing one endpoint subnode. > > +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled > > +- qcom,mdss-slice-height: DSC slice height in pixels > > +- qcom,mdss-slice-width: DSC slice width in pixels > > +- qcom,mdss-slice-per-pkt: DSC slices per packet > > +- qcom,mdss-bit-per-component: DSC bits per component > > +- qcom,mdss-bit-per-pixel: DSC bits per pixel > > +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled > > > > DSI Endpoint properties: > > - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's > > @@ -188,6 +195,14 @@ Example: > > qcom,master-dsi; > > qcom,sync-dual-dsi; > > > > + qcom,mdss-dsc-enabled; > > To me the activation of DSC seems to be a property of the panel. I think there are three parts to the problem 1. Panel needs to support it 2. Host needs to support it 3. Someone needs to decide to use when both the above conditions are met. There are cases where above 1, 2 will be satisfied, but we might be okay without DSC too.. so how to decide when to do DSC :) I feel it is more of a system property. And I also think that these parameters here are host configuration and not really for panel... > > > + qcom,mdss-slice-height = <16>; > > + qcom,mdss-slice-width = <540>; > > + qcom,mdss-slice-per-pkt = <1>; > > + qcom,mdss-bit-per-component = <8>; > > + qcom,mdss-bit-per-pixel = <8>; > > + qcom,mdss-block-prediction-enable; > > Which of these properties relates to the DSC encoder and what needs to > be agreed with the sink? Can't we derive e.g. bpp from the information > we have from the attached panel already? Let me go back and check on this a bit more Thanks -- ~Vinod
On Mon 24 May 02:30 CDT 2021, Vinod Koul wrote: > On 21-05-21, 09:42, Bjorn Andersson wrote: > > On Fri 21 May 07:49 CDT 2021, Vinod Koul wrote: > > > > > DSC enables streams to be compressed before we send to panel. This > > > requires DSC enabled encoder and a panel to be present. So we add this > > > information in board DTS and find if DSC can be enabled and the > > > parameters required to configure DSC are added to binding document along > > > with example > > > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > > > --- > > > .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt > > > index b9a64d3ff184..83d2fb92267e 100644 > > > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt > > > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt > > > @@ -48,6 +48,13 @@ Optional properties: > > > - pinctrl-n: the "sleep" pinctrl state > > > - ports: contains DSI controller input and output ports as children, each > > > containing one endpoint subnode. > > > +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled > > > +- qcom,mdss-slice-height: DSC slice height in pixels > > > +- qcom,mdss-slice-width: DSC slice width in pixels > > > +- qcom,mdss-slice-per-pkt: DSC slices per packet > > > +- qcom,mdss-bit-per-component: DSC bits per component > > > +- qcom,mdss-bit-per-pixel: DSC bits per pixel > > > +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled > > > > > > DSI Endpoint properties: > > > - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's > > > @@ -188,6 +195,14 @@ Example: > > > qcom,master-dsi; > > > qcom,sync-dual-dsi; > > > > > > + qcom,mdss-dsc-enabled; > > > > To me the activation of DSC seems to be a property of the panel. > > I think there are three parts to the problem > 1. Panel needs to support it In the case of DP there's bits to be read in the panel to figure this out, for DSI panels this seems like a property that the panel (driver) should know about. > 2. Host needs to support it Right, so this needs to be known by the driver. My suggestion is that we derive it from the compatible or from the HW version. > 3. Someone needs to decide to use when both the above conditions are > met. > > There are cases where above 1, 2 will be satisfied, but we might be okay > without DSC too.. so how to decide when to do DSC :) > Can we describe those cases? E.g. is it because enabling DSC would not cause a reduction in clock speed that's worth the effort? Or do we only use DSC for DSI when it allows us to squeeze everything into a single link? Regards, Bjorn > I feel it is more of a system property. And I also think that these > parameters here are host configuration and not really for panel... > > > > > > + qcom,mdss-slice-height = <16>; > > > + qcom,mdss-slice-width = <540>; > > > + qcom,mdss-slice-per-pkt = <1>; > > > + qcom,mdss-bit-per-component = <8>; > > > + qcom,mdss-bit-per-pixel = <8>; > > > + qcom,mdss-block-prediction-enable; > > > > Which of these properties relates to the DSC encoder and what needs to > > be agreed with the sink? Can't we derive e.g. bpp from the information > > we have from the attached panel already? > > Let me go back and check on this a bit more > > Thanks > -- > ~Vinod
On 24-05-21, 10:08, Bjorn Andersson wrote: > On Mon 24 May 02:30 CDT 2021, Vinod Koul wrote: > > > On 21-05-21, 09:42, Bjorn Andersson wrote: > > > On Fri 21 May 07:49 CDT 2021, Vinod Koul wrote: > > > > > > > DSC enables streams to be compressed before we send to panel. This > > > > requires DSC enabled encoder and a panel to be present. So we add this > > > > information in board DTS and find if DSC can be enabled and the > > > > parameters required to configure DSC are added to binding document along > > > > with example > > > > > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > > > > --- > > > > .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ > > > > 1 file changed, 15 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt > > > > index b9a64d3ff184..83d2fb92267e 100644 > > > > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt > > > > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt > > > > @@ -48,6 +48,13 @@ Optional properties: > > > > - pinctrl-n: the "sleep" pinctrl state > > > > - ports: contains DSI controller input and output ports as children, each > > > > containing one endpoint subnode. > > > > +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled > > > > +- qcom,mdss-slice-height: DSC slice height in pixels > > > > +- qcom,mdss-slice-width: DSC slice width in pixels > > > > +- qcom,mdss-slice-per-pkt: DSC slices per packet > > > > +- qcom,mdss-bit-per-component: DSC bits per component > > > > +- qcom,mdss-bit-per-pixel: DSC bits per pixel > > > > +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled > > > > > > > > DSI Endpoint properties: > > > > - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's > > > > @@ -188,6 +195,14 @@ Example: > > > > qcom,master-dsi; > > > > qcom,sync-dual-dsi; > > > > > > > > + qcom,mdss-dsc-enabled; > > > > > > To me the activation of DSC seems to be a property of the panel. > > > > I think there are three parts to the problem > > 1. Panel needs to support it > > In the case of DP there's bits to be read in the panel to figure this > out, for DSI panels this seems like a property that the panel (driver) > should know about. Yes panel should know.. > > > 2. Host needs to support it > > Right, so this needs to be known by the driver. My suggestion is that we > derive it from the compatible or from the HW version. Okay > > > 3. Someone needs to decide to use when both the above conditions are > > met. > > > > There are cases where above 1, 2 will be satisfied, but we might be okay > > without DSC too.. so how to decide when to do DSC :) > > > > Can we describe those cases? E.g. is it because enabling DSC would not > cause a reduction in clock speed that's worth the effort? Or do we only > use DSC for DSI when it allows us to squeeze everything into a single > link? I really dont know how and when we should decide that :-| One thing we can do is that if both support then always enable and get benefit of getting power and clock speed reduction. With this, who should have slice and bpp settings, panel or host? Thanks -- ~Vinod
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index b9a64d3ff184..83d2fb92267e 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -48,6 +48,13 @@ Optional properties: - pinctrl-n: the "sleep" pinctrl state - ports: contains DSI controller input and output ports as children, each containing one endpoint subnode. +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled +- qcom,mdss-slice-height: DSC slice height in pixels +- qcom,mdss-slice-width: DSC slice width in pixels +- qcom,mdss-slice-per-pkt: DSC slices per packet +- qcom,mdss-bit-per-component: DSC bits per component +- qcom,mdss-bit-per-pixel: DSC bits per pixel +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled DSI Endpoint properties: - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's @@ -188,6 +195,14 @@ Example: qcom,master-dsi; qcom,sync-dual-dsi; + qcom,mdss-dsc-enabled; + qcom,mdss-slice-height = <16>; + qcom,mdss-slice-width = <540>; + qcom,mdss-slice-per-pkt = <1>; + qcom,mdss-bit-per-component = <8>; + qcom,mdss-bit-per-pixel = <8>; + qcom,mdss-block-prediction-enable; + qcom,mdss-mdp-transfer-time-us = <12000>; pinctrl-names = "default", "sleep";
DSC enables streams to be compressed before we send to panel. This requires DSC enabled encoder and a panel to be present. So we add this information in board DTS and find if DSC can be enabled and the parameters required to configure DSC are added to binding document along with example Signed-off-by: Vinod Koul <vkoul@kernel.org> --- .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) -- 2.26.3