On 22-06-2023 17:37, Mark Brown wrote:
On Thu, Jun 22, 2023 at 05:04:10PM +0530, Sameer Pujar wrote:
From: Sheetal <sheetal@xxxxxxxxxx>
Byte mask for channel-1 of stream-1 is not getting enabled and this
causes failures during AMX use cases. The enable bit is not set during
put() callback of byte map mixer control.
This happens because the byte map value 0 matches the initial state
of byte map array and put() callback returns without doing anything.
Fix the put() callback by actually looking at the byte mask array
to identify if any change is needed and update the fields accordingly.
I'm not quite sure I follow the logic here - I'd have expected this to
mean that there's a bootstrapping issue and that we should be doing some
more initialisation during startup such that the existing code which
checks if there is a change will be doing the right thing?
The issue can happen in subsequent cycles as well if once the user
disables the byte map by putting 256. It happens because of following
reason where 256 value is reset to 0 since the byte map array is tightly
packed and it can't store 256 value.
static int tegra210_amx_put_byte_map() {
...
if (value >= 0 && value <= 255)
mask_val |= (1 << (reg % 32));
else
mask_val &= ~(1 << (reg % 32));
if (mask_val == amx->byte_mask[reg / 32])
return 0;
/* Update byte map and slot */
==> bytes_map[reg] = value % 256;
amx->byte_mask[reg / 32] = mask_val;
return 1;
}
Also update get() callback to return 256 if the byte map is disabled.
This will be a user visible change. It's not clear to me why it's
needed - it seems like it's a hack to push users to do an update in the
case where they want to use channel 1 stream 1?
Though it looks like 256 value is forced, but actually the user sees
whatever value is set before. The 256 value storage is linked to byte
mask value.
I must admit that this is not easily readable. If you suggest to
simplify this, I can check if storage space increase for byte map value
can make it more readable. Thanks for your feedback.
