[FFmpeg-devel] [PATCH v08.01 2/3] fbtile tile/detile, hwcontext_drm detile NonLinear
Lynne
dev at lynne.ee
Mon Jul 13 13:24:23 EEST 2020
Jul 12, 2020, 18:21 by hanishkvc at gmail.com:
> ** fbtile cpu based framebuffer tile/detile helpers
>
> diff --git a/Changelog b/Changelog
> index 20ba03ae8b..0b48858da7 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -6,6 +6,8 @@ version <next>:
> - MacCaption demuxer
> - PGX decoder
> - kmsgrab GetFB2 format_modifier, if user doesnt specify
> +- fbtile cpu based framebuffer tile/detile helpers (Intel TileX|Y|Yf)
> +- hwcontext_drm detiles non linear layouts, if possible
>
Remove upper changelog entry (fbtile). Its purely internal.
> +#include "config.h"
> +#include "avutil.h"
> +#include "common.h"
> +#include "fbtile.h"
> +#if CONFIG_LIBDRM
> +#include <drm_fourcc.h>
> +#endif
> +
> +
> +/**
> + * Ok return value
> + */
> +#define FBT_OK 0
>
Remove this.
> +enum FFFBTileLayout ff_fbtile_getlayoutid(enum FFFBTileFamily family, uint64_t familyTileType)
> +{
> + enum FFFBTileLayout layout = FF_FBTILE_UNKNOWN;
> +
> + switch(family) {
> + case FF_FBTILE_FAMILY_DRM:
> +#if CONFIG_LIBDRM
> + switch(familyTileType) {
> + case DRM_FORMAT_MOD_LINEAR:
> + layout = FF_FBTILE_NONE;
> + break;
> + case I915_FORMAT_MOD_X_TILED:
> + layout = FF_FBTILE_INTEL_XGEN9;
> + break;
> + case I915_FORMAT_MOD_Y_TILED:
> + layout = FF_FBTILE_INTEL_YGEN9;
> + break;
> + case I915_FORMAT_MOD_Yf_TILED:
> + layout = FF_FBTILE_INTEL_YF;
> + break;
> + default:
> + layout = FF_FBTILE_UNKNOWN;
> + break;
> + }
> +#else
> + av_log(NULL, AV_LOG_WARNING, "fbtile:getlayoutid: family[%d] familyTileType[%ld]\n", family, familyTileType);
> +#endif
> + break;
> + default:
> + av_log(NULL, AV_LOG_WARNING, "fbtile:getlayoutid: unknown family[%d] familyTileType[%ld]\n", family, familyTileType);
> + }
> + av_log(NULL, AV_LOG_VERBOSE, "fbtile:getlayoutid: family[%d] familyTileType[%ld] maps to layoutid[%d]\n", family, familyTileType, layout);
> + return layout;
> +}
>
Remove entire function and work using libdrm's modifier defines.
There's absolutely no point in modularity because like I said, no other API is this terminally
bad to require detiling.
> +
> +
> +/**
> + * TileWalk Direction Change Entry
> + * Used to specify the tile walking of subtiles within a tile.
> + */
> +struct FBTWDirChange {
> + int posOffset;
> + int xDelta;
> + int yDelta;
> +};
> +
> +
> +/**
> + * TileWalk, Contains info required for a given tile walking.
> + *
> + * @field bytesPerPixel the bytes per pixel for the image
> + * @field subTileWidth the width of subtile within the tile, in pixels
> + * @field subTileHeight the height of subtile within the tile, in pixels
> + * @field tileWidth the width of the tile, in pixels
> + * @field tileHeight the height of the tile, in pixels
> + * @field numDirChanges the number of dir changes involved in tile walk
> + * @field dirChanges the array of dir changes for the tile walk required
> + */
> +struct FBTileWalk {
> + int bytesPerPixel;
> + int subTileWidth, subTileHeight;
> + int tileWidth, tileHeight;
> + int numDirChanges;
> + struct FBTWDirChange dirChanges[];
> +};
> +
> +
> +/**
> + * Settings for Intel Tile-Yf framebuffer layout.
> + * May need to swap the 4 pixel wide subtile, have to check doc bit more
> + */
> +static struct FBTileWalk tyfTileWalk = {
> + .bytesPerPixel = 4,
> + .subTileWidth = 4, .subTileHeight = 8,
> + .tileWidth = 32, .tileHeight = 32,
> + .numDirChanges = 6,
> + .dirChanges = { {8, 4, 0}, {16, -4, 8}, {32, 4, -8}, {64, -12, 8}, {128, 4, -24}, {256, 4, -24} }
> + };
> +
> +/**
> + * Setting for Intel Tile-X framebuffer layout
> + */
> +static struct FBTileWalk txTileWalk = {
> + .bytesPerPixel = 4,
> + .subTileWidth = 128, .subTileHeight = 8,
> + .tileWidth = 128, .tileHeight = 8,
> + .numDirChanges = 1,
> + .dirChanges = { {8, 128, 0} }
> + };
> +
> +/**
> + * Setting for Intel Tile-Y framebuffer layout
> + * Even thou a simple generic detiling logic doesnt require the
> + * dummy 256 posOffset entry. The pseudo parallel detiling based
> + * opti logic requires to know about the Tile boundry.
> + */
> +static struct FBTileWalk tyTileWalk = {
> + .bytesPerPixel = 4,
> + .subTileWidth = 4, .subTileHeight = 32,
> + .tileWidth = 32, .tileHeight = 32,
> + .numDirChanges = 2,
> + .dirChanges = { {32, 4, 0}, {256, 4, 0} }
> + };
> +
> +
> +/**
> + * Generic Logic to Tile/Detile between tiled and linear layout.
> + *
> + * @param op whether to tile or detile
> + * @param w width of the image
> + * @param h height of the image
> + * @param dst the destination image buffer
> + * @param dstLineSize the size of each row in dst image, in bytes
> + * @param src the source image buffer
> + * @param srcLineSize the size of each row in src image, in bytes
> + * @param tw the structure which contains the tile walk parameters
> + *
> + * @return 0 if detiled, 1 if not
> + */
> +
> +
> +/**
> + * _fbtile_generic_simple tile/detile layout
> + */
> +static int _fbtile_generic_simple(enum FFFBTileOps op,
> + const int w, const int h,
> + uint8_t *dst, const int dstLineSize,
> + uint8_t *src, const int srcLineSize,
> + const int bytesPerPixel,
> + const int subTileWidth, const int subTileHeight,
> + const int tileWidth, const int tileHeight,
> + const int numDirChanges, const struct FBTWDirChange *dirChanges)
> +{
> + int tO, lO;
> + int lX, lY;
> + int cSTL, nSTLines;
> + uint8_t *tld, *lin;
> + int tldLineSize, linLineSize;
> + const int subTileWidthBytes = subTileWidth*bytesPerPixel;
> +
> + if (op == FF_FBTILE_OPS_TILE) {
> + lin = src;
> + linLineSize = srcLineSize;
> + tld = dst;
> + tldLineSize = dstLineSize;
> + } else {
> + tld = src;
> + tldLineSize = srcLineSize;
> + lin = dst;
> + linLineSize = dstLineSize;
> + }
> +
> + // To keep things sane and simple tile layout is assumed to be tightly packed,
> + // so below check is a indirect logical assumption, even thou tldLineSize is not directly mappable at one level
> + if (w*bytesPerPixel != tldLineSize) {
> + av_log(NULL, AV_LOG_ERROR, "fbtile:genericsimp: w%dxh%d, tldLineSize%d, linLineSize%d\n", w, h, tldLineSize, linLineSize);
> + av_log(NULL, AV_LOG_ERROR, "fbtile:genericsimp: dont support tldLineSize | Pitch going beyond width\n");
> + return AVERROR(EINVAL);
> + }
> + tO = 0;
> + lX = 0;
> + lY = 0;
> + nSTLines = (w*h)/subTileWidth; // numSubTileLines
> + cSTL = 0; // curSubTileLine
> + while (cSTL < nSTLines) {
> + lO = lY*linLineSize + lX*bytesPerPixel;
> +#ifdef DEBUG_FBTILE
> + av_log(NULL, AV_LOG_DEBUG, "fbtile:genericsimp: lX%d lY%d; lO%d, tO%d; %d/%d\n", lX, lY, lO, tO, cSTL, nSTLines);
> +#endif
> +
> + for (int k = 0; k < subTileHeight; k++) {
> + if (op == FF_FBTILE_OPS_TILE) {
> + memcpy(tld+tO+k*subTileWidthBytes, lin+lO+k*linLineSize, subTileWidthBytes);
> + } else {
> + memcpy(lin+lO+k*linLineSize, tld+tO+k*subTileWidthBytes, subTileWidthBytes);
> + }
> + }
> + tO = tO + subTileHeight*subTileWidthBytes;
> +
> + cSTL += subTileHeight;
> + for (int i=numDirChanges-1; i>=0; i--) {
> + if ((cSTL%dirChanges[i].posOffset) == 0) {
> + lX += dirChanges[i].xDelta;
> + lY += dirChanges[i].yDelta;
> + break;
> + }
> + }
> + if (lX >= w) {
> + lX = 0;
> + lY += tileHeight;
> + }
> + }
> + return FBT_OK;
> +}
> +
> +
> +static int fbtile_generic_simple(enum FFFBTileOps op,
> + const int w, const int h,
> + uint8_t *dst, const int dstLineSize,
> + uint8_t *src, const int srcLineSize,
> + const struct FBTileWalk *tw)
> +{
> + return _fbtile_generic_simple(op, w, h,
> + dst, dstLineSize, src, srcLineSize,
> + tw->bytesPerPixel,
> + tw->subTileWidth, tw->subTileHeight,
> + tw->tileWidth, tw->tileHeight,
> + tw->numDirChanges, tw->dirChanges);
> +}
>
Remove this wrapper function and just rename _fbtile_generic_simple to
fbtile_generic_simple.
> + av_log(NULL, AV_LOG_WARNING, "fbtile:framecopy: both src [%d] and dst [%d] layouts cant be tiled\n", srcTileLayout, dstTileLayout);
> + }
> + *status = FF_FBTILE_FRAMECOPY_COPYONLY;
> + return av_frame_copy(dst, src);
> +}
> +
> +
> +// vim: set expandtab sts=4: //
>
No weird footers in files please.
> diff --git a/libavutil/fbtile.h b/libavutil/fbtile.h
> new file mode 100644
> index 0000000000..83360952b1
> --- /dev/null
> +++ b/libavutil/fbtile.h
> @@ -0,0 +1,134 @@
> +/*
> + * CPU based Framebuffer Generic Tile DeTile logic
> + * Copyright (c) 2020 C Hanish Menon <HanishKVC>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVUTIL_FBTILE_H
> +#define AVUTIL_FBTILE_H
> +
> +#include <stdint.h>
> +#include "libavutil/pixfmt.h"
> +#include "libavutil/frame.h"
> +
> +/**
> + * @file
> + * @brief CPU based Framebuffer tiler detiler
> + * @author C Hanish Menon <HanishKVC>
> + * @{
> + */
>
No weird headers in files, please.
> +
> +
> +/**
> + * Enable printing of the tile walk
> + */
> +//#define DEBUG_FBTILE 1
>
We do not need per-line printouts, so just remove the av_logs
enabled with this and remove DEBUG_FBTILE.
> +/**
> + * The FBTile related operations
> + */
> +enum FFFBTileOps {
> + FF_FBTILE_OPS_NONE,
> + FF_FBTILE_OPS_TILE,
> + FF_FBTILE_OPS_DETILE,
> + FF_FBTILE_OPS_UNKNOWN,
> +};
>
Tiling could be useful for uploading.
FF_FBTILE_OPS_NONE and FF_FBTILE_OPS_UNKNOWN make no sense.
> +
> +/**
> + * The FBTile layout families
> + * Used to help map from an external subsystem like say drm
> + * to fbtile's internal tile layout id.
> + */
> +enum FFFBTileFamily {
> + FF_FBTILE_FAMILY_DRM,
> + FF_FBTILE_FAMILY_UNKNOWN,
> +};
>
No, remove this. We only need to support DRM.
I'm repeating myself, but _no_other_api_is_or_will_ever_be_this_terminally_bad_.
> +/**
> + * As AVFrame doesnt support tile layout natively, so if detile is successful
> + * the same is notified to any other users by updating the corresponding
> + * hardware AVFrame's tile layout info.
> + * If this is not needed, #define HWCTXDRM_SYNCRELATED_FORMATMODIFIER 0
> + */
> +#ifndef HWCTXDRM_SYNCRELATED_FORMATMODIFIER
> +#define HWCTXDRM_SYNCRELATED_FORMATMODIFIER 1
> +#endif
> +static int drm_transfer_with_detile(const AVFrame *hwAVFrame, AVFrame *dst, AVFrame *src)
> +{
> + int err;
> + uint64_t formatModifier;
> + enum FFFBTileLayout srcFBTileLayout, dstFBTileLayout;
> + enum FFFBTileFrameCopyStatus status;
> + AVDRMFrameDescriptor *drmFrame = NULL;
> +
> + srcFBTileLayout = FF_FBTILE_NONE;
> + dstFBTileLayout = FF_FBTILE_NONE;
> + if (hwAVFrame->format == AV_PIX_FMT_DRM_PRIME) {
> + drmFrame = (AVDRMFrameDescriptor*)hwAVFrame->data[0];
> + formatModifier = drmFrame->objects[0].format_modifier;
> + srcFBTileLayout = ff_fbtile_getlayoutid(FF_FBTILE_FAMILY_DRM, formatModifier);
> + }
> + err = ff_fbtile_frame_copy(dst, dstFBTileLayout, src, srcFBTileLayout, &status);
> +#if HWCTXDRM_SYNCRELATED_FORMATMODIFIER
> + if (!err && (status == FF_FBTILE_FRAMECOPY_TILECOPY)) {
> + if (drmFrame != NULL)
> + drmFrame->objects[0].format_modifier = DRM_FORMAT_MOD_LINEAR;
> + }
>
Don't modify the hardware frame. So there's no need for this.
Remove HWCTXDRM_SYNCRELATED_FORMATMODIFIER.
Also remove the FFFBTileFrameCopyStatus enum, its unneeded.
For logging, just make all functions accept AVHWFramesContext, because this will
not be used outside of a hwcontext and using NULL prevents users from filtering
log messages.
There's still more to go through until this can be merged, particularly regarding making
this as unmodular and hwcontext_drm exlcusive as possible.
More information about the ffmpeg-devel
mailing list