LinuxLists.cc - [PATCH 0/7] staging: vc04_services: Some dead code removal

2018-10-17 09:57:38

by Dave Stevenson

[permalink] [raw]

Subject: Re: [PATCH 0/7] staging: vc04_services: Some dead code removal

On Mon, 15 Oct 2018 at 17:27, Eric Anholt <[email protected]> wrote:
>
> Stefan Wahren <[email protected]> writes:
>
> > Hi Tuomas,
> >
> >> Tuomas Tynkkynen <[email protected]> hat am 4. Oktober 2018 um 11:37 geschrieben:
> >>
> >>
> >> Drop various pieces of dead code from here and there to get rid of
> >> the remaining users of VCHI_CONNECTION_T. After that we get to drop
> >> entire header files worth of unused code.
> >>
> >> I've tested on a Raspberry Pi Model B (bcm2835_defconfig) that
> >> snd-bcm2835 can still play analog audio just fine.
> >>
> >
> > thanks and i'm fine with your patch series:
> >
> > Acked-by: Stefan Wahren <[email protected]>
> >
> > Unfortunately this would break compilation of the downstream vchi
> > drivers like vcsm [1]. Personally i don't want to maintain another
> > one, because i cannot see the gain of the resulting effort.
> >
> > [1] - https://github.com/raspberrypi/linux/tree/rpi-4.14.y/drivers/char/broadcom/vc_sm

I'm happy enough to work around these changes. Once the change is in a
released kernel we can merge a modified version of vc_sm into the
downstream kernel branch. It's not as if it requires big changes.

> I think the main concern would be if we removed things necessary for
> 6by9's new vcsm (the one that will let us do dma-buf sharing between
> media decode and DRM).

The new vcsm uses the same VCHI service as the existing downstream vc_sm driver.
The video codec driver don't use any VCHI functionality over and above
the camera. It goes via a slightly extended version of the
mmal-vchiq.c, which I have split out into a shared module.

> On the other hand, git revert is a thing, so it's not like we actually
> lose anything.

:-)

Dave

2018-10-17 10:20:12

by Dan Carpenter

[permalink] [raw]

Subject: Re: [PATCH 0/7] staging: vc04_services: Some dead code removal

On Sat, Oct 06, 2018 at 12:18:38PM +0200, Stefan Wahren wrote:
> Hi Tuomas,
>
> > Tuomas Tynkkynen <[email protected]> hat am 4. Oktober 2018 um 11:37 geschrieben:
> >
> >
> > Drop various pieces of dead code from here and there to get rid of
> > the remaining users of VCHI_CONNECTION_T. After that we get to drop
> > entire header files worth of unused code.
> >
> > I've tested on a Raspberry Pi Model B (bcm2835_defconfig) that
> > snd-bcm2835 can still play analog audio just fine.
> >
>
> thanks and i'm fine with your patch series:
>
> Acked-by: Stefan Wahren <[email protected]>
>
> Unfortunately this would break compilation of the downstream vchi
> drivers like vcsm [1]. Personally i don't want to maintain another
> one, because i cannot see the gain of the resulting effort.
>
> [1] - https://github.com/raspberrypi/linux/tree/rpi-4.14.y/drivers/char/broadcom/vc_sm

I feel like everyone else already knows the answer but why don't we just
merge that code into staging?

regards,
dan carpenter

2018-10-17 10:53:36

by Stefan Wahren

[permalink] [raw]

Subject: Re: [PATCH 0/7] staging: vc04_services: Some dead code removal

Hi,

Am 17.10.2018 um 11:55 schrieb Dave Stevenson:
> On Mon, 15 Oct 2018 at 17:27, Eric Anholt <[email protected]> wrote:
>> Stefan Wahren <[email protected]> writes:
>>
>>> Hi Tuomas,
>>>
>>>> Tuomas Tynkkynen <[email protected]> hat am 4. Oktober 2018 um 11:37 geschrieben:
>>>>
>>>>
>>>> Drop various pieces of dead code from here and there to get rid of
>>>> the remaining users of VCHI_CONNECTION_T. After that we get to drop
>>>> entire header files worth of unused code.
>>>>
>>>> I've tested on a Raspberry Pi Model B (bcm2835_defconfig) that
>>>> snd-bcm2835 can still play analog audio just fine.
>>>>
>>> thanks and i'm fine with your patch series:
>>>
>>> Acked-by: Stefan Wahren <[email protected]>
>>>
>>> Unfortunately this would break compilation of the downstream vchi
>>> drivers like vcsm [1]. Personally i don't want to maintain another
>>> one, because i cannot see the gain of the resulting effort.
>>>
>>> [1] - https://github.com/raspberrypi/linux/tree/rpi-4.14.y/drivers/char/broadcom/vc_sm
> I'm happy enough to work around these changes. Once the change is in a
> released kernel we can merge a modified version of vc_sm into the
> downstream kernel branch. It's not as if it requires big changes.
>
>> I think the main concern would be if we removed things necessary for
>> 6by9's new vcsm (the one that will let us do dma-buf sharing between
>> media decode and DRM).
> The new vcsm uses the same VCHI service as the existing downstream vc_sm driver.
> The video codec driver don't use any VCHI functionality over and above
> the camera. It goes via a slightly extended version of the
> mmal-vchiq.c, which I have split out into a shared module.

my statement about the old vc_sm based on assumption there wont be a
user of this driver.

In case the camera driver would use the new vc_sm driver, i would be
happier to see this merged in staging than in downstream.

Stefan

>
>> On the other hand, git revert is a thing, so it's not like we actually
>> lose anything.
> :-)
>
> Dave

2018-10-17 15:38:24

Hi Stefan

On Sun, 28 Oct 2018 at 08:31, Stefan Wahren <[email protected]> wrote:
>
> Hi Dave,
>
> > Dave Stevenson <[email protected]> hat am 26. Oktober 2018 um 19:15 geschrieben:
> >
> >
> > Thanks Stefan.
> > I've picked up your latest patches which mean I can get the driver
> > loaded via the (almost) approved method.
> > I do seem to still have issues with not getting the expected address
> > ranges, so the driver/VPU was trying to map cached alias memory. As
> > your patches only came through yesterday I haven't had a chance to dig
> > through why yet. I've done a temporary hack to ensure we always map
> > the uncached alias, but that can't persist.
>
> does it mean with DT probing it worked before and with platform change it's broken?
> Or anything else cause this regression in 4.19?

Yes, probing via DT with the node under soc gave me the correct DMA
addresses (uncached alias). With the platform changes I get the cached
alias.
Both were under 4.14. I'll try again on a later branch.

> > The networking issue has been resolved :-)
> >
> > I've pushed where I've got to to
> > https://github.com/6by9/linux/tree/rpi-4.14.y-codecs-push-pt2b
> > It's a touch messy due to integrating in your patches in the last 24
> > hours. It needs a full rebase so that my changes are on top of yours
> > rather than haphazard.
> > As we're moving to 4.19 fairly soon I may well abandon my 4.14 tree
> > and jump to either that or directly on staging. I'll see where I get
> > to early next week.
>
> Sorry, but there is no need for a quick shot against a downstream 4.14. I assumed you make your changes against upstream linux-next + Phil's and my patches.
>
> You can use https://github.com/anholt/linux/commits/bcm2835-audio until 4.20-rc1 is out.
> Using 4.14 or 4.19 doesn't make any sense to me.

As an employee of Raspberry Pi Trading my first responsibilty is to
them, and that means LTS releases feeding in to the downstream kernel.
If these drivers can be pushed upstream then that's a win as it avoids
divergence, but it is not my main goal. At least 4.19 and 4.20 aren't
far apart so there are likely to be fewer differences.

I had it all working on 4.14, therefore it made sense to see whether
your changes allowed me to load as a platform driver. It did, but with
this little niggle over cache aliases. I'll try again on a later
kernel and try to get some more info.

Dave