[cujomalainey]

Sometimes we want to load a binary block of data without letting
userspace interact with the underlying control. An RO control will do
this exactly by loading the data and providing no write access.

Signed-off-by: Curtis Malainey cujomalainey@chromium.org

[cujomalainey]

Also if anyone can explain why tlv_callback is a must I would appreciate it 😃

[cujomalainey]

Also if anyone can explain why tlv_callback is a must I would appreciate it smiley

or is that simply because SOF is generating controls backed by the driver itself rather than something like a regmap and so the control is tied to function callback in SOF otherwise it would have nothing to actually control?

[lgirdwood]

Also if anyone can explain why tlv_callback is a must I would appreciate it smiley

or is that simply because SOF is generating controls backed by the driver itself rather than something like a regmap and so the control is tied to function callback in SOF otherwise it would have nothing to actually control?

@ujfalusi can you comment on why this is the case?

[lgirdwood]

@ranj063 fyi - will need this in topology2

[gkbldcig]

Can one of the admins verify this patch?

[ujfalusi]

Also if anyone can explain why tlv_callback is a must I would appreciate it smiley

or is that simply because SOF is generating controls backed by the driver itself rather than something like a regmap and so the control is tied to function callback in SOF otherwise it would have nothing to actually control?

@ujfalusi can you comment on why this is the case?

The kernel needs the tlv_callback for byte controls:

		/* TLV bytes controls need standard kcontrol info handler,
		 * TLV callback and extended put/get handlers.
		 */

@cujomalainey, so the idea is that you would have the binary content in the tplg file and when parsed the 'blob' is set to the DSP, it is just a static chunk of data?
I need to check look this up, I'm really not sure how a kcontrol w/o write access behaves. @juimonen, @ranj063, do you have quick hunch on this?

[cujomalainey]

@ujfalusi that is correct, this is a work around for components we do not have confidence in that need to load data. This way they can still receive a custom config payload without being exposed to userspace.

[plbossart]

@cujomalainey I don't think we should even have the ability to write configuration data from the topology in case of RO controls. We are playing with fire here.

[cujomalainey]

@cujomalainey I don't think we should even have the ability to write configuration data from the topology in case of RO controls. We are playing with fire here.

Not in my testing, I can see the cmd handler being called with a blob from the FW.

[cujomalainey]

@plbossart note the volatile RO control also carries a data payload option.

[cujomalainey]

Ping to @plbossart as this is a high priority need

[plbossart]

I don't understand the concept at all but I won't lay on the tracks if this solves a problem. @ranj063 should review this really to check if this solution can be used with topology2 as well, otherwise we'll have the same problem later.

[ranj063]

I don't understand the concept at all but I won't lay on the tracks if this solves a problem. @ranj063 should review this really to check if this solution can be used with topology2 as well, otherwise we'll have the same problem later.

@cujomalainey @plbossart I am aware of the loophole with RO controls that allows us to send initial data to the firmware. I was planning on removing this in the kernel. In the case that the data size is less than the size of max IPC including the size of the component IPC struct, it should be OK. But if thats not the case, it will break what it was intended for. As for tplg2, there would be no difference in behavior

[cujomalainey]

@ranj063 we will be needing this for larger IPCs as well, can we no remove this in the short term and make sure it actually works with large payloads?

[ranj063]

@ranj063 we will be needing this for larger IPCs as well, can we no remove this in the short term and make sure it actually works with large payloads?

@cujomalainey it works with regular and large payloads today. But if I remove performing a set for RO controls, it will not. Is sending larger payloads a requirement even for existing platforms?

[lgirdwood]

@plbossart are we good from kernel side to merge this PR and address any kernel update in another PR or is there a dependency here ?

[plbossart]

@lgirdwood I don't think there's any dependencies at the kernel level, however I am not clear as to who will use this new C_CONTROLBYTES_READONLY macro. @cujomalainey are we missing a patch to show how this macro helps?

[cujomalainey]

I can provide one. Basically it's a drop in replacement for regular bytes control. It will be used in tandem with #5618 as one example. DSM will be another target until we follow up some processes offline. This is temporary.

[cujomalainey]

Apologies for the delay, here is an example usage

CONTROLBYTES_PRIV(DEF_RTNR_PRIV,
   { config data }
)

# RTNR Bytes control with max value of 255
C_CONTROLBYTES_READONLY(DEF_RTNR_BYTES, PIPELINE_ID,
        CONTROLBYTES_OPS(bytes, 258 binds the mixer control to bytes get/put handlers, 258),
        CONTROLBYTES_EXTOPS(258 binds the mixer control to bytes get/put handlers, 258),
        , , ,
        CONTROLBYTES_MAX(, 256),
        ,
        DEF_RTNR_PRIV)

[lgirdwood]

Good for me. @plbossart ?
@ranj063 are we covered for this in tplg2 ?

[plbossart]

Apologies for the delay, here is an example usage

CONTROLBYTES_PRIV(DEF_RTNR_PRIV,
   { config data }
)

# RTNR Bytes control with max value of 255
C_CONTROLBYTES_READONLY(DEF_RTNR_BYTES, PIPELINE_ID,
        CONTROLBYTES_OPS(bytes, 258 binds the mixer control to bytes get/put handlers, 258),
        CONTROLBYTES_EXTOPS(258 binds the mixer control to bytes get/put handlers, 258),
        , , ,
        CONTROLBYTES_MAX(, 256),
        ,
        DEF_RTNR_PRIV)

this is an out-of-tree example @cujomalainey ?

[cujomalainey]

Yes that correct

[ranj063]

Good for me. @plbossart ? @ranj063 are we covered for this in tplg2 ?

@lgirdwood there're no specific changes required for adding a RO byte control in tplg2. It should just work

[plbossart]

we'll likely have to revisit this but approving for now.

[cujomalainey]

we'll likely have to revisit this but approving for now.

Hopefully by that time we won't need it anymore