Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Steve,

On Thu, Mar 26, 2020 at 12:05:38PM -0700, Steve Longerbeam wrote:
> On 3/14/20 3:32 PM, Laurent Pinchart wrote:
> > On Sat, Mar 14, 2020 at 03:30:53PM -0700, Steve Longerbeam wrote:
> >> On 3/11/20 5:47 PM, Laurent Pinchart wrote:
> >>> On Wed, Mar 11, 2020 at 05:16:49PM -0700, Steve Longerbeam wrote:
> >>>> Hi Laurent,
> >>>>
> >>>> I agree that the find/enum format code in imx-utils needs cleanup, it's
> >>>> too confusing. I will be ready to give my ack to the imx-utils patches
> >>>> once I do some smoke testing on a sabre target when I return from vacation.
> >>>>
> >>>> Note that Phillip also submitted a fixup patch to the find/enum format
> >>>> code, IIRC it did more consolidating of the imx_media_pixfmt tables. I
> >>>> can't find it and it has gotten lost, but I tested and provided a
> >>>> reviewed-by at the time.
> >>>
> >>> I've found them in the mail archive. There were 3 patches, Hans said he
> >>> would take the first two, but only the first one got merged. I'll take
> >>> the two others and build on top of them, fixing the issues pointed out
> >>> by the kbuild robot and addressing Hans concerns.
> >>
> >> I found the thread finally. In fact I did some work on this set already,
> >> I fixed the kbuild warnings as well as added some function headers
> >> describing all the format search criteria arguments. Also changed a
> >> local var in enum_formats() to better document the enum algorithm.
> >>
> >> Unless you've started on this work already, I will be returning from
> >> vaca tomorrow and can pick this up again, merging in your patches, as
> >> well as splitting up find|enum_formats() into find|enum_formats() and
> >> find|enum_codes(), which is the remaining suggestion from Hans.
> > I've done this already, I'll try to send the patches on Monday.
> 
> If you don't mind, I've done this work and tested on an imx6 target. So 
> I will post a series.

No worries. I've been a bit delayed indeed. I'll still post my cleanups,
in case they contain anything useful in addition to yours.

> >>>> On 3/10/20 9:18 AM, Laurent Pinchart wrote:
> >>>>> Hello,
> >>>>>
> >>>>> This patch series started as an attempt to fix the format get and set
> >>>>> subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
> >>>>> patch 1/8. Patch 2/8 further cleans up the format-related code in that
> >>>>> subdev.
> >>>>>
> >>>>> Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
> >>>>> fix the format enumeration on the video node at the end of the pipeline.
> >>>>> I realized as part of that effort that there's more work than I
> >>>>> anticipated, and I'm currently evaluating the possible options.
> >>>>> Nonetheless, I think the cleanups make sense even without what I wanted
> >>>>> to build on top of them, so I'm sending them out already.
> >>>>>
> >>>>> Laurent Pinchart (8):
> >>>>>      media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
> >>>>>      media: imx: imx7-mipi-csis: Centralize initialization of pad formats
> >>>>>      media: imx: utils: Inline init_mbus_colorimetry() in its caller
> >>>>>      media: imx: utils: Handle Bayer format lookup through a selection flag
> >>>>>      media: imx: utils: Simplify IPU format lookup and enumeration
> >>>>>      media: imx: utils: Make imx_media_pixfmt handle variable number of
> >>>>>        codes
> >>>>>      media: imx: utils: Remove unneeded argument to (find|enum)_format()
> >>>>>      media: imx: utils: Rename format lookup and enumeration functions
> >>>>>
> >>>>>     drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
> >>>>>     drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
> >>>>>     drivers/staging/media/imx/imx-media-capture.c |  22 +-
> >>>>>     .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
> >>>>>     drivers/staging/media/imx/imx-media-csi.c     |  26 +-
> >>>>>     drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
> >>>>>     drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
> >>>>>     drivers/staging/media/imx/imx-media.h         |  24 +-
> >>>>>     drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
> >>>>>     drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
> >>>>>     10 files changed, 271 insertions(+), 289 deletions(-)

-- 
Regards,

Laurent Pinchart