Re: [Freedreno] [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing

[Marijn Suijten <marijn.suijten@somainline.org>]

On 2022-05-02 13:02:09, Dmitry Baryshkov wrote:
> [snip]
> > How would you represent this in XML?  I was hoping for a method that
> > allows to construct the value in a generic way, without register names,
> > and then simply have a "register macro" that moves (and perhaps masks)
> > the preconstructed value into the right place.  A bit like how `enum`s
> > are currently set up in XML, but with bit ranges for the values and
> > macros to set a value.
> > 
> > I think I've _partially_ found what I was looking for: a `<bitset>`.
> > However, I don't know if we can utilize this multiple times within a
> > single `reg32`, once with an offset for stream1.  Alas, it's just
> > bikeshedding at this point.
> 
> Unfortunately the following naïve patch doesn't work, stream1 bits are 
> still defined in the 0:15 bit space. One would have to modify rnn tool 
> to make sure that it takes into account the low/high parts of the 
> bitfield when generating offsets/masks.
> 
> diff --git a/src/freedreno/registers/dsi/dsi.xml 
> b/src/freedreno/registers/dsi/dsi.xml
> index f2eef4ff41ae..b0166628ad0a 100644
> --- a/src/freedreno/registers/dsi/dsi.xml
> +++ b/src/freedreno/registers/dsi/dsi.xml
> @@ -361,22 +361,19 @@ 
> xsi:schemaLocation="http://nouveau.freedesktop.org/ rules-ng.xsd">
>                  <bitfield name="MAJOR" low="24" high="31" type="uint"/>
>          </reg32>
>          <reg32 offset="0x002d4" name="CPHY_MODE_CTRL"/>
> -       <reg32 offset="0x0029c" name="VIDEO_COMPRESSION_MODE_CTRL">
> -               <bitfield name="WC" low="16" high="31" type="uint"/>
> +       <bitset name="COMPRESSION_MODE_CTRL" inline="true">
>                  <bitfield name="DATATYPE" low="8" high="13" type="uint"/>
>                  <bitfield name="PKT_PER_LINE" low="6" high="7" 
> type="uint"/>
>                  <bitfield name="EOL_BYTE_NUM" low="4" high="5" 
> type="uint"/>
>                  <bitfield name="EN" pos="0" type="boolean"/>
> +       </bitset>
> +       <reg32 offset="0x0029c" name="VIDEO_COMPRESSION_MODE_CTRL">
> +               <bitfield name="WC" low="16" high="31" type="uint"/>
> +               <bitfield name="STREAM0" low="0" high="15" 
> type="COMPRESSION_MODE_CTRL"/>
>          </reg32>
>          <reg32 offset="0x002a4" name="COMMAND_COMPRESSION_MODE_CTRL">
> -               <bitfield name="STREAM1_DATATYPE" low="24" high="29" 
> type="uint"/>
> -               <bitfield name="STREAM1_PKT_PER_LINE" low="22" high="23" 
> type="uint"/>
> -               <bitfield name="STREAM1_EOL_BYTE_NUM" low="20" high="21" 
> type="uint"/>
> -               <bitfield name="STREAM1_EN" pos="16" type="boolean"/>
> -               <bitfield name="STREAM0_DATATYPE" low="8" high="13" 
> type="uint"/>
> -               <bitfield name="STREAM0_PKT_PER_LINE" low="6" high="7" 
> type="uint"/>
> -               <bitfield name="STREAM0_EOL_BYTE_NUM" low="4" high="5" 
> type="uint"/>
> -               <bitfield name="STREAM0_EN" pos="0" type="boolean"/>
> +               <bitfield name="STREAM1" low="16" high="31" 
> type="COMPRESSION_MODE_CTRL"/>
> +               <bitfield name="STREAM0" low="0" high="15" 
> type="COMPRESSION_MODE_CTRL"/>
>          </reg32>
>          <reg32 offset="0x002a8" name="COMMAND_COMPRESSION_MODE_CTRL2">
>                  <bitfield name="STREAM1_SLICE_WIDTH" low="16" high="31" 
> type="uint"/>

This is approximately what I was aiming for.  `inline="true"` does
"inline" all the individual bitfields so that they're prefixed with the
reg32+bitfield name again, right?  That's what I wanted to avoid :)

- Marijn