[PATCH] staging: mt7621-mmc: Add blank line after declaration
From: Nícolas F. R. A. Prado
Correct the following warning from checkpatch.pl:
WARNING: Missing a blank line after declarations
+ struct msdc_host *host = mmc_priv(mmc);
+ msdc_pm(state, (void *)host);
Signed-off-by: Nícolas F. R. A. Prado
---
Hi, this is my first commit. Let me know if there's anything I can do
better.
---
drivers/staging/mt7621-mmc/sd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 0379f9c96..7b66f9b0a 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -1797,6 +1797,7 @@ static void msdc_drv_pm(struct platform_device *pdev,
pm_message_t state)
if (mmc) {
struct msdc_host *host = mmc_priv(mmc);
+
msdc_pm(state, (void *)host);
}
}
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: ICH BRAUCHE IHRE FINANZIELLE UNTERSTÜTZUNG!!
Von: Olivia Nicolas Liebste, Guten Tag und danke für Ihre Aufmerksamkeit. Bitte ich möchte, dass Sie meine E-Mail sorgfältig lesen und mir helfen, dieses Projekt zu bearbeiten. Ich bin Fräulein Olivia Nicolas und ich schreibe demütig für Ihre Partnerschaft und Unterstützung bei der Übertragung und Investition meiner Erbschaft US $ 6.500.000,00 (Sechs Millionen fünfhunderttausend US-Dollar), die mein verstorbener geliebter Vater in einer Bank hinterlegt hat, bevor er starb. Ich möchte Ihnen versichern, dass dieser Fonds von meinem verstorbenen Vater legal erworben wurde und keinen kriminellen Hintergrund hat. Mein Vater hat diesen Fonds legal erworben, bevor er während seiner Geschäftsreise zu Tode vergiftet wurde. Der Tod meines Vaters wurde verdächtigt, von seinen Verwandten geplant zu werden, die während dieser Zeit seiner Geschäftsreise mit ihm reisten. Weil nach 3 Monaten des Todes meines Vaters seine Verwandten alle Eigenschaften meines verstorbenen Vaters in Anspruch nahmen und verkauften. Die Verwandten meines verstorbenen Vaters wissen nichts von den sechs Millionen US-Dollar, die mein verstorbener Vater bei der Bank deponiert hat, und mein verstorbener Vater hat mir vor seinem Tod heimlich gesagt, dass ich in jedem Land nach einem ausländischen Partner suchen sollte meiner Wahl, wo ich dieses Geld für meine eigenen Zwecke übertrage. Bitte helfen Sie mir, dieses Geld für geschäftliche Zwecke in Ihrem Land auf Ihr Konto zu überweisen. Ich habe diese Entscheidung getroffen, weil ich von den Verwandten meines verstorbenen Vaters eine Menge Demütigungen erlitten habe. Zur Zeit habe ich mit dem Direktor der Bank gesprochen, wo mein verstorbener Vater dieses Geld hinterlegt hat. Ich habe dem Bankdirektor erklärt, wie dringend es ist, dass der Fonds ins Ausland transferiert wird, damit ich dieses Land zu meiner Sicherheit verlassen kann. Der Direktor der Bank hat mir versichert, dass der Fonds transferiert wird, sobald ich jemanden überbringe, der ehrlich ist, den Fonds in meinem Namen zu diesem Zweck zu erhalten. Bitte seien Sie versichert, dass die Bank den Fonds auf Ihr Konto überweisen wird und es kein Problem geben wird. Diese Transaktion ist 100% risikofrei und legitim. Ich bin bereit, Ihnen 30% des Gesamtbetrages als Entschädigung für Ihre Bemühungen / Eingaben nach der erfolgreichen Überweisung dieses Fonds auf Ihr Konto anzubieten. Sie werden mir auch helfen, 10% an Wohltätigkeitsorganisationen und Mütterlose Babys in Ihrem Land zu spenden. Bitte, alles was ich möchte, dass du für mich tust, ist, als mein ausländischer Partner zu stehen, so dass die Bank diesen Fonds auf dein Konto überweist, damit ich dieses Land leben kann. Bitte, ich brauche jetzt dringend Hilfe wegen meines jetzigen Zustandes. Auf Ihre volle Annahme, mit mir zu diesem Zweck zu arbeiten, geben Sie bitte Ihr Interesse durch Antwort zurück zu mir, damit ich Ihnen die notwendigen Informationen und die Details, wie weiter verfahren werde, gebe ich Ihnen 30% des Geldes für Ihre Hilfe und Hilfe, damit umzugehen. Ihre dringende Antwort wird geschätzt. Freundliche Grüße Olivia Nicolas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: ICH BRAUCHE IHRE FINANZIELLE UNTERSTÜTZUNG!!
Von: Olivia Nicolas Liebste, Guten Tag und danke für Ihre Aufmerksamkeit. Bitte ich möchte, dass Sie meine E-Mail sorgfältig lesen und mir helfen, dieses Projekt zu bearbeiten. Ich bin Fräulein Olivia Nicolas und ich schreibe demütig für Ihre Partnerschaft und Unterstützung bei der Übertragung und Investition meiner Erbschaft US $ 6.500.000,00 (Sechs Millionen fünfhunderttausend US-Dollar), die mein verstorbener geliebter Vater in einer Bank hinterlegt hat, bevor er starb. Ich möchte Ihnen versichern, dass dieser Fonds von meinem verstorbenen Vater legal erworben wurde und keinen kriminellen Hintergrund hat. Mein Vater hat diesen Fonds legal erworben, bevor er während seiner Geschäftsreise zu Tode vergiftet wurde. Der Tod meines Vaters wurde verdächtigt, von seinen Verwandten geplant zu werden, die während dieser Zeit seiner Geschäftsreise mit ihm reisten. Weil nach 3 Monaten des Todes meines Vaters seine Verwandten alle Eigenschaften meines verstorbenen Vaters in Anspruch nahmen und verkauften. Die Verwandten meines verstorbenen Vaters wissen nichts von den sechs Millionen US-Dollar, die mein verstorbener Vater bei der Bank deponiert hat, und mein verstorbener Vater hat mir vor seinem Tod heimlich gesagt, dass ich in jedem Land nach einem ausländischen Partner suchen sollte meiner Wahl, wo ich dieses Geld für meine eigenen Zwecke übertrage. Bitte helfen Sie mir, dieses Geld für geschäftliche Zwecke in Ihrem Land auf Ihr Konto zu überweisen. Ich habe diese Entscheidung getroffen, weil ich von den Verwandten meines verstorbenen Vaters eine Menge Demütigungen erlitten habe. Zur Zeit habe ich mit dem Direktor der Bank gesprochen, wo mein verstorbener Vater dieses Geld hinterlegt hat. Ich habe dem Bankdirektor erklärt, wie dringend es ist, dass der Fonds ins Ausland transferiert wird, damit ich dieses Land zu meiner Sicherheit verlassen kann. Der Direktor der Bank hat mir versichert, dass der Fonds transferiert wird, sobald ich jemanden überbringe, der ehrlich ist, den Fonds in meinem Namen zu diesem Zweck zu erhalten. Bitte seien Sie versichert, dass die Bank den Fonds auf Ihr Konto überweisen wird und es kein Problem geben wird. Diese Transaktion ist 100% risikofrei und legitim. Ich bin bereit, Ihnen 30% des Gesamtbetrages als Entschädigung für Ihre Bemühungen / Eingaben nach der erfolgreichen Überweisung dieses Fonds auf Ihr Konto anzubieten. Sie werden mir auch helfen, 10% an Wohltätigkeitsorganisationen und Mütterlose Babys in Ihrem Land zu spenden. Bitte, alles was ich möchte, dass du für mich tust, ist, als mein ausländischer Partner zu stehen, so dass die Bank diesen Fonds auf dein Konto überweist, damit ich dieses Land leben kann. Bitte, ich brauche jetzt dringend Hilfe wegen meines jetzigen Zustandes. Auf Ihre volle Annahme, mit mir zu diesem Zweck zu arbeiten, geben Sie bitte Ihr Interesse durch Antwort zurück zu mir, damit ich Ihnen die notwendigen Informationen und die Details, wie weiter verfahren werde, gebe ich Ihnen 30% des Geldes für Ihre Hilfe und Hilfe, damit umzugehen. Ihre dringende Antwort wird geschätzt. Freundliche Grüße Olivia Nicolas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drm/bridge: anx7625: enable DSI EOTP
On Thu, Feb 4, 2021 at 8:07 PM Robert Foss wrote:
>
> Hi Xin,
>
> Thanks for the patch.
>
> On Thu, 28 Jan 2021 at 12:17, Xin Ji wrote:
> >
> > Enable DSI EOTP feature for fixing some panel screen constance
> > shift issue.
> > Removing MIPI flag MIPI_DSI_MODE_EOT_PACKET to enable DSI EOTP.
>
> I don't think I quite understand how removing the
> MIPI_DSI_MODE_EOT_PACKET flag will cause DSI EOTP to be enabled. Could
> you extrapolate on this in the commit message?
That confused me as well, but it turns out that's how the flag is defined:
```
/* disable EoT packets in HS mode */
#define MIPI_DSI_MODE_EOT_PACKET BIT(9)
```
(https://elixir.bootlin.com/linux/latest/source/include/drm/drm_mipi_dsi.h#L129)
I'm almost tempted to put together a mass patch to rename all of these flags...
>
> >
> > Signed-off-by: Xin Ji
> > ---
> > drivers/gpu/drm/bridge/analogix/anx7625.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index 65cc059..e31eeb1b 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > @@ -1334,7 +1334,6 @@ static int anx7625_attach_dsi(struct anx7625_data
> > *ctx)
> > dsi->format = MIPI_DSI_FMT_RGB888;
> > dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> > MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> > - MIPI_DSI_MODE_EOT_PACKET|
> > MIPI_DSI_MODE_VIDEO_HSE;
> >
> > if (mipi_dsi_attach(dsi) < 0) {
> > --
> > 2.7.4
> >
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drm/bridge: anx7625: enable DSI EOTP
On Thu, Feb 4, 2021 at 8:59 PM Andrzej Hajda wrote:
>
>
> W dniu 04.02.2021 o 13:34, Nicolas Boichat pisze:
> > On Thu, Feb 4, 2021 at 8:07 PM Robert Foss wrote:
> >> Hi Xin,
> >>
> >> Thanks for the patch.
> >>
> >> On Thu, 28 Jan 2021 at 12:17, Xin Ji wrote:
> >>> Enable DSI EOTP feature for fixing some panel screen constance
> >>> shift issue.
> >>> Removing MIPI flag MIPI_DSI_MODE_EOT_PACKET to enable DSI EOTP.
> >> I don't think I quite understand how removing the
> >> MIPI_DSI_MODE_EOT_PACKET flag will cause DSI EOTP to be enabled. Could
> >> you extrapolate on this in the commit message?
> > That confused me as well, but it turns out that's how the flag is defined:
> > ```
> > /* disable EoT packets in HS mode */
> > #define MIPI_DSI_MODE_EOT_PACKET BIT(9)
> > ```
> > (https://protect2.fireeye.com/v1/url?k=5bd95ebd-044267fb-5bd8d5f2-0cc47a3003e8-ce9db8ea264d6901&q=1&e=900556dc-d199-4c18-9432-5c3465a98eae&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Finclude%2Fdrm%2Fdrm_mipi_dsi.h%23L129)
> >
> > I'm almost tempted to put together a mass patch to rename all of these
> > flags...
>
>
> Yes that would be good, many of these flags were just copy pasted from
> some hw datasheet, without good analysis how to adapt them to the framework.
I'll look into it (but that shouldn't block this patch).
>
>
> Regards
>
> Andrzej
>
>
> >
> >>> Signed-off-by: Xin Ji
> >>> ---
> >>> drivers/gpu/drm/bridge/analogix/anx7625.c | 1 -
> >>> 1 file changed, 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>> index 65cc059..e31eeb1b 100644
> >>> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>> @@ -1334,7 +1334,6 @@ static int anx7625_attach_dsi(struct anx7625_data
> >>> *ctx)
> >>> dsi->format = MIPI_DSI_FMT_RGB888;
> >>> dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> >>> MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> >>> - MIPI_DSI_MODE_EOT_PACKET|
> >>> MIPI_DSI_MODE_VIDEO_HSE;
> >>>
> >>> if (mipi_dsi_attach(dsi) < 0) {
> >>> --
> >>> 2.7.4
> >>>
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://protect2.fireeye.com/v1/url?k=457f3f39-1ae4067f-457eb476-0cc47a3003e8-b702072da729d8c9&q=1&e=900556dc-d199-4c18-9432-5c3465a98eae&u=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel
> >
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC RESEND 0/3] vp9 v4l2 stateless uapi
Le lundi 26 avril 2021 à 09:38 +0200, Hans Verkuil a écrit : > Hi Andrzej, > > Thank you for working on this! > > On 21/04/2021 12:00, Andrzej Pietrasiewicz wrote: > > Dear All, > > > > This is an RFC on stateless uapi for vp9 decoding with v4l2. This work is > > based on https://lkml.org/lkml/2020/11/2/1043, but has been substantially > > reworked. The important change is that the v4l2 control used to pass > > boolean decoder probabilities has been made unidirectional, and is now > > called V4L2_CID_STATELESS_VP9_COMPRESSED_HDR_PROBS. > > > > In the previous proposal, to queue a frame the userspace must fully dequeue > > the previous one, which effectively results in a forced lockstep behavior > > and defeats vb2's capability to enqueue multiple buffers. Such a design was > > a consequence of backward probability updates being performed by the kernel > > driver (which has direct access to appropriate counter values) but forward > > probability updates being coupled with compressed header parsing performed > > by the userspace. > > > > In vp9 the boolean decoder used to decode the bitstream needs certain > > parameters to work. Those are probabilities, which change with each frame. > > After each frame is decoded it is known how many times a given symbol > > occured in the frame, so the probabilities can be adapted. This process is > > known as backward probabilities update. A next frame header can also > > contain information which modifies probabilities resulting from backward > > update. The said modification is called forward probabilities update. The > > data for backward update is generated by the decoder hardware, while the > > data for forward update is prepared by reading the compressed frame header. > > The natural place to parse something is userspace, while the natural place > > to access hardware-provided counters is the kernel. Such responsibilties > > assignment was used in the original work. > > > > To overcome the lockstep, we moved forward probability updates to the > > kernel, while leaving parsing them in userspace. This way the v4l2 control > > which is used to pass the probs becomes unidirectional (user->kernel) and > > the userspace can keep parsing and enqueueing succeeding frames. > > > > If a particular driver parses the compressed header and does backward > > probability updates on its own then > > V4L2_CID_STATELESS_VP9_COMPRESSED_HDR_PROBS does not need to be used. > > > > This series adds vp9 uapi in proper locations, which means it is a proper, > > "official" uapi, as opposed to staging uapi which was proposed in the above > > mentioned lkml thread. > > Why? I rather liked the way that the other codec APIs started life in a > private header > (like include/media/vp8-ctrls.h) and were given time to mature before moving > them to > the uAPI. Is there a reason why you think that VP9 doesn't need that? I'll be honest, I accepted early code into GStreamer for H264, and it ended up in a nightmare for the users. We now have a released GStreamer that supports kernel API up to 5.9, a blackwhole at 5.10 and finally master catched up and can support 5.11+. It is so complicated for packagers to understand what is going on, that they endup wasting a lot of their time for a single feature in their OS. Same breakage is happening for VP8 in 5.13, even though VP8 has been working great all this time. I will for sure for now on ignore any contribution that depends on staged uAPI. As for FFMPEG, even though now H264 API is table, the maintainers just simply ignore the patches as they have been bitten by the reviewing stuff based on unstable APIs and downstream work. I believe the staged uAPI has been used wrongly in the past. Stuff has been staged quicky right before associated project budget for it was exhausted, so it was in the end a way to look good, and someone else had to pick it up and finish it. Going straight for final API put more pressure on making good research from the start, doing more in-depth reviews and avoiding delaying for multiple years the support. I believe the staging API are confusing even for the Linux projects. Going straight to stable here is a commitment to finish this work and doing it correctly. This specially make sense for VP9, which is a very Open CODEC and were all HW implementation are Google/Hantro derivatives. Also, unlike when this work all started, we do have multiple HW we can look at to validate the API, with more then enough in-depth information to make the right decisions. > > > > > The series adds vp9 support to rkvdec driver. > > > > Rebased onto media_tree. > > > > I kindly ask for your comments. > > > > TODO: > > > > - potentially fine-tune the uAPI (add/remove fields, move between structs) > > - write another driver (intended g2 @ iMX8) The commitment is subtly describe here, the commitment is to implement a second driver, Hantro G2 which has a different design), and that even if we have no use fo
Re: [RFC RESEND 0/3] vp9 v4l2 stateless uapi
Le jeudi 29 avril 2021 à 11:23 +0200, Hans Verkuil a écrit : > On 27/04/2021 01:34, Ezequiel Garcia wrote: > > On Mon, 26 Apr 2021 at 14:38, Nicolas Dufresne wrote: > > > > > > Le lundi 26 avril 2021 à 09:38 +0200, Hans Verkuil a écrit : > > > > Hi Andrzej, > > > > > > > > Thank you for working on this! > > > > > > > > On 21/04/2021 12:00, Andrzej Pietrasiewicz wrote: > > > > > Dear All, > > > > > > > > > > This is an RFC on stateless uapi for vp9 decoding with v4l2. This > > > > > work is based on https://lkml.org/lkml/2020/11/2/1043, but has been > > > > > substantially reworked. The important change is that the v4l2 control > > > > > used to pass boolean decoder probabilities has been made > > > > > unidirectional, and is now called > > > > > V4L2_CID_STATELESS_VP9_COMPRESSED_HDR_PROBS. > > > > > > > > > > In the previous proposal, to queue a frame the userspace must fully > > > > > dequeue the previous one, which effectively results in a forced > > > > > lockstep behavior and defeats vb2's capability to enqueue multiple > > > > > buffers. Such a design was a consequence of backward probability > > > > > updates being performed by the kernel driver (which has direct access > > > > > to appropriate counter values) but forward probability updates being > > > > > coupled with compressed header parsing performed by the userspace. > > > > > > > > > > In vp9 the boolean decoder used to decode the bitstream needs certain > > > > > parameters to work. Those are probabilities, which change with each > > > > > frame. After each frame is decoded it is known how many times a given > > > > > symbol occured in the frame, so the probabilities can be adapted. > > > > > This process is known as backward probabilities update. A next frame > > > > > header can also contain information which modifies probabilities > > > > > resulting from backward update. The said modification is called > > > > > forward probabilities update. The data for backward update is > > > > > generated by the decoder hardware, while the data for forward update > > > > > is prepared by reading the compressed frame header. The natural place > > > > > to parse something is userspace, while the natural place to access > > > > > hardware-provided counters is the kernel. Such responsibilties > > > > > assignment was used in the original work. > > > > > > > > > > To overcome the lockstep, we moved forward probability updates to the > > > > > kernel, while leaving parsing them in userspace. This way the v4l2 > > > > > control which is used to pass the probs becomes unidirectional > > > > > (user->kernel) and the userspace can keep parsing and enqueueing > > > > > succeeding frames. > > > > > > > > > > If a particular driver parses the compressed header and does backward > > > > > probability updates on its own then > > > > > V4L2_CID_STATELESS_VP9_COMPRESSED_HDR_PROBS does not need to be used. > > > > > > > > > > This series adds vp9 uapi in proper locations, which means it is a > > > > > proper, "official" uapi, as opposed to staging uapi which was > > > > > proposed in the above mentioned lkml thread. > > > > > > > > Why? I rather liked the way that the other codec APIs started life in a > > > > private header > > > > (like include/media/vp8-ctrls.h) and were given time to mature before > > > > moving them to > > > > the uAPI. Is there a reason why you think that VP9 doesn't need that? > > > > > > I'll be honest, I accepted early code into GStreamer for H264, and it > > > ended up > > > in a nightmare for the users. We now have a released GStreamer that > > > supports > > > kernel API up to 5.9, a blackwhole at 5.10 and finally master catched up > > > and can > > > support 5.11+. It is so complicated for packagers to understand what is > > > going > > > on, that they endup wasting a lot of their time for a single feature in > > > their > > > OS. Same breakage is happening for VP8 in 5.13, even though VP8 has been > > > working > > > great all
Re: [PATCH v10 6/9] media: uapi: Add a control for HANTRO driver
Le mercredi 05 mai 2021 à 16:18 +0100, John Cox a écrit :
> > The HEVC HANTRO driver needs to know the number of bits to skip at
> > the beginning of the slice header.
> > That is a hardware specific requirement so create a dedicated control
> > for this purpose.
> >
> > Signed-off-by: Benjamin Gaignard
> > ---
> > .../userspace-api/media/drivers/hantro.rst| 19 +++
> > .../userspace-api/media/drivers/index.rst | 1 +
> > include/media/hevc-ctrls.h| 13 +
> > 3 files changed, 33 insertions(+)
> > create mode 100644 Documentation/userspace-api/media/drivers/hantro.rst
> >
> > diff --git a/Documentation/userspace-api/media/drivers/hantro.rst
> > b/Documentation/userspace-api/media/drivers/hantro.rst
> > new file mode 100644
> > index ..cd9754b4e005
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/drivers/hantro.rst
> > @@ -0,0 +1,19 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +Hantro video decoder driver
> > +===
> > +
> > +The Hantro video decoder driver implements the following driver-specific
> > controls:
> > +
> > +``V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP (integer)``
> > +Specifies to Hantro HEVC video decoder driver the number of data (in
> > bits) to
> > +skip in the slice segment header.
> > +If non-IDR, the bits to be skipped go from syntax element
> > "pic_output_flag"
> > +to before syntax element "slice_temporal_mvp_enabled_flag".
> > +If IDR, the skipped bits are just "pic_output_flag"
> > +(separate_colour_plane_flag is not supported).
>
> What happens if it is a dependant_slice_segement or
> output_flag_present_flag? Those flags are all dependant on
> dependant_slice_segement being false. I'm guessing 0 but it maybe
> should be documented.
Zero indeed.
> Likewise if output_flag_present_flag is false pic_output_flag will not
> be coded, so maybe express it as "after slice_type" rather than "before
> pic_output_flag"?
Should work too.
>
> Regards
>
> John Cox
>
> > +.. note::
> > +
> > +This control is not yet part of the public kernel API and
> > +it is expected to change.
> > diff --git a/Documentation/userspace-api/media/drivers/index.rst
> > b/Documentation/userspace-api/media/drivers/index.rst
> > index 1a9038f5f9fa..12e3c512d718 100644
> > --- a/Documentation/userspace-api/media/drivers/index.rst
> > +++ b/Documentation/userspace-api/media/drivers/index.rst
> > @@ -33,6 +33,7 @@ For more details see the file COPYING in the source
> > distribution of Linux.
> >
> > ccs
> > cx2341x-uapi
> > +hantro
> > imx-uapi
> > max2175
> > meye-uapi
> > diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
> > index 8e0109eea454..b713eeed1915 100644
> > --- a/include/media/hevc-ctrls.h
> > +++ b/include/media/hevc-ctrls.h
> > @@ -224,4 +224,17 @@ struct v4l2_ctrl_hevc_decode_params {
> > __u64 flags;
> > };
> >
> > +/* MPEG-class control IDs specific to the Hantro driver as defined by V4L2
> > */
> > +#define
> > V4L2_CID_CODEC_HANTRO_BASE (V4L2_CTRL_CLASS_CODEC
> > | 0x1200)
> > +/*
> > + * V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP -
> > + * the number of data (in bits) to skip in the
> > + * slice segment header.
> > + * If non-IDR, the bits to be skipped go from syntax element
> > "pic_output_flag"
> > + * to before syntax element "slice_temporal_mvp_enabled_flag".
> > + * If IDR, the skipped bits are just "pic_output_flag"
> > + * (separate_colour_plane_flag is not supported).
> > + */
> > +#define V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP
> > (V4L2_CID_CODEC_HANTRO_BASE
> > + 0)
> > +
> > #endif
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC RESEND 2/3] media: uapi: Add VP9 stateless decoder controls
Hi Hans, just a partial reply, I'll let Andrzej extend. Le jeudi 29 avril 2021 à 12:20 +0200, Hans Verkuil a écrit : > > + - ``frame_width_minus_1`` > > + - Add 1 to get the frame width expressed in pixels. > > + * - __u16 > > + - ``frame_height_minus_1`` > > + - Add 1 to get the frame height expressed in pixels. > > These two fields are weird. Isn't this defined by setting the output format? > And why the 'minus_1'? > > > + * - __u16 > > + - ``render_width_minus_1`` > > + - Add 1 to get the expected render width expressed in pixels. This is > > + not used during the decoding process but might be used by HW > > scalers to > > + prepare a frame that's ready for scanout. > > + * - __u16 > > + - render_height_minus_1 > > + - Add 1 to get the expected render height expressed in pixels. This > > is > > + not used during the decoding process but might be used by HW > > scalers to > > + prepare a frame that's ready for scanout. > > No idea what these fields are about. I suspect this can be defined by setting > the capture format, but I'm not sure. We have the same for other codecs. Each codec bitstream include the coded and the display size in some form. For H264/H265 that was passed as as number of macroblock and a crop rectangle. For VP9 value - 1 is as defined in the spec. As 0 is not valid, the boolean coder can save some bits when storing the value. Though, for parameters, we usually start with the way it's encoded first, and change it only if we think it make sense. We'll take note and consider this whenever we have a second driver to look at. Now, why do we include both here. Well in fact, the HW may have some extra constraints. Which means there will be up to 3 frame sizes that matters. We can't also determine if the display/render or coded size will be used for minim CAPTURE format, as this will in fact depends if a post processor will be used or not. So to recap, we limit the CAPTURE format base on this information, and not the opposite. The width/height on OUTPUT FMT has been define as meaningless in the spec (to align with stateful I suppose ?). This way, the driver got all the information aligned with how it works inside the codec, without having to do a translation dance, and then properly implement CAPTURE TRY_FMT base on that. To make an analogy with stateful codec, this replaces the queuing of a frame that contains codec headers. We skip the SRC_CH events, since this is no longer asynchronous. Nicolas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v10 6/9] media: uapi: Add a control for HANTRO driver
Le jeudi 06 mai 2021 à 14:11 +0100, John Cox a écrit : > > On 05/05/2021 17:20, Benjamin Gaignard wrote: > > > > > > Le 05/05/2021 à 16:55, Hans Verkuil a écrit : > > > > On 20/04/2021 14:10, Benjamin Gaignard wrote: > > > > > The HEVC HANTRO driver needs to know the number of bits to skip at > > > > > the beginning of the slice header. > > > > > That is a hardware specific requirement so create a dedicated control > > > > > for this purpose. > > > > > > > > > > Signed-off-by: Benjamin Gaignard > > > > > --- > > > > > .../userspace-api/media/drivers/hantro.rst| 19 > > > > > +++ > > > > > .../userspace-api/media/drivers/index.rst | 1 + > > > > > include/media/hevc-ctrls.h| 13 + > > > > > 3 files changed, 33 insertions(+) > > > > > create mode 100644 Documentation/userspace- > > > > > api/media/drivers/hantro.rst > > > > > > > > > > diff --git a/Documentation/userspace-api/media/drivers/hantro.rst > > > > > b/Documentation/userspace-api/media/drivers/hantro.rst > > > > > new file mode 100644 > > > > > index ..cd9754b4e005 > > > > > --- /dev/null > > > > > +++ b/Documentation/userspace-api/media/drivers/hantro.rst > > > > > @@ -0,0 +1,19 @@ > > > > > +.. SPDX-License-Identifier: GPL-2.0 > > > > > + > > > > > +Hantro video decoder driver > > > > > +=== > > > > > + > > > > > +The Hantro video decoder driver implements the following driver- > > > > > specific controls: > > > > > + > > > > > +``V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP (integer)`` > > > > > +Specifies to Hantro HEVC video decoder driver the number of data > > > > > (in bits) to > > > > > +skip in the slice segment header. > > > > > +If non-IDR, the bits to be skipped go from syntax element > > > > > "pic_output_flag" > > > > > +to before syntax element "slice_temporal_mvp_enabled_flag". > > > > > +If IDR, the skipped bits are just "pic_output_flag" > > > > > +(separate_colour_plane_flag is not supported). > > > > I'm not very keen on this. Without this information the video data > > > > cannot be > > > > decoded, or will it just be suboptimal? > > > > > > Without that information the video can't be decoded. > > > > > > > > > > > The problem is that a generic decoder would have to know that the HW is > > > > a hantro, > > > > and then call this control. If they don't (and are testing on non-hantro > > > > HW), then > > > > it won't work, thus defeating the purpose of the HW independent decoder > > > > API. > > > > > > > > Since hantro is widely used, and if there is no other way to do this > > > > beside explitely > > > > setting this control, then perhaps this should be part of the standard > > > > HEVC API. > > > > Non-hantro drivers that do not need this can just skip it. > > > > > > Even if I put this parameter in decode_params structure that would means > > > that a generic > > > userland decoder will have to know how the compute this value for hantro > > > HW since it > > > isn't something that could be done on kernel side. > > > > But since hantro is very common, any userland decoder will need to calculate > > this anyway. > > So perhaps it is better to have this as part of the decode_params? > > > > I'd like to know what others think about this. > > I don't know exactly what I think on this - its all a bit of a mess. I There is no better way to describe the state of my own opinion about this. > don't think this is going to be the last HEVC decoder that needs some > non-standard setup that can't be trivially extracted from a standard > slice header parse. So if future decoders are going to have to generate > custom attributes to cope with their quirks then Hantro probably should > too. And if Hantro is common then the userspace progs will at least have > a framework for dealing with this sort of thing so when the next oddity > comes along. To add to this, when we moved it out of the decode_params, we were actually making it an example. We use large structure for the common stuff because is convenient, but with the current infrastructure, the cost of adding controls is rather low. So we need to think if we want to hide or highlight what looks like hardware design specific needs. There is nothing particularly wrong in the hardware, as Hantro traditionally parse a lot of the headers, but I suppose they don't really want to implement skip parsers because at some point the hardware becomes quite big and complex, skipping bits is just trivial. One thing I've been discussing with Benjamin yesterday is that while splitting, we also made the data exactly what the HW wants, which is a skip. A more reusable representation would have been to provide two offsets in the header. This way if another HW need a different skip, but with a different stop position, you can share the start position. Though, it's no longer a 1:1 match with how the HW is programmed, so not an easy call. As for having more quirks in more HW,
Re: [PATCH v10 6/9] media: uapi: Add a control for HANTRO driver
Le dimanche 16 mai 2021 à 20:04 -0300, Ezequiel Garcia a écrit : > Hi Hans, > > On Thu, 2021-05-06 at 14:50 +0200, Hans Verkuil wrote: > > On 05/05/2021 17:20, Benjamin Gaignard wrote: > > > > > > Le 05/05/2021 à 16:55, Hans Verkuil a écrit : > > > > On 20/04/2021 14:10, Benjamin Gaignard wrote: > > > > > The HEVC HANTRO driver needs to know the number of bits to skip at > > > > > the beginning of the slice header. > > > > > That is a hardware specific requirement so create a dedicated control > > > > > for this purpose. > > > > > > > > > > Signed-off-by: Benjamin Gaignard > > > > > --- > > > > > .../userspace-api/media/drivers/hantro.rst | 19 > > > > > +++ > > > > > .../userspace-api/media/drivers/index.rst | 1 + > > > > > include/media/hevc-ctrls.h | 13 + > > > > > 3 files changed, 33 insertions(+) > > > > > create mode 100644 > > > > > Documentation/userspace-api/media/drivers/hantro.rst > > > > > > > > > > diff --git a/Documentation/userspace-api/media/drivers/hantro.rst > > > > > b/Documentation/userspace-api/media/drivers/hantro.rst > > > > > new file mode 100644 > > > > > index ..cd9754b4e005 > > > > > --- /dev/null > > > > > +++ b/Documentation/userspace-api/media/drivers/hantro.rst > > > > > @@ -0,0 +1,19 @@ > > > > > +.. SPDX-License-Identifier: GPL-2.0 > > > > > + > > > > > +Hantro video decoder driver > > > > > +=== > > > > > + > > > > > +The Hantro video decoder driver implements the following > > > > > driver-specific controls: > > > > > + > > > > > +``V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP (integer)`` > > > > > + Specifies to Hantro HEVC video decoder driver the number of data > > > > > (in bits) to > > > > > + skip in the slice segment header. > > > > > + If non-IDR, the bits to be skipped go from syntax element > > > > > "pic_output_flag" > > > > > + to before syntax element "slice_temporal_mvp_enabled_flag". > > > > > + If IDR, the skipped bits are just "pic_output_flag" > > > > > + (separate_colour_plane_flag is not supported). > > > > I'm not very keen on this. Without this information the video data > > > > cannot be > > > > decoded, or will it just be suboptimal? > > > > > > Without that information the video can't be decoded. > > > > > > > > > > > The problem is that a generic decoder would have to know that the HW is > > > > a hantro, > > > > and then call this control. If they don't (and are testing on > > > > non-hantro HW), then > > > > it won't work, thus defeating the purpose of the HW independent decoder > > > > API. > > > > > > > > Since hantro is widely used, and if there is no other way to do this > > > > beside explitely > > > > setting this control, then perhaps this should be part of the standard > > > > HEVC API. > > > > Non-hantro drivers that do not need this can just skip it. > > > > > > Even if I put this parameter in decode_params structure that would means > > > that a generic > > > userland decoder will have to know how the compute this value for hantro > > > HW since it > > > isn't something that could be done on kernel side. > > > > But since hantro is very common, any userland decoder will need to > > calculate this anyway. > > So perhaps it is better to have this as part of the decode_params? > > > > I'd like to know what others think about this. > > > > As you know, I'm not a fan of carrying these "unstable" APIs around. > I know it's better than nothing, but I feel they create the illusion > of the interface being supported in mainline. Since it's unstable, > it's difficult for applications to adopt them. > > As Nicolas mentioned, this means neither FFmpeg nor GStreamer will adopt > these APIs, which worries me, as that means we lose two major user bases. > > My personal take from this, is that we need to find ways to stabilize > our stateless codec
Re: [PATCH] media: cedrus: Propagate OUTPUT resolution to CAPTURE
Le jeudi 17 septembre 2020 à 12:39 +0200, Hans Verkuil a écrit : > On 14/05/2020 17:39, Nicolas Dufresne wrote: > > As per spec, the CAPTURE resolution should be automatically set based on > > the OTUPUT resolution. This patch properly propagate width/height to the > > capture when the OUTPUT format is set and override the user provided > > width/height with configured OUTPUT resolution when the CAPTURE fmt is > > updated. > > > > This also prevents userspace from selecting a CAPTURE resolution that is > > too small, avoiding unwanted page faults. > > > > Signed-off-by: Nicolas Dufresne > > --- > > drivers/staging/media/sunxi/cedrus/cedrus_video.c | 7 ++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > > b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > > index 16d82309e7b6..a6d6b15adc2e 100644 > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > > @@ -247,6 +247,8 @@ static int cedrus_try_fmt_vid_cap(struct file *file, > > void *priv, > > return -EINVAL; > > > > pix_fmt->pixelformat = fmt->pixelformat; > > + pix_fmt->width = ctx->src_fmt.width; > > + pix_fmt->height = ctx->src_fmt.height; > > cedrus_prepare_format(pix_fmt); > > > > return 0; > > @@ -319,11 +321,14 @@ static int cedrus_s_fmt_vid_out(struct file *file, > > void *priv, > > break; > > } > > > > - /* Propagate colorspace information to capture. */ > > + /* Propagate format information to capture. */ > > ctx->dst_fmt.colorspace = f->fmt.pix.colorspace; > > ctx->dst_fmt.xfer_func = f->fmt.pix.xfer_func; > > ctx->dst_fmt.ycbcr_enc = f->fmt.pix.ycbcr_enc; > > ctx->dst_fmt.quantization = f->fmt.pix.quantization; > > + ctx->dst_fmt.width = ctx->src_fmt.width; > > + ctx->dst_fmt.height = ctx->src_fmt.height; > > You can only do this if the capture queue isn't busy. > > See also hantro_reset_raw_fmt() where it does that check. > > So this needs a v2. I missed this reply, wasn't expecting a negative third review. I'll copy and paste from there. So basically: - EBUSY if peer vq is busy - EBUSY if pixel format is changed while vq is busy - EBUSY if streaming Obviously, I don't have userspace to exercise any of these cases, but it seems to make sense. I remembered about this patch today as someone reported the kernel crash we get without this patch: --- I tried testing cedrus with gstreamer 0.18 and it managed to crash the kernel on A64. I used: gst-launch-1.0 filesrc location=test.mkv ! matroskademux ! queue ! h264parse ! v4l2slh264dec ! filesink location=aaa.dat Unable to handle kernel paging request at virtual address 8080808080808088 Mem abort info: ESR = 0x9644 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 Data abort info: ISV = 0, ISS = 0x0044 CM = 0, WnR = 1 [8080808080808088] address between user and kernel address ranges Internal error: Oops: 9644 [#1] SMP Modules linked in: modem_power hci_uart btrtl bluetooth ecdh_generic ecc sunxi_cedrus(C) sun8i_ce crypto_engine snd_soc_bt_sco snd_soc_simple_card snd_soc_simple_card_utils snd_soc_simple_amplifier sun50i_codec_analog sun8i_codec sun8i_adda_pr_regmap snd_soc_ec25 sun4i_i2s snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer snd soundcore stk3310 inv_mpu6050_i2c inv_mpu6050 st_magn_i2c st_sensors_i2c st_magn st_sensors industrialio_triggered_buffer kfifo_buf regmap_i2c option usb_wwan usbserial anx7688 ohci_platform ohci_hcd ehci_platform ehci_hcd g_cdc usb_f_acm u_serial usb_f_ecm u_ether libcomposite sunxi phy_generic musb_hdrc udc_core usbcore sun8i_rotate v4l2_mem2mem gc2145 ov5640 sun6i_csi videobuf2_dma_contig v4l2_fwnode videobuf2_memops videobuf2_v4l2 videobuf2_common videodev mc 8723cs(C) cfg80211 rfkill lima gpu_sched goodix CPU: 0 PID: 0 Comm: swapper/0 Tainted: G C5.9.0-rc5- 00386-g4fe2ef82bd0b #62 Hardware name: Pine64 PinePhone (1.2) (DT) pstate: 8085 (Nzcv daIf -PAN -UAO BTYPE=--) pc : v4l2_m2m_buf_remove+0x44/0x90 [v4l2_mem2mem] lr : v4l2_m2m_buf_remove+0x18/0x90 [v4l2_mem2mem] sp : ffc010c8be20 x29: ffc010c8be20 x28: ffc010bb2fc0 x27: 0060 x26: ffc010935e58 x25: ffc010c06a5a x24: 0080 x23: 0005 x22: ffc010c8bf4c x21: ff806ba0d088 x20: ff80687d1800 x19: ff8066c40298 x18: x17: x16: x15: 01b66678fd80 x14: 0204 x13: 0001 x12: 0040 x11: ff806f0c02
[PATCH v2] media: cedrus: Propagate OUTPUT resolution to CAPTURE
As per spec, the CAPTURE resolution should be automatically set based on
the OTUPUT resolution. This patch properly propagate width/height to the
capture when the OUTPUT format is set and override the user provided
width/height with configured OUTPUT resolution when the CAPTURE fmt is
updated.
This also prevents userspace from selecting a CAPTURE resolution that is
too small, avoiding kernel oops.
Signed-off-by: Nicolas Dufresne
Reviewed-by: Ezequiel Garcia
Acked-by: Paul Kocialkowski
Tested-by: Ondrej Jirman
---
.../staging/media/sunxi/cedrus/cedrus_video.c | 29 +--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 16d82309e7b6..667b86dde1ee 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -247,6 +247,8 @@ static int cedrus_try_fmt_vid_cap(struct file *file, void
*priv,
return -EINVAL;
pix_fmt->pixelformat = fmt->pixelformat;
+ pix_fmt->width = ctx->src_fmt.width;
+ pix_fmt->height = ctx->src_fmt.height;
cedrus_prepare_format(pix_fmt);
return 0;
@@ -296,10 +298,30 @@ static int cedrus_s_fmt_vid_out(struct file *file, void
*priv,
{
struct cedrus_ctx *ctx = cedrus_file2ctx(file);
struct vb2_queue *vq;
+ struct vb2_queue *peer_vq;
int ret;
+ ret = cedrus_try_fmt_vid_out(file, priv, f);
+ if (ret)
+ return ret;
+
vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
- if (vb2_is_busy(vq))
+ /*
+* In order to support dynamic resolution change,
+* the decoder admits a resolution change, as long
+* as the pixelformat remains. Can't be done if streaming.
+*/
+ if (vb2_is_streaming(vq) || (vb2_is_busy(vq) &&
+ f->fmt.pix.pixelformat != ctx->src_fmt.pixelformat))
+ return -EBUSY;
+ /*
+* Since format change on the OUTPUT queue will reset
+* the CAPTURE queue, we can't allow doing so
+* when the CAPTURE queue has buffers allocated.
+*/
+ peer_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+ V4L2_BUF_TYPE_VIDEO_CAPTURE);
+ if (vb2_is_busy(peer_vq))
return -EBUSY;
ret = cedrus_try_fmt_vid_out(file, priv, f);
@@ -319,11 +341,14 @@ static int cedrus_s_fmt_vid_out(struct file *file, void
*priv,
break;
}
- /* Propagate colorspace information to capture. */
+ /* Propagate format information to capture. */
ctx->dst_fmt.colorspace = f->fmt.pix.colorspace;
ctx->dst_fmt.xfer_func = f->fmt.pix.xfer_func;
ctx->dst_fmt.ycbcr_enc = f->fmt.pix.ycbcr_enc;
ctx->dst_fmt.quantization = f->fmt.pix.quantization;
+ ctx->dst_fmt.width = ctx->src_fmt.width;
+ ctx->dst_fmt.height = ctx->src_fmt.height;
+ cedrus_prepare_format(&ctx->dst_fmt);
return 0;
}
--
2.26.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] media: cedrus: Propagate OUTPUT resolution to CAPTURE
Le jeudi 17 septembre 2020 à 20:27 -0400, Nicolas Dufresne a écrit :
> As per spec, the CAPTURE resolution should be automatically set based on
> the OTUPUT resolution. This patch properly propagate width/height to the
> capture when the OUTPUT format is set and override the user provided
> width/height with configured OUTPUT resolution when the CAPTURE fmt is
> updated.
>
> This also prevents userspace from selecting a CAPTURE resolution that is
> too small, avoiding kernel oops.
Just in case it wasn't obvious, this is fully reproducible oops
whenever you use GStreamer 1.18. Here's a copy of Ondrej report from
today which thankfully allowed me to realized I had never completed
this patch. Pretty much all kernel that includes Cedrus are to be
affect, though is a staging driver on staging API of course.
---
I tried testing cedrus with gstreamer 1.18 and it managed to crash the
kernel on
A64. I used:
gst-launch-1.0 filesrc location=test.mkv ! matroskademux ! queue !
h264parse ! v4l2slh264dec ! filesink location=aaa.dat
Unable to handle kernel paging request at virtual address
8080808080808088
Mem abort info:
ESR = 0x9644
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x0044
CM = 0, WnR = 1
[8080808080808088] address between user and kernel address ranges
Internal error: Oops: 9644 [#1] SMP
Modules linked in: modem_power hci_uart btrtl bluetooth ecdh_generic
ecc sunxi_cedrus(C) sun8i_ce crypto_engine snd_soc_bt_sco
snd_soc_simple_card snd_soc_simple_card_utils snd_soc_simple_amplifier
sun50i_codec_analog sun8i_codec sun8i_adda_pr_regmap snd_soc_ec25
sun4i_i2s snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer snd
soundcore stk3310 inv_mpu6050_i2c inv_mpu6050 st_magn_i2c
st_sensors_i2c st_magn st_sensors industrialio_triggered_buffer
kfifo_buf regmap_i2c option usb_wwan usbserial anx7688 ohci_platform
ohci_hcd ehci_platform ehci_hcd g_cdc usb_f_acm u_serial usb_f_ecm
u_ether libcomposite sunxi phy_generic musb_hdrc udc_core usbcore
sun8i_rotate v4l2_mem2mem gc2145 ov5640 sun6i_csi videobuf2_dma_contig
v4l2_fwnode videobuf2_memops videobuf2_v4l2 videobuf2_common videodev
mc 8723cs(C) cfg80211 rfkill lima gpu_sched goodix
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G C5.9.0-rc5-
00386-g4fe2ef82bd0b #62
Hardware name: Pine64 PinePhone (1.2) (DT)
pstate: 8085 (Nzcv daIf -PAN -UAO BTYPE=--)
pc : v4l2_m2m_buf_remove+0x44/0x90 [v4l2_mem2mem]
lr : v4l2_m2m_buf_remove+0x18/0x90 [v4l2_mem2mem]
sp : ffc010c8be20
x29: ffc010c8be20 x28: ffc010bb2fc0
x27: 0060 x26: ffc010935e58
x25: ffc010c06a5a x24: 0080
x23: 0005 x22: ffc010c8bf4c
x21: ff806ba0d088 x20: ff80687d1800
x19: ff8066c40298 x18:
x17: x16:
x15: 01b66678fd80 x14: 0204
x13: 0001 x12: 0040
x11: ff806f0c0248 x10: ff806f0c024a
x9 : ffc010bbdac8 x8 : ff806f000270
x7 : x6 : dead0100
x5 : dead0122 x4 :
x3 : 8080808080808080 x2 : 8080808080808080
x1 : ff80641327a8 x0 : 0080
Call trace:
v4l2_m2m_buf_remove+0x44/0x90 [v4l2_mem2mem]
v4l2_m2m_buf_done_and_job_finish+0x34/0x140 [v4l2_mem2mem]
cedrus_irq+0x8c/0xc0 [sunxi_cedrus]
__handle_irq_event_percpu+0x54/0x150
handle_irq_event+0x4c/0xec
handle_fasteoi_irq+0xbc/0x1c0
__handle_domain_irq+0x78/0xdc
gic_handle_irq+0x50/0xa0
el1_irq+0xb8/0x140
arch_cpu_idle+0x10/0x14
cpu_startup_entry+0x24/0x60
rest_init+0xb0/0xbc
arch_call_rest_init+0xc/0x14
start_kernel+0x690/0x6b0
Code: f2fbd5a6 f2fbd5a5 5284 a9400823 (f9000462)
---[ end trace 88233b9a76cdb261 ]---
Kernel panic - not syncing: Fatal exception in interrupt
>
> Signed-off-by: Nicolas Dufresne
> Reviewed-by: Ezequiel Garcia
> Acked-by: Paul Kocialkowski
> Tested-by: Ondrej Jirman
> ---
> .../staging/media/sunxi/cedrus/cedrus_video.c | 29 +--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index 16d82309e7b6..667b86dde1ee 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -247,6 +247,8 @@ static int cedrus_try_fmt_vid_cap(struct file *file, void
> *priv,
> return -EINVAL;
>
> pix_fmt->pixelformat = fmt->pixelformat;
> + pix_fmt->width = ctx->src_fmt.width;
> + pix_fmt->height = ctx->src_fmt.height;
> cedrus_prepare_format(pix_fmt);
>
> return 0;
> @@ -296,10 +298,30 @@ static int cedrus_s_fmt_vid_out(struct file *file, void
> *priv,
> {
> struct cedrus_ctx *ctx = cedrus_file2ctx(file);
Re: [PATCH RFC PKS/PMEM 33/58] fs/cramfs: Utilize new kmap_thread()
On Fri, 9 Oct 2020, ira.we...@intel.com wrote:
> From: Ira Weiny
>
> The kmap() calls in this FS are localized to a single thread. To avoid
> the over head of global PKRS updates use the new kmap_thread() call.
>
> Cc: Nicolas Pitre
> Signed-off-by: Ira Weiny
Acked-by: Nicolas Pitre
> fs/cramfs/inode.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> index 912308600d39..003c014a42ed 100644
> --- a/fs/cramfs/inode.c
> +++ b/fs/cramfs/inode.c
> @@ -247,8 +247,8 @@ static void *cramfs_blkdev_read(struct super_block *sb,
> unsigned int offset,
> struct page *page = pages[i];
>
> if (page) {
> - memcpy(data, kmap(page), PAGE_SIZE);
> - kunmap(page);
> + memcpy(data, kmap_thread(page), PAGE_SIZE);
> + kunmap_thread(page);
> put_page(page);
> } else
> memset(data, 0, PAGE_SIZE);
> @@ -826,7 +826,7 @@ static int cramfs_readpage(struct file *file, struct page
> *page)
>
> maxblock = (inode->i_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> bytes_filled = 0;
> - pgdata = kmap(page);
> + pgdata = kmap_thread(page);
>
> if (page->index < maxblock) {
> struct super_block *sb = inode->i_sb;
> @@ -914,13 +914,13 @@ static int cramfs_readpage(struct file *file, struct
> page *page)
>
> memset(pgdata + bytes_filled, 0, PAGE_SIZE - bytes_filled);
> flush_dcache_page(page);
> - kunmap(page);
> + kunmap_thread(page);
> SetPageUptodate(page);
> unlock_page(page);
> return 0;
>
> err:
> - kunmap(page);
> + kunmap_thread(page);
> ClearPageUptodate(page);
> SetPageError(page);
> unlock_page(page);
> --
> 2.28.0.rc0.12.gb6a658bd00c9
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
Le mardi 29 janvier 2019 à 16:44 +0900, Alexandre Courbot a écrit : > On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski > wrote: > > Hi, > > > > On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: > > > Sent from my iPad > > > > > > > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > > > > wrote: > > > > > > > > Hi, > > > > > > > > > On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: > > > > > I forget a important thing, for the rkvdec and rk hevc decoder, it > > > > > would > > > > > requests cabac table, scaling list, picture parameter set and > > > > > reference > > > > > picture storing in one or various of DMA buffers. I am not talking > > > > > about > > > > > the data been parsed, the decoder would requests a raw data. > > > > > > > > > > For the pps and rps, it is possible to reuse the slice header, just > > > > > let > > > > > the decoder know the offset from the bitstream bufer, I would suggest > > > > > to > > > > > add three properties(with sps) for them. But I think we need a method > > > > > to > > > > > mark a OUTPUT side buffer for those aux data. > > > > > > > > I'm quite confused about the hardware implementation then. From what > > > > you're saying, it seems that it takes the raw bitstream elements rather > > > > than parsed elements. Is it really a stateless implementation? > > > > > > > > The stateless implementation was designed with the idea that only the > > > > raw slice data should be passed in bitstream form to the decoder. For > > > > H.264, it seems that some decoders also need the slice header in raw > > > > bitstream form (because they take the full slice NAL unit), see the > > > > discussions in this thread: > > > > media: docs-rst: Document m2m stateless video decoder interface > > > > > > Stateless just mean it won’t track the previous result, but I don’t > > > think you can define what a date the hardware would need. Even you > > > just build a dpb for the decoder, it is still stateless, but parsing > > > less or more data from the bitstream doesn’t stop a decoder become a > > > stateless decoder. > > > > Yes fair enough, the format in which the hardware decoder takes the > > bitstream parameters does not make it stateless or stateful per-se. > > It's just that stateless decoders should have no particular reason for > > parsing the bitstream on their own since the hardware can be designed > > with registers for each relevant bitstream element to configure the > > decoding pipeline. That's how GPU-based decoder implementations are > > implemented (VAAPI/VDPAU/NVDEC, etc). > > > > So the format we have agreed on so far for the stateless interface is > > to pass parsed elements via v4l2 control structures. > > > > If the hardware can only work by parsing the bitstream itself, I'm not > > sure what the best solution would be. Reconstructing the bitstream in > > the kernel is a pretty bad option, but so is parsing in the kernel or > > having the data both in parsed and raw forms. Do you see another > > possibility? > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > generic interface to an encoded format which the driver needs to > convert into a sequence that the hardware can understand. Typically > this is done by populating hardware-specific structures. Can't we > consider that in this specific instance, the hardware-specific > structure just happens to be identical to the original bitstream > format? At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it would be really really bad. In GStreamer project we have discussed for a while (but have never done anything about) adding the ability through a bitmask to select which part of the stream need to be parsed, as parsing itself was causing some overhead. Maybe similar thing applies, though as per our new design, it's the fourcc that dictate the driver behaviour, we'd need yet another fourcc for drivers that wants the full bitstream (which seems odd if you have already parsed everything, I think this need some clarification). > > I agree that this is not strictly optimal for that particular > hardware, but such is the cost of abstractions, and in this specific > case I don't believe the cost would be particularly high? signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/8] media: cedrus: Detect first slice of a frame
Le vendredi 30 août 2019 à 07:48 +0200, Boris Brezillon a écrit :
> On Thu, 29 Aug 2019 21:04:28 +0200
> Jernej Škrabec wrote:
>
> > Dne ponedeljek, 26. avgust 2019 ob 20:28:31 CEST je Boris Brezillon
> > napisal(a):
> > > Hi Jernej,
> > >
> > > On Thu, 22 Aug 2019 21:44:57 +0200
> > >
> > > Jernej Skrabec wrote:
> > > > When codec supports multiple slices in one frame, VPU has to know when
> > > > first slice of each frame is being processed, presumably to correctly
> > > > clear/set data in auxiliary buffers.
> > > >
> > > > Add first_slice field to cedrus_run structure and set it according to
> > > > timestamps of capture and output buffers. If timestamps are different,
> > > > it's first slice and viceversa.
> > > >
> > > > Signed-off-by: Jernej Skrabec
> > > > ---
> > > >
> > > > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
> > > > drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> > > > 2 files changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > > > b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> > > > 2f017a651848..32cb38e541c6 100644
> > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> > > > @@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
> > > >
> > > > struct cedrus_run {
> > > >
> > > > struct vb2_v4l2_buffer *src;
> > > > struct vb2_v4l2_buffer *dst;
> > > >
> > > > + bool first_slice;
> > > >
> > > > union {
> > > >
> > > > struct cedrus_h264_run h264;
> > > >
> > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > > > b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index
> > > > 56ca4c9ad01c..d7b54accfe83 100644
> > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > > > @@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
> > > >
> > > > run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > > > run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > > >
> > > > + run.first_slice =
> > > > + run.src->vb2_buf.timestamp != run.dst-
> > > vb2_buf.timestamp;
> > >
> > > Can't we use slice->first_mb_in_slice to determine if a slice is the
> > > first? I'd expect ->first_mb_in_slice to be 0 (unless we decide to
> > > support ASO).
> >
> > I looked in all VPU documentation available to me (which isn't much) and
> > there
> > is no indication if ASO is supported or not. Do you have any sample video
> > with
> > out-of-order slices? It's my understanding that this is uncommon.
>
> I'm not entirely sure, but my understanding was that it might be used
> when streaming over network where some packets might be lost and
> re-emitted later on.
>
> > If it's
> > supported, I would leave code as-is.
>
> I remember seeing the ASO acronym mentioned in the hantro G1 spec, but
> at the same time we're doing frame-based decoding, so I guess the HW
> block expects slices to be ordered in that case. Honestly I don't know,
> so let's keep the code as-is.
We had an ASO interrupt when we tried to do slice decoding rather then
frame. I believe on Hantro, the way to do ASO is to actually re-order
in software.
ASO is a feature of baseline profile use to reduce latency. As an
example, VA-API does not support baseline profile (only constrained-
baseline, which excludes ASO).
Nicolas
signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp
Hi Dave,
Le jeudi 27 juin 2019 à 20:55 +0200, Stefan Wahren a écrit :
> From: Dave Stevenson
>
> H264 header come from VC with 0 timestamps, which means they get a
> strange timestamp when processed with VC/kernel start times,
> particularly if used with the inline header option.
> Remember the last frame timestamp and use that if set, or otherwise
> use the kernel start time.
Normally H264 headers are considered to be part of the following frame.
Giving it the timestamp of the previous frame will likely confuse some
userspace and cause an off-by-one in timestamp. I understand this is a
workaround, but am wondering if this can be improved.
>
> Link: https://github.com/raspberrypi/linux/issues/1836
> Signed-off-by: Dave Stevenson
> ---
> .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 14
> --
> .../staging/vc04_services/bcm2835-camera/bcm2835-camera.h | 2 ++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index dce6e6d..0c04815 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -359,7 +359,9 @@ static void buffer_cb(struct vchiq_mmal_instance
> *instance,
> }
> } else {
> if (dev->capture.frame_count) {
> - if (dev->capture.vc_start_timestamp != -1 && pts) {
> + if (dev->capture.vc_start_timestamp != -1) {
> + buf->vb.vb2_buf.timestamp = ktime_get_ns();
> + } else if (pts) {
> ktime_t timestamp;
> s64 runtime_us = pts -
> dev->capture.vc_start_timestamp;
> @@ -372,8 +374,15 @@ static void buffer_cb(struct vchiq_mmal_instance
> *instance,
>ktime_to_ns(timestamp));
> buf->vb.vb2_buf.timestamp =
> ktime_to_ns(timestamp);
> } else {
> - buf->vb.vb2_buf.timestamp = ktime_get_ns();
> + if (dev->capture.last_timestamp) {
> + buf->vb.vb2_buf.timestamp =
> + dev->capture.last_timestamp;
> + } else {
> + buf->vb.vb2_buf.timestamp =
> +
> ktime_to_ns(dev->capture.kernel_start_ts);
> + }
> }
> + dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;
>
> vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
> vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
> @@ -541,6 +550,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned
> int count)
>dev->capture.vc_start_timestamp, parameter_size);
>
> dev->capture.kernel_start_ts = ktime_get();
> + dev->capture.last_timestamp = 0;
>
> /* enable the camera port */
> dev->capture.port->cb_ctx = dev;
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> index 2b5679e..09273b0 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> @@ -90,6 +90,8 @@ struct bm2835_mmal_dev {
> s64 vc_start_timestamp;
> /* Kernel start timestamp for streaming */
> ktime_t kernel_start_ts;
> + /* Timestamp of last frame */
> + u64 last_timestamp;
>
> struct vchiq_mmal_port *port; /* port being used for capture */
> /* camera port being used for capture */
> --
> 2.7.4
>
signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 19/31] staging: bcm2835-camera: Ensure timestamps never go backwards.
Le jeudi 27 juin 2019 à 20:56 +0200, Stefan Wahren a écrit :
> From: Dave Stevenson
>
> There is an awkward situation with H264 header bytes. Currently
> they are returned with a PTS of 0 because they aren't associated
> with a timestamped frame to encode. These are handled by either
> returning the timestamp of the last buffer to have been received,
> or in the case of the first buffer the timestamp taken at
> start_streaming.
> This results in a race where the current frame may have started
> before we take the start time, which results in the first encoded
> frame having an earlier timestamp than the header bytes.
>
> Ensure that we never return a negative delta to the user by checking
> against the previous timestamp.
>
> Signed-off-by: Dave Stevenson
> ---
> drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index 9967df9..6205793 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -387,6 +387,11 @@ static void buffer_cb(struct vchiq_mmal_instance
> *instance,
>ktime_to_ns(dev->capture.kernel_start_ts),
>dev->capture.vc_start_timestamp, pts,
>ktime_to_ns(timestamp));
> + if (timestamp < dev->capture.last_timestamp) {
> + v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
> + "Negative delta - using last time\n");
> + timestamp = dev->capture.last_timestamp;
> + }
Instead of that, maybe you could request a minimum number of buffers,
and not let the header buffer go until you have a proper "following
timestamp" to give it. This way you don't need this hack, and you won't
have an off-by-one in timestamps.
> buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
> } else {
> if (dev->capture.last_timestamp) {
> --
> 2.7.4
>
signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp
Le vendredi 28 juin 2019 à 11:10 +0100, Dave Stevenson a écrit : > Hi Nicolas > > On Thu, 27 Jun 2019 at 20:55, Nicolas Dufresne wrote: > > Hi Dave, > > > > Le jeudi 27 juin 2019 à 20:55 +0200, Stefan Wahren a écrit : > > > From: Dave Stevenson > > > > > > H264 header come from VC with 0 timestamps, which means they get a > > > strange timestamp when processed with VC/kernel start times, > > > particularly if used with the inline header option. > > > Remember the last frame timestamp and use that if set, or otherwise > > > use the kernel start time. > > > > Normally H264 headers are considered to be part of the following frame. > > Giving it the timestamp of the previous frame will likely confuse some > > userspace and cause an off-by-one in timestamp. I understand this is a > > workaround, but am wondering if this can be improved. > > Sorry, slight ambiguity in how I'm reading your comment. > > Are you saying that the header bytes want to be in the same buffer as > the following frame? > I thought this had also been discussed in the V4L2 stateful codec API > threads along with how many encoded frames were allowed in a single > V4L2 buffer. I certainly hadn't seen a statement about the header > bytes being combined with the next frame. > If the behaviour required by V4L2 is that header bytes and following > frame are in the same buffer, then that is relatively easy to achieve > in the firmware. This workaround can remain for older firmware as it > will never trigger if the firmware has combined the frames. The frame alignment is a requirement specific to the stateful codec API. Stateful codec must interpret _H264 format as being one full frame per buffer (1 AU in progressive, and 1 to 2 AU in interlaced), the first frame should include SPS/PPS and any other prefix NALs. You may follow this rule in your capture driver if you want to make it possible to zero-copy the encoded frames from the capture to the decoder. Though, userspace will still have to parse as there is no indication for capture devices of the H264 alignment being used (that imply 1 frame latency). Boris is working on a control for stateless CODEC to control if we are running in full-frame or per slices. I do hope this control will be extended later to allow cameras and decoders to signal their alignment, or simply to allow enabling low-latency modes supported by CODA and ZynMP firmwares. > > Or are you saying that the header bytes remain in their own buffer, > but the timestamp wants to be the same as the next frame? That is > harder to achieve in the firmware, but could probably be done in the > kernel driver by holding on to the header bytes frame until the next > buffer had been received, at which point the timestamp can be copied > across. Possible, but just needs slightly careful handling to ensure > we don't lose buffers accidentally. So this isn't specified by V4L2 itself. So instead I rely on H264 and MPEG TS specification to advance this. This is also the interpretation we have of timestamp in GStreamer (ffmpeg uses out-of-band headers with full frame AVC, so this does not apply). So H264 AUD, SPS, PPS, SEI and other prefix NAL are considered to be the start of a frame. With this interpretation in mind, accumulating them is considered zero-latency. This basically means that if they are to have a timestamp, they would share that timestamp with all the slices of the same frame. In GStreamer, we have the notion of no timestamp, so in such a case we'd leave the timestamp empty and our parsers would pick the first valid timestamp that formed the full frame as being the frame timestamp (it's a bit buggier then that, but that's what it's suppose to do). On top of that, if you don't have any meaningful alignment in your H264 stream, the MPEG TS format states that the timestamp of a buffer should be the timestamp of the first NAL starting within this buffer, or the timestamp of the current NAL if there is not NAL start. By respecting these standards you ensure that latency aware application can work with your driver without causing delays, or worst, having to deal with artificially late frames. I hope this clarify and helps understand my request for "unhacking" the headers timestamps. I had assumed the timestamp came from the driver (instead of from the firmware), sorry if that caused confusion. If merging full frames is easier, I think I would opt for that as it's beneficial to performance when combined with other full frame APIs. Nicolas > > Dave > > > > Link: https://github.com/raspberrypi/linux/issues/1836 > > > Signed-off-by: Dave Stevenson > > > --- > > > .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 14 >
Re: [PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp
Le vendredi 28 juin 2019 à 16:08 +0200, Hans Verkuil a écrit : > On 6/28/19 4:00 PM, Nicolas Dufresne wrote: > > Le vendredi 28 juin 2019 à 11:10 +0100, Dave Stevenson a écrit : > > > Hi Nicolas > > > > > > On Thu, 27 Jun 2019 at 20:55, Nicolas Dufresne > > > wrote: > > > > Hi Dave, > > > > > > > > Le jeudi 27 juin 2019 à 20:55 +0200, Stefan Wahren a écrit : > > > > > From: Dave Stevenson > > > > > > > > > > H264 header come from VC with 0 timestamps, which means they get a > > > > > strange timestamp when processed with VC/kernel start times, > > > > > particularly if used with the inline header option. > > > > > Remember the last frame timestamp and use that if set, or otherwise > > > > > use the kernel start time. > > > > > > > > Normally H264 headers are considered to be part of the following frame. > > > > Giving it the timestamp of the previous frame will likely confuse some > > > > userspace and cause an off-by-one in timestamp. I understand this is a > > > > workaround, but am wondering if this can be improved. > > > > > > Sorry, slight ambiguity in how I'm reading your comment. > > > > > > Are you saying that the header bytes want to be in the same buffer as > > > the following frame? > > > I thought this had also been discussed in the V4L2 stateful codec API > > > threads along with how many encoded frames were allowed in a single > > > V4L2 buffer. I certainly hadn't seen a statement about the header > > > bytes being combined with the next frame. > > > If the behaviour required by V4L2 is that header bytes and following > > > frame are in the same buffer, then that is relatively easy to achieve > > > in the firmware. This workaround can remain for older firmware as it > > > will never trigger if the firmware has combined the frames. > > > > The frame alignment is a requirement specific to the stateful codec > > API. > > Is it? I don't remember it being specified anywhere explicitly. > Here is the latest text: > > https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-encoder.html > > I'll start a new thread about this, since this really needs to be > clarified. Ok, let's clarify this, but before we start, a quick reminder that this is what userspace assumes already, so breaking this will cause regressions all over. > > Regards, > > Hans > > Stateful codec must interpret _H264 format as being one full frame > > per buffer (1 AU in progressive, and 1 to 2 AU in interlaced), the > > first frame should include SPS/PPS and any other prefix NALs. You may > > follow this rule in your capture driver if you want to make it possible > > to zero-copy the encoded frames from the capture to the decoder. > > Though, userspace will still have to parse as there is no indication > > for capture devices of the H264 alignment being used (that imply 1 > > frame latency). Boris is working on a control for stateless CODEC to > > control if we are running in full-frame or per slices. I do hope this > > control will be extended later to allow cameras and decoders to signal > > their alignment, or simply to allow enabling low-latency modes > > supported by CODA and ZynMP firmwares. > > > > > Or are you saying that the header bytes remain in their own buffer, > > > but the timestamp wants to be the same as the next frame? That is > > > harder to achieve in the firmware, but could probably be done in the > > > kernel driver by holding on to the header bytes frame until the next > > > buffer had been received, at which point the timestamp can be copied > > > across. Possible, but just needs slightly careful handling to ensure > > > we don't lose buffers accidentally. > > > > So this isn't specified by V4L2 itself. So instead I rely on H264 and > > MPEG TS specification to advance this. This is also the interpretation > > we have of timestamp in GStreamer (ffmpeg uses out-of-band headers with > > full frame AVC, so this does not apply). > > > > So H264 AUD, SPS, PPS, SEI and other prefix NAL are considered to be > > the start of a frame. With this interpretation in mind, accumulating > > them is considered zero-latency. This basically means that if they are > > to have a timestamp, they would share that timestamp with all the > > slices of the same frame. In GStreamer, we have the notion of no > > timestamp, so in such a case we'd leav
Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
Le mercredi 11 octobre 2017 à 23:08 +0300, Dmitry Osipenko a écrit : > diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra- > vde/TODO > new file mode 100644 > index ..e98bbc7b3c19 > --- /dev/null > +++ b/drivers/staging/tegra-vde/TODO > @@ -0,0 +1,5 @@ > +TODO: > + - Figure out how generic V4L2 API could be utilized by this > driver, > + implement it. > + That is a very interesting effort, I think it's the first time someone is proposing an upstream driver for a Tegra platform. When I look tegra_vde_h264_decoder_ctx, it looks like the only thing that the HW is not parsing is the media header (pps/sps). Is that correct ? I wonder how acceptable it would be to parse this inside the driver. It is no more complex then parsing an EDID. If that was possible, wrapping this driver as a v4l2 mem2mem should be rather simple. As a side effect, you'll automatically get some userspace working, notably GStreamer and FFmpeg. For the case even parsing the headers is too much from a kernel point of view, then I think you should have a look at the following effort. It's a proposal base on yet to be merged Request API. Hugues is also propose a libv4l2 adapter that makes the driver looks like a normal v4l2 m2m, hiding all the userspace parsing and table filling. This though, is long term plan to integrate state-less or parser-less encoders into linux-media. It seems rather overkill for state-full driver that requires parsed headers like PPS/SPS. https://lwn.net/Articles/720797/ regards, Nicolas signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/5] staging: Introduce NVIDIA Tegra video decoder driver
Le dimanche 10 décembre 2017 à 21:56 +0300, Dmitry Osipenko a écrit : > > I've CC-ed Maxime and Giulio as well: they are looking into adding support > > for > > the stateless allwinner codec based on this code as well. There may well be > > opportunities for you to work together, esp. on the userspace side. Note > > that > > Rockchip has the same issue, they too have a stateless HW codec. > > IIUC, we will have to define video decoder parameters in V4L API and then > make a > V4L driver / userspace prototype (ffmpeg for example) that will use the > requests > API for video decoding in order to upstream the requests API. Does it sound > good? Chromium/Chrome already have support for that type of decoder in their tree. In theory, it should just work. Nicolas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] staging: rtl8723bs: make memcmp() calls consistent
rtw_pm_set() uses memcmp() with 5-chars strings and a length of 4 when
parsing extra, and then parses extra+4 as an int:
if (!memcmp(extra, "lps =", 4)) {
sscanf(extra+4, "%u", &mode);
/* ... */
} else if (!memcmp(extra, "ips =", 4)) {
sscanf(extra+4, "%u", &mode);
The space between the key ("lps" and "ips") and the equal sign seems
suspicious. Remove it in order to make the calls to memcmp() consistent.
Signed-off-by: Nicolas Iooss
---
drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
index 3fca0c2d4c8d..ffcfefefc898 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
@@ -4561,10 +4561,10 @@ static int rtw_pm_set(struct net_device *dev,
DBG_871X("[%s] extra = %s\n", __func__, extra);
- if (!memcmp(extra, "lps =", 4)) {
+ if (!memcmp(extra, "lps=", 4)) {
sscanf(extra+4, "%u", &mode);
ret = rtw_pm_set_lps(padapter, mode);
- } else if (!memcmp(extra, "ips =", 4)) {
+ } else if (!memcmp(extra, "ips=", 4)) {
sscanf(extra+4, "%u", &mode);
ret = rtw_pm_set_ips(padapter, mode);
} else {
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event
Le lundi 13 mars 2017 à 10:45 +, Russell King - ARM Linux a écrit : > On Mon, Mar 13, 2017 at 11:02:34AM +0100, Hans Verkuil wrote: > > On 03/11/2017 07:14 PM, Steve Longerbeam wrote: > > > The event must be user visible, otherwise the user has no indication > > > the error, and can't correct it by stream restart. > > > > In that case the driver can detect this and call vb2_queue_error. It's > > what it is there for. > > > > The event doesn't help you since only this driver has this issue. So nobody > > will watch this event, unless it is sw specifically written for this SoC. > > > > Much better to call vb2_queue_error to signal a fatal error (which this > > apparently is) since there are more drivers that do this, and vivid supports > > triggering this condition as well. > > So today, I can fiddle around with the IMX219 registers to help gain > an understanding of how this sensor works. Several of the registers > (such as the PLL setup [*]) require me to disable streaming on the > sensor while changing them. > > This is something I've done many times while testing various ideas, > and is my primary way of figuring out and testing such things. > > Whenever I resume streaming (provided I've let the sensor stop > streaming at a frame boundary) it resumes as if nothing happened. If I > stop the sensor mid-frame, then I get the rolling issue that Steve > reports, but once the top of the frame becomes aligned with the top of > the capture, everything then becomes stable again as if nothing happened. > > The side effect of what you're proposing is that when I disable streaming > at the sensor by poking at its registers, rather than the capture just > stopping, an error is going to be delivered to gstreamer, and gstreamer > is going to exit, taking the entire capture process down. Indeed, there is no recovery attempt in GStreamer code, and it's hard for an higher level programs to handle this. Nothing prevents from adding something of course, but the errors are really un-specific, so it would be something pretty blind. For what it has been tested, this case was never met, usually the error is triggered by a USB camera being un-plugged, a driver failure or even a firmware crash. Most of the time, this is not recoverable. My main concern here based on what I'm reading, is that this driver is not even able to notice immediately that a produced frame was corrupted (because it's out of sync). From usability perspective, this is really bad. Can't the driver derive a clock from some irq and calculate for each frame if the timing was correct ? And if not mark the buffer with V4L2_BUF_FLAG_ERROR ? > > This severely restricts the ability to be able to develop and test > sensor drivers. > > So, I strongly disagree with you. > > Loss of capture frames is not necessarily a fatal error - as I have been > saying repeatedly. In Steve's case, there's some unknown interaction > between the source and iMX6 hardware that is causing the instability, > but that is simply not true of other sources, and I oppose any idea that > we should cripple the iMX6 side of the capture based upon just one > hardware combination where this is a problem. Indeed, it happens all the time with slow USB port and UVC devices. Though, the driver is well aware, and mark the buffers with V4L2_BUF_FLAG_ERROR. > > Steve suggested that the problem could be in the iMX6 CSI block - and I > note comparing Steve's code with the code in FSL's repository that there > are some changes that are missing in Steve's code to do with the CCIR656 > sync code setup, particularly for >8 bit. The progressive CCIR656 8-bit > setup looks pretty similar though - but I think what needs to be asked > is whether the same problem is visible using the FSL/NXP vendor kernel. > > > * - the PLL setup is something that requires research at the moment. > Sony's official position (even to their customers) is that they do not > supply the necessary information, instead they expect customers to tell > them the capture settings they want, and Sony will throw the values into > a spreadsheet, and they'll supply the register settings back to the > customer. Hence, the only way to proceed with a generic driver for > this sensor is to experiment, and experimenting requires the ability to > pause the stream at the sensor while making changes. Take this away, > and we're stuck with the tables-of-register-settings-for-set-of-fixed- > capture-settings approach. I've made a lot of progress away from this > which is all down to the flexibility afforded by _not_ killing the > capture process. > signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging
Le mardi 14 mars 2017 à 15:47 +0100, Benjamin Gaignard a écrit : > Should we use /devi/ion/$heap instead of /dev/ion_$heap ? > I think it would be easier for user to look into one directory rather > then in whole /dev to find the heaps > > > is that we don't have to worry about a limit of 32 possible > > heaps per system (32-bit heap id allocation field). But dealing > > with an ioctl seems easier than names. Userspace might be less > > likely to hardcode random id numbers vs. names as well. > > In the futur I think that heap type will be replaced by a "get caps" > ioctl which will > describe heap capabilities. At least that is my understanding of > kernel part > of "unix memory allocator" project I think what we really need from userspace point of view, is the ability to find a compatible heap for a set of drivers. And this without specific knowledge of the drivers. Nicolas signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)
Le mercredi 15 mars 2017 à 11:50 +0100, Philippe De Muyter a écrit : > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs > > with those, it will likely run with any other application. > > > > I would like to add the 'v4l2src' plugin of gstreamer, and on the > imx6 its While it would be nice if somehow you would get v4l2src to work (in some legacy/emulation mode through libv4l2), the longer plan is to implement smart bin that handle several v4l2src, that can do the required interactions so we can expose similar level of controls as found in Android Camera HAL3, and maybe even further assuming userspace can change the media tree at run-time. We might be a long way from there, specially that some of the features depends on how much the hardware can do. Just being able to figure-out how to build the MC tree dynamically seems really hard when thinking of generic mechanism. Also, Request API will be needed. I think for this one, we'll need some userspace driver that enable the features (not hide them), and that's what I'd be looking for from libv4l2 in this regard. > imx-specific counterpart 'imxv4l2videosrc' from the gstreamer-imx > package > at https://github.com/Freescale/gstreamer-imx, and 'v4l2-ctl'. This one is specific to IMX hardware using vendor driver. You can probably ignore that. Nicolas signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
Le samedi 18 mars 2017 à 20:43 +, Russell King - ARM Linux a écrit : > On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote: > > Can you share your gstreamer pipeline? For now, until > > VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that > > does not attempt to specify a frame rate. I use the attached > > script for testing, which works for me. > > It's nothing more than > > gst-launch-1.0 -v v4l2src ! ! xvimagesink > > in my case, the conversions are bayer2rgbneon. However, this only > shows > you the frame rate negotiated on the pads (which is actually good > enough > to show the issue.) > > How I stumbled across though this was when I was trying to encode: > > gst-launch-1.0 v4l2src device=/dev/video9 ! bayer2rgbneon ! \ > videoconvert ! x264enc speed-preset=1 ! avimux ! \ > filesink location=test.avi > > I noticed that vlc would always say it was playing the resulting AVI > at 30fps. In practice, I have the impression there is a fair reason why framerate enumeration isn't implemented (considering there is only 1 valid rate). Along with the norm fallback, GStreamer could could also consider the currently set framerate as returned by VIDIOC_G_PARM. At the same time, implementing that enumeration shall be straightforward, and will make a large amount of existing userspace work. regards, Nicolas signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
Le dimanche 19 mars 2017 à 00:54 +, Russell King - ARM Linux a écrit : > > > > In practice, I have the impression there is a fair reason why > > framerate > > enumeration isn't implemented (considering there is only 1 valid > > rate). > > That's actually completely incorrect. > > With the capture device interfacing directly with CSI, it's possible > _today_ to select: > > * the CSI sink pad's resolution > * the CSI sink pad's resolution with the width and/or height halved > * the CSI sink pad's frame rate > * the CSI sink pad's frame rate divided by the frame drop factor > > To put it another way, these are possible: > > # v4l2-ctl -d /dev/video10 --list-formats-ext > ioctl: VIDIOC_ENUM_FMT > Index : 0 > Type : Video Capture > Pixel Format: 'RGGB' > Name : 8-bit Bayer RGRG/GBGB > Size: Discrete 816x616 > Interval: Discrete 0.040s (25.000 fps) > Interval: Discrete 0.048s (20.833 fps) > Interval: Discrete 0.050s (20.000 fps) > Interval: Discrete 0.053s (18.750 fps) > Interval: Discrete 0.060s (16.667 fps) > Interval: Discrete 0.067s (15.000 fps) > Interval: Discrete 0.080s (12.500 fps) > Interval: Discrete 0.100s (10.000 fps) > Interval: Discrete 0.120s (8.333 fps) > Interval: Discrete 0.160s (6.250 fps) > Interval: Discrete 0.200s (5.000 fps) > Interval: Discrete 0.240s (4.167 fps) > Size: Discrete 408x616 > > Size: Discrete 816x308 > > Size: Discrete 408x308 > > > These don't become possible as a result of implementing the enums, > they're all already requestable through /dev/video10. Ok that wasn't clear. So basically video9 is a front-end to video10, and it does not proxy the enumerations. I understand this is what you are now fixing. And this has to be fixed, because I can image cases where the front-end could support only a subset of the sub-dev. So having userspace enumerate on another device (and having to find this device by walking the tree) is unlikely to work in all scenarios. regards, Nicolas p.s. This is why caps negotiation is annoyingly complex in GStreamer, specially that there is no shortcut, you connect pads, and they figure- out what format they will use between each other. signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
Le dimanche 19 mars 2017 à 09:55 +, Russell King - ARM Linux a écrit : > 2) would it also make sense to allow gstreamer's v4l2src to try > setting > a these parameters, and only fail if it's unable to set it? IOW, > if > I use: > > gst-launch-1.0 v4l2src device=/dev/video10 ! \ > video/x-bayer,format=RGGB,framerate=20/1 ! ... > > where G_PARM says its currently configured for 25fps, but a S_PARM > with 20fps would actually succeed. In current design, v4l2src will "probe" all possible formats, cache this, and use this information for negotiation. So after the caps has been probed, there will be no TRY_FMT or anything like this happening until it's too late. You have spotted a bug though, it should be reading back the parm structure to validate (and probably produce a not-negotiated error here). Recently, specially for the IMX work done by Pengutronix, there was contributions to enhance this probing to support probing capabilities that are not enumerable (e.g. interlacing, colorimetry) using TRY_FMT. There is no TRY_PARM in the API to implement similar fallback. Also, those ended up creating a massive disaster for slow cameras. We now have UVC cameras that takes 6s or more to start. I have no other choice but to rewrite that now. We will negotiate the non-enumerable at the last minute with TRY_FMT (when the subset is at it's smallest). This will by accident add support for this camera interface, but that wasn't the goal. It would still fail with application that enumerates the possible resolutions and framerate and let you select them with a drop- down (like cheese). In general, I can only conclude that making everything that matter enumerable is the only working way to go for generic userspace. Nicolas signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
Le dimanche 19 mars 2017 à 14:21 +, Russell King - ARM Linux a écrit : > > Can it be a point of failure? > > There's a good reason why I dumped a full debug log using > GST_DEBUG=*:9, > analysed it for the cause of the failure, and tried several different > pipelines, including the standard bayer2rgb plugin. > > Please don't blame this on random stuff after analysis of the logs > _and_ > reading the appropriate plugin code has shown where the problem is. > I > know gstreamer can be very complex, but it's very possible to analyse > the cause of problems and pin them down with detailed logs in > conjunction > with the source code. I read your analyses with GStreamer, and it was all correct. Nicolas signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
Le mardi 21 mars 2017 à 11:36 +, Russell King - ARM Linux a écrit : > warn: v4l2-test-formats.cpp(1187): S_PARM is > supported for buftype 2, but not ENUM_FRAMEINTERVALS > warn: v4l2-test-formats.cpp(1194): S_PARM is > supported but doesn't report V4L2_CAP_TIMEPERFRAME For encoders, the framerate value is used as numerical value to implement bitrate control. So in most cases any interval is accepted. Though, it would be cleaner to just implement the enumeration. It's quite simple when you support everything. Nicolas signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/3] resource: Use list_head to link resource sibling
On Mon, 9 Apr 2018, Rob Herring wrote:
> +Nico who has been working on tinification of the kernel.
>
> On Mon, Apr 9, 2018 at 4:08 AM, Baoquan He wrote:
> > The struct resource uses singly linked list to link siblings. It's not
> > easy to do reverse iteration on sibling list. So replace it with list_head.
>
> Why is reverse iteration needed?
>
> > And code refactoring makes codes in kernel/resource.c more readable than
> > pointer operation.
>
> resource_for_each_* helpers could solve that without the size increase.
>
> > Besides, type of member variables of struct resource, sibling and child, are
> > changed from 'struct resource *' to 'struct list_head'. Kernel size will
> > increase because of those statically defined struct resource instances.
>
> The DT struct device_node also has the same tree structure with
> parent, child, sibling pointers and converting to list_head had been
> on the todo list for a while. ACPI also has some tree walking
> functions (drivers/acpi/acpica/pstree.c). Perhaps there should be a
> common tree struct and helpers defined either on top of list_head or a
> new struct if that saves some size.
>
> >
> > Signed-off-by: Baoquan He
> > ---
> > v2->v3:
> > Make sibling() and first_child() global so that they can be called
> > out of kernel/resource.c to simplify code.
>
> These should probably be inline functions. Or exported if not.
>
> >
> > Fix several code bugs found by kbuild test robot.
> >
> > Got report from lkp that kernel size increased. It's on purpose since
> > the type change of sibling and child inside struct resource{}. For
> > each struct resource variable, it will cost another 16 bytes on x86 64.
>
> The size increase should be mentioned in the commit log. More
> generally, the size increase is 2 pointers.
Tiny kernels have much fewer resources anyway, and usually run on
platforms with 32-bit pointers, so this probably won't matter much in
the end.
This is if reverse iteration is actually needed as you say though.
Unless I'm mistaken, resource iteration doesn't happen that often, and
not in critical paths either.
Making the code clearer while keeping the same structure size could be
considered with the help of llist.h instead.
Nicolas
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: fbtft: fix header guard typo
drivers/staging/fbtft/internal.h header guard tests for __LINUX_FBTFT__INTERNAL_H but then defines __LINUX_FBTFT_INTERNAL_H (only 1 underscore) and uses the same name for the #endif comment. Use the same name everywhere. Signed-off-by: Nicolas Iooss --- drivers/staging/fbtft/internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fbtft/internal.h b/drivers/staging/fbtft/internal.h index f69db8289151..eea0ec5ff4d3 100644 --- a/drivers/staging/fbtft/internal.h +++ b/drivers/staging/fbtft/internal.h @@ -13,7 +13,7 @@ * */ -#ifndef __LINUX_FBTFT__INTERNAL_H +#ifndef __LINUX_FBTFT_INTERNAL_H #define __LINUX_FBTFT_INTERNAL_H void fbtft_sysfs_init(struct fbtft_par *par); -- 2.3.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/2] staging: wilc1000: Add SDIO/SPI 802.11 driver
Le 11/05/2015 07:30, Johnny Kim a écrit : > This driver is for the wilc1000 which is a single chip IEEE 802.11 > b/g/n device. > The driver works together with cfg80211, which is the kernel side of > configuration management for wireless devices because the wilc1000 > chipset is fullmac where the MLME is managed in hardware. > > The driver worked from kernel version 2.6.38 and being now ported > to several others since then. > A TODO file is included as well in this commit. > > Signed-off-by: Johnny Kim > Signed-off-by: Rachel Kim > Signed-off-by: Dean Lee > Signed-off-by: Chris Park > Acked-by: Nicolas Ferre > --- > Changes in v3: > - fix the permissions. > - fix the folder tree. > - forget to add the mailing-list during the previous sending. Hi Greg, Do you have any comment on this v3 series (aka ping ;-)). Bye, > drivers/staging/Kconfig |2 + > drivers/staging/Makefile |1 + > drivers/staging/wilc1000/Kconfig | 55 + > drivers/staging/wilc1000/Makefile | 41 + > drivers/staging/wilc1000/TODO |8 + > drivers/staging/wilc1000/coreconfigsimulator.h| 20 + > drivers/staging/wilc1000/coreconfigurator.c | 2201 ++ > drivers/staging/wilc1000/coreconfigurator.h | 498 ++ > drivers/staging/wilc1000/fifo_buffer.c| 142 + > drivers/staging/wilc1000/fifo_buffer.h| 23 + > drivers/staging/wilc1000/host_interface.c | 8074 > + > drivers/staging/wilc1000/host_interface.h | 1344 > drivers/staging/wilc1000/itypes.h | 60 + > drivers/staging/wilc1000/linux_mon.c | 643 ++ > drivers/staging/wilc1000/linux_wlan.c | 2953 > drivers/staging/wilc1000/linux_wlan_common.h | 170 + > drivers/staging/wilc1000/linux_wlan_sdio.c| 249 + > drivers/staging/wilc1000/linux_wlan_sdio.h| 14 + > drivers/staging/wilc1000/linux_wlan_spi.c | 510 ++ > drivers/staging/wilc1000/linux_wlan_spi.h | 14 + > drivers/staging/wilc1000/wilc_debugfs.c | 185 + > drivers/staging/wilc1000/wilc_errorsupport.h | 84 + > drivers/staging/wilc1000/wilc_event.h | 123 + > drivers/staging/wilc1000/wilc_exported_buf.c | 76 + > drivers/staging/wilc1000/wilc_log.h | 47 + > drivers/staging/wilc1000/wilc_memory.c| 63 + > drivers/staging/wilc1000/wilc_memory.h| 330 + > drivers/staging/wilc1000/wilc_msgqueue.c | 211 + > drivers/staging/wilc1000/wilc_msgqueue.h | 133 + > drivers/staging/wilc1000/wilc_osconfig.h | 55 + > drivers/staging/wilc1000/wilc_oswrapper.h | 133 + > drivers/staging/wilc1000/wilc_platform.h | 181 + > drivers/staging/wilc1000/wilc_sdio.c | 1298 > drivers/staging/wilc1000/wilc_semaphore.c | 70 + > drivers/staging/wilc1000/wilc_semaphore.h | 115 + > drivers/staging/wilc1000/wilc_sleep.c | 36 + > drivers/staging/wilc1000/wilc_sleep.h | 45 + > drivers/staging/wilc1000/wilc_spi.c | 1475 > drivers/staging/wilc1000/wilc_strutils.c | 431 ++ > drivers/staging/wilc1000/wilc_strutils.h | 412 ++ > drivers/staging/wilc1000/wilc_thread.c| 35 + > drivers/staging/wilc1000/wilc_thread.h| 153 + > drivers/staging/wilc1000/wilc_time.c | 163 + > drivers/staging/wilc1000/wilc_time.h | 205 + > drivers/staging/wilc1000/wilc_timer.c | 51 + > drivers/staging/wilc1000/wilc_timer.h | 153 + > drivers/staging/wilc1000/wilc_type.h | 34 + > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4592 > drivers/staging/wilc1000/wilc_wfi_cfgoperations.h | 134 + > drivers/staging/wilc1000/wilc_wfi_netdevice.c | 960 +++ > drivers/staging/wilc1000/wilc_wfi_netdevice.h | 277 + > drivers/staging/wilc1000/wilc_wlan.c | 2434 +++ > drivers/staging/wilc1000/wilc_wlan.h | 321 + > drivers/staging/wilc1000/wilc_wlan_cfg.c | 643 ++ > drivers/staging/wilc1000/wilc_wlan_cfg.h | 33 + > drivers/staging/wilc1000/wilc_wlan_if.h | 991 +++ > 56 files changed, 33704 insertions(+) > create mode 100644 drivers/staging/wilc1000/Kconfig > create mode 100644 drivers/staging/wilc1000/Makefile > create mode 100644 drivers/staging/wilc1000/TODO > create mode 100644 drivers/staging/wilc1000/coreconfigsimulator.h > create mode 100644 drivers/staging/wilc1000/coreconfigurator.c > create mode 100644
Re: [PATCH v3 1/2] staging: wilc1000: Add SDIO/SPI 802.11 driver
Le 18/05/2015 10:04, Dan Carpenter a écrit : > Was the patch sent to the list? Maybe it was rejected for being too > large? Yep is seems: it appears on driverdev-devel: http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-May/069148.html But it seems not on linux-wireless. Bye, -- Nicolas Ferre ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/wilc1000: fix Kconfig dependencies
Le 28/05/2015 16:35, Arnd Bergmann a écrit : > The newly added wilc1000 driver lacks several Kconfig dependencies, > resulting in a multitude of randconfig build errors, e.g.: > > drivers/built-in.o: In function `WILC_WFI_mgmt_tx_cancel_wait': > binder.c:(.text+0x12bd28): undefined reference to > `cfg80211_remain_on_channel_expired' > drivers/built-in.o: In function `WILC_WFI_CfgSetChannel': > binder.c:(.text+0x12c9d8): undefined reference to > `ieee80211_frequency_to_channel' > drivers/built-in.o: In function `WILC_WFI_CfgAlloc': > binder.c:(.text+0x132530): undefined reference to `wiphy_new_nm' > drivers/built-in.o: In function `wilc_netdev_init': > binder.c:(.text+0x1356d0): undefined reference to `register_inetaddr_notifier' > drivers/built-in.o: In function `linux_spi_init': > binder.c:(.text+0x210a68): undefined reference to `spi_register_driver' > > This change ensures that we always have at least one of SPI or MMC > enabled, and are only able to pick an interface that works. It also > adds all the missing dependencies for networking infrastructure > (cfg80211, wext, and ipv4). > > In order to make it readable, I also took the liberty of re-indenting > the Kconfig file to the normal conventions. > > Signed-off-by: Arnd Bergmann Acked-by: Nicolas Ferre Thanks a lot Arnd! Bye, > diff --git a/drivers/staging/wilc1000/Kconfig > b/drivers/staging/wilc1000/Kconfig > index 101f908bc9ed..02381521ff7f 100644 > --- a/drivers/staging/wilc1000/Kconfig > +++ b/drivers/staging/wilc1000/Kconfig > @@ -1,55 +1,57 @@ > config WILC1000 > tristate "WILC1000 support (WiFi only)" > + depends on CFG80211 && WEXT_CORE && INET > + depends on MMC || SPI > ---help--- > - This module only support IEEE 802.11n WiFi. > + This module only support IEEE 802.11n WiFi. > > choice > prompt "Memory Allocation" > depends on WILC1000 > default WILC1000_PREALLOCATE_AT_LOADING_DRIVER > > -config WILC1000_PREALLOCATE_AT_LOADING_DRIVER > -bool "Preallocate memory at loading driver" > ----help--- > -This choice supports static allocation of the memory > -for the receive buffer. The driver will allocate the > RX buffer > -during initial time. The driver will also free the > buffer > -by calling network device stop. > - > -config WILC1000_DYNAMICALLY_ALLOCATE_MEMROY > -bool "Dynamically allocate memory in real time" > ----help--- > -This choice supports dynamic allocation of the memory > -for the receive buffer. The driver will allocate the > RX buffer > -when it is required. > +config WILC1000_PREALLOCATE_AT_LOADING_DRIVER > + bool "Preallocate memory at loading driver" > + ---help--- > + This choice supports static allocation of the memory > + for the receive buffer. The driver will allocate the RX buffer > + during initial time. The driver will also free the buffer > + by calling network device stop. > + > +config WILC1000_DYNAMICALLY_ALLOCATE_MEMROY > +bool "Dynamically allocate memory in real time" > +---help--- > + This choice supports dynamic allocation of the memory > + for the receive buffer. The driver will allocate the RX buffer > + when it is required. > endchoice > > - > choice > -prompt "Bus Type" > -depends on WILC1000 > -default WILC1000_SDIO > - > + prompt "Bus Type" > + depends on WILC1000 > + default WILC1000_SDIO > + > config WILC1000_SDIO > - bool "SDIO support" > - depends on MMC > - ---help--- > - This module adds support for the SDIO interface of > adapters using > - WILC chipset. Select this if your platform is using the > SDIO bus. > + bool "SDIO support" > + depends on MMC > + ---help--- > + This module adds support for the SDIO interface > + of adapters using WILC chipset. Select this if > + your platform is using the SDIO bus. > > config WILC1000_SPI > - bool "SPI support" > - ---help--- > - This module adds support for the SPI interface of > adapters using > - WILC chipset. Select this if your platform is using the > S
Re: [PATCH] media: hantro: Support H264 profile control
Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit :
> The Hantro G1 decoder supports H.264 profiles from Baseline to High, with
> the exception of the Extended profile.
>
> Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the
> applications can query the driver for the list of supported profiles.
Thanks for this patch. Do you think we could also add the LEVEL control
so the profile/level enumeration becomes complete ?
I'm thinking it would be nice if the v4l2 compliance test make sure
that codecs do implement these controls (both stateful and stateless),
it's essential for stack with software fallback, or multiple capable
codec hardware but with different capabilities.
>
> Signed-off-by: Hirokazu Honda
> ---
> drivers/staging/media/hantro/hantro_drv.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/staging/media/hantro/hantro_drv.c
> b/drivers/staging/media/hantro/hantro_drv.c
> index 6d9d41170832..9387619235d8 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = {
> .def = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B,
> .max = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B,
> },
> + }, {
> + .codec = HANTRO_H264_DECODER,
> + .cfg = {
> + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE,
> + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> + .menu_skip_mask =
> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
> + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
> + }
> }, {
> },
> };
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: hantro: Support H264 profile control
Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit :
> On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne
> wrote:
> > Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit :
> > > The Hantro G1 decoder supports H.264 profiles from Baseline to High, with
> > > the exception of the Extended profile.
> > >
> > > Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the
> > > applications can query the driver for the list of supported profiles.
> >
> > Thanks for this patch. Do you think we could also add the LEVEL control
> > so the profile/level enumeration becomes complete ?
> >
> > I'm thinking it would be nice if the v4l2 compliance test make sure
> > that codecs do implement these controls (both stateful and stateless),
> > it's essential for stack with software fallback, or multiple capable
> > codec hardware but with different capabilities.
> >
>
> Level is a difficult story, because it also specifies the number of
> macroblocks per second, but for decoders like this the number of
> macroblocks per second it can handle depends on things the driver
> might be not aware of - clock frequencies, DDR throughput, system
> load, etc.
>
> My take on this is that the decoder driver should advertise the
> highest resolution the decoder can handle due to hardware constraints.
> Performance related things depend on the integration details and
> should be managed elsewhere. For example Android and Chrome OS manage
> expected decoding performance in per-board configuration files.
When you read datasheet, the HW is always rated to maximum level (and
it's a range) with the assumption of a single stream. It seems much
easier to expose this as-is, statically then to start doing some math
with data that isn't fully exposed to the user. This is about filtering
of multiple CODEC instances, it does not need to be rocket science,
specially that the amount of missing data is important (e.g. usage of
tiles, compression, IPP all have an impact on the final performance).
All we want to know in userspace is if this HW is even possibly capable
of LEVEL X, and if not, we'll look for another one.
>
> > > Signed-off-by: Hirokazu Honda
> > > ---
> > > drivers/staging/media/hantro/hantro_drv.c | 10 ++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/staging/media/hantro/hantro_drv.c
> > > b/drivers/staging/media/hantro/hantro_drv.c
> > > index 6d9d41170832..9387619235d8 100644
> > > --- a/drivers/staging/media/hantro/hantro_drv.c
> > > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > > @@ -355,6 +355,16 @@ static const struct hantro_ctrl controls[] = {
> > > .def = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B,
> > > .max = V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B,
> > > },
> > > + }, {
> > > + .codec = HANTRO_H264_DECODER,
> > > + .cfg = {
> > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> > > + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE,
> > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > > + .menu_skip_mask =
> > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
> > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
> > > + }
> > > }, {
> > > },
> > > };
signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: hantro: Support H264 profile control
Le vendredi 10 janvier 2020 à 13:31 +0100, Hans Verkuil a écrit : > On 11/29/19 1:16 AM, Tomasz Figa wrote: > > On Sat, Nov 23, 2019 at 1:52 AM Nicolas Dufresne > > wrote: > > > Le samedi 23 novembre 2019 à 01:00 +0900, Tomasz Figa a écrit : > > > > On Sat, Nov 23, 2019 at 12:09 AM Nicolas Dufresne > > > > wrote: > > > > > Le vendredi 22 novembre 2019 à 14:16 +0900, Hirokazu Honda a écrit : > > > > > > The Hantro G1 decoder supports H.264 profiles from Baseline to High, > > > > > > with > > > > > > the exception of the Extended profile. > > > > > > > > > > > > Expose the V4L2_CID_MPEG_VIDEO_H264_PROFILE control, so that the > > > > > > applications can query the driver for the list of supported > > > > > > profiles. > > > > > > > > > > Thanks for this patch. Do you think we could also add the LEVEL > > > > > control > > > > > so the profile/level enumeration becomes complete ? > > > > > > > > > > I'm thinking it would be nice if the v4l2 compliance test make sure > > > > > that codecs do implement these controls (both stateful and stateless), > > > > > it's essential for stack with software fallback, or multiple capable > > > > > codec hardware but with different capabilities. > > > > > > > > > > > > > Level is a difficult story, because it also specifies the number of > > > > macroblocks per second, but for decoders like this the number of > > > > macroblocks per second it can handle depends on things the driver > > > > might be not aware of - clock frequencies, DDR throughput, system > > > > load, etc. > > > > > > > > My take on this is that the decoder driver should advertise the > > > > highest resolution the decoder can handle due to hardware constraints. > > > > Performance related things depend on the integration details and > > > > should be managed elsewhere. For example Android and Chrome OS manage > > > > expected decoding performance in per-board configuration files. > > > > > > When you read datasheet, the HW is always rated to maximum level (and > > > it's a range) with the assumption of a single stream. It seems much > > > easier to expose this as-is, statically then to start doing some math > > > with data that isn't fully exposed to the user. This is about filtering > > > of multiple CODEC instances, it does not need to be rocket science, > > > specially that the amount of missing data is important (e.g. usage of > > > tiles, compression, IPP all have an impact on the final performance). > > > All we want to know in userspace is if this HW is even possibly capable > > > of LEVEL X, and if not, we'll look for another one. > > > > > > > Agreed, one could potentially define it this way, but would it be > > really useful for the userspace and the users? I guess it could enable > > slightly faster fallback to software decoding in the extreme case of > > the hardware not supporting the level at all, but I suspect that the > > majority of cases would be the hardware just being unusably slow. > > > > Also, as I mentioned before, we already return the range of supported > > resolutions, which in practice should map to the part of the level > > that may depend on hardware capabilities rather than performance, so > > exposing levels as well would add redundancy to the information > > exposed. > > There is a lot of discussion here, but it all revolves around a potential > LEVEL control. > > So I gather everyone is OK with merging this patch? I'm ok with this. For me, the level reflects the real time performance capability as define in the spec, while the width/height constraints usually represent an addressing capabicity, which may or may not operate real-time. > > If not, let me know asap. > > Regards, > > Hans > > > > > > > Signed-off-by: Hirokazu Honda > > > > > > --- > > > > > > drivers/staging/media/hantro/hantro_drv.c | 10 ++ > > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > > > diff --git a/drivers/staging/media/hantro/hantro_drv.c > > > > > > b/drivers/staging/media/hantro/hantro_drv.c > > > > > > index 6d9d41170832..9387619235d8 100644 > > > > >
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Hi,
Just commenting on the mode_fixup function that was added in v7.
On Tue, Feb 25, 2020 at 2:15 PM Xin Ji wrote:
>
> The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed
> for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K.
>
> The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI
> to DP feature. This driver only enabled MIPI DSI/DPI to DP feature.
>
> Signed-off-by: Xin Ji
> ---
> drivers/gpu/drm/bridge/Makefile |2 +-
> drivers/gpu/drm/bridge/analogix/Kconfig |6 +
> drivers/gpu/drm/bridge/analogix/Makefile |1 +
> drivers/gpu/drm/bridge/analogix/anx7625.c | 2172
> +
> drivers/gpu/drm/bridge/analogix/anx7625.h | 410 ++
> 5 files changed, 2590 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c
> create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h
>
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 4934fcf..bcd388a 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
[snip]
> +static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge,
> + const struct drm_display_mode *mode,
> + struct drm_display_mode *adj)
> +{
> + struct anx7625_data *ctx = bridge_to_anx7625(bridge);
> + struct device *dev = &ctx->client->dev;
> + u32 hsync, hfp, hbp, hactive, hblanking;
> + u32 adj_hsync, adj_hfp, adj_hbp, adj_hblanking, delta_adj;
> + u32 vref, adj_clock;
> +
> + DRM_DEV_DEBUG_DRIVER(dev, "drm mode fixup set\n");
> +
> + mutex_lock(&ctx->lock);
Why do you need this lock?
> +
> + hactive = mode->hdisplay;
This is never used, drop it?
> + hsync = mode->hsync_end - mode->hsync_start;
> + hfp = mode->hsync_start - mode->hdisplay;
> + hbp = mode->htotal - mode->hsync_end;
> + hblanking = mode->htotal - mode->hdisplay;
> +
> + DRM_DEV_DEBUG_DRIVER(dev, "before mode fixup\n");
> + DRM_DEV_DEBUG_DRIVER(dev, "hsync(%d),hfp(%d),hbp(%d),clock(%d)\n",
> +hsync,
> +hfp,
> +hbp,
> +adj->clock);
> + DRM_DEV_DEBUG_DRIVER(dev,
> "hsync_start(%d),hsync_end(%d),htotal(%d)\n",
> +adj->hsync_start,
> +adj->hsync_end,
> +adj->htotal);
> +
> + adj_hfp = hfp;
> + adj_hsync = hsync;
> + adj_hbp = hbp;
> + adj_hblanking = hblanking;
> +
> + /* plus 1 if hfp is odd */
A better way to word these comments is to say "hfp needs to be even",
otherwise, you're just repeating what we can already see in the code.
> + if (hfp & 0x1) {
> + adj_hfp = hfp + 1;
adj_hfp -= 1 for consistency?
> + adj_hblanking += 1;
> + }
> +
> + /* minus 1 if hbp is odd */
> + if (hbp & 0x1) {
> + adj_hbp = hbp - 1;
ditto, adj_hbp -= 1;
> + adj_hblanking -= 1;
> + }
> +
> + /* plus 1 if hsync is odd */
> + if (hsync & 0x1) {
> + if (adj_hblanking < hblanking)
> + adj_hsync = hsync + 1;
ditto
> + else
> + adj_hsync = hsync - 1;
ditto
> + }
> +
> + /*
> +* once illegal timing detected, use default HFP, HSYNC, HBP
> +*/
> + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {
should this be adj_hblanking/adj_hfp/adj_hbp?
> + adj_hsync = SYNC_LEN_DEF;
> + adj_hfp = HFP_HBP_DEF;
> + adj_hbp = HFP_HBP_DEF;
> + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal);
> + if (hblanking < HBLANKING_MIN) {
> + delta_adj = HBLANKING_MIN - hblanking;
> + adj_clock = vref * delta_adj * adj->vtotal;
> + adj->clock += DIV_ROUND_UP(adj_clock, 1000);
> + } else {
> + delta_adj = hblanking - HBLANKING_MIN;
> + adj_clock = vref * delta_adj * adj->vtotal;
> + adj->clock -= DIV_ROUND_UP(adj_clock, 1000);
> + }
> +
> + DRM_WARN("illegal hblanking timing, use default.\n");
> + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);
How likely is it that this mode is going to work? Can you just return
false here to reject the mode?
> + } else if (adj_hfp < HP_MIN) {
> + /* adjust hfp if hfp less than HP_MIN */
> + delta_adj = HP_MIN - adj_hfp;
> + adj_hfp = HP_MIN;
> +
> + /*
> +* balance total HBlanking pixel, if HBP hasn't enough space,
"does not have enough space"
> +* adjust HSYNC length, ot
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
On Fri, Apr 24, 2020 at 2:51 PM Xin Ji wrote:
>
> On Thu, Apr 23, 2020 at 07:55:15PM +0800, Nicolas Boichat wrote:
> > Hi,
> >
> > Just commenting on the mode_fixup function that was added in v7.
> >
> [snip]
> > > + /*
> > > +* once illegal timing detected, use default HFP, HSYNC, HBP
> > > +*/
> > > + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {
> >
> > should this be adj_hblanking/adj_hfp/adj_hbp?
> NO, need check original HFP and HBP, if they are not legal, driver need
> set default value to adj_hsync, adj_hfp, adj_hbp.
> >
> > > + adj_hsync = SYNC_LEN_DEF;
> > > + adj_hfp = HFP_HBP_DEF;
> > > + adj_hbp = HFP_HBP_DEF;
> > > + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal);
> > > + if (hblanking < HBLANKING_MIN) {
> > > + delta_adj = HBLANKING_MIN - hblanking;
> > > + adj_clock = vref * delta_adj * adj->vtotal;
> > > + adj->clock += DIV_ROUND_UP(adj_clock, 1000);
> > > + } else {
> > > + delta_adj = hblanking - HBLANKING_MIN;
> > > + adj_clock = vref * delta_adj * adj->vtotal;
> > > + adj->clock -= DIV_ROUND_UP(adj_clock, 1000);
> > > + }
> > > +
> > > + DRM_WARN("illegal hblanking timing, use default.\n");
> > > + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);
> >
> > How likely is it that this mode is going to work? Can you just return
> > false here to reject the mode?
> We want to set the default minimal Hblancking value, then it may display,
> otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better
to reject the mode rather than trying a timing that is definitely
quite different from what the monitor was asking for. No super strong
opinion, I'll let other people on the list weigh in.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v8 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
On Mon, Apr 27, 2020 at 2:18 PM Xin Ji wrote:
>
> The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed
> for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K.
>
> The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI
> to DP feature. This driver only enabled MIPI DSI/DPI to DP feature.
>
> Signed-off-by: Xin Ji
> ---
> drivers/gpu/drm/bridge/Makefile |2 +-
> drivers/gpu/drm/bridge/analogix/Kconfig |6 +
> drivers/gpu/drm/bridge/analogix/Makefile |1 +
> drivers/gpu/drm/bridge/analogix/anx7625.c | 2158
> +
> drivers/gpu/drm/bridge/analogix/anx7625.h | 410 ++
> 5 files changed, 2576 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c
> create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h
>
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 4934fcf..bcd388a 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -12,8 +12,8 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
> obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
> obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
> obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
> -obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
> obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> +obj-y += analogix/
> obj-y += synopsys/
> diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig
> b/drivers/gpu/drm/bridge/analogix/Kconfig
> index e930ff9..b2f127e 100644
> --- a/drivers/gpu/drm/bridge/analogix/Kconfig
> +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
> @@ -2,3 +2,9 @@
> config DRM_ANALOGIX_DP
> tristate
> depends on DRM
> +
> +config ANALOGIX_ANX7625
> + tristate "Analogix MIPI to DP interface support"
> + help
> + ANX7625 is an ultra-low power 4K mobile HD transmitter
> designed
> + for portable devices. It converts MIPI/DPI to DisplayPort1.3
> 4K.
> diff --git a/drivers/gpu/drm/bridge/analogix/Makefile
> b/drivers/gpu/drm/bridge/analogix/Makefile
> index fdbf3fd..8a52867 100644
> --- a/drivers/gpu/drm/bridge/analogix/Makefile
> +++ b/drivers/gpu/drm/bridge/analogix/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_ANALOGIX_ANX7625) += anx7625.o
> analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o
> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> new file mode 100644
> index 000..fff7a49
> [snip]
> +static int anx7625_attach_dsi(struct anx7625_data *ctx)
> +{
> + struct mipi_dsi_host *host;
> + struct mipi_dsi_device *dsi;
> + struct device_node *mipi_host_node;
> + struct device *dev = &ctx->client->dev;
> + const struct mipi_dsi_device_info info = {
> + .type = "anx7625",
> + .channel = 0,
> + .node = NULL,
> + };
> +
> + DRM_DEV_DEBUG_DRIVER(dev, "attach dsi\n");
> +
> + if (ctx->pdata.dsi_supported)
> + mipi_host_node = ctx->pdata.node.mipi_dsi_host_node;
> + else
> + mipi_host_node = ctx->pdata.node.mipi_dpi_host_node;
> +
> + if (!mipi_host_node) {
> + DRM_ERROR("dsi host is not configured.\n");
> + return -EINVAL;
> + }
> +
> + host = of_find_mipi_dsi_host_by_node(mipi_host_node);
I tried this driver when connected to a dpi interface, and this fails,
as of_find_mipi_dsi_host_by_node is not able to find the dpi interface
from the SoC.
I'm not too familiar with how dpi bridges are supposed to work in the
kernel, but should we even call "anx7625_attach_dsi" for DPI
interface?
> + if (!host) {
> + DRM_ERROR("failed to find dsi host.\n");
> + return -EINVAL;
> + }
> +
> + dsi = mipi_dsi_device_register_full(host, &info);
> + if (IS_ERR(dsi)) {
> + DRM_ERROR("failed to create dsi device.\n");
> + return -EINVAL;
> + }
> +
> + dsi->lanes = 4;
> + dsi->format = MIPI_DSI_FMT_RGB888;
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> + MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> + MIPI_DSI_MODE_EOT_PACKET|
> + MIPI_DSI_MODE_VIDEO_HSE;
> +
> + if (mipi_dsi_attach(dsi) < 0) {
> + DRM_ERROR("failed to attach dsi to host.\n");
> + mipi_dsi_device_unregister(dsi);
> + return -EINVAL;
> + }
> +
> + ctx->dsi = dsi;
> +
> + DRM_DEV_DEBUG_DRIVER(dev, "attach dsi succeeded.\n");
> +
> + return 0;
> +}
> +
> [snip]
> +static int anx7625_bridge_attach(struct drm_bridge *bridge)
> +{
> + struct anx7625_data *ctx = bridge_to_anx7625(bridge);
> + int err;
> + struc
Re: [PATCH v5 22/50] mtd: nand: atmel: switch to mtd_ooblayout_ops
Le 30/03/2016 18:14, Boris Brezillon a écrit :
> Implementing the mtd_ooblayout_ops interface is the new way of exposing
> ECC/OOB layout to MTD users.
>
> Signed-off-by: Boris Brezillon
It seems good:
Reviewed-by: Nicolas Ferre
Bye,
> ---
> drivers/mtd/nand/atmel_nand.c | 84
> ---
> 1 file changed, 38 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 321d331..46a601e 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -72,30 +72,44 @@ struct atmel_nand_nfc_caps {
> uint32_t rb_mask;
> };
>
> -/* oob layout for large page size
> +/*
> + * oob layout for large page size
> * bad block info is on bytes 0 and 1
> * the bytes have to be consecutives to avoid
> * several NAND_CMD_RNDOUT during read
> - */
> -static struct nand_ecclayout atmel_oobinfo_large = {
> - .eccbytes = 4,
> - .eccpos = {60, 61, 62, 63},
> - .oobfree = {
> - {2, 58}
> - },
> -};
> -
> -/* oob layout for small page size
> + *
> + * oob layout for small page size
> * bad block info is on bytes 4 and 5
> * the bytes have to be consecutives to avoid
> * several NAND_CMD_RNDOUT during read
> */
> -static struct nand_ecclayout atmel_oobinfo_small = {
> - .eccbytes = 4,
> - .eccpos = {0, 1, 2, 3},
> - .oobfree = {
> - {6, 10}
> - },
> +static int atmel_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *oobregion)
> +{
> + if (section)
> + return -ERANGE;
> +
> + oobregion->length = 4;
> + oobregion->offset = 0;
> +
> + return 0;
> +}
> +
> +static int atmel_ooblayout_free_sp(struct mtd_info *mtd, int section,
> +struct mtd_oob_region *oobregion)
> +{
> + if (section)
> + return -ERANGE;
> +
> + oobregion->offset = 6;
> + oobregion->length = mtd->oobsize - oobregion->offset;
> +
> + return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops atmel_ooblayout_sp_ops = {
> + .ecc = atmel_ooblayout_ecc_sp,
> + .free = atmel_ooblayout_free_sp,
> };
>
> struct atmel_nfc {
> @@ -163,8 +177,6 @@ struct atmel_nand_host {
> int *pmecc_delta;
> };
>
> -static struct nand_ecclayout atmel_pmecc_oobinfo;
> -
> /*
> * Enable NAND.
> */
> @@ -483,22 +495,6 @@ static int pmecc_get_ecc_bytes(int cap, int sector_size)
> return (m * cap + 7) / 8;
> }
>
> -static void pmecc_config_ecc_layout(struct nand_ecclayout *layout,
> - int oobsize, int ecc_len)
> -{
> - int i;
> -
> - layout->eccbytes = ecc_len;
> -
> - /* ECC will occupy the last ecc_len bytes continuously */
> - for (i = 0; i < ecc_len; i++)
> - layout->eccpos[i] = oobsize - ecc_len + i;
> -
> - layout->oobfree[0].offset = PMECC_OOB_RESERVED_BYTES;
> - layout->oobfree[0].length =
> - oobsize - ecc_len - layout->oobfree[0].offset;
> -}
> -
> static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
> {
> int table_size;
> @@ -1013,8 +1009,8 @@ static void atmel_pmecc_core_init(struct mtd_info *mtd)
> {
> struct nand_chip *nand_chip = mtd_to_nand(mtd);
> struct atmel_nand_host *host = nand_get_controller_data(nand_chip);
> + int eccbytes = mtd_ooblayout_count_eccbytes(mtd);
> uint32_t val = 0;
> - struct nand_ecclayout *ecc_layout;
> struct mtd_oob_region oobregion;
>
> pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
> @@ -1065,12 +1061,11 @@ static void atmel_pmecc_core_init(struct mtd_info
> *mtd)
> | PMECC_CFG_AUTO_DISABLE);
> pmecc_writel(host->ecc, CFG, val);
>
> - ecc_layout = nand_chip->ecc.layout;
> pmecc_writel(host->ecc, SAREA, mtd->oobsize - 1);
> mtd_ooblayout_ecc(mtd, 0, &oobregion);
> pmecc_writel(host->ecc, SADDR, oobregion.offset);
> pmecc_writel(host->ecc, EADDR,
> - oobregion.offset + ecc_layout->eccbytes - 1);
> + oobregion.offset + eccbytes - 1);
> /* See datasheet about PMECC Clock Control Register */
> pmecc_writel(host->ecc, CLK, 2);
> pmecc_writel(host->ecc, IDR, 0xff);
> @@ -1292,11 +1287,8 @@ static int atmel_pmecc_nand_init_params(struct
> platform_device *pdev,
> err_no = -EINVAL;
> goto
Re: [PATCH v5 04/50] mtd: nand: atmel: use mtd_ooblayout_xxx() helpers where appropriate
gt;
> ecc_layout = nand_chip->ecc.layout;
> pmecc_writel(host->ecc, SAREA, mtd->oobsize - 1);
> - pmecc_writel(host->ecc, SADDR, ecc_layout->eccpos[0]);
> + mtd_ooblayout_ecc(mtd, 0, &oobregion);
> + pmecc_writel(host->ecc, SADDR, oobregion.offset);
> pmecc_writel(host->ecc, EADDR,
> - ecc_layout->eccpos[ecc_layout->eccbytes - 1]);
> + oobregion.offset + ecc_layout->eccbytes - 1);
> /* See datasheet about PMECC Clock Control Register */
> pmecc_writel(host->ecc, CLK, 2);
> pmecc_writel(host->ecc, IDR, 0xff);
> @@ -1362,12 +1371,12 @@ static int atmel_nand_read_page(struct mtd_info *mtd,
> struct nand_chip *chip,
> {
> int eccsize = chip->ecc.size;
> int eccbytes = chip->ecc.bytes;
> - uint32_t *eccpos = chip->ecc.layout->eccpos;
> uint8_t *p = buf;
> uint8_t *oob = chip->oob_poi;
> uint8_t *ecc_pos;
> int stat;
> unsigned int max_bitflips = 0;
> + struct mtd_oob_region oobregion = {};
>
> /*
>* Errata: ALE is incorrectly wired up to the ECC controller
> @@ -1385,19 +1394,20 @@ static int atmel_nand_read_page(struct mtd_info *mtd,
> struct nand_chip *chip,
> chip->read_buf(mtd, p, eccsize);
>
> /* move to ECC position if needed */
> - if (eccpos[0] != 0) {
> - /* This only works on large pages
> - * because the ECC controller waits for
> - * NAND_CMD_RNDOUTSTART after the
> - * NAND_CMD_RNDOUT.
> - * anyway, for small pages, the eccpos[0] == 0
> + mtd_ooblayout_ecc(mtd, 0, &oobregion);
> + if (oobregion.offset != 0) {
> + /*
> + * This only works on large pages because the ECC controller
> + * waits for NAND_CMD_RNDOUTSTART after the NAND_CMD_RNDOUT.
> + * Anyway, for small pages, the first ECC byte is at offset
> + * 0 in the OOB area.
>*/
> chip->cmdfunc(mtd, NAND_CMD_RNDOUT,
> - mtd->writesize + eccpos[0], -1);
> + mtd->writesize + oobregion.offset, -1);
> }
>
> /* the ECC controller needs to read the ECC just after the data */
> - ecc_pos = oob + eccpos[0];
> + ecc_pos = oob + oobregion.offset;
> chip->read_buf(mtd, ecc_pos, eccbytes);
>
> /* check if there's an error */
>
--
Nicolas Ferre
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RESEND PATCH] drivers: android: correct the size of struct binder_uintptr_t for BC_DEAD_BINDER_DONE
From: Lisa Du
There's one point was missed in the patch commit da49889deb34 ("staging:
binder: Support concurrent 32 bit and 64 bit processes."). When configure
BINDER_IPC_32BIT, the size of binder_uintptr_t was 32bits, but size of
void * is 64bit on 64bit system. Correct it here.
Signed-off-by: Lisa Du
Signed-off-by: Nicolas Boichat
---
This patch was first sent upstream here: https://lkml.org/lkml/2015/5/28/747,
but was not picked up (probably because it was part of a bigger series that
refactored binder).
This patch fixes issues when using 64-bit binder API on 32-bit kernels.
drivers/android/binder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 796301a..16288e7 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2081,7 +2081,7 @@ static int binder_thread_write(struct binder_proc *proc,
if (get_user(cookie, (binder_uintptr_t __user *)ptr))
return -EFAULT;
- ptr += sizeof(void *);
+ ptr += sizeof(cookie);
list_for_each_entry(w, &proc->delivered_death, entry) {
struct binder_ref_death *tmp_death =
container_of(w, struct binder_ref_death, work);
--
2.7.0.rc3.207.g0ac5344
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] usb: gadget: atmel_usba_udc: add missing ret value check
Le 07/07/2015 16:02, Robert Baldyga a écrit :
> Add missing return value check. In case of error print debug message
> and return error code.
>
> Signed-off-by: Robert Baldyga
Yes, it's indeed missing:
Acked-by: Nicolas Ferre
Thanks, bye.
> ---
> drivers/usb/gadget/udc/atmel_usba_udc.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
> b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 4095cce0..37d414e 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -1989,6 +1989,10 @@ static struct usba_ep * atmel_udc_of_init(struct
> platform_device *pdev,
> ep->can_isoc = of_property_read_bool(pp, "atmel,can-isoc");
>
> ret = of_property_read_string(pp, "name", &name);
> + if (ret) {
> + dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
> + goto err;
> + }
> ep->ep.name = name;
>
> ep->ep_regs = udc->regs + USBA_EPT_BASE(i);
>
--
Nicolas Ferre
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] MAINTAINERS: Update maintainer entry for wilc1000
From: Aditya Shankar Take the maintenance of the Atmel WIFI staging driver wilc1000. Former maintainers are no more with Atmel. Reported-by: Loic Lefort Signed-off-by: Aditya Shankar Signed-off-by: Ganesh Krishna Signed-off-by: Nicolas Ferre --- Hi Luis, Greg, I'd like that we don't delete the entry for this wilc1000 driver but change it with the patch below. After the Microchip - Atmel merger, Aditya and Ganesh will take over the maintenance of this driver and will continue the work that our former colleagues started. Thanks, bye. MAINTAINERS | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 51447a517095..9142e08ba8ae 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10931,12 +10931,8 @@ S: Odd Fixes F: drivers/staging/vt665?/ STAGING - WILC1000 WIFI DRIVER -M: Johnny Kim -M: Austin Shin -M: Chris Park -M: Tony Cho -M: Glen Lee -M: Leo Kim +M: Aditya Shankar +M: Ganesh Krishna L: linux-wirel...@vger.kernel.org S: Supported F: drivers/staging/wilc1000/ -- 2.9.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RESEND PATCH] MAINTAINERS: Update maintainer entry for wilc1000
From: Aditya Shankar Take the maintenance of the Atmel WIFI staging driver wilc1000. Former maintainers are no more with Atmel. Reported-by: Loic Lefort Signed-off-by: Aditya Shankar Signed-off-by: Ganesh Krishna Acked-by: Luis de Bethencourt Signed-off-by: Nicolas Ferre --- Hi all, Just an elaborate "ping" with the Acked-by tag from Luis added. As noted by Luis, previous version of this patch is here: https://lkml.org/lkml/2016/6/27/382 Thanks, bye. MAINTAINERS | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 33d7e2252ceb..b0d5c77c9db2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11202,12 +11202,8 @@ S: Odd Fixes F: drivers/staging/vt665?/ STAGING - WILC1000 WIFI DRIVER -M: Johnny Kim -M: Austin Shin -M: Chris Park -M: Tony Cho -M: Glen Lee -M: Leo Kim +M: Aditya Shankar +M: Ganesh Krishna L: linux-wirel...@vger.kernel.org S: Supported F: drivers/staging/wilc1000/ -- 2.9.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] staging: rtl8192u: do not use undefined $(TOPDIR) in Makefile
drivers/staging/rtl8192u/ieee80211/Makefile uses $(TOPDIR) to configure an include directory, without defining this variable first. The path which is configured is therefore "/drivers/net/wireless", which does not seem to be the intended path. Remove the offending line. Signed-off-by: Nicolas Iooss --- drivers/staging/rtl8192u/ieee80211/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/rtl8192u/ieee80211/Makefile b/drivers/staging/rtl8192u/ieee80211/Makefile index b5d0c2eb045b..9e3f432e5355 100644 --- a/drivers/staging/rtl8192u/ieee80211/Makefile +++ b/drivers/staging/rtl8192u/ieee80211/Makefile @@ -1,7 +1,6 @@ NIC_SELECT = RTL8192U -ccflags-y := -I$(TOPDIR)/drivers/net/wireless -ccflags-y += -O2 +ccflags-y := -O2 ccflags-y += -DJACKSON_NEW_8187 -DJACKSON_NEW_RX ieee80211-rsl-objs := ieee80211_rx.o \ -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] staging/atomisp: fix header guards
Files input_formatter_subsystem_defs.h begin with:
#ifndef _if_subsystem_defs_h
#define _if_subsystem_defs_h__
and end with:
#endif /* _if_subsystem_defs_h__ */
The intent seems to have been to use _if_subsystem_defs_h__ everywhere
but two underscores are missing in the initial #ifndef.
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Nicolas Iooss
---
.../css2400/css_2400_system/hrt/input_formatter_subsystem_defs.h| 2 +-
.../css2400/css_2401_csi2p_system/hrt/input_formatter_subsystem_defs.h | 2 +-
.../css2400/css_2401_system/hrt/input_formatter_subsystem_defs.h| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/input_formatter_subsystem_defs.h
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/input_formatter_subsystem_defs.h
index 16bfe1d80bc9..7766f78cd123 100644
---
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/input_formatter_subsystem_defs.h
+++
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2400_system/hrt/input_formatter_subsystem_defs.h
@@ -12,7 +12,7 @@
* more details.
*/
-#ifndef _if_subsystem_defs_h
+#ifndef _if_subsystem_defs_h__
#define _if_subsystem_defs_h__
#define HIVE_IFMT_GP_REGS_INPUT_SWITCH_LUT_REG_00
diff --git
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_csi2p_system/hrt/input_formatter_subsystem_defs.h
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_csi2p_system/hrt/input_formatter_subsystem_defs.h
index 16bfe1d80bc9..7766f78cd123 100644
---
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_csi2p_system/hrt/input_formatter_subsystem_defs.h
+++
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_csi2p_system/hrt/input_formatter_subsystem_defs.h
@@ -12,7 +12,7 @@
* more details.
*/
-#ifndef _if_subsystem_defs_h
+#ifndef _if_subsystem_defs_h__
#define _if_subsystem_defs_h__
#define HIVE_IFMT_GP_REGS_INPUT_SWITCH_LUT_REG_00
diff --git
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_system/hrt/input_formatter_subsystem_defs.h
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_system/hrt/input_formatter_subsystem_defs.h
index 16bfe1d80bc9..7766f78cd123 100644
---
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_system/hrt/input_formatter_subsystem_defs.h
+++
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/css_2401_system/hrt/input_formatter_subsystem_defs.h
@@ -12,7 +12,7 @@
* more details.
*/
-#ifndef _if_subsystem_defs_h
+#ifndef _if_subsystem_defs_h__
#define _if_subsystem_defs_h__
#define HIVE_IFMT_GP_REGS_INPUT_SWITCH_LUT_REG_00
--
2.14.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging/olpc_dcon: fix checkpatch warnings
WARNING: Missing a blank line after declarations
Signed-off-by: Nicolas Joseph
---
drivers/staging/olpc_dcon/olpc_dcon.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c
b/drivers/staging/olpc_dcon/olpc_dcon.c
index 26b4ec5..eb83b28 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon.c
@@ -210,6 +210,7 @@ static void dcon_sleep(struct dcon_priv *dcon, bool sleep)
if (sleep) {
u8 pm = 0;
+
x = olpc_ec_cmd(EC_DCON_POWER_MODE, &pm, 1, NULL, 0);
if (x)
pr_warn("unable to force dcon to power down: %d!\n", x);
@@ -240,6 +241,7 @@ static void dcon_sleep(struct dcon_priv *dcon, bool sleep)
static void dcon_load_holdoff(struct dcon_priv *dcon)
{
struct timespec delta_t, now;
+
while (1) {
getnstimeofday(&now);
delta_t = timespec_sub(now, dcon->load_time);
@@ -399,14 +401,15 @@ static ssize_t dcon_mode_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct dcon_priv *dcon = dev_get_drvdata(dev);
+
return sprintf(buf, "%4.4X\n", dcon->disp_mode);
}
static ssize_t dcon_sleep_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
-
struct dcon_priv *dcon = dev_get_drvdata(dev);
+
return sprintf(buf, "%d\n", dcon->asleep);
}
@@ -414,6 +417,7 @@ static ssize_t dcon_freeze_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct dcon_priv *dcon = dev_get_drvdata(dev);
+
return sprintf(buf, "%d\n", dcon->curr_src == DCON_SOURCE_DCON ? 1 : 0);
}
@@ -421,6 +425,7 @@ static ssize_t dcon_mono_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct dcon_priv *dcon = dev_get_drvdata(dev);
+
return sprintf(buf, "%d\n", dcon->mono);
}
@@ -534,6 +539,7 @@ static int dcon_bl_update(struct backlight_device *dev)
static int dcon_bl_get(struct backlight_device *dev)
{
struct dcon_priv *dcon = bl_get_data(dev);
+
return dcon->bl_val;
}
--
2.0.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/olpc_dcon: fix checkpatch warnings
On 23/05/2014 14:42, Greg Kroah-Hartman wrote: > All of the tabs were stripped out and replaced with spaces, making it > impossible to apply this patch :( > > Can you please fix up your email client and resend? Sorry, I rebase and resend it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging/olpc_dcon: fix checkpatch warnings
WARNING: Missing a blank line after declarations
Signed-off-by: Nicolas Joseph
---
drivers/staging/olpc_dcon/olpc_dcon.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c
b/drivers/staging/olpc_dcon/olpc_dcon.c
index 26b4ec5..eb83b28 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon.c
@@ -210,6 +210,7 @@ static void dcon_sleep(struct dcon_priv *dcon, bool sleep)
if (sleep) {
u8 pm = 0;
+
x = olpc_ec_cmd(EC_DCON_POWER_MODE, &pm, 1, NULL, 0);
if (x)
pr_warn("unable to force dcon to power down: %d!\n", x);
@@ -240,6 +241,7 @@ static void dcon_sleep(struct dcon_priv *dcon, bool sleep)
static void dcon_load_holdoff(struct dcon_priv *dcon)
{
struct timespec delta_t, now;
+
while (1) {
getnstimeofday(&now);
delta_t = timespec_sub(now, dcon->load_time);
@@ -399,14 +401,15 @@ static ssize_t dcon_mode_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct dcon_priv *dcon = dev_get_drvdata(dev);
+
return sprintf(buf, "%4.4X\n", dcon->disp_mode);
}
static ssize_t dcon_sleep_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
-
struct dcon_priv *dcon = dev_get_drvdata(dev);
+
return sprintf(buf, "%d\n", dcon->asleep);
}
@@ -414,6 +417,7 @@ static ssize_t dcon_freeze_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct dcon_priv *dcon = dev_get_drvdata(dev);
+
return sprintf(buf, "%d\n", dcon->curr_src == DCON_SOURCE_DCON ? 1 : 0);
}
@@ -421,6 +425,7 @@ static ssize_t dcon_mono_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct dcon_priv *dcon = dev_get_drvdata(dev);
+
return sprintf(buf, "%d\n", dcon->mono);
}
@@ -534,6 +539,7 @@ static int dcon_bl_update(struct backlight_device *dev)
static int dcon_bl_get(struct backlight_device *dev)
{
struct dcon_priv *dcon = bl_get_data(dev);
+
return dcon->bl_val;
}
--
2.0.0.rc4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: vt6656: fix leaks in error path
Fix memory leaks in ioctl error handling paths. Signed-off-by: Nicolas Thery --- drivers/staging/vt6656/hostap.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/vt6656/hostap.c b/drivers/staging/vt6656/hostap.c index 506e5d3..a25bfef 100644 --- a/drivers/staging/vt6656/hostap.c +++ b/drivers/staging/vt6656/hostap.c @@ -742,7 +742,8 @@ int vt6656_hostap_ioctl(struct vnt_private *pDevice, struct iw_point *p) case VIAWGET_HOSTAPD_MLME: DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "VIAWGET_HOSTAPD_MLME\n"); - return -EOPNOTSUPP; + ret = -EOPNOTSUPP; + goto out; case VIAWGET_HOSTAPD_SET_GENERIC_ELEMENT: DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "VIAWGET_HOSTAPD_SET_GENERIC_ELEMENT\n"); @@ -751,7 +752,8 @@ int vt6656_hostap_ioctl(struct vnt_private *pDevice, struct iw_point *p) case VIAWGET_HOSTAPD_SCAN_REQ: DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "VIAWGET_HOSTAPD_SCAN_REQ\n"); - return -EOPNOTSUPP; + ret = -EOPNOTSUPP; + goto out; case VIAWGET_HOSTAPD_STA_CLEAR_STATS: DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "VIAWGET_HOSTAPD_STA_CLEAR_STATS\n"); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: vt6656: fix leaks in error path
Sorry I didn't know staging patches should be based against linux-next. Please disregard this patch and the previous one. Best regards Nicolas On 30 May 2014 21:23, Christian Engelmayer wrote: > On Fri, 30 May 2014 20:47:44 +0200, Nicolas Thery wrote: >> Fix memory leaks in ioctl error handling paths. >> >> Signed-off-by: Nicolas Thery >> --- >> drivers/staging/vt6656/hostap.c | 6 -- >> 1 file changed, 4 insertions(+), 2 deletions(-) > > This doesn't apply against staging-next. > > The changeset itself seems to be identical to commit 721b79d1 (staging: > vt6656: > fix potential leak in vt6656_hostap_ioctl()) - > https://lkml.org/lkml/2014/5/7/833 > > The affected file has been remove later in commit a30d534b (staging: vt6656: > Remove dead code hostap.) - > http://www.spinics.net/lists/linux-wireless/msg122509.html > > Regards, > Christian ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] fix coding style issues
Done as task 10 of the eudyptula challenge.
Signed-off-by: Nicolas Koch
---
drivers/staging/octeon/ethernet-mem.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/octeon/ethernet-mem.c
b/drivers/staging/octeon/ethernet-mem.c
index bf666b0..964da86 100644
--- a/drivers/staging/octeon/ethernet-mem.c
+++ b/drivers/staging/octeon/ethernet-mem.c
@@ -46,9 +46,10 @@
static int cvm_oct_fill_hw_skbuff(int pool, int size, int elements)
{
int freed = elements;
- while (freed) {
+ while (freed) {
struct sk_buff *skb = dev_alloc_skb(size + 256);
+
if (unlikely(skb == NULL))
break;
skb_reserve(skb, 256 - (((unsigned long)skb->data) & 0x7f));
@@ -81,10 +82,10 @@ static void cvm_oct_free_hw_skbuff(int pool, int size, int
elements)
if (elements < 0)
pr_warn("Freeing of pool %u had too many skbuffs (%d)\n",
-pool, elements);
+ pool, elements);
else if (elements > 0)
pr_warn("Freeing of pool %u is missing %d skbuffs\n",
- pool, elements);
+ pool, elements);
}
/**
@@ -115,7 +116,7 @@ static int cvm_oct_fill_hw_memory(int pool, int size, int
elements)
memory = kmalloc(size + 256, GFP_ATOMIC);
if (unlikely(memory == NULL)) {
pr_warn("Unable to allocate %u bytes for FPA pool %d\n",
- elements * size, pool);
+ elements * size, pool);
break;
}
fpa = (char *)(((unsigned long)memory + 256) & ~0x7fUL);
@@ -136,6 +137,7 @@ static void cvm_oct_free_hw_memory(int pool, int size, int
elements)
{
char *memory;
char *fpa;
+
do {
fpa = cvmx_fpa_alloc(pool);
if (fpa) {
@@ -157,6 +159,7 @@ static void cvm_oct_free_hw_memory(int pool, int size, int
elements)
int cvm_oct_mem_fill_fpa(int pool, int size, int elements)
{
int freed;
+
if (USE_SKBUFFS_IN_HW && pool == CVMX_FPA_PACKET_POOL)
freed = cvm_oct_fill_hw_skbuff(pool, size, elements);
else
--
1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch] Staging: octeon: minor style cleanups
Done as task 10 of the eudyptula challenge.
Signed-off-by: Nicolas Koch
---
drivers/staging/octeon/ethernet-mem.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/octeon/ethernet-mem.c
b/drivers/staging/octeon/ethernet-mem.c
index bf666b0..964da86 100644
--- a/drivers/staging/octeon/ethernet-mem.c
+++ b/drivers/staging/octeon/ethernet-mem.c
@@ -46,9 +46,10 @@
static int cvm_oct_fill_hw_skbuff(int pool, int size, int elements)
{
int freed = elements;
- while (freed) {
+ while (freed) {
struct sk_buff *skb = dev_alloc_skb(size + 256);
+
if (unlikely(skb == NULL))
break;
skb_reserve(skb, 256 - (((unsigned long)skb->data) & 0x7f));
@@ -81,10 +82,10 @@ static void cvm_oct_free_hw_skbuff(int pool, int size, int
elements)
if (elements < 0)
pr_warn("Freeing of pool %u had too many skbuffs (%d)\n",
-pool, elements);
+ pool, elements);
else if (elements > 0)
pr_warn("Freeing of pool %u is missing %d skbuffs\n",
- pool, elements);
+ pool, elements);
}
/**
@@ -115,7 +116,7 @@ static int cvm_oct_fill_hw_memory(int pool, int size, int
elements)
memory = kmalloc(size + 256, GFP_ATOMIC);
if (unlikely(memory == NULL)) {
pr_warn("Unable to allocate %u bytes for FPA pool %d\n",
- elements * size, pool);
+ elements * size, pool);
break;
}
fpa = (char *)(((unsigned long)memory + 256) & ~0x7fUL);
@@ -136,6 +137,7 @@ static void cvm_oct_free_hw_memory(int pool, int size, int
elements)
{
char *memory;
char *fpa;
+
do {
fpa = cvmx_fpa_alloc(pool);
if (fpa) {
@@ -157,6 +159,7 @@ static void cvm_oct_free_hw_memory(int pool, int size, int
elements)
int cvm_oct_mem_fill_fpa(int pool, int size, int elements)
{
int freed;
+
if (USE_SKBUFFS_IN_HW && pool == CVMX_FPA_PACKET_POOL)
freed = cvm_oct_fill_hw_skbuff(pool, size, elements);
else
--
1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: include missing header
This patch fixes the following sparse warnings: drivers/staging/rtl8188eu/hal/phy.c:46:5: warning: symbol 'phy_query_bb_reg' was not declared. Should it be static? drivers/staging/rtl8188eu/hal/phy.c:56:6: warning: symbol 'phy_set_bb_reg' was not declared. Should it be static? drivers/staging/rtl8188eu/hal/phy.c:129:5: warning: symbol 'phy_query_rf_reg' was not declared. Should it be static? drivers/staging/rtl8188eu/hal/phy.c:140:6: warning: symbol 'phy_set_rf_reg' was not declared. Should it be static? drivers/staging/rtl8188eu/hal/phy.c:228:6: warning: symbol 'phy_set_tx_power_level' was not declared. Should it be static? drivers/staging/rtl8188eu/hal/phy.c:324:6: warning: symbol 'phy_set_bw_mode' was not declared. Should it be static? drivers/staging/rtl8188eu/hal/phy.c:360:6: warning: symbol 'phy_sw_chnl' was not declared. Should it be static? Signed-off-by: Nicolas Thery --- drivers/staging/rtl8188eu/hal/phy.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/rtl8188eu/hal/phy.c b/drivers/staging/rtl8188eu/hal/phy.c index 7291b46..3b569b0 100644 --- a/drivers/staging/rtl8188eu/hal/phy.c +++ b/drivers/staging/rtl8188eu/hal/phy.c @@ -24,6 +24,7 @@ #include #include #include +#include #define MAX_PRECMD_CNT 16 #define MAX_RFDEPENDCMD_CNT 16 -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 31/44] arm: Register with kernel poweroff handler
On 07/10/2014 07:28, Guenter Roeck : > Register with kernel poweroff handler instead of setting pm_power_off > directly. Always use register_poweroff_handler_simple as there is no > indication that more than one poweroff handler is registered. > > If the poweroff handler only resets the system or puts the CPU in sleep mode, > select a priority of 0 to indicate that the poweroff handler is one of last > resort. If the poweroff handler powers off the system, select a priority > of 128. > > Cc: Russell King > Cc: Andrew Victor > Cc: Nicolas Ferre > Cc: Jean-Christophe Plagniol-Villard > Cc: Stephen Warren > Cc: Christian Daudt > Cc: Matt Porter > Cc: Anton Vorontsov > Cc: Rob Herring > Cc: Shawn Guo > Cc: Sascha Hauer > Cc: Imre Kaloz > Cc: Krzysztof Halasa > Cc: Tony Lindgren > Cc: Jason Cooper > Cc: Andrew Lunn > Cc: Sebastian Hesselbarth > Cc: Eric Miao > Cc: Haojian Zhuang > Cc: Robert Jarzmik > Cc: Marek Vasut > Cc: Ben Dooks > Cc: Kukjin Kim > Cc: Linus Walleij > Cc: Tony Prisk > Cc: Stefano Stabellini > Signed-off-by: Guenter Roeck > --- > arch/arm/kernel/psci.c | 2 +- > arch/arm/mach-at91/board-gsia18s.c | 2 +- > arch/arm/mach-at91/setup.c | 4 ++-- For the 2 files above: NAK just because: for the first one -> this file will be removed during 3.19 dev cycle. for the second one -> the poweroff handlers are removed during the 3.18 cycle and converted in the appropriate poweroff drivers (in drivers/power/reset/at91-poweroff.c). So, can you remove these changes from your patch? Thanks for your work, bye. > arch/arm/mach-bcm/board_bcm2835.c | 2 +- > arch/arm/mach-cns3xxx/cns3420vb.c | 2 +- > arch/arm/mach-cns3xxx/core.c | 2 +- > arch/arm/mach-highbank/highbank.c | 2 +- > arch/arm/mach-imx/mach-mx31moboard.c | 2 +- > arch/arm/mach-iop32x/em7210.c | 2 +- > arch/arm/mach-iop32x/glantank.c| 2 +- > arch/arm/mach-iop32x/iq31244.c | 2 +- > arch/arm/mach-iop32x/n2100.c | 2 +- > arch/arm/mach-ixp4xx/dsmg600-setup.c | 2 +- > arch/arm/mach-ixp4xx/nas100d-setup.c | 2 +- > arch/arm/mach-ixp4xx/nslu2-setup.c | 2 +- > arch/arm/mach-omap2/board-omap3touchbook.c | 2 +- > arch/arm/mach-orion5x/board-mss2.c | 2 +- > arch/arm/mach-orion5x/dns323-setup.c | 6 +++--- > arch/arm/mach-orion5x/kurobox_pro-setup.c | 2 +- > arch/arm/mach-orion5x/ls-chl-setup.c | 2 +- > arch/arm/mach-orion5x/ls_hgl-setup.c | 2 +- > arch/arm/mach-orion5x/lsmini-setup.c | 2 +- > arch/arm/mach-orion5x/mv2120-setup.c | 2 +- > arch/arm/mach-orion5x/net2big-setup.c | 2 +- > arch/arm/mach-orion5x/terastation_pro2-setup.c | 2 +- > arch/arm/mach-orion5x/ts209-setup.c| 2 +- > arch/arm/mach-orion5x/ts409-setup.c| 2 +- > arch/arm/mach-pxa/corgi.c | 2 +- > arch/arm/mach-pxa/mioa701.c| 2 +- > arch/arm/mach-pxa/poodle.c | 2 +- > arch/arm/mach-pxa/spitz.c | 2 +- > arch/arm/mach-pxa/tosa.c | 2 +- > arch/arm/mach-pxa/viper.c | 2 +- > arch/arm/mach-pxa/z2.c | 6 +++--- > arch/arm/mach-pxa/zeus.c | 6 +++--- > arch/arm/mach-s3c24xx/mach-gta02.c | 2 +- > arch/arm/mach-s3c24xx/mach-jive.c | 2 +- > arch/arm/mach-s3c24xx/mach-vr1000.c| 2 +- > arch/arm/mach-s3c64xx/mach-smartq.c| 2 +- > arch/arm/mach-sa1100/generic.c | 2 +- > arch/arm/mach-sa1100/simpad.c | 2 +- > arch/arm/mach-u300/regulator.c | 2 +- > arch/arm/mach-vt8500/vt8500.c | 2 +- > arch/arm/xen/enlighten.c | 2 +- > 44 files changed, 51 insertions(+), 51 deletions(-) > > diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c > index f73891b..dd58d86 100644 > --- a/arch/arm/kernel/psci.c > +++ b/arch/arm/kernel/psci.c > @@ -264,7 +264,7 @@ static int psci_0_2_init(struct device_node *np) > > arm_pm_restart = psci_sys_reset; > > - pm_power_off = psci_sys_poweroff; > + register_poweroff_handler_simple(psci_sys_poweroff, 128); > > out_put_node: > of_node_put(np); > diff --git a/arch/arm/mach-at91/board-gsia18s.c > b/arch/arm/mach-at91/board-gsia18s.c > index b729dd1..6722e66 100644 > --- a/arch/arm/mach-at91/board-gsia18s.c > +++ b/arch/arm/mach-at91/board-gsia18s.c > @@ -521,
Re: [PATCHv3 3/3] drm: bridge: anx78xx: Add anx78xx driver support by analogix.
Hi Enric,
Partial review for now, thanks for you work.
Best,
On Thu, Sep 10, 2015 at 06:35:52PM +0200, Enric Balletbo i Serra wrote:
> At the moment it only supports ANX7814.
>
> The ANX7814 is an ultra-low power Full-HD (1080p60) SlimPort transmitter
> designed for portable devices.
>
> This driver adds initial support and supports HDMI to DP pass-through mode.
>
> Signed-off-by: Enric Balletbo i Serra
> ---
Please include a revision log here, stating what you changed between each
version.
> drivers/gpu/drm/bridge/Kconfig |2 +
> drivers/gpu/drm/bridge/Makefile |1 +
> drivers/gpu/drm/bridge/anx78xx/Kconfig |7 +
> drivers/gpu/drm/bridge/anx78xx/Makefile |4 +
> drivers/gpu/drm/bridge/anx78xx/anx78xx.h | 44 +
> drivers/gpu/drm/bridge/anx78xx/anx78xx_main.c| 241 ++
> drivers/gpu/drm/bridge/anx78xx/slimport_tx_drv.c | 3198
> ++
> drivers/gpu/drm/bridge/anx78xx/slimport_tx_drv.h | 215 ++
> drivers/gpu/drm/bridge/anx78xx/slimport_tx_reg.h | 786 ++
> 9 files changed, 4498 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/anx78xx/Kconfig
> create mode 100644 drivers/gpu/drm/bridge/anx78xx/Makefile
> create mode 100644 drivers/gpu/drm/bridge/anx78xx/anx78xx.h
> create mode 100644 drivers/gpu/drm/bridge/anx78xx/anx78xx_main.c
> create mode 100644 drivers/gpu/drm/bridge/anx78xx/slimport_tx_drv.c
> create mode 100644 drivers/gpu/drm/bridge/anx78xx/slimport_tx_drv.h
> create mode 100644 drivers/gpu/drm/bridge/anx78xx/slimport_tx_reg.h
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 2de52a5..aa6fe12 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -29,4 +29,6 @@ config DRM_PARADE_PS8622
> ---help---
> Parade eDP-LVDS bridge chip driver.
>
> +source "drivers/gpu/drm/bridge/anx78xx/Kconfig"
> +
> endmenu
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index e2eef1c..e5bd38b 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -3,3 +3,4 @@ ccflags-y := -Iinclude/drm
> obj-$(CONFIG_DRM_DW_HDMI) += dw_hdmi.o
> obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> +obj-$(CONFIG_DRM_ANX78XX) += anx78xx/
> diff --git a/drivers/gpu/drm/bridge/anx78xx/Kconfig
> b/drivers/gpu/drm/bridge/anx78xx/Kconfig
> new file mode 100644
> index 000..08f9c08
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/anx78xx/Kconfig
> @@ -0,0 +1,7 @@
> +config DRM_ANX78XX
> + tristate "Analogix ANX78XX bridge"
> + help
> + ANX78XX is a HD video transmitter chip over micro-USB
> + connector for smartphone device.
> +
> +
> diff --git a/drivers/gpu/drm/bridge/anx78xx/Makefile
> b/drivers/gpu/drm/bridge/anx78xx/Makefile
> new file mode 100644
> index 000..a843733
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/anx78xx/Makefile
> @@ -0,0 +1,4 @@
> +obj-${CONFIG_DRM_ANX78XX} := anx78xx.o
> +
> +anx78xx-y += anx78xx_main.o
> +anx78xx-y += slimport_tx_drv.o
> diff --git a/drivers/gpu/drm/bridge/anx78xx/anx78xx.h
> b/drivers/gpu/drm/bridge/anx78xx/anx78xx.h
> new file mode 100644
> index 000..4f6dd1d
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/anx78xx/anx78xx.h
> @@ -0,0 +1,44 @@
> +/*
> + * Copyright(c) 2015, Analogix Semiconductor. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#ifndef __ANX78xx_H
> +#define __ANX78xx_H
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define AUX_ERR 1
> +#define AUX_OK 0
> +
> +struct anx78xx_platform_data {
> + struct gpio_desc *gpiod_pd;
> + struct gpio_desc *gpiod_reset;
> + spinlock_t lock;
> +};
> +
> +struct anx78xx {
> + struct i2c_client *client;
> + struct anx78xx_platform_data *pdata;
> + struct delayed_work work;
> + struct workqueue_struct *workqueue;
> + struct mutex lock;
> +};
> +
> +void anx78xx_poweron(struct anx78xx *data);
> +void anx78xx_poweroff(struct anx78xx *data);
> +
> +#endif
> diff --git a/drivers/gpu/drm/bridge/anx78xx/anx78xx_main.c
> b/drivers/gpu/drm/bridge/anx78xx/anx78xx_main.c
> new file mode 100644
> index 000..b92d2bc
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/anx78xx/anx78xx_main.c
> @@ -0,0 +1,241 @@
> +/*
> + * Copyright(c) 2015, Analogix Semiconductor. All rights reserved.
> + *
> + * This program is free software; you can redistribute
[PATCH] staging/rtl8192u: remove unused function
Remove N_DBPSOfRate used in ComputeTxTime, remove by
commit 742728f97a99 ("staging: rtl8192u: remove unused function.")
Signed-off-by: Nicolas Joseph
---
drivers/staging/rtl8192u/r8192U_core.c | 44 --
1 file changed, 44 deletions(-)
diff --git a/drivers/staging/rtl8192u/r8192U_core.c
b/drivers/staging/rtl8192u/r8192U_core.c
index b143b36..80a6a4f 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -1227,50 +1227,6 @@ inline u8 rtl8192_IsWirelessBMode(u16 rate)
return 0;
}
-u16 N_DBPSOfRate(u16 DataRate)
-{
- u16 N_DBPS = 24;
-
- switch (DataRate) {
- case 60:
- N_DBPS = 24;
- break;
-
- case 90:
- N_DBPS = 36;
- break;
-
- case 120:
- N_DBPS = 48;
- break;
-
- case 180:
- N_DBPS = 72;
- break;
-
- case 240:
- N_DBPS = 96;
- break;
-
- case 360:
- N_DBPS = 144;
- break;
-
- case 480:
- N_DBPS = 192;
- break;
-
- case 540:
- N_DBPS = 216;
- break;
-
- default:
- break;
- }
-
- return N_DBPS;
-}
-
short rtl819xU_tx_cmd(struct net_device *dev, struct sk_buff *skb)
{
struct r8192_priv *priv = ieee80211_priv(dev);
--
2.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/rtl8192u: remove unused function
Le 14/09/15 à 14:25, Raphaël Beamonte a écrit : > Not sure the commit message is really clear. Something like this seems > more clean to me: > "Remove unused function N_DBPSOfRate. This function was only > used by function ComputeTxTime that was removed in a previous > commit." Indeed, is more clear. I repost my path with this commit message, thank you. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging/rtl8192u: remove unused function
Remove unused function N_DBPSOfRate. This function was only used by
function ComputeTxTime that was removed in the previous
commit 742728f97a99 ("staging: rtl8192u: remove unused function.")
Signed-off-by: Nicolas Joseph
---
drivers/staging/rtl8192u/r8192U_core.c | 44 --
1 file changed, 44 deletions(-)
diff --git a/drivers/staging/rtl8192u/r8192U_core.c
b/drivers/staging/rtl8192u/r8192U_core.c
index b143b36..80a6a4f 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -1227,50 +1227,6 @@ inline u8 rtl8192_IsWirelessBMode(u16 rate)
return 0;
}
-u16 N_DBPSOfRate(u16 DataRate)
-{
- u16 N_DBPS = 24;
-
- switch (DataRate) {
- case 60:
- N_DBPS = 24;
- break;
-
- case 90:
- N_DBPS = 36;
- break;
-
- case 120:
- N_DBPS = 48;
- break;
-
- case 180:
- N_DBPS = 72;
- break;
-
- case 240:
- N_DBPS = 96;
- break;
-
- case 360:
- N_DBPS = 144;
- break;
-
- case 480:
- N_DBPS = 192;
- break;
-
- case 540:
- N_DBPS = 216;
- break;
-
- default:
- break;
- }
-
- return N_DBPS;
-}
-
short rtl819xU_tx_cmd(struct net_device *dev, struct sk_buff *skb)
{
struct r8192_priv *priv = ieee80211_priv(dev);
--
2.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: wilc1000: Modify null check routine
Le 17/09/2015 10:50, Tony Cho a écrit :
> From: Leo Kim
>
> This patch modify null check routine.
> - Null check error non return. (Handle_RcvdGnrlAsyncInfo)
It doesn't parse...
Is it fixing a bug? What were the consequences without the return?
Bye,
> Signed-off-by: Leo Kim
> Signed-off-by: Tony Cho
> ---
> drivers/staging/wilc1000/host_interface.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c
> b/drivers/staging/wilc1000/host_interface.c
> index 6fdf392..a9eaa8f 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -2403,8 +2403,10 @@ static s32 Handle_RcvdGnrlAsyncInfo(tstrWILC_WFIDrv
> *drvHandler, tstrRcvdGnrlAsy
> s32 s32Err = 0;
> tstrWILC_WFIDrv *pstrWFIDrv = (tstrWILC_WFIDrv *) drvHandler;
>
> - if (pstrWFIDrv == NULL)
> + if (!pstrWFIDrv) {
> PRINT_ER("Driver handler is NULL\n");
> + return -EFAULT;
> + }
> PRINT_D(GENERIC_DBG, "Current State = %d,Received state = %d\n",
> pstrWFIDrv->enuHostIFstate,
> pstrRcvdGnrlAsyncInfo->pu8Buffer[7]);
>
>
--
Nicolas Ferre
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging/rdma/hfi1: do not use u8 to store a 32-bit integer
hfi1_rc_hdrerr() stores the result of be32_to_cpu() into opcode, which is a local variable declared as u8. Later this variable is used in a 24-bit logical right shift, which makes clang complains (when building an allmodconfig kernel with LLVMLinux patches): drivers/staging/rdma/hfi1/rc.c:2399:9: warning: shift count >= width of type [-Wshift-count-overflow] opcode >>= 24; ^ ~~ All of this lead to the point that opcode may have been designed to be a 32-bit integer instead of an 8-bit one. Therefore make this variable u32. Signed-off-by: Nicolas Iooss --- drivers/staging/rdma/hfi1/rc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rdma/hfi1/rc.c b/drivers/staging/rdma/hfi1/rc.c index 632dd5ba7dfd..1e9caebb0281 100644 --- a/drivers/staging/rdma/hfi1/rc.c +++ b/drivers/staging/rdma/hfi1/rc.c @@ -2383,7 +2383,7 @@ void hfi1_rc_hdrerr( struct hfi1_other_headers *ohdr; struct hfi1_ibport *ibp = to_iport(qp->ibqp.device, qp->port_num); int diff; - u8 opcode; + u32 opcode; u32 psn; /* Check for GRH */ -- 2.5.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ft1000: fix some checkpatch complaints
This patch fixes the following errors and warnings:
ft1000_proc.c:26: WARNING: Use #include instead of
ft1000_proc.c:27: WARNING: Use #include instead of
ft1000_proc.c:33: ERROR: Macros with multiple statements should be enclosed in
a do - while loop
ft1000_proc.c:40: ERROR: Macros with multiple statements should be enclosed in
a do - while loop
ft1000_proc.c:49: WARNING: static const char * array should probably be static
const char * const
ft1000_proc.c:53: WARNING: static const char * array should probably be static
const char * const
Signed-off-by: Nicolas Thery
---
drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
index 88f6f9c..28a2744 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
@@ -23,34 +23,38 @@
#include
#include
#include
-#include
-#include
+#include
+#include
#include "ft1000.h"
#define FT1000_PROC "ft1000"
#define MAX_FILE_LEN 255
-#define seq_putx(m, message, size, var) \
+#define seq_putx(m, message, size, var) do { \
seq_printf(m, message); \
for (i = 0; i < (size - 1); i++) { \
seq_printf(m, "%02x:", var[i]); \
} \
- seq_printf(m, "%02x\n", var[i])
+ seq_printf(m, "%02x\n", var[i]); \
+} while (0)
-#define seq_putd(m, message, size, var) \
+#define seq_putd(m, message, size, var) do { \
seq_printf(m, message); \
for (i = 0; i < (size - 1); i++) { \
seq_printf(m, "%d.", var[i]); \
} \
- seq_printf(m, "%d\n", var[i])
+ seq_printf(m, "%d\n", var[i]); \
+} while (0)
static int ft1000ReadProc(struct seq_file *m, void *v)
{
- static const char *status[] = {
+ static const char * const status[] = {
"Idle (Disconnect)", "Searching", "Active (Connected)",
"Waiting for L2", "Sleep", "No Coverage", "", ""
};
- static const char *signal[] = { "", "*", "**", "***", "" };
+ static const char * const signal[] = {
+ "", "*", "**", "***", ""
+ };
struct net_device *dev = m->private;
struct ft1000_info *info = netdev_priv(dev);
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: ft1000: fix some checkpatch complaints
On Sun, Jul 27, 2014 at 10:35:38AM -0700, Greg KH wrote:
> On Sun, Jul 27, 2014 at 10:23:53AM -0700, Joe Perches wrote:
> > On Sun, 2014-07-27 at 19:08 +0200, Nicolas Thery wrote:
> > > This patch fixes the following errors and warnings:
> > []
> > > diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
> > > b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
> > []
> > > @@ -23,34 +23,38 @@
> > []
> > > -#define seq_putx(m, message, size, var) \
> > > +#define seq_putx(m, message, size, var) do { \
> > > seq_printf(m, message); \
> > > for (i = 0; i < (size - 1); i++) { \
> > > seq_printf(m, "%02x:", var[i]); \
> > > } \
> > > - seq_printf(m, "%02x\n", var[i])
> > > + seq_printf(m, "%02x\n", var[i]); \
> > > +} while (0)
> >
> > Ideally, these wouldn't depend on an external
> > i variable.
> >
> > Maybe something like:
> >
> > #define seq_putx(m, message, size, var) \
> > do { \
> > int _i; \
> > seq_printf(m, message); \
> > for (_i = 0; _i < (size - 1); _i++) \
> > seq_printf(m, "%02x:", var[_i]); \
> > seq_printf(m, "%02x\n", var[_i]); \
> > } while (0)
> >
> > > -#define seq_putd(m, message, size, var) \
> > > +#define seq_putd(m, message, size, var) do { \
> > > seq_printf(m, message); \
> > > for (i = 0; i < (size - 1); i++) { \
> > > seq_printf(m, "%d.", var[i]); \
> > > } \
> > > - seq_printf(m, "%d\n", var[i])
> > > + seq_printf(m, "%d\n", var[i]); \
> > > +} while (0)
> >
> > Maybe later change these to use the recently introduced
> > seq_hex_dump: https://lkml.org/lkml/2014/7/11/269
> >
>
> How about removing the proc files entirely, as I doubt they are really
> needed :)
Okay. I'll do this.
Thanks to both of you for the review.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ft1000: remove procfs entries
This patch started as a fix to some checkpatch complaints in ft1000
procfs code but Greg suggested to remove the procfs entries altogether:
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2014-July/055594.html
Signed-off-by: Nicolas Thery
---
drivers/staging/ft1000/ft1000-pcmcia/Makefile | 3 +-
drivers/staging/ft1000/ft1000-pcmcia/ft1000.h | 2 -
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 4 -
drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c | 211 --
drivers/staging/ft1000/ft1000-usb/Makefile | 2 +-
drivers/staging/ft1000/ft1000-usb/ft1000_proc.c| 241 -
drivers/staging/ft1000/ft1000-usb/ft1000_usb.c | 8 -
drivers/staging/ft1000/ft1000-usb/ft1000_usb.h | 3 -
drivers/staging/ft1000/ft1000.h| 2 -
9 files changed, 2 insertions(+), 474 deletions(-)
delete mode 100644 drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
delete mode 100644 drivers/staging/ft1000/ft1000-usb/ft1000_proc.c
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/Makefile
b/drivers/staging/ft1000/ft1000-pcmcia/Makefile
index 660b7a5..715de3f 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/Makefile
+++ b/drivers/staging/ft1000/ft1000-pcmcia/Makefile
@@ -1,3 +1,2 @@
obj-$(CONFIG_FT1000_PCMCIA) = ft1000_pcmcia.o
-ft1000_pcmcia-y := ft1000_hw.o ft1000_dnld.o ft1000_proc.o ft1000_cs.o
-
+ft1000_pcmcia-y := ft1000_hw.o ft1000_dnld.o ft1000_cs.o
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000.h
b/drivers/staging/ft1000/ft1000-pcmcia/ft1000.h
index 0c21ac6..1d52738 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000.h
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000.h
@@ -47,8 +47,6 @@ extern struct net_device *init_ft1000_card(struct
pcmcia_device *link,
extern void stop_ft1000_card(struct net_device *dev);
extern int card_download(struct net_device *dev, const u8 *pFileStart,
size_t FileLength);
-extern void ft1000InitProc(struct net_device *dev);
-extern void ft1000CleanupProc(struct net_device *dev);
extern u16 ft1000_read_dpram(struct net_device *dev, int offset);
extern void card_bootload(struct net_device *dev);
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
index a6158be..21f09fe 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
@@ -19,8 +19,6 @@
#include
#include
-#include
-
#include
#include
#include
@@ -2102,7 +2100,6 @@ void stop_ft1000_card(struct net_device *dev)
release_region(dev->base_addr,256);
release_firmware(fw_entry);
flarion_ft1000_cnt--;
- ft1000CleanupProc(dev);
}
@@ -2247,7 +2244,6 @@ struct net_device *init_ft1000_card(struct pcmcia_device
*link,
ft1000_enable_interrupts(dev);
- ft1000InitProc(dev);
ft1000_card_present = 1;
dev->ethtool_ops = &ops;
printk(KERN_INFO "ft1000: %s: addr 0x%04lx irq %d, MAC addr %pM\n",
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
deleted file mode 100644
index 88f6f9c..000
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_proc.c
+++ /dev/null
@@ -1,211 +0,0 @@
-/*---
- FT1000 driver for Flarion Flash OFDM NIC Device
-
- Copyright (C) 2006 Patrik Ostrihon, All rights reserved.
- Copyright (C) 2006 ProWeb Consulting, a.s, All rights reserved.
-
- This program is free software; you can redistribute it and/or modify it
- under the terms of the GNU General Public License as published by the Free
- Software Foundation; either version 2 of the License, or (at your option)
any
- later version. This program is distributed in the hope that it will be
useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY
- or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- more details. You should have received a copy of the GNU General Public
- License along with this program; if not, write to the
- Free Software Foundation, Inc., 59 Temple Place -
- Suite 330, Boston, MA 02111-1307, USA.
--*/
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include "ft1000.h"
-
-#define FT1000_PROC "ft1000"
-#define MAX_FILE_LEN 255
-
-#define seq_putx(m, message, size, var) \
- seq_printf(m, message); \
- for (i = 0; i < (size - 1); i++) { \
- seq_printf(m, "%02x:", var[i]); \
- } \
- seq_printf(m, "%02x\n", var[i])
-
-#define seq_putd(m, message, size, var) \
- seq_printf(m, message); \
- for (i = 0; i < (size - 1); i++) { \
-
Re: [PATCH v2 0/3] Initial driver support for Xilinx M2M Video Scaler
Le mercredi 21 février 2018 à 14:43 -0800, Rohit Athavale a écrit : > This patch series has three commits : > - Driver support for the Xilinx M2M Video Scaler IP > - TODO document > - DT binding doc > > Changes in HW register map is expected as the IP undergoes changes. > This is a first attempt at the driver as an early prototype. > > This is a M2M Video Scaler IP that uses polyphases filters to perform > video scaling. The driver will be used by an application like a > gstreamer plugin. I'm hoping you know all this already, but just in case, rebasing your driver on videobuf2-v4l2.h interface would be automatically supported by GStreamer, and likely a better proposal for upstreaming. There few drivers already that could be use as an inspiration. ./drivers/media/platform/vim2m.c: Which demonstrate the API ./drivers/media/platform/exynos4-is/: Exynos4 imaging functions ./drivers/media/platform/exynos-gsc/: Exynos4 scaler (and more) ./drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c: MediaTek CSC/Scale ./drivers/media/platform/s5p-g2d/g2d.c: A 2D blitter iirc ? . . . I don't know them all, I have developped the GStreamer code with the Exynos4/5 platform, but also had success report on IMX6 (not upstream yet apparently). With the framework, you'll gain DMABuf with very little code. > > Change Log: > > v2 > - Cc'ing linux-media mailing list as suggested by Dan Carpenter. >Dan wanted to see if someone from linux-media can review the >driver interface in xm2m_vscale.c to see if it makes sense. > - Another question would be the right place to keep the driver, >in drivers/staging/media or drivers/staging/ > - Dropped empty mmap_open, mmap_close ops. > - Removed incorrect DMA_SHARED_BUFFER select from Kconfig > v1 - Initial version > > > Rohit Athavale (3): > staging: xm2mvscale: Driver support for Xilinx M2M Video Scaler > staging: xm2mvscale: Add TODO for the driver > Documentation: devicetree: bindings: Add DT binding doc for xm2mvsc > driver > > drivers/staging/Kconfig| 2 + > drivers/staging/Makefile | 1 + > .../devicetree/bindings/xm2mvscaler.txt| 25 + > drivers/staging/xm2mvscale/Kconfig | 11 + > drivers/staging/xm2mvscale/Makefile| 3 + > drivers/staging/xm2mvscale/TODO| 18 + > drivers/staging/xm2mvscale/ioctl_xm2mvsc.h | 134 +++ > drivers/staging/xm2mvscale/scaler_hw_xm2m.c| 945 > + > drivers/staging/xm2mvscale/scaler_hw_xm2m.h| 152 > drivers/staging/xm2mvscale/xm2m_vscale.c | 768 > + > drivers/staging/xm2mvscale/xvm2mvsc_hw_regs.h | 204 + > 11 files changed, 2263 insertions(+) > create mode 100644 > drivers/staging/xm2mvscale/Documentation/devicetree/bindings/xm2mvsca > ler.txt > create mode 100644 drivers/staging/xm2mvscale/Kconfig > create mode 100644 drivers/staging/xm2mvscale/Makefile > create mode 100644 drivers/staging/xm2mvscale/TODO > create mode 100644 drivers/staging/xm2mvscale/ioctl_xm2mvsc.h > create mode 100644 drivers/staging/xm2mvscale/scaler_hw_xm2m.c > create mode 100644 drivers/staging/xm2mvscale/scaler_hw_xm2m.h > create mode 100644 drivers/staging/xm2mvscale/xm2m_vscale.c > create mode 100644 drivers/staging/xm2mvscale/xvm2mvsc_hw_regs.h > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] staging: xm2mvscale: Driver support for Xilinx M2M Video Scaler
Le mardi 20 mars 2018 à 00:46 +, Rohit Athavale a écrit : > Hi Hans, > > Thanks for taking the time to take a look at this. > > > This should definitely use the V4L2 API. I guess it could be added > > to staging/media with a big fat TODO that this should be converted > > to > > the V4L2 mem2mem framework. > > > > But it makes no sense to re-invent the V4L2 streaming API :-) > > > > drivers/media/platform/mx2_emmaprp.c does something similar to > > this. > > It's a little bit outdated (not using the latest m2m helper > > functions) > > but it is a good starting point. > > I looked at the mx2_emmaprp.c and the Samsung G-Scaler M2M driver. > IMHO, the main difference between > the Hardware registers/capabilities is that mx2_emmaprp driver or the > gsc driver, have one scaling "channel" > if we might call it. Whereas the HW/IP I have in mind has 4-8 scaling > channels. > > By a scaling channel, I mean an entity of the HW or IP, that can take > the following parameters : > - Input height, stride , width, color format, input Y and Cb/Cr > physically contiguous memory pointers > - Output height, stride, width, color format, output Y and Cb/Cr > physically contiguous memory pointers > > Based on the above parameters, when the above are provided and the IP > is started, we get an interrupt on completion. > I'm sure you are familiar with this model. However, in the case of > this IP, there could be 4-8 such channels and a single interrupt > on the completion of the all 4-8 scaling operations. > > In this IP, we are trying to have 4-8 input sources being scaled by > this single piece of hardware, by time multiplexing. > > An example use case is : > > Four applications (sources) will feed (enqueue) 4 input buffers to > the scaler, the scaler driver will synchronize the programming of > these buffers, when the number of buffers received by the driver > meets our batch size (say a batch size of 4), it will kick start the > IP. The four applications will poll on the fd, upon receiving an > interrupt from the hardware the poll will unblock. And all four > applications can dequeue their respective buffers and display them on > a sink. You should think of a better scheduling model, it will be really hard to design userspace that collaborate in order to optimize the IP usage. I think a better approach would be to queue while the IP is busy. These queues can then be sorted and prioritized. > > But each "channel" can be set to do accept its own individual input > and output formats. When I went through : > https://www.kernel.org/doc/html/v4.14/media/uapi/v4l/open.html#multip > le-opens > > It appears, once an application has invoked VIDIOC_REQBUFS or > VIDIOC_CREATE_BUFS, other applications cannot VIDIOC_S_FMT on them. > However to maximize the available number of channels, it would be > necessary to allow several applications to be able to > perform VIDIOC_S_FMT on the device node in the case of this hardware > as different channels can be expected to deal with different scaling > operations. This does not apply to M2M devices. Each time userspace open an M2M device, it will get a different instance (unless there is no more resource available). What drivers like Samsung FIMC, GSCALER, MFC. etc. do, is that they limit the number of instances (open calls) to the number of streams they can handle in parallel. They don't seem to share an IRQ when doing batch though. > > One option is to create a logical /dev/videoX node for each such > channel, and have a parent driver perform the interrupt handling, > batch size setting and other such common functionalities. Is there a > way to allow multiple applications talk to the same video device > node/file handle without creating logical video nodes for each > channel ? FIMC used to expose a node per instance and it was terribly hard to use. I don't think this is a good idea. > > Please let me know if the description of HW is not clear. I will look > forward to hear comments from you. > > > > > So for this series: > > > > Nacked-by: Hans Verkuil > > > > If this was added to drivers/staging/media instead and with an > > updated > > TODO, then we can accept it, but we need to see some real effort > > afterwards > > to switch this to the right API. Otherwise it will be removed again > > after a few kernel cycles. > > > > Many thanks for providing a pathway to get this into > drivers/staging/media > > I will drop this series, and re-send with the driver being placed in > drivers/staging/media. > I'll add some references to this conversation, so a new reviewer gets > some context of what > was discussed. In the meanwhile I will look into re-writing this to > utilize the M2M V4L2 API. > > > Regards, > > > > Hans > > > Best Regards, > Rohit > > > > -Original Message- > > From: Hans Verkuil [mailto:hverk...@xs4all.nl] > > Sent: Friday, March 09, 2018 3:58 AM > > To: Greg KH ; Rohit Athavale > > > > Cc: de...@driverdev.osuosl.org; linux-me.
[PATCH] media: cedrus: Propagate OUTPUT resolution to CAPTURE
As per spec, the CAPTURE resolution should be automatically set based on the OTUPUT resolution. This patch properly propagate width/height to the capture when the OUTPUT format is set and override the user provided width/height with configured OUTPUT resolution when the CAPTURE fmt is updated. This also prevents userspace from selecting a CAPTURE resolution that is too small, avoiding unwanted page faults. Signed-off-by: Nicolas Dufresne --- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c index 16d82309e7b6..a6d6b15adc2e 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c @@ -247,6 +247,8 @@ static int cedrus_try_fmt_vid_cap(struct file *file, void *priv, return -EINVAL; pix_fmt->pixelformat = fmt->pixelformat; + pix_fmt->width = ctx->src_fmt.width; + pix_fmt->height = ctx->src_fmt.height; cedrus_prepare_format(pix_fmt); return 0; @@ -319,11 +321,14 @@ static int cedrus_s_fmt_vid_out(struct file *file, void *priv, break; } - /* Propagate colorspace information to capture. */ + /* Propagate format information to capture. */ ctx->dst_fmt.colorspace = f->fmt.pix.colorspace; ctx->dst_fmt.xfer_func = f->fmt.pix.xfer_func; ctx->dst_fmt.ycbcr_enc = f->fmt.pix.ycbcr_enc; ctx->dst_fmt.quantization = f->fmt.pix.quantization; + ctx->dst_fmt.width = ctx->src_fmt.width; + ctx->dst_fmt.height = ctx->src_fmt.height; + cedrus_prepare_format(&ctx->dst_fmt); return 0; } -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: cedrus: Propagate OUTPUT resolution to CAPTURE
Le jeudi 14 mai 2020 à 11:39 -0400, Nicolas Dufresne a écrit : > As per spec, the CAPTURE resolution should be automatically set based on > the OTUPUT resolution. This patch properly propagate width/height to the > capture when the OUTPUT format is set and override the user provided > width/height with configured OUTPUT resolution when the CAPTURE fmt is > updated. > > This also prevents userspace from selecting a CAPTURE resolution that is > too small, avoiding unwanted page faults. Side note, this patch is based on top of "Samuel Holland " patches: [PATCH v3 1/2] media: cedrus: Program output format during each run [PATCH v3 2/2] media: cedrus: Implement runtime PM As the patchset also fixes concurrent decoding issues. This was tested using GStreamer implementation, which strictly follow the Stateless CODEC spec and expect OUTPUT to CAPTURE width/height propagation. > > Signed-off-by: Nicolas Dufresne > --- > drivers/staging/media/sunxi/cedrus/cedrus_video.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > index 16d82309e7b6..a6d6b15adc2e 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > @@ -247,6 +247,8 @@ static int cedrus_try_fmt_vid_cap(struct file *file, void > *priv, > return -EINVAL; > > pix_fmt->pixelformat = fmt->pixelformat; > + pix_fmt->width = ctx->src_fmt.width; > + pix_fmt->height = ctx->src_fmt.height; > cedrus_prepare_format(pix_fmt); > > return 0; > @@ -319,11 +321,14 @@ static int cedrus_s_fmt_vid_out(struct file *file, void > *priv, > break; > } > > - /* Propagate colorspace information to capture. */ > + /* Propagate format information to capture. */ > ctx->dst_fmt.colorspace = f->fmt.pix.colorspace; > ctx->dst_fmt.xfer_func = f->fmt.pix.xfer_func; > ctx->dst_fmt.ycbcr_enc = f->fmt.pix.ycbcr_enc; > ctx->dst_fmt.quantization = f->fmt.pix.quantization; > + ctx->dst_fmt.width = ctx->src_fmt.width; > + ctx->dst_fmt.height = ctx->src_fmt.height; > + cedrus_prepare_format(&ctx->dst_fmt); > > return 0; > } ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: cedrus: Add support for VP8 decoding
Le mercredi 20 mai 2020 à 23:01 +0200, Jernej Skrabec a écrit :
> VP8 in Cedrus shares same engine as H264.
>
> Note that it seems necessary to call bitstream parsing functions,
> to parse frame header, otherwise decoded image is garbage. This is
> contrary to what is driver supposed to do. However, values are not
> really used, so this might be acceptable. It's possible that bitstream
Have you verified that all values passed through controls are not used
? To remain a stateless driver, there is no requirement for parsed data
to be used, the only requirement is that the reference are used.
Otherwise doing parallel decoding of two stream of different stream
would be broken. Have you verified that parallel decoding is working as
expected ?
> parsing functions set some internal VPU state, which is later necessary
> for proper decoding. Biggest suspect is "VP8 probs update" trigger.
>
> Signed-off-by: Jernej Skrabec
> ---
> drivers/staging/media/sunxi/cedrus/Makefile | 3 +-
> drivers/staging/media/sunxi/cedrus/cedrus.c | 8 +
> drivers/staging/media/sunxi/cedrus/cedrus.h | 15 +
> .../staging/media/sunxi/cedrus/cedrus_dec.c | 5 +
> .../staging/media/sunxi/cedrus/cedrus_hw.c| 1 +
> .../staging/media/sunxi/cedrus/cedrus_regs.h | 80 ++
> .../staging/media/sunxi/cedrus/cedrus_video.c | 9 +
> .../staging/media/sunxi/cedrus/cedrus_vp8.c | 699 ++
> 8 files changed, 819 insertions(+), 1 deletion(-)
> create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
>
> diff --git a/drivers/staging/media/sunxi/cedrus/Makefile
> b/drivers/staging/media/sunxi/cedrus/Makefile
> index 1bce49d3e7e2..a647b3690bf8 100644
> --- a/drivers/staging/media/sunxi/cedrus/Makefile
> +++ b/drivers/staging/media/sunxi/cedrus/Makefile
> @@ -2,4 +2,5 @@
> obj-$(CONFIG_VIDEO_SUNXI_CEDRUS) += sunxi-cedrus.o
>
> sunxi-cedrus-y = cedrus.o cedrus_video.o cedrus_hw.o cedrus_dec.o \
> - cedrus_mpeg2.o cedrus_h264.o cedrus_h265.o
> + cedrus_mpeg2.o cedrus_h264.o cedrus_h265.o \
> + cedrus_vp8.o
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
> b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index b320aa058f74..e6b864f05364 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -142,6 +142,13 @@ static const struct cedrus_control cedrus_controls[] = {
> .codec = CEDRUS_CODEC_H265,
> .required = false,
> },
> + {
> + .cfg = {
> + .id = V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER,
> + },
> + .codec = CEDRUS_CODEC_VP8,
> + .required = true,
> + },
> };
>
> #define CEDRUS_CONTROLS_COUNTARRAY_SIZE(cedrus_controls)
> @@ -388,6 +395,7 @@ static int cedrus_probe(struct platform_device *pdev)
> dev->dec_ops[CEDRUS_CODEC_MPEG2] = &cedrus_dec_ops_mpeg2;
> dev->dec_ops[CEDRUS_CODEC_H264] = &cedrus_dec_ops_h264;
> dev->dec_ops[CEDRUS_CODEC_H265] = &cedrus_dec_ops_h265;
> + dev->dec_ops[CEDRUS_CODEC_VP8] = &cedrus_dec_ops_vp8;
>
> mutex_init(&dev->dev_mutex);
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index f8264953dd04..353cb04b4ce5 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -35,6 +35,7 @@ enum cedrus_codec {
> CEDRUS_CODEC_MPEG2,
> CEDRUS_CODEC_H264,
> CEDRUS_CODEC_H265,
> + CEDRUS_CODEC_VP8,
> CEDRUS_CODEC_LAST,
> };
>
> @@ -76,6 +77,10 @@ struct cedrus_h265_run {
> const struct v4l2_ctrl_hevc_scaling_matrix *scaling_matrix;
> };
>
> +struct cedrus_vp8_run {
> + const struct v4l2_ctrl_vp8_frame_header *slice_params;
> +};
> +
> struct cedrus_run {
> struct vb2_v4l2_buffer *src;
> struct vb2_v4l2_buffer *dst;
> @@ -84,6 +89,7 @@ struct cedrus_run {
> struct cedrus_h264_run h264;
> struct cedrus_mpeg2_run mpeg2;
> struct cedrus_h265_run h265;
> + struct cedrus_vp8_run vp8;
> };
> };
>
> @@ -141,6 +147,14 @@ struct cedrus_ctx {
> void*entry_points_buf;
> dma_addr_t entry_points_buf_addr;
> } h265;
> + struct {
> + unsigned intlast_frame_p_type;
> + unsigned intlast_filter_type;
> + unsigned intlast_sharpness_level;
> +
> + u8 *entropy_probs_buf;
> + dma_addr_t entropy_probs_buf_dma;
> + } vp8;
> } codec;
> };
>
> @@ -188,6 +202,7 @@ struct cedrus_dev {
> extern struct cedrus_dec_ops cedrus_dec_ops_mpeg2;
> extern struct cedrus_dec_ops cedrus_dec_ops_h264;
> extern struct cedrus_dec_ops cedrus
Re: [PATCH 1/3] media: uapi: h264: update reference lists
Sorry, missed one thing.
Le vendredi 05 juin 2020 à 13:08 -0400, Nicolas Dufresne a écrit :
> Le jeudi 04 juin 2020 à 20:57 +0200, Jernej Skrabec a écrit :
> > When dealing with with interlaced frames, reference lists must tell if
> > each particular reference is meant for top or bottom field. This info
> > is currently not provided at all in the H264 related controls.
> >
> > Make reference lists hold a structure which will also hold flags along
> > index into DPB array. Flags will tell if reference is meant for top or
> > bottom field.
> >
> > Currently the only user of these lists is Cedrus which is just compile
> > fixed here. Actual usage of newly introduced flags will come in
> > following commit.
> >
> > Signed-off-by: Jernej Skrabec
>
> This looks like the right approach to me and is extensible if anything
> else is needed for MVC and SVC special referencing (at least will be
> enough for what H.264 actually supports in this regard).
>
> Reviewed-by: Nicolas Dufresne
>
> > ---
> > .../media/v4l/ext-ctrls-codec.rst | 40 ++-
> > .../staging/media/sunxi/cedrus/cedrus_h264.c | 6 +--
> > include/media/h264-ctrls.h| 12 +-
> > 3 files changed, 51 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index d0d506a444b1..6c36d298db20 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -1843,10 +1843,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > * - __u32
> >- ``slice_group_change_cycle``
> >-
> > -* - __u8
> > +* - struct :c:type:`v4l2_h264_reference`
> >- ``ref_pic_list0[32]``
> >- Reference picture list after applying the per-slice modifications
> > -* - __u8
> > +* - struct :c:type:`v4l2_h264_reference`
> >- ``ref_pic_list1[32]``
> >- Reference picture list after applying the per-slice modifications
> > * - __u32
> > @@ -1926,6 +1926,42 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> >- ``chroma_offset[32][2]``
> >-
> >
> > +``Picture Reference``
> > +
> > +.. c:type:: v4l2_h264_reference
> > +
> > +.. cssclass:: longtable
> > +
> > +.. flat-table:: struct v4l2_h264_reference
> > +:header-rows: 0
> > +:stub-columns: 0
> > +:widths: 1 1 2
> > +
> > +* - __u16
> > + - ``flags``
> > + - See :ref:`Picture Reference Flags `
> > +* - __u8
> > + - ``index``
> > + -
> > +
> > +.. _h264_reference_flags:
> > +
> > +``Picture Reference Flags``
> > +
> > +.. cssclass:: longtable
> > +
> > +.. flat-table::
> > +:header-rows: 0
> > +:stub-columns: 0
> > +:widths: 1 1 2
> > +
> > +* - ``V4L2_H264_REFERENCE_FLAG_TOP_FIELD``
> > + - 0x0001
> > + -
> > +* - ``V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD``
> > + - 0x0002
> > + -
> > +
> > ``V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS (struct)``
> > Specifies the decode parameters (as extracted from the bitstream)
> > for the associated H264 slice data. This includes the necessary
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > index 54ee2aa423e2..cce527bbdf86 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > @@ -166,8 +166,8 @@ static void cedrus_write_frame_list(struct cedrus_ctx
> > *ctx,
> >
> > static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> >struct cedrus_run *run,
> > - const u8 *ref_list, u8 num_ref,
> > - enum cedrus_h264_sram_off sram)
> > + const struct v4l2_h264_reference *ref_list,
> > + u8 num_ref, enum cedrus_h264_sram_off sram)
> > {
> > const struct v4l2_ctrl_h264_decode_params *decode =
> > run->h264.decode_params;
> > struct vb2_queue *cap_q;
> > @@ -188,7 +188,7 @@ static void _cedrus_write_ref_list(struct cedrus_ctx
> > *ctx,
> > int buf_idx;
> > u8 dpb_idx;
> &g
Re: [PATCH 1/3] media: uapi: h264: update reference lists
Le jeudi 04 juin 2020 à 20:57 +0200, Jernej Skrabec a écrit :
> When dealing with with interlaced frames, reference lists must tell if
> each particular reference is meant for top or bottom field. This info
> is currently not provided at all in the H264 related controls.
>
> Make reference lists hold a structure which will also hold flags along
> index into DPB array. Flags will tell if reference is meant for top or
> bottom field.
>
> Currently the only user of these lists is Cedrus which is just compile
> fixed here. Actual usage of newly introduced flags will come in
> following commit.
>
> Signed-off-by: Jernej Skrabec
This looks like the right approach to me and is extensible if anything
else is needed for MVC and SVC special referencing (at least will be
enough for what H.264 actually supports in this regard).
Reviewed-by: Nicolas Dufresne
> ---
> .../media/v4l/ext-ctrls-codec.rst | 40 ++-
> .../staging/media/sunxi/cedrus/cedrus_h264.c | 6 +--
> include/media/h264-ctrls.h| 12 +-
> 3 files changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index d0d506a444b1..6c36d298db20 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -1843,10 +1843,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> * - __u32
>- ``slice_group_change_cycle``
>-
> -* - __u8
> +* - struct :c:type:`v4l2_h264_reference`
>- ``ref_pic_list0[32]``
>- Reference picture list after applying the per-slice modifications
> -* - __u8
> +* - struct :c:type:`v4l2_h264_reference`
>- ``ref_pic_list1[32]``
>- Reference picture list after applying the per-slice modifications
> * - __u32
> @@ -1926,6 +1926,42 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
>- ``chroma_offset[32][2]``
>-
>
> +``Picture Reference``
> +
> +.. c:type:: v4l2_h264_reference
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_h264_reference
> +:header-rows: 0
> +:stub-columns: 0
> +:widths: 1 1 2
> +
> +* - __u16
> + - ``flags``
> + - See :ref:`Picture Reference Flags `
> +* - __u8
> + - ``index``
> + -
> +
> +.. _h264_reference_flags:
> +
> +``Picture Reference Flags``
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> +:header-rows: 0
> +:stub-columns: 0
> +:widths: 1 1 2
> +
> +* - ``V4L2_H264_REFERENCE_FLAG_TOP_FIELD``
> + - 0x0001
> + -
> +* - ``V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD``
> + - 0x0002
> + -
> +
> ``V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS (struct)``
> Specifies the decode parameters (as extracted from the bitstream)
> for the associated H264 slice data. This includes the necessary
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> index 54ee2aa423e2..cce527bbdf86 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -166,8 +166,8 @@ static void cedrus_write_frame_list(struct cedrus_ctx
> *ctx,
>
> static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> struct cedrus_run *run,
> -const u8 *ref_list, u8 num_ref,
> -enum cedrus_h264_sram_off sram)
> +const struct v4l2_h264_reference *ref_list,
> +u8 num_ref, enum cedrus_h264_sram_off sram)
> {
> const struct v4l2_ctrl_h264_decode_params *decode =
> run->h264.decode_params;
> struct vb2_queue *cap_q;
> @@ -188,7 +188,7 @@ static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> int buf_idx;
> u8 dpb_idx;
>
> - dpb_idx = ref_list[i];
> + dpb_idx = ref_list[i].index;
> dpb = &decode->dpb[dpb_idx];
>
> if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
> diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
> index 080fd1293c42..9b1cbc9bc38e 100644
> --- a/include/media/h264-ctrls.h
> +++ b/include/media/h264-ctrls.h
> @@ -140,6 +140,14 @@ struct v4l2_h264_pred_weight_table {
> #define V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED 0x04
> #define V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH 0x08
>
> +#define V4
Re: [PATCH 2/3] media: cedrus: h264: Properly configure reference field
Le jeudi 04 juin 2020 à 20:57 +0200, Jernej Skrabec a écrit :
> When interlaced H264 content is being decoded, references must indicate
> which field is being referenced. Currently this was done by checking
> capture buffer flags. However, that is not correct because capture
> buffer may hold both fields.
>
> Fix this by checking newly introduced flags in reference lists.
>
> Signed-off-by: Jernej Skrabec
Perhaps an additional patch could cleanup the miss-leading comment in
v4l2_h264_dpb_entry definition.
Reviewed-by: Nicolas Dufresne
> ---
> drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> index cce527bbdf86..c87717d17ec5 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -183,7 +183,6 @@ static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> for (i = 0; i < num_ref; i++) {
> const struct v4l2_h264_dpb_entry *dpb;
> const struct cedrus_buffer *cedrus_buf;
> - const struct vb2_v4l2_buffer *ref_buf;
> unsigned int position;
> int buf_idx;
> u8 dpb_idx;
> @@ -198,12 +197,11 @@ static void _cedrus_write_ref_list(struct cedrus_ctx
> *ctx,
> if (buf_idx < 0)
> continue;
>
> - ref_buf = to_vb2_v4l2_buffer(cap_q->bufs[buf_idx]);
> - cedrus_buf = vb2_v4l2_to_cedrus_buffer(ref_buf);
> + cedrus_buf = vb2_to_cedrus_buffer(cap_q->bufs[buf_idx]);
> position = cedrus_buf->codec.h264.position;
>
> sram_array[i] |= position << 1;
> - if (ref_buf->field == V4L2_FIELD_BOTTOM)
> + if (ref_list[i].flags & V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD)
> sram_array[i] |= BIT(0);
> }
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] media: uapi: cedrus: Fix decoding interlaced H264 content
Le samedi 06 juin 2020 à 09:46 -0300, Ezequiel Garcia a écrit : > Hi Jernej, > > On Thu, 4 Jun 2020 at 15:55, Jernej Skrabec wrote: > > Currently H264 interlaced content it's not properly decoded on Cedrus. > > There are two reasons for this: > > 1. slice parameters control doesn't provide enough information > > 2. bug in frame list construction in Cedrus driver > > > > As described in commit message in patch 1, references stored in > > reference lists should tell if reference targets top or bottom field. > > However, this information is currently not provided. Patch 1 adds > > it in form of flags which are set for each reference. Patch 2 then > > uses those flags in Cedrus driver. > > > > Frame list construction is fixed in patch 3. > > > > This solution was extensively tested using Kodi on LibreELEC with A64, > > H3, H5 and H6 SoCs in slightly different form (flags were transmitted > > in MSB bits in index). > > > > So, if I understand correctly the field needs to be passed per-reference, > and the current per-DPB entry is not good? For interlaced content we reference fields separately. That's the reason there is 32 entries in l0/l1. Though, as we decode both fields in the same buffer (interleaved), the DPB indice is not sufficient to inform the decoder what we are referencing. Understand that a top field can be used to decode the next bottom field. This even make sense as they are closer on the capture timeline. This covers slice based decoders. The flags to indicate presence of top/bottom fields in DPB array seems only useful to frame base decoders. This is so that decoder can avoid using lost fields when constructing it's own l0/l1 internally. > > If you could point at the userspace code for this, it would be interesting > to take a look. > > > Note: I'm not 100% sure if flags for both, top and bottom fields are > > needed. Any input here would be welcome. > > > > Given enum v4l2_field is already part of the uAPI, perhaps it makes > sense to just reuse that for the field type? Maybe it's an overkill, > but it would make sense to reuse the concepts and types that > already exist. > > We can still add a reserved field to make this new reference type > extensive. I think it's fine to create new flag for this, as your solution would require extending a reference to 24bit (this patch extend to 16bits). Though indeed, we could combine frame and TOP and reserve more bits for future use. > > Thanks, > Ezequiel > > > > Please take a look. > > > > Best regards, > > Jernej > > > > Jernej Skrabec (3): > > media: uapi: h264: update reference lists > > media: cedrus: h264: Properly configure reference field > > media: cedrus: h264: Fix frame list construction > > > > .../media/v4l/ext-ctrls-codec.rst | 40 ++- > > .../staging/media/sunxi/cedrus/cedrus_h264.c | 27 +++-- > > include/media/h264-ctrls.h| 12 +- > > 3 files changed, 62 insertions(+), 17 deletions(-) > > > > -- > > 2.27.0 > > > > > > ___ > > linux-arm-kernel mailing list > > linux-arm-ker...@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] media: uapi: cedrus: Fix decoding interlaced H264 content
Le dimanche 07 juin 2020 à 16:21 -0400, Nicolas Dufresne a écrit : > Le samedi 06 juin 2020 à 09:46 -0300, Ezequiel Garcia a écrit : > > Hi Jernej, > > > > On Thu, 4 Jun 2020 at 15:55, Jernej Skrabec wrote: > > > Currently H264 interlaced content it's not properly decoded on Cedrus. > > > There are two reasons for this: > > > 1. slice parameters control doesn't provide enough information > > > 2. bug in frame list construction in Cedrus driver > > > > > > As described in commit message in patch 1, references stored in > > > reference lists should tell if reference targets top or bottom field. > > > However, this information is currently not provided. Patch 1 adds > > > it in form of flags which are set for each reference. Patch 2 then > > > uses those flags in Cedrus driver. > > > > > > Frame list construction is fixed in patch 3. > > > > > > This solution was extensively tested using Kodi on LibreELEC with A64, > > > H3, H5 and H6 SoCs in slightly different form (flags were transmitted > > > in MSB bits in index). > > > > > > > So, if I understand correctly the field needs to be passed per-reference, > > and the current per-DPB entry is not good? > > For interlaced content we reference fields separately. That's the > reason there is 32 entries in l0/l1. Though, as we decode both fields > in the same buffer (interleaved), the DPB indice is not sufficient to > inform the decoder what we are referencing. Understand that a top field > can be used to decode the next bottom field. This even make sense as > they are closer on the capture timeline. This covers slice based > decoders. > > The flags to indicate presence of top/bottom fields in DPB array seems > only useful to frame base decoders. This is so that decoder can avoid > using lost fields when constructing it's own l0/l1 internally. > > > If you could point at the userspace code for this, it would be interesting > > to take a look. > > > > > Note: I'm not 100% sure if flags for both, top and bottom fields are > > > needed. Any input here would be welcome. > > > > > > > Given enum v4l2_field is already part of the uAPI, perhaps it makes > > sense to just reuse that for the field type? Maybe it's an overkill, > > but it would make sense to reuse the concepts and types that > > already exist. > > > > We can still add a reserved field to make this new reference type > > extensive. > > I think it's fine to create new flag for this, as your solution would > require extending a reference to 24bit (this patch extend to 16bits). > Though indeed, we could combine frame and TOP and reserve more bits for > future use. Sorry for the oversight, the flags is 16bits, so we already use 24bits. But looking at "enum v4l2_field", which is not a flag, seems like a miss-fit. It would create such a confusion, as V4L2_FIELD_SEQ_TB/BT can still be used with this interface, even though we still need to say if we reference TOP or BOTTOM. Only V4L2_FIELD_ALTERNATE is not supported. But as you can see, "enum v4l2_fiel" is really meant to describe the layout of the interleaved frame rather then signalling a field directly. > > > Thanks, > > Ezequiel > > > > > > > Please take a look. > > > > > > Best regards, > > > Jernej > > > > > > Jernej Skrabec (3): > > > media: uapi: h264: update reference lists > > > media: cedrus: h264: Properly configure reference field > > > media: cedrus: h264: Fix frame list construction > > > > > > .../media/v4l/ext-ctrls-codec.rst | 40 ++- > > > .../staging/media/sunxi/cedrus/cedrus_h264.c | 27 +++-- > > > include/media/h264-ctrls.h| 12 +- > > > 3 files changed, 62 insertions(+), 17 deletions(-) > > > > > > -- > > > 2.27.0 > > > > > > > > > ___ > > > linux-arm-kernel mailing list > > > linux-arm-ker...@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] Detect and remove trace_printk
trace_printk is meant as a debugging tool, and should not be compiled into production code without specific debug Kconfig options enabled or source code changes. Patches 1 to 3 remove/disable trace_printk that should not be enabled by default. Patch 4 adds a config option that can be used to detect such trace_printk at compile time (instead of printing a nasty warning at runtime). Nicolas Boichat (4): usb: cdns3: gadget: Replace trace_printk by dev_dbg media: atomisp: Replace trace_printk by pr_info media: camss: vfe: Use trace_printk for debugging only kernel/trace: Add TRACING_ALLOW_PRINTK config option drivers/gpu/drm/i915/Kconfig.debug | 4 ++-- .../media/platform/qcom/camss/camss-vfe-4-1.c | 2 ++ .../media/platform/qcom/camss/camss-vfe-4-7.c | 2 ++ drivers/staging/media/atomisp/pci/hmm/hmm.c | 10 +- drivers/usb/cdns3/gadget.c | 2 +- fs/f2fs/Kconfig | 1 + include/linux/kernel.h | 17 - kernel/trace/Kconfig| 10 ++ samples/Kconfig | 2 ++ 9 files changed, 41 insertions(+), 9 deletions(-) -- 2.27.0.212.ge8ba1cc988-goog ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/4] kernel/trace: Add TRACING_ALLOW_PRINTK config option
trace_printk is meant as a debugging tool, and should not be
compiled into production code without specific debug Kconfig
options enabled, or source code changes, as indicated by the
warning that shows up on boot if any trace_printk is called:
** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **
** **
** trace_printk() being used. Allocating extra memory. **
** **
** This means that this is a DEBUG kernel and it is **
** unsafe for production use. **
If this option is set to n, the kernel will generate a
build-time error if trace_printk is used.
Note that the code to handle trace_printk is still present,
so this does not prevent people from compiling out-of-tree
kernel modules with the option set to =y, or BPF programs.
Signed-off-by: Nicolas Boichat
---
Changes since v1:
- Use static_assert instead of __static_assert (Jason Gunthorpe)
- Fix issues that can be detected by this patch (running some
randconfig in a loop, kernel test robot, or manual inspection),
by:
- Making some debug config options that use trace_printk depend
on the new config option.
- Adding 3 patches before this one.
There is a question from Alexei whether the warning is warranted,
and it's possibly too strongly worded, but the fact is, we do
not want trace_printk to be sprinkled in kernel code by default,
unless a very specific Kconfig command is enabled (or preprocessor
macro).
There's at least 3 reasons that I can come up with:
1. trace_printk introduces some overhead.
2. If the kernel keeps adding always-enabled trace_printk, it will
be much harder for developers to make use of trace_printk for
debugging.
3. People may assume that trace_printk is for debugging only, and
may accidentally output sensitive data (theoritical at this
stage).
drivers/gpu/drm/i915/Kconfig.debug | 4 ++--
fs/f2fs/Kconfig| 1 +
include/linux/kernel.h | 17 -
kernel/trace/Kconfig | 10 ++
samples/Kconfig| 2 ++
5 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.debug
b/drivers/gpu/drm/i915/Kconfig.debug
index 1cb28c20807c59d..fa30f9bdc82311f 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -84,7 +84,7 @@ config DRM_I915_ERRLOG_GEM
config DRM_I915_TRACE_GEM
bool "Insert extra ftrace output from the GEM internals"
depends on DRM_I915_DEBUG_GEM
- select TRACING
+ depends on TRACING_ALLOW_PRINTK
default n
help
Enable additional and verbose debugging output that will spam
@@ -98,7 +98,7 @@ config DRM_I915_TRACE_GEM
config DRM_I915_TRACE_GTT
bool "Insert extra ftrace output from the GTT internals"
depends on DRM_I915_DEBUG_GEM
- select TRACING
+ depends on TRACING_ALLOW_PRINTK
default n
help
Enable additional and verbose debugging output that will spam
diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index d13c5c6a978769b..d161d96cc1b7418 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -80,6 +80,7 @@ config F2FS_IO_TRACE
bool "F2FS IO tracer"
depends on F2FS_FS
depends on FUNCTION_TRACER
+ depends on TRACING_ALLOW_PRINTK
help
F2FS IO trace is based on a function trace, which gathers process
information and block IO patterns in the filesystem level.
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 196607aaf653082..8abce95b0c95a0e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -721,10 +721,15 @@ do {
\
#define trace_printk(fmt, ...) \
do { \
char ___STR[] = __stringify((__VA_ARGS__)); \
+ \
+ static_assert( \
+ IS_ENABLED(CONFIG_TRACING_ALLOW_PRINTK),\
+ "trace_printk called, please enable
CONFIG_TRACING_ALLOW_PRINTK."); \
+ \
if (sizeof(___STR) > 3) \
do_trace_printk(fmt, ##__VA_ARGS__);\
else\
- trace_puts(fmt);\
+ do_trace_puts(fmt); \
} while (0)
#define do_trace_printk(fmt, args...) \
@@ -773,6 +778,13 @@ int __trace_printk(unsigned long ip, const char *fmt, ...);
*/
#define trace_puts(str) ({ \
+ static_assert(
[PATCH 2/4] media: atomisp: Replace trace_printk by pr_info
trace_printk should not be used in production code, replace it
call with pr_info.
Signed-off-by: Nicolas Boichat
---
drivers/staging/media/atomisp/pci/hmm/hmm.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c
b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index 42fef17798622f1..2bd39b4939f16d2 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -735,11 +735,11 @@ ia_css_ptr hmm_host_vaddr_to_hrt_vaddr(const void *ptr)
void hmm_show_mem_stat(const char *func, const int line)
{
- trace_printk("tol_cnt=%d usr_size=%d res_size=%d res_cnt=%d sys_size=%d
dyc_thr=%d dyc_size=%d.\n",
-hmm_mem_stat.tol_cnt,
-hmm_mem_stat.usr_size, hmm_mem_stat.res_size,
-hmm_mem_stat.res_cnt, hmm_mem_stat.sys_size,
-hmm_mem_stat.dyc_thr, hmm_mem_stat.dyc_size);
+ pr_info("tol_cnt=%d usr_size=%d res_size=%d res_cnt=%d sys_size=%d
dyc_thr=%d dyc_size=%d.\n",
+ hmm_mem_stat.tol_cnt,
+ hmm_mem_stat.usr_size, hmm_mem_stat.res_size,
+ hmm_mem_stat.res_cnt, hmm_mem_stat.sys_size,
+ hmm_mem_stat.dyc_thr, hmm_mem_stat.dyc_size);
}
void hmm_init_mem_stat(int res_pgnr, int dyc_en, int dyc_pgnr)
--
2.27.0.212.ge8ba1cc988-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] media: camss: vfe: Use trace_printk for debugging only
trace_printk should not be used in production code. Since
tracing interrupts is presumably latency sensitive, pr_dbg is
not appropriate, so guard the call with a preprocessor symbol
that can be defined for debugging purpose.
Signed-off-by: Nicolas Boichat
---
drivers/media/platform/qcom/camss/camss-vfe-4-1.c | 2 ++
drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
index 174a36be6f5d866..0c57171fae4f9e9 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
@@ -936,8 +936,10 @@ static irqreturn_t vfe_isr(int irq, void *dev)
vfe->ops->isr_read(vfe, &value0, &value1);
+#ifdef CAMSS_VFE_TRACE_IRQ
trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n",
value0, value1);
+#endif
if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK)
vfe->isr_ops.reset_ack(vfe);
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
index 0dca8bf9281e774..307675925e5c779 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
@@ -1058,8 +1058,10 @@ static irqreturn_t vfe_isr(int irq, void *dev)
vfe->ops->isr_read(vfe, &value0, &value1);
+#ifdef CAMSS_VFE_TRACE_IRQ
trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n",
value0, value1);
+#endif
if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK)
vfe->isr_ops.reset_ack(vfe);
--
2.27.0.212.ge8ba1cc988-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
trace_printk should not be used in production code, replace it
call with dev_dbg.
Signed-off-by: Nicolas Boichat
---
Unclear why a trace_printk was used in the first place, it's
possible that some rate-limiting is necessary here.
drivers/usb/cdns3/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
index 5e24c2e57c0d8c8..c303ab7c62d1651 100644
--- a/drivers/usb/cdns3/gadget.c
+++ b/drivers/usb/cdns3/gadget.c
@@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device
*priv_dev,
if ((priv_req->flags & REQUEST_INTERNAL) ||
(priv_ep->flags & EP_TDLCHK_EN) ||
priv_ep->use_streams) {
- trace_printk("Blocking external request\n");
+ dev_dbg(priv_dev->dev, "Blocking external request\n");
return ret;
}
}
--
2.27.0.212.ge8ba1cc988-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v13 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP
On Tue, Jun 9, 2020 at 3:20 PM Xin Ji wrote:
>
> The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed
> for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K.
>
> Signed-off-by: Xin Ji
> ---
> drivers/gpu/drm/bridge/analogix/Kconfig |9 +
> drivers/gpu/drm/bridge/analogix/Makefile |1 +
> drivers/gpu/drm/bridge/analogix/anx7625.c | 1999
> +
> drivers/gpu/drm/bridge/analogix/anx7625.h | 397 ++
> 4 files changed, 2406 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c
> create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h
>
> [snip]
> +static int anx7625_parse_dt(struct device *dev,
> + struct anx7625_platform_data *pdata)
> +{
> + struct device_node *np = dev->of_node;
> + struct device_node *panel_node, *out_ep;
> +
> + pdata->node.mipi_dsi_host_node = of_graph_get_remote_node(np, 0, 0);
> + if (!pdata->node.mipi_dsi_host_node) {
> + DRM_DEV_ERROR(dev, "fail to get internal panel.\n");
> + return -EPROBE_DEFER;
This does not look correct. I don't think of_graph_get_remote_node
will ever return NULL if the device tree is configured properly, and
it's useless to retry later (EPROBE_DEFER). You should just fail (e.g.
return EINVAL).
> + }
> +
> + of_node_put(pdata->node.mipi_dsi_host_node);
You are using pdata->node.mipi_dsi_host_node in other places in the
code, so I don't think it's ok to call of_node_put?
> + DRM_DEV_DEBUG_DRIVER(dev, "found dsi host node.\n");
> +
> + pdata->node.panel_node = of_graph_get_port_by_id(np, 1);
> + if (!pdata->node.panel_node) {
> + DRM_DEV_ERROR(dev, "fail to get panel node.\n");
> + return -EPROBE_DEFER;
-EINVAL.
> + }
> +
> + of_node_put(pdata->node.panel_node);
> + out_ep = of_get_child_by_name(pdata->node.panel_node,
> + "endpoint");
> + if (!out_ep) {
> + DRM_DEV_DEBUG_DRIVER(dev, "cannot get endpoint.\n");
DRM_DEV_ERROR seems more appropriate
> + return -EPROBE_DEFER;
-EINVAL
> + }
> +
> + panel_node = of_graph_get_remote_port_parent(out_ep);
> + of_node_put(out_ep);
> + pdata->panel = of_drm_find_panel(panel_node);
> + DRM_DEV_DEBUG_DRIVER(dev, "get panel node.\n");
> +
> + of_node_put(panel_node);
> + if (IS_ERR_OR_NULL(pdata->panel))
> + return -EPROBE_DEFER;
of_drm_find_panel cannot return NULL, so, do this instead:
if (IS_ERR(pdata->panel))
return PTR_ERR(pdata->panel);
(which actually _may_ return EPROBE_DEFER)
> +
> + return 0;
> +}
> [snip]
> +static int anx7625_i2c_probe(struct i2c_client *client,
> +const struct i2c_device_id *id)
> +{
> + struct anx7625_data *platform;
> + struct anx7625_platform_data *pdata;
> + int ret = 0;
> + struct device *dev = &client->dev;
> +
> + if (!i2c_check_functionality(client->adapter,
> +I2C_FUNC_SMBUS_I2C_BLOCK)) {
> + DRM_DEV_ERROR(dev, "anx7625's i2c bus doesn't support\n");
> + return -ENODEV;
> + }
> +
> + platform = kzalloc(sizeof(*platform), GFP_KERNEL);
> + if (!platform) {
> + DRM_DEV_ERROR(dev, "fail to allocate driver data\n");
> + return -ENOMEM;
> + }
> +
> + pdata = &platform->pdata;
> +
> + ret = anx7625_parse_dt(dev, pdata);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "fail to parse devicetree.\n");
Please do not print this error (or at least not if err == -EPROBE_DEFER).
> + goto free_platform;
> + }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] media: uapi: h264: update reference lists
Le mercredi 08 juillet 2020 à 17:57 +0200, Jernej Škrabec a écrit :
> Hi!
>
> Dne sreda, 08. julij 2020 ob 15:28:52 CEST je Ezequiel Garcia napisal(a):
> > Hello Jernej,
> >
> > I'd like to post a new H264 uAPI cleanup series soon,
> > would you mind resending this, or otherwise do you
> > mind if I include this patch in the series?
>
> I don't mind at all. Currently my focus was elsewhere...
>
> > See below for a tiny comment.
> >
> > On Thu, 4 Jun 2020 at 15:55, Jernej Skrabec wrote:
> > > When dealing with with interlaced frames, reference lists must tell if
> > > each particular reference is meant for top or bottom field. This info
> > > is currently not provided at all in the H264 related controls.
> > >
> > > Make reference lists hold a structure which will also hold flags along
> > > index into DPB array. Flags will tell if reference is meant for top or
> > > bottom field.
> > >
> > > Currently the only user of these lists is Cedrus which is just compile
> > > fixed here. Actual usage of newly introduced flags will come in
> > > following commit.
> > >
> > > Signed-off-by: Jernej Skrabec
> > > ---
> > >
> > > .../media/v4l/ext-ctrls-codec.rst | 40 ++-
> > > .../staging/media/sunxi/cedrus/cedrus_h264.c | 6 +--
> > > include/media/h264-ctrls.h| 12 +-
> > > 3 files changed, 51 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst index
> > > d0d506a444b1..6c36d298db20 100644
> > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > @@ -1843,10 +1843,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type
> > > ->
> > > * - __u32
> > >
> > >- ``slice_group_change_cycle``
> > >-
> > >
> > > -* - __u8
> > > +* - struct :c:type:`v4l2_h264_reference`
> > >
> > >- ``ref_pic_list0[32]``
> > >- Reference picture list after applying the per-slice modifications
> > >
> > > -* - __u8
> > > +* - struct :c:type:`v4l2_h264_reference`
> > >
> > >- ``ref_pic_list1[32]``
> > >- Reference picture list after applying the per-slice modifications
> > >
> > > * - __u32
> > >
> > > @@ -1926,6 +1926,42 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type
> > > -
> > >
> > >- ``chroma_offset[32][2]``
> > >-
> > >
> > > +``Picture Reference``
> > > +
> > > +.. c:type:: v4l2_h264_reference
> > > +
> > > +.. cssclass:: longtable
> > > +
> > > +.. flat-table:: struct v4l2_h264_reference
> > > +:header-rows: 0
> > > +:stub-columns: 0
> > > +:widths: 1 1 2
> > > +
> > > +* - __u16
> > > + - ``flags``
> > > + - See :ref:`Picture Reference Flags `
> > > +* - __u8
> > > + - ``index``
> > > + -
> > > +
> > > +.. _h264_reference_flags:
> > > +
> > > +``Picture Reference Flags``
> > > +
> > > +.. cssclass:: longtable
> > > +
> > > +.. flat-table::
> > > +:header-rows: 0
> > > +:stub-columns: 0
> > > +:widths: 1 1 2
> > > +
> > > +* - ``V4L2_H264_REFERENCE_FLAG_TOP_FIELD``
> > > + - 0x0001
> > > + -
> > > +* - ``V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD``
> > > + - 0x0002
> > > + -
> > > +
> > >
> > > ``V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS (struct)``
> > >
> > > Specifies the decode parameters (as extracted from the bitstream)
> > > for the associated H264 slice data. This includes the necessary
> > >
> > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index
> > > 54ee2aa423e2..cce527bbdf86 100644
> > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > > @@ -166,8 +166,8 @@ static void cedrus_write_frame_list(struct cedrus_ctx
> > > *ctx,>
> > > static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> > >
> > >struct cedrus_run *run,
> > >
> > > - const u8 *ref_list, u8 num_ref,
> > > - enum cedrus_h264_sram_off sram)
> > > + const struct v4l2_h264_reference
> > > *ref_list, + u8 num_ref, enum
> > > cedrus_h264_sram_off sram)>
> > > {
> > >
> > > const struct v4l2_ctrl_h264_decode_params *decode =
> > > run->h264.decode_params; struct vb2_queue *cap_q;
> > >
> > > @@ -188,7 +188,7 @@ static void _cedrus_write_ref_list(struct cedrus_ctx
> > > *ctx,>
> > > int buf_idx;
> > > u8 dpb_idx;
> > >
> > > - dpb_idx = ref_list[i];
> > > + dpb_idx = ref_list[i].index;
> > >
> > > dpb = &decode->dpb[dpb_idx];
> > >
> > >
[RESEND PATCH] media: atomisp: Replace trace_printk by pr_info
trace_printk should not be used in production code, replace it
call with pr_info.
Signed-off-by: Nicolas Boichat
---
Sent this before as part of a series (whose 4th patch was a
change that allows to detect such trace_printk), but maybe it's
easier to get individual maintainer attention by splitting it.
drivers/staging/media/atomisp/pci/hmm/hmm.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c
b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index 42fef17798622f1..2bd39b4939f16d2 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -735,11 +735,11 @@ ia_css_ptr hmm_host_vaddr_to_hrt_vaddr(const void *ptr)
void hmm_show_mem_stat(const char *func, const int line)
{
- trace_printk("tol_cnt=%d usr_size=%d res_size=%d res_cnt=%d sys_size=%d
dyc_thr=%d dyc_size=%d.\n",
-hmm_mem_stat.tol_cnt,
-hmm_mem_stat.usr_size, hmm_mem_stat.res_size,
-hmm_mem_stat.res_cnt, hmm_mem_stat.sys_size,
-hmm_mem_stat.dyc_thr, hmm_mem_stat.dyc_size);
+ pr_info("tol_cnt=%d usr_size=%d res_size=%d res_cnt=%d sys_size=%d
dyc_thr=%d dyc_size=%d.\n",
+ hmm_mem_stat.tol_cnt,
+ hmm_mem_stat.usr_size, hmm_mem_stat.res_size,
+ hmm_mem_stat.res_cnt, hmm_mem_stat.sys_size,
+ hmm_mem_stat.dyc_thr, hmm_mem_stat.dyc_size);
}
void hmm_init_mem_stat(int res_pgnr, int dyc_en, int dyc_pgnr)
--
2.27.0.383.g050319c2ae-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
On Thu, Jul 23, 2020 at 9:17 PM Felipe Balbi wrote:
>
> Nicolas Boichat writes:
>
> > trace_printk should not be used in production code, replace it
> > call with dev_dbg.
> >
> > Signed-off-by: Nicolas Boichat
> >
> > ---
> >
> > Unclear why a trace_printk was used in the first place, it's
> > possible that some rate-limiting is necessary here.
> >
> > drivers/usb/cdns3/gadget.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> > index 5e24c2e57c0d8c8..c303ab7c62d1651 100644
> > --- a/drivers/usb/cdns3/gadget.c
> > +++ b/drivers/usb/cdns3/gadget.c
> > @@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device
> > *priv_dev,
> > if ((priv_req->flags & REQUEST_INTERNAL) ||
> > (priv_ep->flags & EP_TDLCHK_EN) ||
> > priv_ep->use_streams) {
> > - trace_printk("Blocking external request\n");
> > + dev_dbg(priv_dev->dev, "Blocking external request\n");
>
> Instead, I would suggest adding a proper trace event here; one that
> includes "priv_ep->flags" in the output.
The patch was already merged by Greg
(https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/usb/cdns3/gadget.c?id=b3a5ce874c2619c9b8a6c5bbcfefdb95e0227600),
but feel free to do that as a follow-up CL.
Looks like Peter -- the main author, is ok with dev_dbg (also,
apologies for missing the R-b tag when I sent a v2 -- which is the one
that was merged by Greg).
Thanks,
>
> --
> balbi
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RESEND PATCH] media: atomisp: Replace trace_printk by pr_info
On Fri, Jul 10, 2020 at 3:03 PM Greg Kroah-Hartman wrote: > > On Fri, Jul 10, 2020 at 02:45:29PM +0800, Nicolas Boichat wrote: > > trace_printk should not be used in production code, replace it > > call with pr_info. > > > > Signed-off-by: Nicolas Boichat > > --- > > Sent this before as part of a series (whose 4th patch was a > > change that allows to detect such trace_printk), but maybe it's > > easier to get individual maintainer attention by splitting it. > > Mauro should take this soon: > > Acked-by: Greg Kroah-Hartman Mauro: did you get a chance to look at this? (and the other similar patch "media: camss: vfe: Use trace_printk for debugging only") Thanks! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RESEND PATCH] media: atomisp: Replace trace_printk by pr_info
On Fri, Jul 24, 2020 at 8:41 PM Nicolas Boichat wrote: > > On Fri, Jul 10, 2020 at 3:03 PM Greg Kroah-Hartman > wrote: > > > > On Fri, Jul 10, 2020 at 02:45:29PM +0800, Nicolas Boichat wrote: > > > trace_printk should not be used in production code, replace it > > > call with pr_info. > > > > > > Signed-off-by: Nicolas Boichat > > > --- > > > Sent this before as part of a series (whose 4th patch was a > > > change that allows to detect such trace_printk), but maybe it's > > > easier to get individual maintainer attention by splitting it. > > > > Mauro should take this soon: > > > > Acked-by: Greg Kroah-Hartman > > Mauro: did you get a chance to look at this? (and the other similar > patch "media: camss: vfe: Use trace_printk for debugging only") Mauro: Another gentle ping. Thanks. > Thanks! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RESEND PATCH] media: atomisp: Replace trace_printk by pr_info
On Fri, Aug 7, 2020 at 2:28 PM Greg Kroah-Hartman wrote: > > On Fri, Aug 07, 2020 at 09:50:23AM +0800, Nicolas Boichat wrote: > > On Fri, Jul 24, 2020 at 8:41 PM Nicolas Boichat > > wrote: > > > > > > On Fri, Jul 10, 2020 at 3:03 PM Greg Kroah-Hartman > > > wrote: > > > > > > > > On Fri, Jul 10, 2020 at 02:45:29PM +0800, Nicolas Boichat wrote: > > > > > trace_printk should not be used in production code, replace it > > > > > call with pr_info. > > > > > > > > > > Signed-off-by: Nicolas Boichat > > > > > --- > > > > > Sent this before as part of a series (whose 4th patch was a > > > > > change that allows to detect such trace_printk), but maybe it's > > > > > easier to get individual maintainer attention by splitting it. > > > > > > > > Mauro should take this soon: > > > > > > > > Acked-by: Greg Kroah-Hartman > > > > > > Mauro: did you get a chance to look at this? (and the other similar > > > patch "media: camss: vfe: Use trace_printk for debugging only") > > > > Mauro: Another gentle ping. Thanks. > > It's the middle of the merge window, maintainers can't do anything until > after 5.9-rc1 is out, sorry. Huh, wait, looks like Mauro _did_ pick it (found it in this email "[GIT PULL for v5.8-rc7] media fixes"). My bad then, I was expecting an ack ,-) Thanks! > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RESEND PATCH] media: atomisp: Replace trace_printk by pr_info
On Fri, Aug 7, 2020 at 4:04 PM Mauro Carvalho Chehab wrote: > > Em Fri, 7 Aug 2020 14:51:12 +0800 > Nicolas Boichat escreveu: > > > On Fri, Aug 7, 2020 at 2:28 PM Greg Kroah-Hartman > > wrote: > > > > > > On Fri, Aug 07, 2020 at 09:50:23AM +0800, Nicolas Boichat wrote: > > > > On Fri, Jul 24, 2020 at 8:41 PM Nicolas Boichat > > > > wrote: > > > > > > > > > > On Fri, Jul 10, 2020 at 3:03 PM Greg Kroah-Hartman > > > > > wrote: > > > > > > > > > > > > On Fri, Jul 10, 2020 at 02:45:29PM +0800, Nicolas Boichat wrote: > > > > > > > trace_printk should not be used in production code, replace it > > > > > > > call with pr_info. > > > > > > > > > > > > > > Signed-off-by: Nicolas Boichat > > > > > > > --- > > > > > > > Sent this before as part of a series (whose 4th patch was a > > > > > > > change that allows to detect such trace_printk), but maybe it's > > > > > > > easier to get individual maintainer attention by splitting it. > > > > > > > > > > > > Mauro should take this soon: > > > > > > > > > > > > Acked-by: Greg Kroah-Hartman > > > > > > > > > > Mauro: did you get a chance to look at this? (and the other similar > > > > > patch "media: camss: vfe: Use trace_printk for debugging only") > > > > > > > > Mauro: Another gentle ping. Thanks. > > > > > > It's the middle of the merge window, maintainers can't do anything until > > > after 5.9-rc1 is out, sorry. > > > > Huh, wait, looks like Mauro _did_ pick it (found it in this email > > "[GIT PULL for v5.8-rc7] media fixes"). > > > > My bad then, I was expecting an ack ,-) > > Never expect acks. Kernel maintainers usually don't send them. For some reasons I'm working mainly with maintainers who do ,-) I'll adjust my expectations, thanks. > Yet, in the case of media, you should probably have received > an automatic e-mail from our patchwork instance. Nope, didn't receive anything. But I'm happy to blame gmail for that... Anyway, I'll ping you again after the merge window closes about "media: camss: vfe: Use trace_printk for debugging only" (I _think_ that one didn't get merged). Hopefully not too many other trace_printks made it through the cracks in the meantime ,-) Thanks, have a good weekend, > > Thanks, > Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed
We added a config option CONFIG_TRACING_ALLOW_PRINTK to make sure
that no extra trace_printk gets added to the kernel, let's use
that in this driver to guard the trace_printk call.
Signed-off-by: Nicolas Boichat
---
Technically, we could only initialize the trace_printk buffers
when the print env is switched, to avoid the build error and
unconditional boot-time warning, but I assume this printing
framework will eventually get removed when the driver moves out
of staging?
drivers/staging/media/atomisp/pci/atomisp_compat_css20.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index 1b2b2c68025b4cc..020519dca1324ab 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -165,11 +165,13 @@ static int atomisp_css2_dbg_print(const char *fmt,
va_list args)
return 0;
}
+#ifdef CONFIG_TRACING_ALLOW_PRINTK
static int atomisp_css2_dbg_ftrace_print(const char *fmt, va_list args)
{
ftrace_vprintk(fmt, args);
return 0;
}
+#endif
static int atomisp_css2_err_print(const char *fmt, va_list args)
{
@@ -865,9 +867,11 @@ static inline int __set_css_print_env(struct
atomisp_device *isp, int opt)
if (opt == 0)
isp->css_env.isp_css_env.print_env.debug_print = NULL;
+#ifdef CONFIG_TRACING_ALLOW_PRINTK
else if (opt == 1)
isp->css_env.isp_css_env.print_env.debug_print =
atomisp_css2_dbg_ftrace_print;
+#endif
else if (opt == 2)
isp->css_env.isp_css_env.print_env.debug_print =
atomisp_css2_dbg_print;
--
2.28.0.220.ged08abb693-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed
On Thu, Aug 20, 2020 at 10:23 PM Steven Rostedt wrote: > > On Thu, 20 Aug 2020 17:14:12 +0800 > Nicolas Boichat wrote: > > > Technically, we could only initialize the trace_printk buffers > > when the print env is switched, to avoid the build error and > > unconditional boot-time warning, but I assume this printing > > framework will eventually get removed when the driver moves out > > of staging? > > Perhaps this should be converting into a trace event. Look at what bpf > did for their bpf_trace_printk(). > > The more I think about it, the less I like this series. To make it clear, the primary goal of this series is to get rid of trace_printk sprinkled in the kernel by making sure some randconfig builds fail. Since my v2, there already has been one more added (the one that this patch removes), so I'd like to land 2/3 ASAP to prevent even more from being added. Looking at your reply on 1/3, I think we are aligned on that goal? Is there some other approach you'd recommend? Now, I'm not pretending my fixes are the best possible ones, but I would much rather have the burden of converting to trace events on the respective driver maintainers. (btw is there a short documentation/tutorial that I could link to in these patches, to help developers understand what is the recommended way now?) Thanks, > > -- Steve ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed
On Fri, Aug 21, 2020 at 8:36 AM Steven Rostedt wrote: > > On Fri, 21 Aug 2020 08:13:00 +0800 > Nicolas Boichat wrote: > > > On Thu, Aug 20, 2020 at 10:23 PM Steven Rostedt wrote: > > > > > > On Thu, 20 Aug 2020 17:14:12 +0800 > > > Nicolas Boichat wrote: > > > > > > > Technically, we could only initialize the trace_printk buffers > > > > when the print env is switched, to avoid the build error and > > > > unconditional boot-time warning, but I assume this printing > > > > framework will eventually get removed when the driver moves out > > > > of staging? > > > > > > Perhaps this should be converting into a trace event. Look at what bpf > > > did for their bpf_trace_printk(). > > > > > > The more I think about it, the less I like this series. > > > > To make it clear, the primary goal of this series is to get rid of > > trace_printk sprinkled in the kernel by making sure some randconfig > > builds fail. Since my v2, there already has been one more added (the > > one that this patch removes), so I'd like to land 2/3 ASAP to prevent > > even more from being added. > > > > Looking at your reply on 1/3, I think we are aligned on that goal? Is > > there some other approach you'd recommend? > > > > Now, I'm not pretending my fixes are the best possible ones, but I > > would much rather have the burden of converting to trace events on the > > respective driver maintainers. (btw is there a short > > documentation/tutorial that I could link to in these patches, to help > > developers understand what is the recommended way now?) > > > > I like the goal, but I guess I never articulated the problem I have > with the methodology. > > trace_printk() is meant to be a debugging tool. Something that people > can and do sprinkle all over the kernel to help them find a bug in > areas that are called quite often (where printk() is way too slow). > > The last thing I want them to deal with is adding a trace_printk() with > their distro's config (or a config from someone that triggered the bug) > only to have the build to fail, because they also need to add a config > value. > > I add to the Cc a few developers I know that use trace_printk() in this > fashion. I'd like to hear their view on having to add a config option > to make trace_printk work before they test a config that is sent to > them. Gotcha, thanks. I have also used trace_printk in the past, as uncommitted changes (and understand the usefulness ,-)). And in Chrome OS team here, developers have also raised this concern: how do we make the developer flow convenient so that we can add trace_printk to our code for debugging, without having to flip back that config option, and _yet_ make sure that no trace_printk ever makes it into our production kernels. We have creative ways of making that work (portage USE flags and stuff). But I'm not sure about other flows, and your concern is totally valid... Some other approaches/ideas: 1. Filter all lkml messages that contain trace_printk. Already found 1 instance, and I can easily reply to those with a semi-canned answer, if I remember to check that filter regularly (not sustainable in the long run...). 2. Integration into some kernel test robot? (I will not roll my own for this ,-)) It may be a bit difficult as some debug config options do enable trace_printk, and that's ok. 3. In Chromium OS, I can add a unit test (i.e. something outside of the normal kernel build system), but that'll only catch regressions downstream (or when we happen to backport patches). Down the line, #3 catches what I care about the most (Chromium OS issues: we had production kernels for a few days/weeks showing that splat on boot), but it'd be nice to have something upstream that benefits everyone. Thanks, > > -- Steve ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed
On Fri, Aug 21, 2020 at 9:57 AM Steven Rostedt wrote: > > On Fri, 21 Aug 2020 09:39:19 +0800 > Nicolas Boichat wrote: > > > On Fri, Aug 21, 2020 at 8:36 AM Steven Rostedt wrote: > > > > > > On Fri, 21 Aug 2020 08:13:00 +0800 > > > Nicolas Boichat wrote: > > > > > > > On Thu, Aug 20, 2020 at 10:23 PM Steven Rostedt > > > > wrote: > > > > > > > > > > On Thu, 20 Aug 2020 17:14:12 +0800 > > > > > Nicolas Boichat wrote: > > > > > > > > > > > Technically, we could only initialize the trace_printk buffers > > > > > > when the print env is switched, to avoid the build error and > > > > > > unconditional boot-time warning, but I assume this printing > > > > > > framework will eventually get removed when the driver moves out > > > > > > of staging? > > > > > > > > > > Perhaps this should be converting into a trace event. Look at what bpf > > > > > did for their bpf_trace_printk(). > > > > > > > > > > The more I think about it, the less I like this series. > > > > > > > > To make it clear, the primary goal of this series is to get rid of > > > > trace_printk sprinkled in the kernel by making sure some randconfig > > > > builds fail. Since my v2, there already has been one more added (the > > > > one that this patch removes), so I'd like to land 2/3 ASAP to prevent > > > > even more from being added. > > > > > > > > Looking at your reply on 1/3, I think we are aligned on that goal? Is > > > > there some other approach you'd recommend? > > > > > > > > Now, I'm not pretending my fixes are the best possible ones, but I > > > > would much rather have the burden of converting to trace events on the > > > > respective driver maintainers. (btw is there a short > > > > documentation/tutorial that I could link to in these patches, to help > > > > developers understand what is the recommended way now?) > > > > > > > > > > I like the goal, but I guess I never articulated the problem I have > > > with the methodology. > > > > > > trace_printk() is meant to be a debugging tool. Something that people > > > can and do sprinkle all over the kernel to help them find a bug in > > > areas that are called quite often (where printk() is way too slow). > > > > > > The last thing I want them to deal with is adding a trace_printk() with > > > their distro's config (or a config from someone that triggered the bug) > > > only to have the build to fail, because they also need to add a config > > > value. > > > > > > I add to the Cc a few developers I know that use trace_printk() in this > > > fashion. I'd like to hear their view on having to add a config option > > > to make trace_printk work before they test a config that is sent to > > > them. > > > > Gotcha, thanks. I have also used trace_printk in the past, as > > uncommitted changes (and understand the usefulness ,-)). And in Chrome > > OS team here, developers have also raised this concern: how do we make > > the developer flow convenient so that we can add trace_printk to our > > code for debugging, without having to flip back that config option, > > and _yet_ make sure that no trace_printk ever makes it into our > > production kernels. We have creative ways of making that work (portage > > USE flags and stuff). But I'm not sure about other flows, and your > > concern is totally valid... > > > > Some other approaches/ideas: > > 1. Filter all lkml messages that contain trace_printk. Already found > > 1 instance, and I can easily reply to those with a semi-canned answer, > > if I remember to check that filter regularly (not sustainable in the > > long run...). > > Added Joe Perches to the thread. > > We can update checkpatch.pl to complain about a trace_printk() that it > finds in the added code. Oh, that's a good and simple idea. > > > 2. Integration into some kernel test robot? (I will not roll my own > > for this ,-)) It may be a bit difficult as some debug config options > > do enable trace_printk, and that's ok. > > 3. In Chromium OS, I can add a unit test (i.e. something outside of > > the normal kernel build system), but that'll only catch regressions > > downstream (or when we happen to backport patches). > > > > Down the li
Re: [PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed
On Fri, Aug 21, 2020 at 10:36 AM Joe Perches wrote: > > On Thu, 2020-08-20 at 21:57 -0400, Steven Rostedt wrote: > > On Fri, 21 Aug 2020 09:39:19 +0800 > > Nicolas Boichat wrote: > [] > > > Some other approaches/ideas: > > > 1. Filter all lkml messages that contain trace_printk. Already found > > > 1 instance, and I can easily reply to those with a semi-canned answer, > > > if I remember to check that filter regularly (not sustainable in the > > > long run...). > > > > Added Joe Perches to the thread. > > > > We can update checkpatch.pl to complain about a trace_printk() that it > > finds in the added code. > > Why? > > I don't see much value in a trace_printk checkpatch warning. > tracing is still dependent on CONFIG_TRACING otherwise > trace_printk is an if (0) > > ELI5 please. This is my "new" canned answer to this: Please do not use trace_printk in production code [1,2], it is only meant for debug use. Consider using trace events, or dev_dbg. [1] https://elixir.bootlin.com/linux/v5.8/source/kernel/trace/trace.c#L3158 [2] https://elixir.bootlin.com/linux/v5.8/source/include/linux/kernel.h#L766 I also had arguments in patch 2/3 notes: There's at least 3 reasons that I can come up with: 1. trace_printk introduces some overhead. [some users, e.g. Android/Chrome OS, want CONFIG_TRACING but _not_ that extra overhead] 2. If the kernel keeps adding always-enabled trace_printk, it will be much harder for developers to make use of trace_printk for debugging. 3. People may assume that trace_printk is for debugging only, and may accidentally output sensitive data (theoretical at this stage). (we'll need to summarize that somehow if we want to add to checkpatch.pl) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed
On Fri, Aug 21, 2020 at 4:48 PM David Laight wrote: > > From: Steven Rostedt > > Sent: 21 August 2020 01:36 > > On Fri, 21 Aug 2020 08:13:00 +0800 > > Nicolas Boichat wrote: > > > > > On Thu, Aug 20, 2020 at 10:23 PM Steven Rostedt > > > wrote: > > > > > > > > On Thu, 20 Aug 2020 17:14:12 +0800 > > > > Nicolas Boichat wrote: > > > > > > > > > Technically, we could only initialize the trace_printk buffers > > > > > when the print env is switched, to avoid the build error and > > > > > unconditional boot-time warning, but I assume this printing > > > > > framework will eventually get removed when the driver moves out > > > > > of staging? > > > > > > > > Perhaps this should be converting into a trace event. Look at what bpf > > > > did for their bpf_trace_printk(). > > > > > > > > The more I think about it, the less I like this series. > > > > > > To make it clear, the primary goal of this series is to get rid of > > > trace_printk sprinkled in the kernel by making sure some randconfig > > > builds fail. Since my v2, there already has been one more added (the > > > one that this patch removes), so I'd like to land 2/3 ASAP to prevent > > > even more from being added. > > > > > > Looking at your reply on 1/3, I think we are aligned on that goal? Is > > > there some other approach you'd recommend? > > > > > > Now, I'm not pretending my fixes are the best possible ones, but I > > > would much rather have the burden of converting to trace events on the > > > respective driver maintainers. (btw is there a short > > > documentation/tutorial that I could link to in these patches, to help > > > developers understand what is the recommended way now?) > > > > > > > I like the goal, but I guess I never articulated the problem I have > > with the methodology. > > > > trace_printk() is meant to be a debugging tool. Something that people > > can and do sprinkle all over the kernel to help them find a bug in > > areas that are called quite often (where printk() is way too slow). > > > > The last thing I want them to deal with is adding a trace_printk() with > > their distro's config (or a config from someone that triggered the bug) > > only to have the build to fail, because they also need to add a config > > value. > > > > I add to the Cc a few developers I know that use trace_printk() in this > > fashion. I'd like to hear their view on having to add a config option > > to make trace_printk work before they test a config that is sent to > > them. > > Is it worth having three compile-time options: > 1) trace_printk() ignored. CONFIG_TRACE=n (now) > 2) trace_printk() enabled. CONFIG_TRACE=y (now) > 3) trace_printk() generates a compile time error. CONFIG_TRACE=y and CONFIG_TRACING_ALLOW_PRINTK=n (my patch) > > Normal kernel builds would ignore calls. > Either a config option or a module option (etc) would enable it. > A config option that 'rand-config' builds would turn on would > generate compile-time errors. Yes, a config option is exactly what my patch 2/2 does. And see Steven's argument. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 > 1PT, UK > Registration No: 1397386 (Wales) > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed
On Fri, Aug 21, 2020 at 7:32 PM David Laight wrote: > > From: Nicolas Boichat > > Sent: 21 August 2020 11:28 > > > > On Fri, Aug 21, 2020 at 4:48 PM David Laight > > wrote: > > > > > > From: Steven Rostedt > > > > Sent: 21 August 2020 01:36 > > > > On Fri, 21 Aug 2020 08:13:00 +0800 > > > > Nicolas Boichat wrote: > > > > > > > > > On Thu, Aug 20, 2020 at 10:23 PM Steven Rostedt > > > > > wrote: > > > > > > > > > > > > On Thu, 20 Aug 2020 17:14:12 +0800 > > > > > > Nicolas Boichat wrote: > > > > > > > > > > > > > Technically, we could only initialize the trace_printk buffers > > > > > > > when the print env is switched, to avoid the build error and > > > > > > > unconditional boot-time warning, but I assume this printing > > > > > > > framework will eventually get removed when the driver moves out > > > > > > > of staging? > > > > > > > > > > > > Perhaps this should be converting into a trace event. Look at what > > > > > > bpf > > > > > > did for their bpf_trace_printk(). > > > > > > > > > > > > The more I think about it, the less I like this series. > > > > > > > > > > To make it clear, the primary goal of this series is to get rid of > > > > > trace_printk sprinkled in the kernel by making sure some randconfig > > > > > builds fail. Since my v2, there already has been one more added (the > > > > > one that this patch removes), so I'd like to land 2/3 ASAP to prevent > > > > > even more from being added. > > > > > > > > > > Looking at your reply on 1/3, I think we are aligned on that goal? Is > > > > > there some other approach you'd recommend? > > > > > > > > > > Now, I'm not pretending my fixes are the best possible ones, but I > > > > > would much rather have the burden of converting to trace events on the > > > > > respective driver maintainers. (btw is there a short > > > > > documentation/tutorial that I could link to in these patches, to help > > > > > developers understand what is the recommended way now?) > > > > > > > > > > > > > I like the goal, but I guess I never articulated the problem I have > > > > with the methodology. > > > > > > > > trace_printk() is meant to be a debugging tool. Something that people > > > > can and do sprinkle all over the kernel to help them find a bug in > > > > areas that are called quite often (where printk() is way too slow). > > > > > > > > The last thing I want them to deal with is adding a trace_printk() with > > > > their distro's config (or a config from someone that triggered the bug) > > > > only to have the build to fail, because they also need to add a config > > > > value. > > > > > > > > I add to the Cc a few developers I know that use trace_printk() in this > > > > fashion. I'd like to hear their view on having to add a config option > > > > to make trace_printk work before they test a config that is sent to > > > > them. > > > > > > Is it worth having three compile-time options: > > > 1) trace_printk() ignored. > > > > CONFIG_TRACE=n (now) > > > > > 2) trace_printk() enabled. > > > > CONFIG_TRACE=y (now) > > > > > 3) trace_printk() generates a compile time error. > > > > CONFIG_TRACE=y and CONFIG_TRACING_ALLOW_PRINTK=n (my patch) > > > > > > > > Normal kernel builds would ignore calls. > > > Either a config option or a module option (etc) would enable it. > > > A config option that 'rand-config' builds would turn on would > > > generate compile-time errors. > > > > Yes, a config option is exactly what my patch 2/2 does. And see > > Steven's argument. > > But you were adding #ifs to you code to enable the traces. > That is just horrid. Yeah this patch 3/3 is not the best (if I understand what you mean), 1/3 type of #ifdef is actually fairly common in the kernel (an ifdef that you can enable manually in the same file, _not_ a config option). Steven's point is that both 1/3 and 3/3 should be replaced by trace events anyway. > > What you want is CONFIG_HJHJVLKHCVKIYVKIIYVYK
Re: [PATCH v4 3/3] media: atomisp: Only use trace_printk if allowed
On Fri, Aug 21, 2020 at 11:01 AM Steven Rostedt wrote:
>
> On Fri, 21 Aug 2020 10:39:02 +0800
> Nicolas Boichat wrote:
>
> > I'm not sure how that helps? I mean, the use case you have in mind is
> > somebody reusing a distro/random config and not being able to use
> > trace_printk, right? If that config has CONFIG_DISABLE_TRACE_PRINTK=y,
> > then the developer will still need to flip that back.
> >
> > Note that the option I'm added has default=y (_allow_ trace_printk),
> > so I don't think default y or default n really matters?
>
> Ideally, the production system doesn't have it set. It only sets it to
> make sure that it's clean before sending out. But then it can add it
> back before production. Yeah, it's pretty much cutting hairs between
> the two. I don't like either one.
>
> Really, if you are worried about this, just add your patch to your
> local tree. I'm not sure this is something that can be fixed upstream.
>
> Another idea is to add something like below, and build with:
>
> make CHECK_TRACE_PRINT=1
>
> This way it is a build command line option that causes the build to
> fail if trace_printk() is added.
>
> This way production systems can add this to make sure their kernels are
> free of trace_printk() but it doesn't affect the config that is used.
(for some reason I missed this reply in the thread ,-P)
Thanks, that's quite a nice idea, I'll try it out (not sure if we
still want that build_assert trick). We'd lose the automatic detection
of issues through randconfig kernel test robot, but hopefully if
enough distros enable it they'll start filing reports about issues.
And maybe we can use that in combination with a checkpatch.pl check.
(it turns out I'm having a hard time finding a good spot for this test
in our Chrome OS build infra, so an upstream-friendly solution would
be much better ,-P)
>
> -- Steve
>
> [ Not even compiled tested! ]
>
> diff --git a/Makefile b/Makefile
> index 2057c92a6205..5714a738879d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -91,6 +91,13 @@ else
>Q = @
> endif
>
> +ifeq ("$(origin CHECK_TRACE_PRINTK)", "command line")
> + KBUILD_NO_TRACE_PRINTK = $(NO_TRACE_PRINTK)
> +endif
> +ifndef KBUILD_NO_TRACE_PRINTK
> + KBUILD_NO_TRACE_PRINTK = 0
> +endif
> +
> # If the user is running make -s (silent mode), suppress echoing of
> # commands
>
> @@ -839,6 +846,10 @@ KBUILD_AFLAGS += -gz=zlib
> KBUILD_LDFLAGS += --compress-debug-sections=zlib
> endif
>
> +ifeq ($(KBUILD_NO_TRACE_PRINTK),1)
> +KBUILD_CFLAGS += -DNO_TRACE_PRINTK
> +endif
> +
> KBUILD_CFLAGS += $(DEBUG_CFLAGS)
> export DEBUG_CFLAGS
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 500def620d8f..bee432547d26 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -680,11 +680,14 @@ extern void tracing_stop(void);
> static inline __printf(1, 2)
> void trace_printk_check_format(const char *fmt, ...)
> {
> +#ifdef NO_TRACE_PRINTK
> + extern void __no_trace_printk_on_build(void);
> + __no_trace_printk_on_build();
> +#endif
> }
> #define __trace_printk_check_format(fmt, args...) \
> do { \
> - if (0) \
> - trace_printk_check_format(fmt, ##args); \
> + trace_printk_check_format(fmt, ##args); \
> } while (0)
>
> /**
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
