Re: [RFC PATCH v5 1/4] dpll: Add DPLL framework base functions

[Jiri Pirko <jiri@resnulli.us>]

Thu, Jan 19, 2023 at 06:16:13PM CET, jiri@resnulli.us wrote:
>Tue, Jan 17, 2023 at 07:00:48PM CET, vadfed@meta.com wrote:

[...]


>>+/**
>>+ * dpll_cmd - Commands supported by the dpll generic netlink family
>>+ *
>>+ * @DPLL_CMD_UNSPEC - invalid message type
>>+ * @DPLL_CMD_DEVICE_GET - Get list of dpll devices (dump) or attributes of
>>+ *	single dpll device and it's pins
>>+ * @DPLL_CMD_DEVICE_SET - Set attributes for a dpll
>>+ * @DPLL_CMD_PIN_SET - Set attributes for a pin
>>+ **/
>>+enum dpll_cmd {
>>+	DPLL_CMD_UNSPEC,
>>+	DPLL_CMD_DEVICE_GET,
>>+	DPLL_CMD_DEVICE_SET,
>>+	DPLL_CMD_PIN_SET,
>
>Have pin get to get list of pins, then you can have 1:1 mapping to
>events and loose the enum dpll_event_change. This is the usual way to do
>stuff. Events have the same cmd and message format as get.

I was thinking about that a bit more.
1) There is 1:n relationship between PIN and DPLL(s).
2) The pin configuration is independent on DPLL, with an
   exeption of PRIO.

Therefore as I suggested in the reply to this patch, the pin should be
separate entity, allocated and having ops unrelated to DPLL. It is just
registered to the DPLLs that are using the pin.

The pin ops should not have dpll pointer as arg, again with exception of
PRIO.

DPLL_CMD_DEVICE_GET should not contain pins at all.

There should be DPLL_CMD_PIN_GET added which can dump and will be used
to get the list of pins in the system.
- if DPLL handle is passed to DPLL_CMD_PIN_GET, it will dump only pins
  related to the specified DPLL.

DPLL_CMD_PIN_GET message will contain pin-specific attrs and will have a
list of connected DPLLs:
       DPLLA_PIN_IDX
       DPLLA_PIN_DESCRIPTION
       DPLLA_PIN_TYPE
       DPLLA_PIN_SIGNAL_TYPE
       DPLLA_PIN_SIGNAL_TYPE_SUPPORTED
       DPLLA_PIN_CUSTOM_FREQ
       DPLLA_PIN_MODE
       DPLLA_PIN_MODE_SUPPORTED
       DPLLA_PIN_PARENT_IDX
       DPLLA_PIN_DPLL    (nested)
          DPLLA_DPLL_HANDLE   "dpll_0"
          DPLLA_PIN_PRIO    1
       DPLLA_PIN_DPLL    (nested)
          DPLLA_DPLL_HANDLE   "dpll_1"
          DPLLA_PIN_PRIO    2

Please take the names lightly. My point is to show 2 nests for 2
DPLLS connected, on each the pin has different prio.

Does this make sense?

One issue to be solved is the pin indexing. As pin would be separate
entity, the indexing would be global and therefore not predictable. We
would have to figure it out differntly. Pehaps something like this:

$ dpll dev show
pci/0000:08:00.0: dpll 1             first dpll on 0000:08:00.0
pci/0000:08:00.0: dpll 2             second dpll on the same pci device
pci/0000:09:00.0: dpll 1             first dpll on 0000:09:00.0
pci/0000:09:00.0: dpll 2             second dpll on the same pci device

$ dpll pin show
pci/0000:08:00.0: pin 1 desc SOMELABEL_A
  dpll 1:                          This refers to DPLL 1 on the same pci device
    prio 80
  dpll 2:                          This refers to DPLL 2 on the same pci device
    prio 100
pci/0000:08:00.0: pin 2 desc SOMELABEL_B
  dpll 1:
    prio 80
  dpll 2:
    prio 100
pci/0000:08:00.0: pin 3 desc SOMELABEL_C
  dpll 1:
    prio 80
  dpll 2:
    prio 100
pci/0000:08:00.0: pin 4 desc SOMELABEL_D
  dpll 1:
    prio 80
  dpll 2:
    prio 100
pci/0000:09:00.0: pin 1 desc SOMEOTHERLABEL_A
  dpll 1:
    prio 80
  dpll 2:
    prio 100
pci/0000:09:00.0: pin 2 desc SOMEOTHERLABEL_B
  dpll 1:
    prio 80
  dpll 2:
    prio 100
pci/0000:09:00.0: pin 3 desc SOMEOTHERLABEL_C
  dpll 1:
    prio 80
  dpll 2:
    prio 100
pci/0000:09:00.0: pin 4 desc SOMEOTHERLABEL_C
  dpll 1:
    prio 80
  dpll 2:
    prio 100

Note there are 2 groups of pins, one for each pci device.

Setting some attribute command would looks like:
To set DPLL mode:
$ dpll dev set pci/0000:08:00.0 dpll 1 mode freerun
   netlink:
   DPLL_CMD_DEVICE_SET
      DPLLA_BUS_NAME "pci"
      DPLLA_DEV_NAME "0000:08:00.0"
      DPLLA_DPLL_INDEX 1
      DPLLA_DPLL_MODE 3

$ dpll dev set pci/0000:08:00.0 dpll 2 mode automatic


To set signal frequency in HZ:
$ dpll pin set pci/0000:08:00.0 pin 3 frequency 10000000
   netlink:
   DPLL_CMD_PIN_SET
      DPLLA_BUS_NAME "pci"
      DPLLA_DEV_NAME "0000:08:00.0"
      DPLLA_PIN_INDEX 3
      DPLLA_PIN_FREQUENCY 10000000

$ dpll pin set pci/0000:08:00.0 pin 1 frequency 1


To set individual of one pin for 2 DPLLs:
$ dpll pin set pci/0000:08:00.0 pin 1 dpll 1 prio 40
   netlink:
   DPLL_CMD_PIN_SET
      DPLLA_BUS_NAME "pci"
      DPLLA_DEV_NAME "0000:08:00.0"
      DPLLA_PIN_INDEX 1
      DPLLA_DPLL_INDEX 1
      DPLLA_PIN_PRIO 40

$ dpll pin set pci/0000:08:00.0 pin 1 dpll 2 prio 80


Isn't this neat?


[...]