Am Dienstag, dem 07.12.2021 um 08:07 -0600 schrieb Adam Ford:

quoted

quoted

quoted

quoted

quoted

[...]

quoted

quoted

quoted

quoted

quoted

I think it's important to clarify that you are breaking support
for the previous device-tree binding. Not only because of the compatible
string change, but because the binding is now quite different.

Note that in the past Benjamin tried to avoid this.
IIRC, his proposal was backwards compatible.

I was looking at how to do this, but the stand-alone vpu bindings did
the reset and the fuses manually, it causes issues.  When the block
control is enabled those memory locations for the resets and fuses are
assigned to the blk-ctrl driver, so attempting to access them from a
different driver was a violation.

When I started looking at this, the stand-alone VPU was trying to be
both G1 and G2, but when I was testing it before I made changes, I
didn't see the G2 function at all. When I was done the G2 seemed to
work, so it seems like this is an improvement.  If you want me to keep
the previous compatible flag, I could rename the nxp,imx8mq-vpu-g1
back to nxp,imx8mq-vpu and remove the reset/fuse controls, but I'd
have to add the blk-ctrl reference, so it seemed to me like a better
alternative to deprecate the old binding/driver and replace it with
the new one because of the significant changes.  Since I'd like to
rebase the i.MX8M Mini I did on this work, it seemed weird to have
nxp,imx8mq-vpu, nxp,imx8mq-vpu-g2, nxp,imx8mm-vpu-g1, and
nxp,imx8mm-vpu-g2 where the only one without a Gx name was the
original 8MQ binding but limited to G1 functionality and the G2
stripped out.

I would very much appreciate if we could keep the driver code for the
old binding. It does not need to have any new additional functionality,
but just keep the existing G1 h.264 decode when booted on a old DT with
the old VPU description and no blk-ctrl, so we don't regress
functionality when a new kernel is booted with a old DT.

New functionality with the G2 can depend on the new VPU binding and the
blk-ctrl driver.

How does that work when both the original VPU and the blk-ctrl attempt
to manipulate the reset and clock lines?  The original binding for the
vpu was assigned:

reg = <0x38300000 0x10000>,
          <0x38310000 0x10000>,
          <0x38320000 0x10000>;
reg-names = "g1", "g2", "ctrl";

If G2 is going to run from 38310000 and vpu-blk-ctrl run from
38320000, they'll collide.

It's not going to work, but it also doesn't have to. Either you have a
old DT where the VPU driver will poke the blk-ctrl registers, but no
blk-ctrl driver, or you have a new DT where the VPU driver leaves the
blk-ctrl region alone and the blk-ctrl driver needs to handle it.

Just don't support mixing the old VPU DT binding with the new blk-ctrl
way of doing things. The only thing that needs to keep working is a
unchannged old DT, where the VPU uses the old binding, but no blk-ctrl
is present as a separate node.

I think I understand.  I'll leave the old code for the old binding in
the driver and add the new code for the new bindings with blk-ctrl.
Once the device tree is migrated to the new bindings, the old one
becomes harmless, but still works with old device trees lacking the
blk-ctrl.  That makes sense.  In my head, I wasn't thinking about
mixing older device trees.

Exactly. While most people don't use it this way, the kernel and DT are
supposed to be independent, e.g. the DTB could be included in the
device firmware, while the kernel could be updated via a distro.

To not break this use-case without a good reason, new kernels should
try to not regress functionality with a existing binary DT. We can
mandate DT updates for new functionality (like being able to use the G2
core only with the new blk-ctrl + split VPU binding), but we shouldn't
break existing features if there isn't a very good reason to do so.
Keeping the bit of code in the VPU driver to keep the old G1 only
decoding working with a new kernel isn't much of a burden IMHO, so we
should try to keep it alive.

Regards,
Lucas


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel