>-----Original Message----- >From: Zhenyu Wang [mailto:zhenyuw@xxxxxxxxxxxxxxx] >Sent: Wednesday, May 31, 2017 1:12 PM >To: Chen, Xiaoguang <xiaoguang.chen@xxxxxxxxx> >Cc: alex.williamson@xxxxxxxxxx; kraxel@xxxxxxxxxx; chris@xxxxxxxxxxxxxxxxxx; >intel-gfx@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >zhenyuw@xxxxxxxxxxxxxxx; Lv, Zhiyuan <zhiyuan.lv@xxxxxxxxx>; intel-gvt- >dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>; Tian, Kevin ><kevin.tian@xxxxxxxxx> >Subject: Re: [PATCH v6 3/6] drm/i915/gvt: Frame buffer decoder support for GVT- >g > >On 2017.05.27 16:38:49 +0800, Xiaoguang Chen wrote: >> decode frambuffer attributes of primary, cursor and sprite plane >> >> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/gvt/Makefile | 3 +- >> drivers/gpu/drm/i915/gvt/display.c | 2 +- >> drivers/gpu/drm/i915/gvt/display.h | 2 + >> drivers/gpu/drm/i915/gvt/fb_decoder.c | 479 >> ++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/gvt/fb_decoder.h | 166 ++++++++++++ >> drivers/gpu/drm/i915/gvt/gvt.h | 1 + >> 6 files changed, 651 insertions(+), 2 deletions(-) create mode >> 100644 drivers/gpu/drm/i915/gvt/fb_decoder.c >> create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.h >> >> diff --git a/drivers/gpu/drm/i915/gvt/Makefile >> b/drivers/gpu/drm/i915/gvt/Makefile >> index b123c20..192ca26 100644 >> --- a/drivers/gpu/drm/i915/gvt/Makefile >> +++ b/drivers/gpu/drm/i915/gvt/Makefile >> @@ -1,7 +1,8 @@ >> GVT_DIR := gvt >> GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o >firmware.o \ >> interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \ >> - execlist.o scheduler.o sched_policy.o render.o cmd_parser.o >> + execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \ >> + fb_decoder.o >> >> ccflags-y += -I$(src) -I$(src)/$(GVT_DIR) -Wall >> i915-y += $(addprefix $(GVT_DIR)/, >$(GVT_SOURCE)) >> diff --git a/drivers/gpu/drm/i915/gvt/display.c >> b/drivers/gpu/drm/i915/gvt/display.c >> index e0261fc..f5f63c5 100644 >> --- a/drivers/gpu/drm/i915/gvt/display.c >> +++ b/drivers/gpu/drm/i915/gvt/display.c >> @@ -67,7 +67,7 @@ static int edp_pipe_is_enabled(struct intel_vgpu *vgpu) >> return 1; >> } >> >> -static int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe) >> +int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe) >> { >> struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv; >> >> diff --git a/drivers/gpu/drm/i915/gvt/display.h >> b/drivers/gpu/drm/i915/gvt/display.h >> index d73de22..b46b868 100644 >> --- a/drivers/gpu/drm/i915/gvt/display.h >> +++ b/drivers/gpu/drm/i915/gvt/display.h >> @@ -179,4 +179,6 @@ int intel_vgpu_init_display(struct intel_vgpu >> *vgpu, u64 resolution); void intel_vgpu_reset_display(struct >> intel_vgpu *vgpu); void intel_vgpu_clean_display(struct intel_vgpu >> *vgpu); >> >> +int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe); >> + >> #endif >> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c >> b/drivers/gpu/drm/i915/gvt/fb_decoder.c >> new file mode 100644 >> index 0000000..d4404fd >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c >> @@ -0,0 +1,479 @@ >> +/* >> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved. >> + * >> + * Permission is hereby granted, free of charge, to any person >> +obtaining a >> + * copy of this software and associated documentation files (the >> +"Software"), >> + * to deal in the Software without restriction, including without >> +limitation >> + * the rights to use, copy, modify, merge, publish, distribute, >> +sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom >> +the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice (including >> +the next >> + * paragraph) shall be included in all copies or substantial portions >> +of the >> + * Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> +EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> +MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO >EVENT >> +SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, >DAMAGES >> +OR OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> +ARISING FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER >> +DEALINGS IN THE >> + * SOFTWARE. >> + * >> + * Authors: >> + * Kevin Tian <kevin.tian@xxxxxxxxx> >> + * >> + * Contributors: >> + * Bing Niu <bing.niu@xxxxxxxxx> >> + * Xu Han <xu.han@xxxxxxxxx> >> + * Ping Gao <ping.a.gao@xxxxxxxxx> >> + * Xiaoguang Chen <xiaoguang.chen@xxxxxxxxx> >> + * Yang Liu <yang2.liu@xxxxxxxxx> >> + * >> + */ >> + >> +#include <uapi/drm/drm_fourcc.h> >> +#include "i915_drv.h" >> +#include "gvt.h" >> + >> +/* The below definitions are required by guest. */ // [63:0] x:R:G:B >> +16:16:16:16 little endian #define DRM_FORMAT_XRGB161616_GVT >> +fourcc_code('X', 'R', '4', '8') // [63:0] x:B:G:R 16:16:16:16 little >> +endian #define DRM_FORMAT_XBGR161616_GVT fourcc_code('X', 'B', '4', >> +'8') >> + > >Should be added to drm_fourcc? Will add to drm_fourcc. > >> +#define FORMAT_NUM 16 > >PRIMARY_FORMAT_NUM > >> +struct pixel_format { >> + int drm_format; /* Pixel format in DRM definition */ >> + int bpp; /* Bits per pixel, 0 indicates invalid */ >> + char *desc; /* The description */ >> +}; >> + >> +/* non-supported format has bpp default to 0 */ static struct >> +pixel_format primary_pixel_formats[FORMAT_NUM] = { >> + [0x2] = {DRM_FORMAT_C8, 8, "8-bit Indexed"}, >> + [0x5] = {DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"}, >> + [0x6] = {DRM_FORMAT_XRGB8888, 32, >> + "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"}, >> + [0x8] = {DRM_FORMAT_XBGR2101010, 32, >> + "32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"}, >> + [0xa] = {DRM_FORMAT_XRGB2101010, 32, >> + "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"}, >> + [0xc] = {DRM_FORMAT_XRGB161616_GVT, 64, >> + "64-bit RGBX Floating Point(16:16:16:16 MSB-X:B:G:R)"}, >> + [0xe] = {DRM_FORMAT_XBGR8888, 32, >> + "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"}, }; > >might explicitly say this is bdw_pixel_formats? Good suggestion :) > >> + >> +/* non-supported format has bpp default to 0 */ static struct >> +pixel_format skl_pixel_formats[] = { >> + {DRM_FORMAT_YUYV, 16, "16-bit packed YUYV (8:8:8:8 MSB- >V:Y2:U:Y1)"}, >> + {DRM_FORMAT_UYVY, 16, "16-bit packed UYVY (8:8:8:8 MSB- >Y2:V:Y1:U)"}, >> + {DRM_FORMAT_YVYU, 16, "16-bit packed YVYU (8:8:8:8 MSB- >U:Y2:V:Y1)"}, >> + {DRM_FORMAT_VYUY, 16, "16-bit packed VYUY (8:8:8:8 MSB- >Y2:U:Y1:V)"}, >> + >> + {DRM_FORMAT_C8, 8, "8-bit Indexed"}, >> + {DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"}, >> + {DRM_FORMAT_ABGR8888, 32, "32-bit RGBA (8:8:8:8 MSB-A:B:G:R)"}, >> + {DRM_FORMAT_XBGR8888, 32, "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"}, >> + >> + {DRM_FORMAT_ARGB8888, 32, "32-bit BGRA (8:8:8:8 MSB-A:R:G:B)"}, >> + {DRM_FORMAT_XRGB8888, 32, "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"}, >> + {DRM_FORMAT_XBGR2101010, 32, "32-bit RGBX (2:10:10:10 MSB- >X:B:G:R)"}, >> + {DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 >> +MSB-X:R:G:B)"}, >> + >> + {DRM_FORMAT_XRGB161616_GVT, 64, >> + "64-bit XRGB (16:16:16:16 MSB-X:R:G:B)"}, >> + {DRM_FORMAT_XBGR161616_GVT, 64, >> + "64-bit XBGR (16:16:16:16 MSB-X:B:G:R)"}, >> + >> + /* non-supported format has bpp default to 0 */ >> + {0, 0, NULL}, >> +}; >> + >> +static int skl_format_to_drm(int format, bool rgb_order, bool alpha, >> + int yuv_order) >> +{ >> + int skl_pixel_formats_index = 14; >> + >> + switch (format) { >> + case PLANE_CTL_FORMAT_INDEXED: >> + skl_pixel_formats_index = 4; >> + break; >> + case PLANE_CTL_FORMAT_RGB_565: >> + skl_pixel_formats_index = 5; >> + break; >> + case PLANE_CTL_FORMAT_XRGB_8888: >> + if (rgb_order) >> + skl_pixel_formats_index = alpha ? 6 : 7; >> + else >> + skl_pixel_formats_index = alpha ? 8 : 9; >> + break; >> + case PLANE_CTL_FORMAT_XRGB_2101010: >> + skl_pixel_formats_index = rgb_order ? 10 : 11; >> + break; >> + >> + case PLANE_CTL_FORMAT_XRGB_16161616F: >> + skl_pixel_formats_index = rgb_order ? 12 : 13; >> + break; >> + >> + case PLANE_CTL_FORMAT_YUV422: >> + skl_pixel_formats_index = yuv_order >> 16; >> + if (skl_pixel_formats_index > 3) >> + return -EINVAL; >> + break; >> + >> + default: >> + break; >> + } >> + >> + return skl_pixel_formats_index; >> +} >> + >> +static u32 intel_vgpu_get_stride(struct intel_vgpu *vgpu, int pipe, >> + u32 tiled, int stride_mask, int bpp) { >> + struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv; >> + >> + u32 stride_reg = vgpu_vreg(vgpu, DSPSTRIDE(pipe)) & stride_mask; >> + u32 stride = stride_reg; >> + >> + if (IS_SKYLAKE(dev_priv)) { >> + switch (tiled) { >> + case PLANE_CTL_TILED_LINEAR: >> + stride = stride_reg * 64; >> + break; >> + case PLANE_CTL_TILED_X: >> + stride = stride_reg * 512; >> + break; >> + case PLANE_CTL_TILED_Y: >> + stride = stride_reg * 128; >> + break; >> + case PLANE_CTL_TILED_YF: >> + if (bpp == 8) >> + stride = stride_reg * 64; >> + else if (bpp == 16 || bpp == 32 || bpp == 64) >> + stride = stride_reg * 128; >> + else >> + gvt_dbg_core("skl: unsupported bpp:%d\n", bpp); >> + break; >> + default: >> + gvt_dbg_core("skl: unsupported tile format:%x\n", >> + tiled); >> + } >> + } >> + >> + return stride; >> +} >> + >> +static int intel_vgpu_decode_primary_plane_format(struct intel_vgpu *vgpu, >> + int pipe, struct intel_vgpu_primary_plane_format *plane) { >> + u32 val, fmt; >> + struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv; >> + >> + val = vgpu_vreg(vgpu, DSPCNTR(pipe)); >> + plane->enabled = !!(val & DISPLAY_PLANE_ENABLE); >> + if (!plane->enabled) >> + return 0; >> + >> + if (IS_SKYLAKE(dev_priv)) { >> + plane->tiled = (val & PLANE_CTL_TILED_MASK) >> >> + _PLANE_CTL_TILED_SHIFT; >> + fmt = skl_format_to_drm( >> + val & PLANE_CTL_FORMAT_MASK, >> + val & PLANE_CTL_ORDER_RGBX, >> + val & PLANE_CTL_ALPHA_MASK, >> + val & PLANE_CTL_YUV422_ORDER_MASK); >> + plane->bpp = skl_pixel_formats[fmt].bpp; >> + plane->drm_format = skl_pixel_formats[fmt].drm_format; >> + } else { >> + plane->tiled = !!(val & DISPPLANE_TILED); >> + fmt = (val & DISPPLANE_PIXFORMAT_MASK) >> >_PRI_PLANE_FMT_SHIFT; >> + plane->bpp = primary_pixel_formats[fmt].bpp; >> + plane->drm_format = primary_pixel_formats[fmt].drm_format; >> + } >> + >> + if ((IS_SKYLAKE(dev_priv) && !skl_pixel_formats[fmt].bpp) >> + || (!IS_SKYLAKE(dev_priv) && >> + !primary_pixel_formats[fmt].bpp)) { > >Just if (!plane->bpp)? Good suggestion :) > >> + gvt_vgpu_err("Non-supported pixel format (0x%x)\n", fmt); >> + return -EINVAL; >> + } >> + >> + plane->hw_format = fmt; >> + >> + plane->base = vgpu_vreg(vgpu, DSPSURF(pipe)) & GTT_PAGE_MASK; >> + >> + plane->stride = intel_vgpu_get_stride(vgpu, pipe, (plane->tiled << 10), >> + (IS_SKYLAKE(dev_priv)) ? (_PRI_PLANE_STRIDE_MASK >> 6) : >> + _PRI_PLANE_STRIDE_MASK, plane->bpp); >> + >> + plane->width = (vgpu_vreg(vgpu, PIPESRC(pipe)) & >_PIPE_H_SRCSZ_MASK) >> >> + _PIPE_H_SRCSZ_SHIFT; >> + plane->width += 1; >> + plane->height = (vgpu_vreg(vgpu, PIPESRC(pipe)) & >> + _PIPE_V_SRCSZ_MASK) >> _PIPE_V_SRCSZ_SHIFT; >> + plane->height += 1; /* raw height is one minus the real value */ >> + >> + val = vgpu_vreg(vgpu, DSPTILEOFF(pipe)); >> + plane->x_offset = (val & _PRI_PLANE_X_OFF_MASK) >> >> + _PRI_PLANE_X_OFF_SHIFT; >> + plane->y_offset = (val & _PRI_PLANE_Y_OFF_MASK) >> >> + _PRI_PLANE_Y_OFF_SHIFT; >> + >> + return 0; >> +} >> + >> +#define CURSOR_MODE_NUM (1 << 6) > >CURSOR_FORMAT_NUM > >> +struct cursor_mode_format { >> + int drm_format; /* Pixel format in DRM definition */ >> + u8 bpp; /* Bits per pixel; 0 indicates invalid */ >> + u32 width; /* In pixel */ >> + u32 height; /* In lines */ >> + char *desc; /* The description */ >> +}; >> + >> +/* non-supported format has bpp default to 0 */ static struct >> +cursor_mode_format cursor_pixel_formats[CURSOR_MODE_NUM] = { >> + [0x22] = {DRM_FORMAT_ARGB8888, 32, 128, 128, "128x128 32bpp >ARGB"}, >> + [0x23] = {DRM_FORMAT_ARGB8888, 32, 256, 256, "256x256 32bpp >ARGB"}, >> + [0x27] = {DRM_FORMAT_ARGB8888, 32, 64, 64, "64x64 32bpp ARGB"}, >> + [0x7] = {DRM_FORMAT_ARGB8888, 32, 64, 64, "64x64 32bpp ARGB"}, }; >> + >> +static int intel_vgpu_decode_cursor_plane_format(struct intel_vgpu *vgpu, >> + int pipe, struct intel_vgpu_cursor_plane_format *plane) { >> + u32 val, mode; >> + u32 alpha_plane, alpha_force; >> + struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv; >> + >> + val = vgpu_vreg(vgpu, CURCNTR(pipe)); >> + mode = val & CURSOR_MODE; >> + plane->enabled = (mode != CURSOR_MODE_DISABLE); >> + if (!plane->enabled) >> + return 0; >> + >> + if (!cursor_pixel_formats[mode].bpp) { >> + gvt_vgpu_err("Non-supported cursor mode (0x%x)\n", mode); >> + return -EINVAL; >> + } >> + plane->mode = mode; >> + plane->bpp = cursor_pixel_formats[mode].bpp; >> + plane->drm_format = cursor_pixel_formats[mode].drm_format; >> + plane->width = cursor_pixel_formats[mode].width; >> + plane->height = cursor_pixel_formats[mode].height; >> + >> + alpha_plane = (val & _CURSOR_ALPHA_PLANE_MASK) >> >> + _CURSOR_ALPHA_PLANE_SHIFT; >> + alpha_force = (val & _CURSOR_ALPHA_FORCE_MASK) >> >> + _CURSOR_ALPHA_FORCE_SHIFT; >> + if (alpha_plane || alpha_force) >> + gvt_dbg_core("alpha_plane=0x%x, alpha_force=0x%x\n", >> + alpha_plane, alpha_force); >> + >> + plane->base = vgpu_vreg(vgpu, CURBASE(pipe)) & GTT_PAGE_MASK; >> + >> + val = vgpu_vreg(vgpu, CURPOS(pipe)); >> + plane->x_pos = (val & _CURSOR_POS_X_MASK) >> >_CURSOR_POS_X_SHIFT; >> + plane->x_sign = (val & _CURSOR_SIGN_X_MASK) >> >_CURSOR_SIGN_X_SHIFT; >> + plane->y_pos = (val & _CURSOR_POS_Y_MASK) >> >_CURSOR_POS_Y_SHIFT; >> + plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >> >_CURSOR_SIGN_Y_SHIFT; >> + >> + return 0; >> +} >> + >> +#define FORMAT_NUM_SRRITE (1 << 3) >> + > >SPRITE_FORMAT_NUM > >> +static struct pixel_format sprite_pixel_formats[FORMAT_NUM_SRRITE] = { >> + [0x0] = {DRM_FORMAT_YUV422, 16, "YUV 16-bit 4:2:2 packed"}, >> + [0x1] = {DRM_FORMAT_XRGB2101010, 32, "RGB 32-bit 2:10:10:10"}, >> + [0x2] = {DRM_FORMAT_XRGB8888, 32, "RGB 32-bit 8:8:8:8"}, >> + [0x3] = {DRM_FORMAT_XRGB161616_GVT, 64, >> + "RGB 64-bit 16:16:16:16 Floating Point"}, >> + [0x4] = {DRM_FORMAT_AYUV, 32, >> + "YUV 32-bit 4:4:4 packed (8:8:8:8 MSB-X:Y:U:V)"}, }; >> + >> +static int intel_vgpu_decode_sprite_plane_format(struct intel_vgpu *vgpu, >> + int pipe, struct intel_vgpu_sprite_plane_format *plane) { >> + u32 val, fmt; >> + u32 width; >> + u32 color_order, yuv_order; >> + int drm_format; >> + >> + val = vgpu_vreg(vgpu, SPRCTL(pipe)); >> + plane->enabled = !!(val & SPRITE_ENABLE); >> + if (!plane->enabled) >> + return 0; >> + >> + plane->tiled = !!(val & SPRITE_TILED); >> + color_order = !!(val & SPRITE_RGB_ORDER_RGBX); >> + yuv_order = (val & SPRITE_YUV_BYTE_ORDER_MASK) >> >> + _SPRITE_YUV_ORDER_SHIFT; >> + >> + fmt = (val & SPRITE_PIXFORMAT_MASK) >> _SPRITE_FMT_SHIFT; >> + if (!sprite_pixel_formats[fmt].bpp) { >> + gvt_vgpu_err("Non-supported pixel format (0x%x)\n", fmt); >> + return -EINVAL; >> + } >> + plane->hw_format = fmt; >> + plane->bpp = sprite_pixel_formats[fmt].bpp; >> + drm_format = sprite_pixel_formats[fmt].drm_format; >> + >> + /* Order of RGB values in an RGBxxx buffer may be ordered RGB or >> + * BGR depending on the state of the color_order field >> + */ >> + if (!color_order) { >> + if (drm_format == DRM_FORMAT_XRGB2101010) >> + drm_format = DRM_FORMAT_XBGR2101010; >> + else if (drm_format == DRM_FORMAT_XRGB8888) >> + drm_format = DRM_FORMAT_XBGR8888; >> + } >> + >> + if (drm_format == DRM_FORMAT_YUV422) { >> + switch (yuv_order) { >> + case 0: > >no extra space > >> + drm_format = DRM_FORMAT_YUYV; >> + break; >> + case 1: >> + drm_format = DRM_FORMAT_UYVY; >> + break; >> + case 2: >> + drm_format = DRM_FORMAT_YVYU; >> + break; >> + case 3: >> + drm_format = DRM_FORMAT_VYUY; >> + break; >> + default: >> + /* yuv_order has only 2 bits */ >> + break; >> + } >> + } >> + >> + plane->drm_format = drm_format; >> + >> + plane->base = vgpu_vreg(vgpu, SPRSURF(pipe)) & GTT_PAGE_MASK; >> + plane->width = vgpu_vreg(vgpu, SPRSTRIDE(pipe)) & >> + _SPRITE_STRIDE_MASK; >> + plane->width /= plane->bpp / 8; /* raw width in bytes */ >> + >> + val = vgpu_vreg(vgpu, SPRSIZE(pipe)); >> + plane->height = (val & _SPRITE_SIZE_HEIGHT_MASK) >> >> + _SPRITE_SIZE_HEIGHT_SHIFT; >> + width = (val & _SPRITE_SIZE_WIDTH_MASK) >> >_SPRITE_SIZE_WIDTH_SHIFT; >> + plane->height += 1; /* raw height is one minus the real value */ >> + width += 1; /* raw width is one minus the real value */ >> + if (plane->width != width) >> + gvt_dbg_core("sprite_plane: plane->width=%d, width=%d\n", >> + plane->width, width); > >should report error? OK. > >> +static int intel_vgpu_decode_fb_format(struct intel_gvt *gvt, int id, >> + struct intel_vgpu_fb_format *fb) >> +{ >> + int i; >> + struct intel_vgpu *vgpu = NULL; >> + int ret = 0; >> + struct drm_i915_private *dev_priv = gvt->dev_priv; >> + >> + if (!fb) >> + return -EINVAL; >> + >> + /* TODO: use fine-grained refcnt later */ >> + mutex_lock(&gvt->lock); >> + >> + for_each_active_vgpu(gvt, vgpu, i) >> + if (vgpu->id == id) >> + break; >> + >> + if (!vgpu) { >> + gvt_vgpu_err("Invalid vgpu ID (%d)\n", id); >> + mutex_unlock(&gvt->lock); >> + return -ENODEV; >> + } >> + >> + for (i = 0; i < I915_MAX_PIPES; i++) { >> + struct intel_vgpu_pipe_format *pipe = &fb->pipes[i]; >> + u32 ddi_func_ctl = vgpu_vreg(vgpu, TRANS_DDI_FUNC_CTL(i)); >> + >> + if (!(ddi_func_ctl & TRANS_DDI_FUNC_ENABLE)) { >> + pipe->ddi_port = DDI_PORT_NONE; >> + } else { >> + u32 port = (ddi_func_ctl & TRANS_DDI_PORT_MASK) >> >> + TRANS_DDI_PORT_SHIFT; >> + if (port <= DDI_PORT_E) >> + pipe->ddi_port = port; >> + else >> + pipe->ddi_port = DDI_PORT_NONE; >> + } >> + >> + ret |= intel_vgpu_decode_primary_plane_format(vgpu, >> + i, &pipe->primary); >> + ret |= intel_vgpu_decode_sprite_plane_format(vgpu, >> + i, &pipe->sprite); >> + ret |= intel_vgpu_decode_cursor_plane_format(vgpu, >> + i, &pipe->cursor); >> + >> + if (ret) { >> + gvt_vgpu_err("Decode format error for pipe(%d)\n", i); >> + ret = -EINVAL; >> + break; >> + } >> + } >> + >> + mutex_unlock(&gvt->lock); >> + >> + return ret; >> +} >> + >> +/** >> + * intel_vgpu_decode_plane - Decode plane based on plane id >> + * @dev: drm device >> + * @vgpu: input vgpu >> + * This function is called for decoding plane >> + * >> + * Returns: >> + * pipe on success, NULL if failed. >> + */ >> +struct intel_vgpu_pipe_format *intel_vgpu_decode_plane(struct drm_device >*dev, >> + struct intel_vgpu *vgpu) >> +{ >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + struct intel_vgpu_fb_format fb; >> + struct intel_vgpu_pipe_format *pipe; >> + int i; >> + >> + if (intel_vgpu_decode_fb_format(dev_priv->gvt, vgpu->id, &fb)) >> + return NULL; >> + >> + for (i = 0; i < I915_MAX_PIPES; i++) >> + if (pipe_is_enabled(vgpu, i)) >> + break; >> + if (i >= I915_MAX_PIPES) { >> + gvt_dbg_core("No enabled pipes\n"); >> + return NULL; >> + } > >Looks this check should be moved in above decode_fb_format function which >doesn't check if pipe is actually enabled. Good suggestion :) > >> + pipe = &fb.pipes[i]; >> + >> + if (!pipe || !pipe->primary.enabled) { >> + gvt_dbg_core("Invalid pipe_id :%d\n", i); >> + return NULL; >> + } > >And this function is to find primary plane actually? Should name it like >intel_vgpu_decode_primary_plane(). Not exactly. This function will decode plane based on the input plane id in our case: primary plane and cursor plane. > >> + >> + return pipe; >> +} >> + >> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.h >> b/drivers/gpu/drm/i915/gvt/fb_decoder.h >> new file mode 100644 >> index 0000000..04300af >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.h >> + >> +struct intel_vgpu_pipe_format { >> + struct intel_vgpu_primary_plane_format primary; >> + struct intel_vgpu_sprite_plane_format sprite; >> + struct intel_vgpu_cursor_plane_format cursor; >> + enum DDI_PORT ddi_port; /* the DDI port that pipe is connected to >> +*/ }; >> + >> +struct intel_vgpu_fb_format { >> + struct intel_vgpu_pipe_format pipes[4]; >> +}; > >Should use MAX_PIPE definition. > >-- >Open Source Technology Center, Intel ltd. > >$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx