Re: [Freedreno] [PATCH 1/4] drm/msm: add some cec register bitfield details

2023-04-20 Thread Arnaud Vrac
Le jeu. 20 avr. 2023 à 02:30, Dmitry Baryshkov
 a écrit :
>
> On 20/04/2023 03:27, Abhinav Kumar wrote:
> >
> >
> > On 4/19/2023 5:21 PM, Dmitry Baryshkov wrote:
> >> On 20/04/2023 03:17, Abhinav Kumar wrote:
> >>>
> >>>
> >>> On 4/19/2023 5:11 PM, Dmitry Baryshkov wrote:
>  On 20/04/2023 03:10, Abhinav Kumar wrote:
> >
> >
> > On 4/19/2023 4:53 PM, Dmitry Baryshkov wrote:
> >> On 18/04/2023 21:10, Arnaud Vrac wrote:
> >>> The register names and bitfields were determined from the downstream
> >>> msm-4.4 driver.
> >>>
> >>> Signed-off-by: Arnaud Vrac 
> >>> ---
> >>>   drivers/gpu/drm/msm/hdmi/hdmi.xml.h | 62
> >>> -
> >>>   1 file changed, 61 insertions(+), 1 deletion(-)
> >>
> >> I have opened MR against Mesa at
> >> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22588.
> >>
> >> The patch is:
> >>
> >> Reviewed-by: Dmitry Baryshkov 
> >>
> >> Minor nit below
> >>
> >
> > Also, shouldnt the register updates be done using rnn tool instead
> > of manual edits?
> 
>  We usually update the rnn and ask Rob to pull it at the beginning of
>  the cycle.
> 
> >>>
> >>> Sorry, I didnt get this. So you are saying, we will accept manual
> >>> edits and then replace it with the tool generated xml later? I was
> >>> not aware of that, because previously I was always asked by Rob to
> >>> use the tool to generate the xml and push that.
> >>
> >> We accept manual edits for the patchset (so that one can test it), but
> >> before merging the patchset we ask Rob to pull the xml.
> >>
> >
> > Interesting, and Rob generates the xml that time or who does that?
> >
> > The MR you have created updates the freedreno/registers which is just to
> > keep the XML in the driver and mesa in sync.
> >
> > But I am trying to understand who generates the updated xml to merge it
> > with the patchset if its not the developer who does that anymore.
>
> In this case I went on and created the MR as Arnaud didn't create one.
> Yes, usually we do this on our own when updating the register file (in
> other words: I usually edit the xml, then regen the xml.h, then add it
> to the patchset).

Ok thanks, I wasn't sure in which order to do this, thanks for posting
the MR on mesa. The changes in hdmi.xml.h I posted are not manually
edited, they were generated using the gen_header.py script in mesa (I
omitted the top comments changes about envytools which are not present
anymore though).

>
> >
> >>>
> >
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
> >>> b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
> >>> index 973b460486a5a..b4dd6e8cba6b7 100644
> >>> --- a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
> >>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
> >>> @@ -76,6 +76,13 @@ enum hdmi_acr_cts {
> >>>   ACR_48 = 3,
> >>>   };
> >>> +enum hdmi_cec_tx_status {
> >>> +CEC_TX_OK = 0,
> >>> +CEC_TX_NACK = 1,
> >>> +CEC_TX_ARB_LOSS = 2,
> >>> +CEC_TX_MAX_RETRIES = 3,
> >>> +};
> >>> +
> >>>   #define REG_HDMI_CTRL0x
> >>>   #define HDMI_CTRL_ENABLE0x0001
> >>>   #define HDMI_CTRL_HDMI0x0002
> >>> @@ -476,20 +483,73 @@ static inline uint32_t
> >>> HDMI_DDC_REF_REFTIMER(uint32_t val)
> >>>   #define REG_HDMI_HDCP_SW_LOWER_AKSV0x0288
> >>>   #define REG_HDMI_CEC_CTRL0x028c
> >>> +#define HDMI_CEC_CTRL_ENABLE0x0001
> >>> +#define HDMI_CEC_CTRL_SEND_TRIGGER0x0002
> >>> +#define HDMI_CEC_CTRL_FRAME_SIZE__MASK0x01f0
> >>> +#define HDMI_CEC_CTRL_FRAME_SIZE__SHIFT4
> >>> +static inline uint32_t HDMI_CEC_CTRL_FRAME_SIZE(uint32_t val)
> >>> +{
> >>> +return ((val) << HDMI_CEC_CTRL_FRAME_SIZE__SHIFT) &
> >>> HDMI_CEC_CTRL_FRAME_SIZE__MASK;
> >>> +}
> >>> +#define HDMI_CEC_CTRL_LINE_OE0x0200
> >>>   #define REG_HDMI_CEC_WR_DATA0x0290
> >>> +#define HDMI_CEC_WR_DATA_BROADCAST0x0001
> >>> +#define HDMI_CEC_WR_DATA_DATA__MASK0xff00
> >>> +#define HDMI_CEC_WR_DATA_DATA__SHIFT8
> >>> +static inline uint32_t HDMI_CEC_WR_DATA_DATA(uint32_t val)
> >>> +{
> >>> +return ((val) << HDMI_CEC_WR_DATA_DATA__SHIFT) &
> >>> HDMI_CEC_WR_DATA_DATA__MASK;
> >>> +}
> >>> -#define REG_HDMI_CEC_CEC_RETRANSMIT0x0294
> >>> +#define REG_HDMI_CEC_RETRANSMIT0x0294
> >>> +#define HDMI_CEC_RETRANSMIT_ENABLE0x0001
> >>> +#define HDMI_CEC_RETRANSMIT_COUNT__MASK0x00fe
> >>> +#define HDMI_CEC_RETRANSMIT_COUNT__SHIFT1
> 

Re: [Freedreno] [PATCH 1/4] drm/msm: add some cec register bitfield details

2023-04-19 Thread Dmitry Baryshkov

On 20/04/2023 03:27, Abhinav Kumar wrote:



On 4/19/2023 5:21 PM, Dmitry Baryshkov wrote:

On 20/04/2023 03:17, Abhinav Kumar wrote:



On 4/19/2023 5:11 PM, Dmitry Baryshkov wrote:

On 20/04/2023 03:10, Abhinav Kumar wrote:



On 4/19/2023 4:53 PM, Dmitry Baryshkov wrote:

On 18/04/2023 21:10, Arnaud Vrac wrote:

The register names and bitfields were determined from the downstream
msm-4.4 driver.

Signed-off-by: Arnaud Vrac 
---
  drivers/gpu/drm/msm/hdmi/hdmi.xml.h | 62 
-

  1 file changed, 61 insertions(+), 1 deletion(-)


I have opened MR against Mesa at 
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22588.


The patch is:

Reviewed-by: Dmitry Baryshkov 

Minor nit below



Also, shouldnt the register updates be done using rnn tool instead 
of manual edits?


We usually update the rnn and ask Rob to pull it at the beginning of 
the cycle.




Sorry, I didnt get this. So you are saying, we will accept manual 
edits and then replace it with the tool generated xml later? I was 
not aware of that, because previously I was always asked by Rob to 
use the tool to generate the xml and push that.


We accept manual edits for the patchset (so that one can test it), but 
before merging the patchset we ask Rob to pull the xml.




Interesting, and Rob generates the xml that time or who does that?

The MR you have created updates the freedreno/registers which is just to 
keep the XML in the driver and mesa in sync.


But I am trying to understand who generates the updated xml to merge it 
with the patchset if its not the developer who does that anymore.


In this case I went on and created the MR as Arnaud didn't create one. 
Yes, usually we do this on our own when updating the register file (in 
other words: I usually edit the xml, then regen the xml.h, then add it 
to the patchset).










diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h 
b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h

index 973b460486a5a..b4dd6e8cba6b7 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
@@ -76,6 +76,13 @@ enum hdmi_acr_cts {
  ACR_48 = 3,
  };
+enum hdmi_cec_tx_status {
+    CEC_TX_OK = 0,
+    CEC_TX_NACK = 1,
+    CEC_TX_ARB_LOSS = 2,
+    CEC_TX_MAX_RETRIES = 3,
+};
+
  #define REG_HDMI_CTRL    0x
  #define HDMI_CTRL_ENABLE    0x0001
  #define HDMI_CTRL_HDMI    0x0002
@@ -476,20 +483,73 @@ static inline uint32_t 
HDMI_DDC_REF_REFTIMER(uint32_t val)

  #define REG_HDMI_HDCP_SW_LOWER_AKSV    0x0288
  #define REG_HDMI_CEC_CTRL    0x028c
+#define HDMI_CEC_CTRL_ENABLE    0x0001
+#define HDMI_CEC_CTRL_SEND_TRIGGER    0x0002
+#define HDMI_CEC_CTRL_FRAME_SIZE__MASK    0x01f0
+#define HDMI_CEC_CTRL_FRAME_SIZE__SHIFT    4
+static inline uint32_t HDMI_CEC_CTRL_FRAME_SIZE(uint32_t val)
+{
+    return ((val) << HDMI_CEC_CTRL_FRAME_SIZE__SHIFT) & 
HDMI_CEC_CTRL_FRAME_SIZE__MASK;

+}
+#define HDMI_CEC_CTRL_LINE_OE    0x0200
  #define REG_HDMI_CEC_WR_DATA    0x0290
+#define HDMI_CEC_WR_DATA_BROADCAST    0x0001
+#define HDMI_CEC_WR_DATA_DATA__MASK    0xff00
+#define HDMI_CEC_WR_DATA_DATA__SHIFT    8
+static inline uint32_t HDMI_CEC_WR_DATA_DATA(uint32_t val)
+{
+    return ((val) << HDMI_CEC_WR_DATA_DATA__SHIFT) & 
HDMI_CEC_WR_DATA_DATA__MASK;

+}
-#define REG_HDMI_CEC_CEC_RETRANSMIT    0x0294
+#define REG_HDMI_CEC_RETRANSMIT    0x0294
+#define HDMI_CEC_RETRANSMIT_ENABLE    0x0001
+#define HDMI_CEC_RETRANSMIT_COUNT__MASK    0x00fe
+#define HDMI_CEC_RETRANSMIT_COUNT__SHIFT    1
+static inline uint32_t HDMI_CEC_RETRANSMIT_COUNT(uint32_t val)
+{
+    return ((val) << HDMI_CEC_RETRANSMIT_COUNT__SHIFT) & 
HDMI_CEC_RETRANSMIT_COUNT__MASK;

+}
  #define REG_HDMI_CEC_STATUS    0x0298
+#define HDMI_CEC_STATUS_BUSY    0x0001
+#define HDMI_CEC_STATUS_TX_FRAME_DONE    0x0008
+#define HDMI_CEC_STATUS_TX_STATUS__MASK    0x00f0
+#define HDMI_CEC_STATUS_TX_STATUS__SHIFT    4
+static inline uint32_t HDMI_CEC_STATUS_TX_STATUS(enum 
hdmi_cec_tx_status val)

+{
+    return ((val) << HDMI_CEC_STATUS_TX_STATUS__SHIFT) & 
HDMI_CEC_STATUS_TX_STATUS__MASK;

+}
  #define REG_HDMI_CEC_INT    0x029c
+#define HDMI_CEC_INT_TX_DONE    0x0001
+#define HDMI_CEC_INT_TX_DONE_MASK    0x0002
+#define HDMI_CEC_INT_TX_ERROR    0x0004
+#define HDMI_CEC_INT_TX_ERROR_MASK    0x0008
+#define HDMI_CEC_INT_MONITOR    0x0010
+#define HDMI_CEC_INT_MONITOR_MASK    0x0020
+#define HDMI_CEC_INT_RX_DONE    0x0040
+#define HDMI_CEC_INT_RX_DONE_MASK 

Re: [Freedreno] [PATCH 1/4] drm/msm: add some cec register bitfield details

2023-04-19 Thread Abhinav Kumar




On 4/19/2023 5:21 PM, Dmitry Baryshkov wrote:

On 20/04/2023 03:17, Abhinav Kumar wrote:



On 4/19/2023 5:11 PM, Dmitry Baryshkov wrote:

On 20/04/2023 03:10, Abhinav Kumar wrote:



On 4/19/2023 4:53 PM, Dmitry Baryshkov wrote:

On 18/04/2023 21:10, Arnaud Vrac wrote:

The register names and bitfields were determined from the downstream
msm-4.4 driver.

Signed-off-by: Arnaud Vrac 
---
  drivers/gpu/drm/msm/hdmi/hdmi.xml.h | 62 
-

  1 file changed, 61 insertions(+), 1 deletion(-)


I have opened MR against Mesa at 
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22588.


The patch is:

Reviewed-by: Dmitry Baryshkov 

Minor nit below



Also, shouldnt the register updates be done using rnn tool instead 
of manual edits?


We usually update the rnn and ask Rob to pull it at the beginning of 
the cycle.




Sorry, I didnt get this. So you are saying, we will accept manual 
edits and then replace it with the tool generated xml later? I was not 
aware of that, because previously I was always asked by Rob to use the 
tool to generate the xml and push that.


We accept manual edits for the patchset (so that one can test it), but 
before merging the patchset we ask Rob to pull the xml.




Interesting, and Rob generates the xml that time or who does that?

The MR you have created updates the freedreno/registers which is just to 
keep the XML in the driver and mesa in sync.


But I am trying to understand who generates the updated xml to merge it 
with the patchset if its not the developer who does that anymore.








diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h 
b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h

index 973b460486a5a..b4dd6e8cba6b7 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
@@ -76,6 +76,13 @@ enum hdmi_acr_cts {
  ACR_48 = 3,
  };
+enum hdmi_cec_tx_status {
+    CEC_TX_OK = 0,
+    CEC_TX_NACK = 1,
+    CEC_TX_ARB_LOSS = 2,
+    CEC_TX_MAX_RETRIES = 3,
+};
+
  #define REG_HDMI_CTRL    0x
  #define HDMI_CTRL_ENABLE    0x0001
  #define HDMI_CTRL_HDMI    0x0002
@@ -476,20 +483,73 @@ static inline uint32_t 
HDMI_DDC_REF_REFTIMER(uint32_t val)

  #define REG_HDMI_HDCP_SW_LOWER_AKSV    0x0288
  #define REG_HDMI_CEC_CTRL    0x028c
+#define HDMI_CEC_CTRL_ENABLE    0x0001
+#define HDMI_CEC_CTRL_SEND_TRIGGER    0x0002
+#define HDMI_CEC_CTRL_FRAME_SIZE__MASK    0x01f0
+#define HDMI_CEC_CTRL_FRAME_SIZE__SHIFT    4
+static inline uint32_t HDMI_CEC_CTRL_FRAME_SIZE(uint32_t val)
+{
+    return ((val) << HDMI_CEC_CTRL_FRAME_SIZE__SHIFT) & 
HDMI_CEC_CTRL_FRAME_SIZE__MASK;

+}
+#define HDMI_CEC_CTRL_LINE_OE    0x0200
  #define REG_HDMI_CEC_WR_DATA    0x0290
+#define HDMI_CEC_WR_DATA_BROADCAST    0x0001
+#define HDMI_CEC_WR_DATA_DATA__MASK    0xff00
+#define HDMI_CEC_WR_DATA_DATA__SHIFT    8
+static inline uint32_t HDMI_CEC_WR_DATA_DATA(uint32_t val)
+{
+    return ((val) << HDMI_CEC_WR_DATA_DATA__SHIFT) & 
HDMI_CEC_WR_DATA_DATA__MASK;

+}
-#define REG_HDMI_CEC_CEC_RETRANSMIT    0x0294
+#define REG_HDMI_CEC_RETRANSMIT    0x0294
+#define HDMI_CEC_RETRANSMIT_ENABLE    0x0001
+#define HDMI_CEC_RETRANSMIT_COUNT__MASK    0x00fe
+#define HDMI_CEC_RETRANSMIT_COUNT__SHIFT    1
+static inline uint32_t HDMI_CEC_RETRANSMIT_COUNT(uint32_t val)
+{
+    return ((val) << HDMI_CEC_RETRANSMIT_COUNT__SHIFT) & 
HDMI_CEC_RETRANSMIT_COUNT__MASK;

+}
  #define REG_HDMI_CEC_STATUS    0x0298
+#define HDMI_CEC_STATUS_BUSY    0x0001
+#define HDMI_CEC_STATUS_TX_FRAME_DONE    0x0008
+#define HDMI_CEC_STATUS_TX_STATUS__MASK    0x00f0
+#define HDMI_CEC_STATUS_TX_STATUS__SHIFT    4
+static inline uint32_t HDMI_CEC_STATUS_TX_STATUS(enum 
hdmi_cec_tx_status val)

+{
+    return ((val) << HDMI_CEC_STATUS_TX_STATUS__SHIFT) & 
HDMI_CEC_STATUS_TX_STATUS__MASK;

+}
  #define REG_HDMI_CEC_INT    0x029c
+#define HDMI_CEC_INT_TX_DONE    0x0001
+#define HDMI_CEC_INT_TX_DONE_MASK    0x0002
+#define HDMI_CEC_INT_TX_ERROR    0x0004
+#define HDMI_CEC_INT_TX_ERROR_MASK    0x0008
+#define HDMI_CEC_INT_MONITOR    0x0010
+#define HDMI_CEC_INT_MONITOR_MASK    0x0020
+#define HDMI_CEC_INT_RX_DONE    0x0040
+#define HDMI_CEC_INT_RX_DONE_MASK    0x0080
  #define REG_HDMI_CEC_ADDR    0x02a0
  #define REG_HDMI_CEC_TIME    0x02a4
+#define HDMI_CEC_TIME_ENABLE    0x0001
+#define HDMI_CEC_TIME_SIGNAL_FREE_TIME__MASK    0xff80
+#define