[PATCH v3] drm/ast: Add Atomic gamma lut support for aspeed
The current ast driver only supports legacy gamma interface. This also fixes a Gnome3/Wayland error which incorrectly adds gamma to atomic commit: "Page flip discarded: CRTC property (GAMMA_LUT) not found" I only tested remotely, so I wasn't able to check that it had an effect on the VGA output. But when activating "Night Light" in Gnome, ast_crtc_load_lut() is called. v2: use the same functions as mgag200. handle 16bits color mode. v3: Check gamma_lut size in atomic check. Signed-off-by: Jocelyn Falempe Tested-by: Thomas Zimmermann Reviewed-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_mode.c | 103 +++-- 1 file changed, 86 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 214b10178454..874b356ce37a 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -49,6 +49,8 @@ #include "ast_drv.h" #include "ast_tables.h" +#define AST_LUT_SIZE 256 + static inline void ast_load_palette_index(struct ast_private *ast, u8 index, u8 red, u8 green, u8 blue) @@ -63,20 +65,71 @@ static inline void ast_load_palette_index(struct ast_private *ast, ast_io_read8(ast, AST_IO_SEQ_PORT); } -static void ast_crtc_load_lut(struct ast_private *ast, struct drm_crtc *crtc) +static void ast_crtc_set_gamma_linear(struct ast_private *ast, + const struct drm_format_info *format) { - u16 *r, *g, *b; int i; - if (!crtc->enabled) - return; + switch (format->format) { + case DRM_FORMAT_RGB565: + /* Use better interpolation, to take 32 values from 0 to 255 */ + for (i = 0; i < AST_LUT_SIZE / 8; i++) + ast_load_palette_index(ast, + i, + i * 8 + i / 4, + i * 4 + i / 16, + i * 8 + i / 4); + /* Green has one more bit, so add padding with 0 for red and blue. */ + for (i = AST_LUT_SIZE / 8; i < AST_LUT_SIZE / 4; i++) + ast_load_palette_index(ast, i, 0, i * 4 + i / 16, 0); + break; + case DRM_FORMAT_C8: /* In this case, gamma table is used as color palette */ + case DRM_FORMAT_RGB888: + case DRM_FORMAT_XRGB: + for (i = 0; i < AST_LUT_SIZE; i++) + ast_load_palette_index(ast, i, i, i, i); + break; + default: + drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma correction\n", + &format->format); + break; + } +} - r = crtc->gamma_store; - g = r + crtc->gamma_size; - b = g + crtc->gamma_size; +static void ast_crtc_set_gamma(struct ast_private *ast, + const struct drm_format_info *format, + struct drm_color_lut *lut) +{ + int i; - for (i = 0; i < 256; i++) - ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8); + switch (format->format) { + case DRM_FORMAT_RGB565: + /* Use better interpolation, to take 32 values from lut[0] to lut[255] */ + for (i = 0; i < AST_LUT_SIZE / 8; i++) + ast_load_palette_index(ast, + i, + lut[i * 8 + i / 4].red >> 8, + lut[i * 4 + i / 16].green >> 8, + lut[i * 8 + i / 4].blue >> 8); + /* Green has one more bit, so add padding with 0 for red and blue. */ + for (i = AST_LUT_SIZE / 8; i < AST_LUT_SIZE / 4; i++) + ast_load_palette_index(ast, i, 0, lut[i * 4 + i / 16].green >> 8, 0); + break; + case DRM_FORMAT_C8: /* In this case, gamma table is used as color palette */ + case DRM_FORMAT_RGB888: + case DRM_FORMAT_XRGB: + for (i = 0; i < AST_LUT_SIZE; i++) + ast_load_palette_index(ast, + i, + lut[i].red >> 8, + lut[i].green >> 8, + lut[i].blue >> 8); + break; + default: + drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma correction\n", + &format->format); + break; + } } static bool ast_get_vbios_mode_info(const struct drm_format_info *format, @@ -1018,9 +1071,11 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
Re: [PATCH v2] drm/ast: Add Atomic gamma lut support for aspeed
On 29/09/2022 17:27, Thomas Zimmermann wrote: Hi Am 29.09.22 um 15:20 schrieb Jocelyn Falempe: The current ast driver only supports legacy gamma interface. This also fixes a Gnome3/Wayland error which incorrectly adds gamma to atomic commit: "Page flip discarded: CRTC property (GAMMA_LUT) not found" I only tested remotely, so I wasn't able to check that it had an effect on the VGA output. But when activating "Night Light" in Gnome, ast_crtc_load_lut() is called. v2: use the same functions as mgag200. handle 16bits color mode. Signed-off-by: Jocelyn Falempe That works well on my AST2300 test system. The one thing missing is the CRTC's atomic_check code. See https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/mgag200/mgag200_mode.c#n597 Thanks for the test and confirmation that it's working. I'm adding the check and will send a v3 soon. Best regards, -- Jocelyn for what to do. With this test implemented, you can add Tested-by: Thomas Zimmermann Reviewed-by: Thomas Zimmermann to the patch. Best regards Thomas --- drivers/gpu/drm/ast/ast_mode.c | 94 -- 1 file changed, 77 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 214b10178454..06ea13c3a9b4 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -49,6 +49,8 @@ #include "ast_drv.h" #include "ast_tables.h" +#define AST_LUT_SIZE 256 + static inline void ast_load_palette_index(struct ast_private *ast, u8 index, u8 red, u8 green, u8 blue) @@ -63,20 +65,71 @@ static inline void ast_load_palette_index(struct ast_private *ast, ast_io_read8(ast, AST_IO_SEQ_PORT); } -static void ast_crtc_load_lut(struct ast_private *ast, struct drm_crtc *crtc) +static void ast_crtc_set_gamma_linear(struct ast_private *ast, + const struct drm_format_info *format) { - u16 *r, *g, *b; int i; - if (!crtc->enabled) - return; + switch (format->format) { + case DRM_FORMAT_RGB565: + /* Use better interpolation, to take 32 values from 0 to 255 */ + for (i = 0; i < AST_LUT_SIZE / 8; i++) + ast_load_palette_index(ast, + i, + i * 8 + i / 4, + i * 4 + i / 16, + i * 8 + i / 4); + /* Green has one more bit, so add padding with 0 for red and blue. */ + for (i = AST_LUT_SIZE / 8; i < AST_LUT_SIZE / 4; i++) + ast_load_palette_index(ast, i, 0, i * 4 + i / 16, 0); + break; + case DRM_FORMAT_C8: /* In this case, gamma table is used as color palette */ + case DRM_FORMAT_RGB888: + case DRM_FORMAT_XRGB: + for (i = 0; i < AST_LUT_SIZE; i++) + ast_load_palette_index(ast, i, i, i, i); + break; + default: + drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma correction\n", + &format->format); + break; + } +} - r = crtc->gamma_store; - g = r + crtc->gamma_size; - b = g + crtc->gamma_size; +static void ast_crtc_set_gamma(struct ast_private *ast, + const struct drm_format_info *format, + struct drm_color_lut *lut) +{ + int i; - for (i = 0; i < 256; i++) - ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8); + switch (format->format) { + case DRM_FORMAT_RGB565: + /* Use better interpolation, to take 32 values from lut[0] to lut[255] */ + for (i = 0; i < AST_LUT_SIZE / 8; i++) + ast_load_palette_index(ast, + i, + lut[i * 8 + i / 4].red >> 8, + lut[i * 4 + i / 16].green >> 8, + lut[i * 8 + i / 4].blue >> 8); + /* Green has one more bit, so add padding with 0 for red and blue. */ + for (i = AST_LUT_SIZE / 8; i < AST_LUT_SIZE / 4; i++) + ast_load_palette_index(ast, i, 0, lut[i * 4 + i / 16].green >> 8, 0); + break; + case DRM_FORMAT_C8: /* In this case, gamma table is used as color palette */ + case DRM_FORMAT_RGB888: + case DRM_FORMAT_XRGB: + for (i = 0; i < AST_LUT_SIZE; i++) + ast_load_palette_index(ast, + i, + lut[i].red >> 8, + lut[i].green >> 8, + lut[i].blue >> 8); + break; + default: + drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma correction\n", + &format->format); + break; + } } static bool ast_get_vbios_mode_info(const struct drm_format_info *format, @@ -1018,9 +1071,11 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode) ast_set_color_reg(ast, format); ast_set_vbios_color_reg(ast, format, vbios_mode_
Re: [PATCH v2 2/2] drm/tests: Split drm_test_dp_mst_sideband_msg_req_decode into parameterized tests
On Fri, Sep 30, 2022 at 6:33 AM Michał Winiarski wrote: > > On Tue, Sep 27, 2022 at 07:12:06PM -0300, Maíra Canal wrote: > > The drm_test_dp_mst_sideband_msg_req_decode repeats the same test > > structure with different parameters. This could be better represented > > by parameterized tests, provided by KUnit. > > > > In order to convert the tests to parameterized tests, the test case for > > the client ID was changed: instead of using get_random_bytes to generate > > the client ID, the client ID is now hardcoded in the test case. > > Generally "random" usage is not incompatible with parameterized tests, we can > create parameterized tests that use random data. > The idea is to pass a function that generates the actual param (where we have > a > pointer to function as one of the members in "params" struct). > > For example, see "random_dp_query_enc_client_id" usage here: > https://lore.kernel.org/dri-devel/20220117232259.180459-7-michal.winiar...@intel.com/ > > In this case, we just compare data going in with data going out (and the data > itself is not transformed in any way), so it doesn't really matter for > coverage > and we can hardcode. > > -Michał FWIW, while the uses of randomness in DRM tests so far haven't concerned me much, I think we'll eventually want to have some way of ensuring the inputs to tests are deterministic. My thoughts are that (at some point) we'll add a kunit_random() function or similar, which will use a pseudorandom number generator which can be set to a deterministic seed before each test case. That way, there'd be a way to reproduce an error easily if it occurred. (Of course, there'd be a way of setting different or random seeds to preserve the extra coverage you'd otherwise get.) I don't think this is something worth holding up or changing existing tests at the moment, but having tests behave deterministically is definitely desirable, so +1 to avoiding get_random_bytes() if it's not giving you any real benefit. We've also had a few requests in the past for being able to pass in a custom set of parameters from userspace, which opens up some other interesting possibilities, though it's not a priority at the moment. Cheers, -- David smime.p7s Description: S/MIME Cryptographic Signature
Re: [RFC PATCH v3] media: mediatek: vcodec: support stateless AV1 decoder
Dear Daniel, Thanks for your good suggestion! I have updated v4 to fix your comment. Changes from v3: - modify comment for struct vdec_av1_slice_slot - add define SEG_LVL_ALT_Q - change use_lr/use_chroma_lr parse from av1 spec - use ARRAY_SIZE to replace size for loop_filter_level and loop_filter_mode_deltas - change array size of loop_filter_mode_deltas from 4 to 2 - add define SECONDARY_FILTER_STRENGTH_NUM_BITS - change some hex values from upper case to lower case - change *dpb_sz equal to V4L2_AV1_TOTAL_REFS_PER_FRAME + 1 - convert vb2_find_timestamp to vb2_find_buffer - test by av1 fluster, result is 173/239 detail in link: https://patchwork.kernel.org/project/linux-mediatek/patch/20220930033000.22579-1-xiaoyong...@mediatek.com/ thanks ! Xiaoyong Lu On Thu, 2022-09-22 at 13:36 -0300, Daniel Almeida wrote: > Hi Xiaoyong. > > Comments below (other code removed for brevity) > > +/** > + * struct vdec_av1_slice_slot - slot info need save in global > instance > + * @frame_info: frame info for each slot > + * @timestamp: time stamp info > + */ > +struct vdec_av1_slice_slot { > + struct vdec_av1_slice_frame_info > frame_info[AV1_MAX_FRAME_BUF_COUNT]; > + u64 timestamp[AV1_MAX_FRAME_BUF_COUNT]; > +}; > > nit: slot info that needs to be saved in the global instance > > +static int vdec_av1_slice_get_qindex(struct > vdec_av1_slice_uncompressed_header *uh, > + int segmentation_id) > +{ > + struct vdec_av1_slice_seg *seg = &uh->seg; > + struct vdec_av1_slice_quantization *quant = &uh->quant; > + int data = 0, qindex = 0; > + > + if (seg->segmentation_enabled && > + (seg->feature_enabled_mask[segmentation_id] & BIT(0))) { > + data = seg->feature_data[segmentation_id][0]; > > > Maybe you should replace the 0 above by SEG_LVL_ALT_Q to be more > explicit. Same goes for BIT(0). > > +static void vdec_av1_slice_setup_lr(struct vdec_av1_slice_lr *lr, > + struct > v4l2_av1_loop_restoration *ctrl_lr) > +{ > + int i; > + > + for (i = 0; i < V4L2_AV1_NUM_PLANES_MAX; i++) { > + lr->frame_restoration_type[i] = ctrl_lr- > >frame_restoration_type[i]; > + lr->loop_restoration_size[i] = ctrl_lr- > >loop_restoration_size[i]; > + } > + lr->use_lr = !!lr->frame_restoration_type[0]; > + lr->use_chroma_lr = !!lr->frame_restoration_type[1]; > +} > > From a first glance, this looks a bit divergent from the spec? > > for ( i = 0; i < NumPlanes; i++ ) { > lr_type > FrameRestorationType[i] = Remap_Lr_Type[lr_type] > if ( FrameRestorationType[i] != RESTORE_NONE ) { > UsesLr = 1 > if ( i > 0 ) { > usesChromaLr = 1 > } > } > } > > I will include these two variables in the next iteration of the uapi > if > computing them in the driver is problematic. > > +static void vdec_av1_slice_setup_lf(struct > vdec_av1_slice_loop_filter *lf, > + struct v4l2_av1_loop_filter > *ctrl_lf) > +{ > + int i; > + > + for (i = 0; i < 4; i++) > + lf->loop_filter_level[i] = ctrl_lf->level[i]; > + > + for (i = 0; i < V4L2_AV1_TOTAL_REFS_PER_FRAME; i++) > + lf->loop_filter_ref_deltas[i] = ctrl_lf->ref_deltas[i]; > + > + for (i = 0; i < 2; i++) > + lf->loop_filter_mode_deltas[i] = ctrl_lf- > >mode_deltas[i]; > + > + lf->loop_filter_sharpness = ctrl_lf->sharpness; > + lf->loop_filter_delta_enabled = > +BIT_FLAG(ctrl_lf, > V4L2_AV1_LOOP_FILTER_FLAG_DELTA_ENABLED); > +} > > Maybe ARRAY_SIZE can be of use in the loop indices here? > > +static void vdec_av1_slice_setup_cdef(struct vdec_av1_slice_cdef > *cdef, > + struct v4l2_av1_cdef *ctrl_cdef) > +{ > + int i; > + > + cdef->cdef_damping = ctrl_cdef->damping_minus_3 + 3; > + cdef->cdef_bits = ctrl_cdef->bits; > + > + for (i = 0; i < V4L2_AV1_CDEF_MAX; i++) { > + if (ctrl_cdef->y_sec_strength[i] == 4) > + ctrl_cdef->y_sec_strength[i] -= 1; > + > + if (ctrl_cdef->uv_sec_strength[i] == 4) > + ctrl_cdef->uv_sec_strength[i] -= 1; > + > + cdef->cdef_y_strength[i] = ctrl_cdef->y_pri_strength[i] > << 2 | > +ctrl_cdef- > >y_sec_strength[i]; > + cdef->cdef_uv_strength[i] = ctrl_cdef- > >uv_pri_strength[i] << 2 | > + ctrl_cdef- > >uv_sec_strength[i]; > + } > +} > > Maybe: > > #define SECONDARY_FILTER_STRENGTH_NUM_BITS 2 > > + cdef->cdef_y_strength[i] = ctrl_cdef->y_pri_strength[i] > << > SECONDARY_FILTER_STRENGTH_NUM_BITS | > +ctrl_cdef- > >y_sec_strength[i]; > + cdef->cdef_uv_strength[i] = ctrl_cdef- > >uv_pri_strength[i] << > SECONDARY_FILTER_STRENGTH_NUM_BITS | > +
[linux-next:master] BUILD REGRESSION 1c6c4f42b3de4f18ea96d62950d0e266ca35a055
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 1c6c4f42b3de4f18ea96d62950d0e266ca35a055 Add linux-next specific files for 20220929 Error/Warning reports: https://lore.kernel.org/linux-mm/202209150141.wgbakqmx-...@intel.com https://lore.kernel.org/llvm/202209300609.14wj5tgf-...@intel.com https://lore.kernel.org/llvm/202209300825.jcuh1oum-...@intel.com Error/Warning: (recently discovered and may have been fixed) /kbuild/src/minority/drivers/gpu/drm/msm/msm_gem_shrinker.c:29:28: error: '__GFP_ATOMIC' undeclared (first use in this function); did you mean 'GFP_ATOMIC'? ERROR: modpost: "devm_iio_channel_get_all" [drivers/power/supply/mt6370-charger.ko] undefined! ERROR: modpost: "devm_ioremap_resource" [drivers/dma/fsl-edma.ko] undefined! ERROR: modpost: "devm_ioremap_resource" [drivers/dma/idma64.ko] undefined! ERROR: modpost: "devm_ioremap_resource" [drivers/dma/qcom/hdma.ko] undefined! ERROR: modpost: "devm_memremap" [drivers/misc/open-dice.ko] undefined! ERROR: modpost: "devm_memunmap" [drivers/misc/open-dice.ko] undefined! ERROR: modpost: "devm_platform_ioremap_resource" [drivers/char/xillybus/xillybus_of.ko] undefined! ERROR: modpost: "devm_platform_ioremap_resource" [drivers/clk/xilinx/clk-xlnx-clock-wizard.ko] undefined! ERROR: modpost: "iio_read_channel_processed" [drivers/power/supply/mt6370-charger.ko] undefined! ERROR: modpost: "ioremap" [drivers/tty/ipwireless/ipwireless.ko] undefined! ERROR: modpost: "iounmap" [drivers/net/ethernet/8390/pcnet_cs.ko] undefined! ERROR: modpost: "iounmap" [drivers/tty/ipwireless/ipwireless.ko] undefined! arch/arm64/kernel/alternative.c:199:6: warning: no previous prototype for 'apply_alternatives_vdso' [-Wmissing-prototypes] arch/arm64/kernel/alternative.c:295:14: warning: no previous prototype for 'alt_cb_patch_nops' [-Wmissing-prototypes] depmod: ERROR: Cycle detected: nf_conntrack -> nf_nat -> nf_conntrack depmod: ERROR: Found 2 modules in dependency cycles! drivers/gpu/drm/msm/msm_gem_shrinker.c:29:28: error: '__GFP_ATOMIC' undeclared (first use in this function); did you mean 'GFP_ATOMIC'? drivers/pinctrl/pinctrl-amd.c:288 amd_gpio_dbg_show() warn: format string contains non-ascii character '\x9a' drivers/pinctrl/pinctrl-amd.c:288 amd_gpio_dbg_show() warn: format string contains non-ascii character '\xa1' drivers/pinctrl/pinctrl-amd.c:370 amd_gpio_dbg_show() warn: format string contains non-ascii character '\x95' grep: smatch_trinity_*: No such file or directory Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | `-- drivers-gpu-drm-msm-msm_gem_shrinker.c:error:__GFP_ATOMIC-undeclared-(first-use-in-this-function) |-- arc-allyesconfig | `-- drivers-gpu-drm-msm-msm_gem_shrinker.c:error:__GFP_ATOMIC-undeclared-(first-use-in-this-function) |-- arm-allyesconfig | `-- drivers-gpu-drm-msm-msm_gem_shrinker.c:error:__GFP_ATOMIC-undeclared-(first-use-in-this-function) |-- arm-defconfig | `-- drivers-gpu-drm-msm-msm_gem_shrinker.c:error:__GFP_ATOMIC-undeclared-(first-use-in-this-function) |-- arm64-allyesconfig | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-alt_cb_patch_nops | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-apply_alternatives_vdso | `-- drivers-gpu-drm-msm-msm_gem_shrinker.c:error:__GFP_ATOMIC-undeclared-(first-use-in-this-function) |-- arm64-buildonly-randconfig-r001-20220926 | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-alt_cb_patch_nops | `-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-apply_alternatives_vdso |-- arm64-buildonly-randconfig-r002-20220926 | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-alt_cb_patch_nops | `-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-apply_alternatives_vdso |-- arm64-randconfig-r002-20220926 | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-alt_cb_patch_nops | `-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-apply_alternatives_vdso |-- arm64-randconfig-r033-20220926 | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-alt_cb_patch_nops | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-apply_alternatives_vdso | `-- drivers-gpu-drm-msm-msm_gem_shrinker.c:error:__GFP_ATOMIC-undeclared-(first-use-in-this-function) |-- ia64-allmodconfig | `-- drivers-gpu-drm-msm-msm_gem_shrinker.c:error:__GFP_ATOMIC-undeclared-(first-use-in-this-function) |-- m68k-allmodconfig | `-- drivers-gpu-drm-msm-msm_gem_shrinker.c:error:__GFP_ATOMIC-undeclared-(first-use-in-this-function) |-- m68k-allyesconfig | `-- drivers-gpu-drm-msm-msm_gem_shrin
[PATCH -next 2/2] drm/amd/display: clean up one inconsistent indenting
clean up one inconsistent indenting Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2321 Reported-by: Abaci Robot Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c index 559e563d5bc1..f04595b750ab 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c @@ -852,7 +852,7 @@ static struct hubbub *dcn301_hubbub_create(struct dc_context *ctx) vmid->masks = &vmid_masks; } -hubbub3->num_vmid = res_cap_dcn301.num_vmid; + hubbub3->num_vmid = res_cap_dcn301.num_vmid; return &hubbub3->base; } -- 2.20.1.7.g153144c
[PATCH -next 1/2] drm/amd/display: clean up one inconsistent indenting
clean up one inconsistent indenting Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2238 Reported-by: Abaci Robot Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c b/drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c index aed0f689cbbf..b19774f4c78d 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c @@ -1656,7 +1656,7 @@ static bool dcn321_resource_construct( #undef REG_STRUCT #define REG_STRUCT dccg_regs - dccg_regs_init(); + dccg_regs_init(); ctx->dc_bios->regs = &bios_regs; -- 2.20.1.7.g153144c
[PATCH 1/3] drm/i915: Fix __gen125_emit_bb_start() without WA
ce->wa_bb_page is allocated only for graphics version 12. However __gen125_emit_bb_start() is used for any graphics version >= 12.50. For the currently supported platforms this is not an issue, but for future ones there's a mismatch causing the jump to `wa_offset + DG2_PREDICATE_RESULT_BB` to be invalid since wa_offset is not correct. As in other places in the driver, check for graphics version "greater or equal" to future-proof the support for new platforms. Cc: Matt Roper Cc: Matthew Auld Cc: Chris Wilson Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 2 ++ drivers/gpu/drm/i915/gt/intel_lrc.c | 19 +-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 31a2fbd8c4a8..e000eaf8abed 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -405,6 +405,8 @@ static int __gen125_emit_bb_start(struct i915_request *rq, u32 wa_offset = lrc_indirect_bb(ce); u32 *cs; + GEM_BUG_ON(!ce->wa_bb_page); + cs = intel_ring_begin(rq, 12); if (IS_ERR(cs)) return PTR_ERR(cs); diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index e84ef3859934..3515882a91fb 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -825,19 +825,18 @@ static int lrc_ring_cmd_buf_cctl(const struct intel_engine_cs *engine) static u32 lrc_ring_indirect_offset_default(const struct intel_engine_cs *engine) { - switch (GRAPHICS_VER(engine->i915)) { - default: - MISSING_CASE(GRAPHICS_VER(engine->i915)); - fallthrough; - case 12: + if (GRAPHICS_VER(engine->i915) >= 12) return GEN12_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT; - case 11: + else if (GRAPHICS_VER(engine->i915) >= 11) return GEN11_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT; - case 9: + else if (GRAPHICS_VER(engine->i915) >= 9) return GEN9_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT; - case 8: + else if (GRAPHICS_VER(engine->i915) >= 8) return GEN8_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT; - } + + GEM_BUG_ON(GRAPHICS_VER(engine->i915) < 8); + + return 0; } static void @@ -1092,7 +1091,7 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine) if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) context_size += I915_GTT_PAGE_SIZE; /* for redzone */ - if (GRAPHICS_VER(engine->i915) == 12) { + if (GRAPHICS_VER(engine->i915) >= 12) { ce->wa_bb_page = context_size / PAGE_SIZE; context_size += PAGE_SIZE; } -- 2.37.3
[PATCH 0/3] drm/i915: Improve register state context init
Some small improvements to future-proof the initialization around the register state context. Lucas De Marchi (3): drm/i915: Fix __gen125_emit_bb_start() without WA drm/i915/gt: Document function to decode register state context drm/i915/gt: Fix platform prefix drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 26 +-- drivers/gpu/drm/i915/gt/gen8_engine_cs.h | 12 +++--- .../drm/i915/gt/intel_execlists_submission.c | 4 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 43 ++- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- 5 files changed, 56 insertions(+), 31 deletions(-) -- 2.37.3
[PATCH 2/3] drm/i915/gt: Document function to decode register state context
It's no obviously clear how the encode/decode of the per platform tables is done. Document it so while adding tables for new platforms people can be confident they right things is being done. Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/i915/gt/intel_lrc.c | 24 1 file changed, 24 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 3515882a91fb..7771a19008c6 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -20,6 +20,30 @@ #include "intel_ring.h" #include "shmem_utils.h" +/* + * The per-platform tables are u8-encoded in @data. Decode @data and set the + * addresses' offset and commands in @regs. The following encoding is used + * for each byte. There are 2 steps: decoding commands and decoding addresses. + * + * Commands: + * [7]: create NOPs - number of NOPs are set in lower bits + * [6]: When creating MI_LOAD_REGISTER_IMM command, allow to set + * MI_LRI_FORCE_POSTED + * [5:0]: Number of NOPs or registers to set values to in case of + *MI_LOAD_REGISTER_IMM + * + * Addresses: these are decoded after a MI_LOAD_REGISTER_IMM command by "count" + * number of registers. They are set by using the REG/REG16 macros: the former + * is used for offsets smaller than 0x200 while the latter is for values bigger + * than that. Those macros already set all the bits documented below correctly: + * + * [7]: When a register offset needs more than 6 bits, use additional bytes, to + * follow, for the lower bits + * [6:0]: Register offset, without considering the engine base. + * + * This function only tweaks the commands and register offsets. Values are not + * filled out. + */ static void set_offsets(u32 *regs, const u8 *data, const struct intel_engine_cs *engine, -- 2.37.3
[PATCH 3/3] drm/i915/gt: Fix platform prefix
Different handling for XeHP and later platforms should be using the xehp prefix, not gen125. Rename them. Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 24 +-- drivers/gpu/drm/i915/gt/gen8_engine_cs.h | 12 +- .../drm/i915/gt/intel_execlists_submission.c | 4 ++-- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index e000eaf8abed..e1c76e5bfa82 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -396,10 +396,10 @@ int gen8_emit_init_breadcrumb(struct i915_request *rq) return 0; } -static int __gen125_emit_bb_start(struct i915_request *rq, - u64 offset, u32 len, - const unsigned int flags, - u32 arb) +static int __xehp_emit_bb_start(struct i915_request *rq, + u64 offset, u32 len, + const unsigned int flags, + u32 arb) { struct intel_context *ce = rq->context; u32 wa_offset = lrc_indirect_bb(ce); @@ -437,18 +437,18 @@ static int __gen125_emit_bb_start(struct i915_request *rq, return 0; } -int gen125_emit_bb_start_noarb(struct i915_request *rq, - u64 offset, u32 len, - const unsigned int flags) +int xehp_emit_bb_start_noarb(struct i915_request *rq, +u64 offset, u32 len, +const unsigned int flags) { - return __gen125_emit_bb_start(rq, offset, len, flags, MI_ARB_DISABLE); + return __xehp_emit_bb_start(rq, offset, len, flags, MI_ARB_DISABLE); } -int gen125_emit_bb_start(struct i915_request *rq, -u64 offset, u32 len, -const unsigned int flags) +int xehp_emit_bb_start(struct i915_request *rq, + u64 offset, u32 len, + const unsigned int flags) { - return __gen125_emit_bb_start(rq, offset, len, flags, MI_ARB_ENABLE); + return __xehp_emit_bb_start(rq, offset, len, flags, MI_ARB_ENABLE); } int gen8_emit_bb_start_noarb(struct i915_request *rq, diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.h b/drivers/gpu/drm/i915/gt/gen8_engine_cs.h index e4d24c811dd6..655e5c00ddc2 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.h +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.h @@ -32,12 +32,12 @@ int gen8_emit_bb_start(struct i915_request *rq, u64 offset, u32 len, const unsigned int flags); -int gen125_emit_bb_start_noarb(struct i915_request *rq, - u64 offset, u32 len, - const unsigned int flags); -int gen125_emit_bb_start(struct i915_request *rq, -u64 offset, u32 len, -const unsigned int flags); +int xehp_emit_bb_start_noarb(struct i915_request *rq, +u64 offset, u32 len, +const unsigned int flags); +int xehp_emit_bb_start(struct i915_request *rq, + u64 offset, u32 len, + const unsigned int flags); u32 *gen8_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs); u32 *gen12_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs); diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index c718e6dc40b5..0187bc72310d 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -3471,9 +3471,9 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine) if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50)) { if (intel_engine_has_preemption(engine)) - engine->emit_bb_start = gen125_emit_bb_start; + engine->emit_bb_start = xehp_emit_bb_start; else - engine->emit_bb_start = gen125_emit_bb_start_noarb; + engine->emit_bb_start = xehp_emit_bb_start_noarb; } else { if (intel_engine_has_preemption(engine)) engine->emit_bb_start = gen8_emit_bb_start; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 0ef295a94060..d81f68fef9a8 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -4094,7 +4094,7 @@ static void guc_default_vfuncs(struct intel_engine_cs *engine) engine->emit_bb_start = gen8_emit_bb_start; if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50)) - engine->emit_
Re: [Intel-gfx] [PATCH 05/16] drm/i915/vm_bind: Implement bind and unbind of object
Hi Niranjana, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-tip/drm-tip] [cannot apply to drm-intel/for-linux-next drm/drm-next drm-exynos/exynos-drm-next drm-misc/drm-misc-next linus/master v6.0-rc7 next-20220927] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Niranjana-Vishwanathapura/drm-i915-vm_bind-Add-VM_BIND-functionality/20220928-142242 base: git://anongit.freedesktop.org/drm/drm-tip drm-tip config: x86_64-randconfig-a013-20220926 compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/087e1b7e812c6983f49cdc0102baa8fcc67c48b3 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Niranjana-Vishwanathapura/drm-i915-vm_bind-Add-VM_BIND-functionality/20220928-142242 git checkout 087e1b7e812c6983f49cdc0102baa8fcc67c48b3 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All errors (new ones prefixed by >>): >> drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c:18:1: error: unused >> function 'i915_vm_bind_it_iter_next' [-Werror,-Wunused-function] INTERVAL_TREE_DEFINE(struct i915_vma, rb, u64, __subtree_last, ^ include/linux/interval_tree_generic.h:151:33: note: expanded from macro 'INTERVAL_TREE_DEFINE' ITSTATIC ITSTRUCT * \ ^ :13:1: note: expanded from here i915_vm_bind_it_iter_next ^ 1 error generated. vim +/i915_vm_bind_it_iter_next +18 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c 17 > 18 INTERVAL_TREE_DEFINE(struct i915_vma, rb, u64, __subtree_last, 19 START, LAST, static inline, i915_vm_bind_it) 20 -- 0-DAY CI Kernel Test Service https://01.org/lkp # # Automatically generated file; DO NOT EDIT. # Linux/x86_64 6.0.0-rc7 Kernel Configuration # CONFIG_CC_VERSION_TEXT="clang version 14.0.6 (git://gitmirror/llvm_project f28c006a5895fc0e329fe15fead81e37457cb1d1)" CONFIG_GCC_VERSION=0 CONFIG_CC_IS_CLANG=y CONFIG_CLANG_VERSION=140006 CONFIG_AS_IS_LLVM=y CONFIG_AS_VERSION=140006 CONFIG_LD_VERSION=0 CONFIG_LD_IS_LLD=y CONFIG_LLD_VERSION=140006 CONFIG_CC_CAN_LINK=y CONFIG_CC_CAN_LINK_STATIC=y CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y CONFIG_TOOLS_SUPPORT_RELR=y CONFIG_CC_HAS_ASM_INLINE=y CONFIG_CC_HAS_NO_PROFILE_FN_ATTR=y CONFIG_PAHOLE_VERSION=123 CONFIG_CONSTRUCTORS=y CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_TABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 # CONFIG_COMPILE_TEST is not set # CONFIG_WERROR is not set CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_BUILD_SALT="" CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y CONFIG_HAVE_KERNEL_ZSTD=y # CONFIG_KERNEL_GZIP is not set # CONFIG_KERNEL_BZIP2 is not set CONFIG_KERNEL_LZMA=y # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set # CONFIG_KERNEL_ZSTD is not set CONFIG_DEFAULT_INIT="" CONFIG_DEFAULT_HOSTNAME="(none)" CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_SYSVIPC_COMPAT=y # CONFIG_POSIX_MQUEUE is not set CONFIG_WATCH_QUEUE=y CONFIG_CROSS_MEMORY_ATTACH=y # CONFIG_USELIB is not set # CONFIG_AUDIT is not set CONFIG_HAVE_ARCH_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK=y CONFIG_GENERIC_PENDING_IRQ=y CONFIG_GENERIC_IRQ_MIGRATION=y CONFIG_HARDIRQS_SW_RESEND=y CONFIG_GENERIC_IRQ_CHIP=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_SIM=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_GENERIC_MSI_IRQ_DOMAIN=y CONFIG_IRQ_MSI_IOMMU=y CONFIG_GENERIC_IRQ_MATRIX_ALLOCATOR=y CONFIG_GENERIC_IRQ_RESERVATION_MODE=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y # CONFIG_GENERIC_IRQ_DEBUGFS is not set # end of IRQ subsystem CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_ARCH_CLOCKSOURCE_INIT=y CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y CONFIG_HAVE_POSIX_CPU_TIM
[PATCH v2] drm/bridge: ps8640: Add software to support aux defer
This chip can not handle aux defer if the host directly program its aux registers to access edid/dpcd. So we need let software to handle the aux defer situation. Signed-off-by: Jason Yen --- Changes in v2: - Add aux defer handler - Remove incorrect statements drivers/gpu/drm/bridge/parade-ps8640.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c index 31e88cb39f8a..76ada237096d 100644 --- a/drivers/gpu/drm/bridge/parade-ps8640.c +++ b/drivers/gpu/drm/bridge/parade-ps8640.c @@ -286,7 +286,6 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux, } switch (data & SWAUX_STATUS_MASK) { - /* Ignore the DEFER cases as they are already handled in hardware */ case SWAUX_STATUS_NACK: case SWAUX_STATUS_I2C_NACK: /* @@ -303,6 +302,14 @@ static ssize_t ps8640_aux_transfer_msg(struct drm_dp_aux *aux, case SWAUX_STATUS_ACKM: len = data & SWAUX_M_MASK; break; + case SWAUX_STATUS_DEFER: + case SWAUX_STATUS_I2C_DEFER: + if (is_native_aux) + msg->reply |= DP_AUX_NATIVE_REPLY_DEFER; + else + msg->reply |= DP_AUX_I2C_REPLY_DEFER; + len = data & SWAUX_M_MASK; + break; case SWAUX_STATUS_INVALID: return -EOPNOTSUPP; case SWAUX_STATUS_TIMEOUT: -- 2.34.1
[PATCH v8] drm/sched: Add FIFO sched policy to run queue
From: Andrey Grodzovsky When many entities are competing for the same run queue on the same scheduler, we observe an unusually long wait times and some jobs get starved. This has been observed on GPUVis. The issue is due to the Round Robin policy used by schedulers to pick up the next entity's job queue for execution. Under stress of many entities and long job queues within entity some jobs could be stuck for very long time in it's entity's queue before being popped from the queue and executed while for other entities with smaller job queues a job might execute earlier even though that job arrived later then the job in the long queue. Fix: Add FIFO selection policy to entities in run queue, chose next entity on run queue in such order that if job on one entity arrived earlier then job on another entity the first job will start executing earlier regardless of the length of the entity's job queue. v2: Switch to rb tree structure for entities based on TS of oldest job waiting in the job queue of an entity. Improves next entity extraction to O(1). Entity TS update O(log N) where N is the number of entities in the run-queue Drop default option in module control parameter. v3: Various cosmetical fixes and minor refactoring of fifo update function. (Luben) v4: Switch drm_sched_rq_select_entity_fifo to in order search (Luben) v5: Fix up drm_sched_rq_select_entity_fifo loop (Luben) v6: Add missing drm_sched_rq_remove_fifo_locked v7: Fix ts sampling bug and more cosmetic stuff (Luben) v8: Fix module parameter string (Luben) Cc: Luben Tuikov Cc: Christian König Cc: Direct Rendering Infrastructure - Development Cc: AMD Graphics Signed-off-by: Andrey Grodzovsky Tested-by: Yunxiang Li (Teddy) Signed-off-by: Luben Tuikov Reviewed-by: Luben Tuikov --- drivers/gpu/drm/scheduler/sched_entity.c | 20 + drivers/gpu/drm/scheduler/sched_main.c | 96 +++- include/drm/gpu_scheduler.h | 32 3 files changed, 145 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 6b25b2f4f5a308..7060e4ed5a3148 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -73,6 +73,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity, entity->priority = priority; entity->sched_list = num_sched_list > 1 ? sched_list : NULL; entity->last_scheduled = NULL; + RB_CLEAR_NODE(&entity->rb_tree_node); if(num_sched_list) entity->rq = &sched_list[0]->sched_rq[entity->priority]; @@ -443,6 +444,19 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct drm_sched_entity *entity) smp_wmb(); spsc_queue_pop(&entity->job_queue); + + /* +* Update the entity's location in the min heap according to +* the timestamp of the next job, if any. +*/ + if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) { + struct drm_sched_job *next; + + next = to_drm_sched_job(spsc_queue_peek(&entity->job_queue)); + if (next) + drm_sched_rq_update_fifo(entity, next->submit_ts); + } + return sched_job; } @@ -507,6 +521,7 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job) atomic_inc(entity->rq->sched->score); WRITE_ONCE(entity->last_user, current->group_leader); first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node); + sched_job->submit_ts = ktime_get(); /* first job wakes up scheduler */ if (first) { @@ -518,8 +533,13 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job) DRM_ERROR("Trying to push to a killed entity\n"); return; } + drm_sched_rq_add_entity(entity->rq, entity); spin_unlock(&entity->rq_lock); + + if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) + drm_sched_rq_update_fifo(entity, sched_job->submit_ts); + drm_sched_wakeup(entity->rq->sched); } } diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 4f2395d1a79182..ce86b03e838699 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -62,6 +62,55 @@ #define to_drm_sched_job(sched_job)\ container_of((sched_job), struct drm_sched_job, queue_node) +int drm_sched_policy = DRM_SCHED_POLICY_RR; + +/** + * DOC: sched_policy (int) + * Used to override default entities scheduling policy in a run queue. + */ +MODULE_PARM_DESC(sched_policy, "Specify schedule policy for entities on a runqueue, " __stringify(DRM_SCHED_POLICY_RR) " = Round Robin (default), " __stringify(DRM_SCHED_POLICY_FIFO) " = use FIFO."); +module_param_named(sched_policy, drm_sched_policy, int, 0444); + +static __always_inli
Re: [PATCH v3 4/4] drm: lcdif: Add support for YUV planes
Hi Laurent, On Thu, 2022-09-29 at 23:42 +0300, Laurent Pinchart wrote: > From: Kieran Bingham > > The LCDIF includes a color space converter that supports YUV input. Use > it to support YUV planes, either through the converter if the output > format is RGB, or in conversion bypass mode otherwise. > > Signed-off-by: Kieran Bingham > Signed-off-by: Laurent Pinchart > Reviewed-by: Marek Vasut > Reviewed-by: Kieran Bingham > --- > Changes since v2: > > - Fix YUV to RGB coefficients > - List floating point coefficient values in comment > - Express CSC coefficients with three hex digits > - Move | to the end of the line > - Add comment header before RGB formats > > Changes since v1: > > - Support all YCbCr encodings and quantization ranges > - Drop incorrect comment > --- > drivers/gpu/drm/mxsfb/lcdif_kms.c | 260 - > drivers/gpu/drm/mxsfb/lcdif_regs.h | 5 +- > 2 files changed, 224 insertions(+), 41 deletions(-) Hmmm, 'git am' fails to apply this one to drm-misc-next but it's ok with v6.0-rc1. I should have tried drm-misc-next with v2. > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c > b/drivers/gpu/drm/mxsfb/lcdif_kms.c > index 08e4880ec6cf..81454b53936f 100644 > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -32,13 +33,119 @@ > /* > - > * CRTC > */ > + > +/* > + * Despite the reference manual stating the opposite, the D1, D2 and D3 > offset > + * values are added to Y, U and V, not subtracted. They must thus be > programmed > + * with negative values. Can you add the generic conversion equations with [A-D][1-3] coefficient symbols as comments in code, like imx-pxp.c does? It will improve code readability better and tell which coefficient is which. Also for RGB to YCbCr conversion if you want to do that with this series. Sorry for not pointing this out in v2 review cycle. > + */ > +static const u32 lcdif_yuv2rgb_coeffs[3][2][6] = { [...] > + /* YUYV Formats */ 'YUV Formats' would be better? Again, failed to catch this in v2... Regards, Liu Ying > + case DRM_FORMAT_YUYV: > + writel(CTRLDESCL0_5_BPP_YCbCr422 | > CTRLDESCL0_5_YUV_FORMAT_VY2UY1, > +lcdif->base + LCDC_V8_CTRLDESCL0_5); > + in_yuv = true; > + break; > + case DRM_FORMAT_YVYU: > + writel(CTRLDESCL0_5_BPP_YCbCr422 | > CTRLDESCL0_5_YUV_FORMAT_UY2VY1, > +lcdif->base + LCDC_V8_CTRLDESCL0_5); > + in_yuv = true; > + break; > + case DRM_FORMAT_UYVY: > + writel(CTRLDESCL0_5_BPP_YCbCr422 | > CTRLDESCL0_5_YUV_FORMAT_Y2VY1U, > +lcdif->base + LCDC_V8_CTRLDESCL0_5); > + in_yuv = true; > + break; > + case DRM_FORMAT_VYUY: > + writel(CTRLDESCL0_5_BPP_YCbCr422 | > CTRLDESCL0_5_YUV_FORMAT_Y2UY1V, > +lcdif->base + LCDC_V8_CTRLDESCL0_5); > + in_yuv = true; > + break; > + > + default: > + dev_err(drm->dev, "Unknown pixel format 0x%x\n", format); > + break; > + }
[RFC PATCH v4] media: mediatek: vcodec: support stateless AV1 decoder
Add mediatek av1 decoder linux driver which use the stateless API in MT8195. Signed-off-by: Xiaoyong Lu --- Changes from v3: - modify comment for struct vdec_av1_slice_slot - add define SEG_LVL_ALT_Q - change use_lr/use_chroma_lr parse from av1 spec - use ARRAY_SIZE to replace size for loop_filter_level and loop_filter_mode_deltas - change array size of loop_filter_mode_deltas from 4 to 2 - add define SECONDARY_FILTER_STRENGTH_NUM_BITS - change some hex values from upper case to lower case - change *dpb_sz equal to V4L2_AV1_TOTAL_REFS_PER_FRAME + 1 - convert vb2_find_timestamp to vb2_find_buffer - test by av1 fluster, result is 173/239 Changes from v2: - Match with av1 uapi v3 modify - test by av1 fluster, result is 173/239 --- Reference series: [1]: v3 of this series is presend by Daniel Almeida. message-id: 20220825225312.564619-1-daniel.alme...@collabora.com .../media/platform/mediatek/vcodec/Makefile |1 + .../vcodec/mtk_vcodec_dec_stateless.c | 47 +- .../platform/mediatek/vcodec/mtk_vcodec_drv.h |1 + .../vcodec/vdec/vdec_av1_req_lat_if.c | 2235 + .../platform/mediatek/vcodec/vdec_drv_if.c|4 + .../platform/mediatek/vcodec/vdec_drv_if.h|1 + .../platform/mediatek/vcodec/vdec_msg_queue.c | 27 + .../platform/mediatek/vcodec/vdec_msg_queue.h |4 + 8 files changed, 2319 insertions(+), 1 deletion(-) create mode 100644 drivers/media/platform/mediatek/vcodec/vdec/vdec_av1_req_lat_if.c diff --git a/drivers/media/platform/mediatek/vcodec/Makefile b/drivers/media/platform/mediatek/vcodec/Makefile index 93e7a343b5b0e..7537259130072 100644 --- a/drivers/media/platform/mediatek/vcodec/Makefile +++ b/drivers/media/platform/mediatek/vcodec/Makefile @@ -10,6 +10,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \ vdec/vdec_vp8_req_if.o \ vdec/vdec_vp9_if.o \ vdec/vdec_vp9_req_lat_if.o \ + vdec/vdec_av1_req_lat_if.o \ vdec/vdec_h264_req_if.o \ vdec/vdec_h264_req_common.o \ vdec/vdec_h264_req_multi_if.o \ diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c index c45bd2599bb2d..802b2f046ffc2 100644 --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c @@ -107,11 +107,51 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = { }, .codec_type = V4L2_PIX_FMT_VP9_FRAME, }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_SEQUENCE, + + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_FRAME, + + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_TILE_GROUP_ENTRY, + .dims = { V4L2_AV1_MAX_TILE_COUNT }, + + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_PROFILE, + .min = V4L2_STATELESS_AV1_PROFILE_MAIN, + .def = V4L2_STATELESS_AV1_PROFILE_MAIN, + .max = V4L2_STATELESS_AV1_PROFILE_PROFESSIONAL, + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, + { + .cfg = { + .id = V4L2_CID_STATELESS_AV1_LEVEL, + .min = V4L2_STATELESS_AV1_LEVEL_2_0, + .def = V4L2_STATELESS_AV1_LEVEL_2_0, + .max = V4L2_STATELESS_AV1_LEVEL_7_3, + }, + .codec_type = V4L2_PIX_FMT_AV1_FRAME, + }, }; #define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls) -static struct mtk_video_fmt mtk_video_formats[5]; +static struct mtk_video_fmt mtk_video_formats[6]; static struct mtk_video_fmt default_out_format; static struct mtk_video_fmt default_cap_format; @@ -351,6 +391,7 @@ static void mtk_vcodec_add_formats(unsigned int fourcc, case V4L2_PIX_FMT_H264_SLICE: case V4L2_PIX_FMT_VP8_FRAME: case V4L2_PIX_FMT_VP9_FRAME: + case V4L2_PIX_FMT_AV1_FRAME: mtk_video_formats[count_formats].fourcc = fourcc; mtk_video_formats[count_formats].type = MTK_FMT_DEC; mtk_video_formats[count_formats].num_planes = 1; @@ -407,6 +448,10 @@ static void mtk_vcodec_get_supported_formats(struct mtk_vcodec_ctx *ctx) mtk_vcodec_add_formats(V4L2_PIX_FMT_VP9_FRAME, ctx); out_format_count++; } + if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_AV1_FRAME) { + mtk_vcodec_add_formats(V4L
RE: [PATCH 02/13] drm/scheduler: add drm_sched_job_add_resv_dependencies
[AMD Official Use Only - General] Hi Christian This looks on kernel revision 5.15, currently, the Zork use 5.4 Google also comment that kernel 5.15 fix the issue. I'm not sure the kernel have rev plan to 5.15 or not We will discuss this on 10/3. Do you suggest that use kernel 5.15 and merge patch to test ? Jay -Original Message- From: Christian König Sent: Thursday, September 29, 2022 9:21 PM To: dri-devel@lists.freedesktop.org Cc: Grodzovsky, Andrey ; Chien, WenChieh (Jay) ; Wang, Chester ; Tuikov, Luben ; Koenig, Christian Subject: [PATCH 02/13] drm/scheduler: add drm_sched_job_add_resv_dependencies Add a new function to update job dependencies from a resv obj. Signed-off-by: Christian König --- drivers/gpu/drm/scheduler/sched_main.c | 49 ++ include/drm/gpu_scheduler.h| 5 +++ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index e0ab14e0fb6b..6e2cd0f906b2 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -685,32 +685,28 @@ int drm_sched_job_add_dependency(struct drm_sched_job *job, EXPORT_SYMBOL(drm_sched_job_add_dependency); /** - * drm_sched_job_add_implicit_dependencies - adds implicit dependencies as job - * dependencies + * drm_sched_job_add_resv_dependencies - add all fences from the resv + to the job * @job: scheduler job to add the dependencies to - * @obj: the gem object to add new dependencies from. - * @write: whether the job might write the object (so we need to depend on - * shared fences in the reservation object). + * @resv: the dma_resv object to get the fences from + * @usage: the dma_resv_usage to use to filter the fences * - * This should be called after drm_gem_lock_reservations() on your array of - * GEM objects used in the job but before updating the reservations with your - * own fences. + * This adds all fences matching the given usage from @resv to @job. + * Must be called with the @resv lock held. * * Returns: * 0 on success, or an error on failing to expand the array. */ -int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, - struct drm_gem_object *obj, - bool write) +int drm_sched_job_add_resv_dependencies(struct drm_sched_job *job, + struct dma_resv *resv, + enum dma_resv_usage usage) { struct dma_resv_iter cursor; struct dma_fence *fence; int ret; - dma_resv_assert_held(obj->resv); + dma_resv_assert_held(resv); - dma_resv_for_each_fence(&cursor, obj->resv, dma_resv_usage_rw(write), - fence) { + dma_resv_for_each_fence(&cursor, resv, usage, fence) { /* Make sure to grab an additional ref on the added fence */ dma_fence_get(fence); ret = drm_sched_job_add_dependency(job, fence); @@ -721,8 +717,31 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, } return 0; } -EXPORT_SYMBOL(drm_sched_job_add_implicit_dependencies); +EXPORT_SYMBOL(drm_sched_job_add_resv_dependencies); +/** + * drm_sched_job_add_implicit_dependencies - adds implicit dependencies as job + * dependencies + * @job: scheduler job to add the dependencies to + * @obj: the gem object to add new dependencies from. + * @write: whether the job might write the object (so we need to depend +on + * shared fences in the reservation object). + * + * This should be called after drm_gem_lock_reservations() on your +array of + * GEM objects used in the job but before updating the reservations +with your + * own fences. + * + * Returns: + * 0 on success, or an error on failing to expand the array. + */ +int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, + struct drm_gem_object *obj, + bool write) +{ + return drm_sched_job_add_resv_dependencies(job, obj->resv, + dma_resv_usage_rw(write)); +} +EXPORT_SYMBOL(drm_sched_job_add_implicit_dependencies); /** * drm_sched_job_cleanup - clean up scheduler job resources diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..3315e5be7791 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -32,6 +32,8 @@ #define MAX_WAIT_SCHED_ENTITY_Q_EMPTY msecs_to_jiffies(1000) +enum dma_resv_usage; +struct dma_resv; struct drm_gem_object; struct drm_gpu_scheduler; @@ -474,6 +476,9 @@ int drm_sched_job_init(struct drm_sched_job *job, void drm_sched_job_arm(struct drm_sched_job *job); int drm_sched_job_add_dependency(struct drm_sched_job *job, struct dma_f
RE: [PATCH v4 15/15] vfio: Add struct device to vfio_device
> From: Alex Williamson > Sent: Friday, September 30, 2022 2:24 AM > > On Thu, 29 Sep 2022 14:49:56 -0300 > Jason Gunthorpe wrote: > > > On Thu, Sep 29, 2022 at 10:55:19AM -0600, Alex Williamson wrote: > > > Hi Kevin, > > > > > > This introduced the regression discovered here: > > > > > > > https://lore.kernel.org/all/20220928125650.0a2ea297.alex.williamson@redh > at.com/ > > > > > > Seems we're not releasing the resources when removing an mdev. This is > > > a regression, so it needs to be fixed or reverted before the merge > > > window. Thanks, > > > > My guess at the fix for this: > > > > https://lore.kernel.org/r/0-v1-013609965fe8+9d- > vfio_gvt_unregister_...@nvidia.com > > Indeed this seems to work I'll look for acks and further reviews from > Intel folks. Thanks! > Thanks Jason for baking a quick fix! I've acked it.
Re: [PATCH 2/7] mm: Free device private pages have zero refcount
Alistair Popple wrote: > > Dan Williams writes: > > > Alistair Popple wrote: > >> > >> Jason Gunthorpe writes: > >> > >> > On Mon, Sep 26, 2022 at 04:03:06PM +1000, Alistair Popple wrote: > >> >> Since 27674ef6c73f ("mm: remove the extra ZONE_DEVICE struct page > >> >> refcount") device private pages have no longer had an extra reference > >> >> count when the page is in use. However before handing them back to the > >> >> owning device driver we add an extra reference count such that free > >> >> pages have a reference count of one. > >> >> > >> >> This makes it difficult to tell if a page is free or not because both > >> >> free and in use pages will have a non-zero refcount. Instead we should > >> >> return pages to the drivers page allocator with a zero reference count. > >> >> Kernel code can then safely use kernel functions such as > >> >> get_page_unless_zero(). > >> >> > >> >> Signed-off-by: Alistair Popple > >> >> --- > >> >> arch/powerpc/kvm/book3s_hv_uvmem.c | 1 + > >> >> drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 1 + > >> >> drivers/gpu/drm/nouveau/nouveau_dmem.c | 1 + > >> >> lib/test_hmm.c | 1 + > >> >> mm/memremap.c| 5 - > >> >> mm/page_alloc.c | 6 ++ > >> >> 6 files changed, 10 insertions(+), 5 deletions(-) > >> > > >> > I think this is a great idea, but I'm surprised no dax stuff is > >> > touched here? > >> > >> free_zone_device_page() shouldn't be called for pgmap->type == > >> MEMORY_DEVICE_FS_DAX so I don't think we should have to worry about DAX > >> there. Except that the folio code looks like it might have introduced a > >> bug. AFAICT put_page() always calls > >> put_devmap_managed_page(&folio->page) but folio_put() does not (although > >> folios_put() does!). So it seems folio_put() won't end up calling > >> __put_devmap_managed_page_refs() as I think it should. > >> > >> I think you're right about the change to __init_zone_device_page() - I > >> should limit it to DEVICE_PRIVATE/COHERENT pages only. But I need to > >> look at Dan's patch series more closely as I suspect it might be better > >> to rebase this patch on top of that. > > > > Apologies for the delay I was travelling the past few days. Yes, I think > > this patch slots in nicely to avoid the introduction of an init_mode > > [1]: > > > > https://lore.kernel.org/nvdimm/166329940343.2786261.6047770378829215962.st...@dwillia2-xfh.jf.intel.com/ > > > > Mind if I steal it into my series? > > No problem, although I notice Andrew has already merged it into > mm-unstable. If you end up rebasing your series on top of mine I think > all that's needed is a patch somewhere in your series to drop the > various `if (pgmap->type == MEMORY_DEVICE_*)` I added to (hopefully) > avoid breaking DAX. Assuming DAX takes a pagemap reference on struct > page allocation something like below. Yeah, I'll go that route and rebase on top of -mm. Thanks again.
Re: [Freedreno] [PATCH -next] drm/msm/msm_gem_shrinker: fix compile error in can_block()
Hi, On 2022/9/30 4:38, Rob Clark wrote: On Thu, Sep 29, 2022 at 4:51 AM Akhil P Oommen wrote: On 9/29/2022 3:00 PM, Yang Yingliang wrote: I got the compile error: drivers/gpu/drm/msm/msm_gem_shrinker.c: In function ‘can_block’: drivers/gpu/drm/msm/msm_gem_shrinker.c:29:21: error: ‘__GFP_ATOMIC’ undeclared (first use in this function); did you mean ‘GFP_ATOMIC’? if (sc->gfp_mask & __GFP_ATOMIC) ^~~~ GFP_ATOMIC drivers/gpu/drm/msm/msm_gem_shrinker.c:29:21: note: each undeclared identifier is reported only once for each function it appears in __GFP_ATOMIC is dropped by commit 6708fe6bec50 ("mm: discard __GFP_ATOMIC"). Use __GFP_HIGH instead. Fixes: 025d27239a2f ("drm/msm/gem: Evict active GEM objects when necessary") Signed-off-by: Yang Yingliang --- drivers/gpu/drm/msm/msm_gem_shrinker.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c b/drivers/gpu/drm/msm/msm_gem_shrinker.c index 58e0513be5f4..6a0de6cdb82b 100644 --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c @@ -26,7 +26,7 @@ static bool can_swap(void) static bool can_block(struct shrink_control *sc) { - if (sc->gfp_mask & __GFP_ATOMIC) + if (sc->gfp_mask & __GFP_HIGH) return false; return current_is_kswapd() || (sc->gfp_mask & __GFP_RECLAIM); } Reviewed-by: Akhil P Oommen Somehow the original patch didn't show up in my inbox, but I've sent this: https://patchwork.freedesktop.org/series/109255/ When __GFP_ATOMIC is not dropped, if __GFP_KSWAPD_RECLAIM is set, it allows sleep(can_block() returns true). In your patch case, if __GFP_KSWAPD_RECLAIM is set but __GFP_DIRECT_RECLAIM is not set, it don't allows sleep(can_blcok() returns false). It's different from earlier behavior. Thanks, Yang I guess __GFP_HIGH could also be used to detect GFP_ATOMIC, but checking that direct reclaim is ok seems safer (ie. it should always be safe to sleep in that case) BR, -R -Akhil. .
RE: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
> From: Jason Gunthorpe > Sent: Friday, September 30, 2022 1:49 AM > > When converting to directly create the vfio_device the mdev driver has to > put a vfio_register_emulated_iommu_dev() in the probe() and a pairing > vfio_unregister_group_dev() in the remove. > > This was missed for gvt, add it. > > Cc: sta...@vger.kernel.org > Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use > vfio_register_emulated_iommu_dev") > Reported-by: Alex Williamson > Signed-off-by: Jason Gunthorpe Reviewed-by: Kevin Tian
Re: [PATCH v5 1/3] drm/msm/dp: cleared DP_DOWNSPREAD_CTRL register before start link training
On 9/12/2022 12:25 PM, Dmitry Baryshkov wrote: On 12/09/2022 22:21, Kuogee Hsieh wrote: On 9/12/2022 11:39 AM, Dmitry Baryshkov wrote: On 12/09/2022 19:23, Kuogee Hsieh wrote: DOWNSPREAD_CTRL (0x107) shall be cleared to 0 upon power-on reset or an upstream device disconnect. This patch will enforce this rule by always cleared DOWNSPREAD_CTRL register to 0 before start link training. At rare case that DP MSA timing parameters may be mis-interpreted by the sink which causes audio sampling rate be calculated wrongly and cause audio did not work at sink if DOWNSPREAD_CTRL register is not cleared to 0. Changes in v2: 1) fix spelling at commit text 2) merge ssc variable into encoding[0] Changes in v3: -- correct spelling of DOWNSPREAD_CTRL -- replace err with len of ssize_t Changes in v4: -- split into 2 patches Fixes: 154b5a7da0fd ("drm/msm/dp: add displayPort driver support") Fixes tag is wrong here. It should be: Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support") Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/dp/dp_ctrl.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index ab6aa13..2c74c59 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1245,8 +1245,7 @@ static int dp_ctrl_link_train(struct dp_ctrl_private *ctrl, { int ret = 0; const u8 *dpcd = ctrl->panel->dpcd; - u8 encoding = DP_SET_ANSI_8B10B; - u8 ssc; + u8 encoding[] = { 0, DP_SET_ANSI_8B10B }; u8 assr; struct dp_link_info link_info = {0}; @@ -1258,13 +1257,11 @@ static int dp_ctrl_link_train(struct dp_ctrl_private *ctrl, dp_aux_link_configure(ctrl->aux, &link_info); - if (drm_dp_max_downspread(dpcd)) { - ssc = DP_SPREAD_AMP_0_5; - drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, &ssc, 1); - } + if (drm_dp_max_downspread(dpcd)) + encoding[0] |= DP_SPREAD_AMP_0_5; It would be simpler to call drm_dp_dpcd_write(ssc, DP_DOWNSPREAD_CTRL, 1) unconditionally here. You won't have to change the encoding/DP_MAIN_LINK_CHANNEL_CODING_SET/etc. The difference is one write with 2 bytes against two writes with one byte each. I think it is more efficient to combine two bytes into one write since these two bytes are consecutive address. I probably wouldn't do so, nevertheless: Reviewed-by: Dmitry Baryshkov - drm_dp_dpcd_write(ctrl->aux, DP_MAIN_LINK_CHANNEL_CODING_SET, - &encoding, 1); + /* config DOWNSPREAD_CTRL and MAIN_LINK_CHANNEL_CODING_SET */ + drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, encoding, 2); if (drm_dp_alternate_scrambler_reset_cap(dpcd)) { assr = DP_ALTERNATE_SCRAMBLER_RESET_ENABLE;
Re: [PATCH v2 2/2] drm/panel: simple: Use dev_err_probe() to simplify code
在 2022/9/30 5:08, Doug Anderson 写道: Hi, On Wed, Sep 28, 2022 at 6:57 PM Yuan Can wrote: In the probe path, dev_err() can be replaced with dev_err_probe() which will check if error code is -EPROBE_DEFER and prints the error name. It also sets the defer probe reason which can be checked later through debugfs. Signed-off-by: Yuan Can Reviewed-by: Douglas Anderson --- drivers/gpu/drm/panel/panel-simple.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) Pushed to drm-misc-next: c9b48b91e2fb drm/panel: simple: Use dev_err_probe() to simplify code For the other patches in your original series, I'll assume folks paying more attention to those panel drivers will commit them. If they totally stall for a while, though, then yell and I can try taking a look at them. :-) That's great, Thanks!😁 -- Best regards, Yuan Can
Re: [PATCH 2/7] mm: Free device private pages have zero refcount
Dan Williams writes: > Alistair Popple wrote: >> >> Jason Gunthorpe writes: >> >> > On Mon, Sep 26, 2022 at 04:03:06PM +1000, Alistair Popple wrote: >> >> Since 27674ef6c73f ("mm: remove the extra ZONE_DEVICE struct page >> >> refcount") device private pages have no longer had an extra reference >> >> count when the page is in use. However before handing them back to the >> >> owning device driver we add an extra reference count such that free >> >> pages have a reference count of one. >> >> >> >> This makes it difficult to tell if a page is free or not because both >> >> free and in use pages will have a non-zero refcount. Instead we should >> >> return pages to the drivers page allocator with a zero reference count. >> >> Kernel code can then safely use kernel functions such as >> >> get_page_unless_zero(). >> >> >> >> Signed-off-by: Alistair Popple >> >> --- >> >> arch/powerpc/kvm/book3s_hv_uvmem.c | 1 + >> >> drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 1 + >> >> drivers/gpu/drm/nouveau/nouveau_dmem.c | 1 + >> >> lib/test_hmm.c | 1 + >> >> mm/memremap.c| 5 - >> >> mm/page_alloc.c | 6 ++ >> >> 6 files changed, 10 insertions(+), 5 deletions(-) >> > >> > I think this is a great idea, but I'm surprised no dax stuff is >> > touched here? >> >> free_zone_device_page() shouldn't be called for pgmap->type == >> MEMORY_DEVICE_FS_DAX so I don't think we should have to worry about DAX >> there. Except that the folio code looks like it might have introduced a >> bug. AFAICT put_page() always calls >> put_devmap_managed_page(&folio->page) but folio_put() does not (although >> folios_put() does!). So it seems folio_put() won't end up calling >> __put_devmap_managed_page_refs() as I think it should. >> >> I think you're right about the change to __init_zone_device_page() - I >> should limit it to DEVICE_PRIVATE/COHERENT pages only. But I need to >> look at Dan's patch series more closely as I suspect it might be better >> to rebase this patch on top of that. > > Apologies for the delay I was travelling the past few days. Yes, I think > this patch slots in nicely to avoid the introduction of an init_mode > [1]: > > https://lore.kernel.org/nvdimm/166329940343.2786261.6047770378829215962.st...@dwillia2-xfh.jf.intel.com/ > > Mind if I steal it into my series? No problem, although I notice Andrew has already merged it into mm-unstable. If you end up rebasing your series on top of mine I think all that's needed is a patch somewhere in your series to drop the various `if (pgmap->type == MEMORY_DEVICE_*)` I added to (hopefully) avoid breaking DAX. Assuming DAX takes a pagemap reference on struct page allocation something like below. --- diff --git a/mm/memremap.c b/mm/memremap.c index 421bec3a29ee..da1a0e0abb8b 100644 --- a/mm/memremap.c +++ b/mm/memremap.c @@ -507,15 +507,7 @@ void free_zone_device_page(struct page *page) page->mapping = NULL; page->pgmap->ops->page_free(page); - if (page->pgmap->type != MEMORY_DEVICE_PRIVATE && - page->pgmap->type != MEMORY_DEVICE_COHERENT) - /* -* Reset the page count to 1 to prepare for handing out the page -* again. -*/ - set_page_count(page, 1); - else - put_dev_pagemap(page->pgmap); + put_dev_pagemap(page->pgmap); } void zone_device_page_init(struct page *page) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 014dbdf54d62..3e5ff06700ca 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -6816,9 +6816,7 @@ static void __ref __init_zone_device_page(struct page *page, unsigned long pfn, * ZONE_DEVICE pages are released directly to the driver page allocator * which will set the page count to 1 when allocating the page. */ - if (pgmap->type == MEMORY_DEVICE_PRIVATE || - pgmap->type == MEMORY_DEVICE_COHERENT) - set_page_count(page, 0); + set_page_count(page, 0); } /*
[git pull] drm fixes for 6.0 final
Hi Linus, Last set of fixes for 6.0 hopefully, minor bridge fixes, i915 fixes, and a bunch of amdgpu fixes for new IP blocks, along with a couple of regression fixes. Hopefully all set for merge window next week. Dave. drm-fixes-2022-09-30-1: drm fixes for 6.0 final amdgpu: - GC 11.x fixes - SMU 13.x fixes - DCN 3.1.4 fixes - DCN 3.2.x fixes - GC 9.x fix - Fence fix - SR-IOV supend/resume fix - PSR regression fix i915: - Restrict forced preemption to the active context - Restrict perf_limit_reasons to the supported platforms - gen11+ bridge: - analogix: Revert earlier suspend fix - lt8912b: Fix corrupt display output The following changes since commit f76349cf41451c5c42a99f18a9163377e4b364ff: Linux 6.0-rc7 (2022-09-25 14:01:02 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-09-30-1 for you to fetch changes up to 6643b3836f3908c4f77883b2fae72451e85cf3ca: Merge tag 'drm-intel-fixes-2022-09-29' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2022-09-30 09:28:58 +1000) drm fixes for 6.0 final amdgpu: - GC 11.x fixes - SMU 13.x fixes - DCN 3.1.4 fixes - DCN 3.2.x fixes - GC 9.x fix - Fence fix - SR-IOV supend/resume fix - PSR regression fix i915: - Restrict forced preemption to the active context - Restrict perf_limit_reasons to the supported platforms - gen11+ bridge: - analogix: Revert earlier suspend fix - lt8912b: Fix corrupt display output Alvin Lee (1): drm/amd/display: Update DCN32 to use new SR latencies Aric Cyr (1): drm/amd/display: Fix audio on display after unplugging another Ashutosh Dixit (1): drm/i915/gt: Perf_limit_reasons are only available for Gen11+ Bokun Zhang (1): drm/amdgpu: Add amdgpu suspend-resume code path under SRIOV Brian Norris (1): Revert "drm: bridge: analogix/dp: add panel prepare/unprepare in suspend/resume time" Chris Wilson (1): drm/i915/gt: Restrict forced preemption to the active context Dave Airlie (3): Merge tag 'drm-misc-fixes-2022-09-29' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'amd-drm-fixes-6.0-2022-09-29' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Merge tag 'drm-intel-fixes-2022-09-29' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Eric Bernstein (1): drm/amd/display: Remove assert for odm transition case Evan Quan (3): drm/amdgpu: avoid gfx register accessing during gfxoff drm/amd/pm: enable gfxoff feature for SMU 13.0.0 drm/amd/pm: use adverse selection for dpm features unsupported by driver Francesco Dolcini (1): drm/bridge: lt8912b: fix corrupted image output Graham Sider (3): drm/amdkfd: fix MQD init for GFX11 in init_mqd drm/amdgpu: pass queue size and is_aql_queue to MES drm/amdkfd: fix dropped interrupt in kfd_int_process_v11 Jiadong.Zhu (2): drm/amdgpu: Correct the position in patch_cond_exec drm/amdgpu: Remove fence_process in count_emitted Leo Li (1): drm/amd/display: Prevent OTG shutdown during PSR SU Nicholas Kazlauskas (3): drm/amd/display: Do DIO FIFO enable after DP video stream enable drm/amd/display: Wrap OTG disable workaround with FIFO control drm/amd/display: Add explicit FIFO disable for DP blank Philippe Schenker (2): drm/bridge: lt8912b: add vsync hsync drm/bridge: lt8912b: set hdmi or dvi mode Samson Tam (1): drm/amd/display: fill in clock values when DPM is not enabled Taimur Hassan (3): drm/amd/display: Avoid avoid unnecessary pixel rate divider programming drm/amd/display: Fix typo in get_pixel_rate_div drm/amd/display: Avoid unnecessary pixel rate divider programming drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 4 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 27 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h| 2 + drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 4 + drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 4 + .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 + drivers/gpu/drm/amd/amdkfd/kfd_int_process_v11.c | 6 +- drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c | 4 + .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c | 8 +- .../amd/display/dc/clk_mgr/dcn314/dcn314_clk_mgr.c | 11 ++- .../amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c | 14 .../amd/display/dc/dce110/dce110_hw_sequencer.c| 6 +- .../gpu/drm/amd/display/dc/dcn314/dcn314_dccg.c| 47 .../display/dc/dcn314/dcn314_dio_stream_encoder.c | 25 -- drivers/gpu/drm/amd/display/dc/dcn32/dcn32_dccg.c | 53 + .../gpu/drm/amd/display/dc/dcn32/dcn32_hubbub.c| 10 ++- .../gpu/drm/amd/d
Re: [PATCH v4.1] drm/i915/mtl: Define engine context layouts
On Wed, Sep 28, 2022 at 08:55:11AM -0700, Radhakrishna Sripada wrote: From: Matt Roper The part of the media and blitter engine contexts that we care about for setting up an initial state on MTL are nearly similar to DG2 (and PVC). The difference being PRT_BB_STATE being replaced with NOP. For render/compute engines, the part of the context images are nearly the same, although the layout had a very slight change --- one POSH register was removed and the placement of some LRI/noops adjusted slightly to compensate. v2: - Dg2, mtl xcs offsets slightly vary. Use a separate offsets array(Bala) - Add missing nop in xcs offsets(Bala) v3: - Fix the spacing for nop in xcs offset(MattR) v4: - Fix rcs register offset(MattR) v4.1: - Fix commit message(Lucas) Bspec: 46261, 46260, 45585 Cc: Balasubramani Vivekanandan Cc: Licas De Marchi Signed-off-by: Matt Roper Signed-off-by: Radhakrishna Sripada Reviewed-by: Lucas De Marchi Lucas De Marchi
Re: [PATCH] drm: do not call detect for connectors which are forced on
Hello Michael, Thank you for the patch. Sorry for the late reply, I wasn't on the CC list so I didn't notice it. On Fri, Aug 26, 2022 at 11:11:21AM +0200, Michael Rodin wrote: > "detect" should not be called and its return value shall not be used when a > connector is forced as hinted in the commit d50ba256b5f1 ("drm/kms: start > adding command line interface using fb.") and the commit 6fe14acd496e > ("drm: Document drm_connector_funcs"). One negative side effect of doing > this is observed on the RCar3 SoCs which use the dw-hdmi driver. It > continues executing drm_helper_hpd_irq_event even if its connector is > forced to ON. As consequence drm_helper_hpd_irq_event calls "detect" so the > connector status is updated to "disconnected": > > [ 420.201527] [drm:drm_helper_hpd_irq_event] [CONNECTOR:76:HDMI-A-1] status > updated from connected to disconnected > > This status is corrected by drm_helper_probe_single_connector_modes shortly > after this because this function checks if a connector is forced: > > [ 420.218703] [drm:drm_helper_probe_single_connector_modes] > [CONNECTOR:76:HDMI-A-1] status updated from disconnected to connected > > To avoid similar issues this commit adapts functions which call "detect" > so they check if a connector is forced and return the correct status. > > Fixes: 949f08862d66 ("drm: Make the connector .detect() callback optional") Is this really the right fixes tag ? The call to .detect() in drm_helper_hpd_irq_event() predates that commit. > Signed-off-by: Michael Rodin > --- > drivers/gpu/drm/drm_probe_helper.c | 16 ++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > b/drivers/gpu/drm/drm_probe_helper.c > index bb427c5a4f1f..1691047d0472 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -289,7 +289,12 @@ drm_helper_probe_detect_ctx(struct drm_connector > *connector, bool force) > retry: > ret = drm_modeset_lock(&connector->dev->mode_config.connection_mutex, > &ctx); > if (!ret) { > - if (funcs->detect_ctx) > + if (connector->force == DRM_FORCE_ON || > + connector->force == DRM_FORCE_ON_DIGITAL) > + ret = connector_status_connected; > + else if (connector->force == DRM_FORCE_OFF) > + ret = connector_status_disconnected; > + else if (funcs->detect_ctx) > ret = funcs->detect_ctx(connector, &ctx, force); > else if (connector->funcs->detect) > ret = connector->funcs->detect(connector, force); > @@ -340,7 +345,14 @@ drm_helper_probe_detect(struct drm_connector *connector, > if (ret) > return ret; > > - if (funcs->detect_ctx) > + if (connector->force == DRM_FORCE_ON || > + connector->force == DRM_FORCE_ON_DIGITAL) > + ret = connector_status_connected; > + else if (connector->force == DRM_FORCE_OFF) > + ret = connector_status_disconnected; > + else if (funcs->detect_ctx) > + ret = funcs->detect_ctx(connector, ctx, force); > + else if (funcs->detect_ctx) > ret = funcs->detect_ctx(connector, ctx, force); Those two conditions are identical. > else if (connector->funcs->detect) > ret = connector->funcs->detect(connector, force); The same logic is used in two places in this patch. Could this be factored out to a separate function ? It may even be possible to refactor drm_helper_probe_detect() and drm_helper_probe_detect_ctx() to share more code between the two functions. This being said, I'm not sure this is the right fix. Beside the i915 and nouveau drivers, the only callers of drm_helper_probe_detect() are drm_helper_probe_single_connector_modes(), output_poll_execute() and check_connector_changed() in this file. The first two functions already check connector->force and skip the call to drm_helper_probe_detect() if the connector is forced. Only check_connector_changed() is missing that check. Wouldn't it be simpler to return false in that function if connector->force is set ? Another question is whether it is valid for the dw-hdmi driver to call drm_helper_hpd_irq_event() when the connector status is forced. Shouldn't HPD events be ignored in that case ? The detection code has grown quite complex over time, I would really appreciate input from Maxime and Maarten on this. -- Regards, Laurent Pinchart
Re: [Intel-gfx] [PATCH] drm/i915: Fix CFI violations in gt_sysfs
On Fri, Sep 30, 2022 at 12:34:41AM +0200, Andrzej Hajda wrote: > On 22.09.2022 21:51, Nathan Chancellor wrote: > > When booting with clang's kernel control flow integrity series [1], > > there are numerous violations when accessing the files under > > /sys/devices/pci:00/:00:02.0/drm/card0/gt/gt0: > > > >$ cd /sys/devices/pci:00/:00:02.0/drm/card0/gt/gt0 > > > >$ grep . * > >id:0 > >punit_req_freq_mhz:350 > >rc6_enable:1 > >rc6_residency_ms:214934 > >rps_act_freq_mhz:1300 > >rps_boost_freq_mhz:1300 > >rps_cur_freq_mhz:350 > >rps_max_freq_mhz:1300 > >rps_min_freq_mhz:350 > >rps_RP0_freq_mhz:1300 > >rps_RP1_freq_mhz:350 > >rps_RPn_freq_mhz:350 > >throttle_reason_pl1:0 > >throttle_reason_pl2:0 > >throttle_reason_pl4:0 > >throttle_reason_prochot:0 > >throttle_reason_ratl:0 > >throttle_reason_status:0 > >throttle_reason_thermal:0 > >throttle_reason_vr_tdc:0 > >throttle_reason_vr_thermalert:0 > > > >$ sudo dmesg &| grep "CFI failure at" > >[ 214.595903] CFI failure at kobj_attr_show+0x19/0x30 (target: > > id_show+0x0/0x70 [i915]; expected type: 0xc527b809) > >[ 214.596064] CFI failure at kobj_attr_show+0x19/0x30 (target: > > punit_req_freq_mhz_show+0x0/0x40 [i915]; expected type: 0xc527b809) > >[ 214.596407] CFI failure at kobj_attr_show+0x19/0x30 (target: > > rc6_enable_show+0x0/0x40 [i915]; expected type: 0xc527b809) > >[ 214.596528] CFI failure at kobj_attr_show+0x19/0x30 (target: > > rc6_residency_ms_show+0x0/0x270 [i915]; expected type: 0xc527b809) > >[ 214.596682] CFI failure at kobj_attr_show+0x19/0x30 (target: > > act_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) > >[ 214.596792] CFI failure at kobj_attr_show+0x19/0x30 (target: > > boost_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) > >[ 214.596893] CFI failure at kobj_attr_show+0x19/0x30 (target: > > cur_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) > >[ 214.596996] CFI failure at kobj_attr_show+0x19/0x30 (target: > > max_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) > >[ 214.597099] CFI failure at kobj_attr_show+0x19/0x30 (target: > > min_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) > >[ 214.597198] CFI failure at kobj_attr_show+0x19/0x30 (target: > > RP0_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) > >[ 214.597301] CFI failure at kobj_attr_show+0x19/0x30 (target: > > RP1_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) > >[ 214.597405] CFI failure at kobj_attr_show+0x19/0x30 (target: > > RPn_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) > >[ 214.597538] CFI failure at kobj_attr_show+0x19/0x30 (target: > > throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) > >[ 214.597701] CFI failure at kobj_attr_show+0x19/0x30 (target: > > throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) > >[ 214.597836] CFI failure at kobj_attr_show+0x19/0x30 (target: > > throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) > >[ 214.597952] CFI failure at kobj_attr_show+0x19/0x30 (target: > > throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) > >[ 214.598071] CFI failure at kobj_attr_show+0x19/0x30 (target: > > throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) > >[ 214.598177] CFI failure at kobj_attr_show+0x19/0x30 (target: > > throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) > >[ 214.598307] CFI failure at kobj_attr_show+0x19/0x30 (target: > > throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) > >[ 214.598439] CFI failure at kobj_attr_show+0x19/0x30 (target: > > throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) > >[ 214.598542] CFI failure at kobj_attr_show+0x19/0x30 (target: > > throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) > > > > With kCFI, indirect calls are validated against their expected type > > versus actual type and failures occur when the two types do not match. > > Have you tried this tool with drm subsytem, IIRC there are also similar > cases with callbacks expecting ptr to different struct than actually passed. Yes, I have also run a kCFI kernel on an AMD system that I have and I have not seen any failures from them. I only have AMD and Intel systems with graphics so there could be other problems lurking in other drivers. > > The ultimate issue is that these sysfs functions are expecting to be > > called via dev_attr_show() but they may also be called via > > kobj_attr_show(), as certain files are created under two different > > kobjects that have two different sysfs_ops in intel_gt_sysfs_register(), > > hence the warnings above. When accessing the gt_ files under > > /sys/devices/pci:00/:00:02.0/drm/card0, which are using the same > > sysfs functio
Re: Nested AVIC design (was:Re: [RFC PATCH v3 04/19] KVM: x86: mmu: allow to enable write tracking externally)
On Mon, Aug 08, 2022, Maxim Levitsky wrote: > Hi Sean, Paolo, and everyone else who wants to review my nested AVIC work. Before we dive deep into design details, I think we should first decide whether or not nested AVIC is worth pursing/supporting. - Rome has a ucode/silicon bug with no known workaround and no anticipated fix[*]; AMD's recommended "workaround" is to disable AVIC. - AVIC is not available in Milan, which may or may not be related to the aforementioned bug. - AVIC is making a comeback on Zen4, but Zen4 comes with x2AVIC. - x2APIC is likely going to become ubiquitous, e.g. Intel is effectively requiring x2APIC to fudge around xAPIC bugs. - It's actually quite realistic to effectively force the guest to use x2APIC, at least if it's a Linux guest. E.g. turn x2APIC on in BIOS, which is often (always?) controlled by the host, and Linux will use x2APIC. In other words, given that AVIC is well on its way to becoming a "legacy" feature, IMO there needs to be a fairly strong use case to justify taking on this much code and complexity. ~1500 lines of code to support a feature that has historically been buggy _without_ nested support is going to require a non-trivial amount of effort to review, stabilize, and maintain. [*] 1235 "Guest With AVIC (Advanced Virtual Interrupt Controller) Enabled May Fail to Process IPI (Inter-Processor Interrupt) Until Guest Is Re-Scheduled" in https://www.amd.com/system/files/TechDocs/56323-PUB_1.00.pdf
Re: [Intel-gfx] [PATCH] drm/i915: Fix CFI violations in gt_sysfs
On 22.09.2022 21:51, Nathan Chancellor wrote: When booting with clang's kernel control flow integrity series [1], there are numerous violations when accessing the files under /sys/devices/pci:00/:00:02.0/drm/card0/gt/gt0: $ cd /sys/devices/pci:00/:00:02.0/drm/card0/gt/gt0 $ grep . * id:0 punit_req_freq_mhz:350 rc6_enable:1 rc6_residency_ms:214934 rps_act_freq_mhz:1300 rps_boost_freq_mhz:1300 rps_cur_freq_mhz:350 rps_max_freq_mhz:1300 rps_min_freq_mhz:350 rps_RP0_freq_mhz:1300 rps_RP1_freq_mhz:350 rps_RPn_freq_mhz:350 throttle_reason_pl1:0 throttle_reason_pl2:0 throttle_reason_pl4:0 throttle_reason_prochot:0 throttle_reason_ratl:0 throttle_reason_status:0 throttle_reason_thermal:0 throttle_reason_vr_tdc:0 throttle_reason_vr_thermalert:0 $ sudo dmesg &| grep "CFI failure at" [ 214.595903] CFI failure at kobj_attr_show+0x19/0x30 (target: id_show+0x0/0x70 [i915]; expected type: 0xc527b809) [ 214.596064] CFI failure at kobj_attr_show+0x19/0x30 (target: punit_req_freq_mhz_show+0x0/0x40 [i915]; expected type: 0xc527b809) [ 214.596407] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_enable_show+0x0/0x40 [i915]; expected type: 0xc527b809) [ 214.596528] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_residency_ms_show+0x0/0x270 [i915]; expected type: 0xc527b809) [ 214.596682] CFI failure at kobj_attr_show+0x19/0x30 (target: act_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) [ 214.596792] CFI failure at kobj_attr_show+0x19/0x30 (target: boost_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) [ 214.596893] CFI failure at kobj_attr_show+0x19/0x30 (target: cur_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) [ 214.596996] CFI failure at kobj_attr_show+0x19/0x30 (target: max_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) [ 214.597099] CFI failure at kobj_attr_show+0x19/0x30 (target: min_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) [ 214.597198] CFI failure at kobj_attr_show+0x19/0x30 (target: RP0_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) [ 214.597301] CFI failure at kobj_attr_show+0x19/0x30 (target: RP1_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) [ 214.597405] CFI failure at kobj_attr_show+0x19/0x30 (target: RPn_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809) [ 214.597538] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) [ 214.597701] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) [ 214.597836] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) [ 214.597952] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) [ 214.598071] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) [ 214.598177] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) [ 214.598307] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) [ 214.598439] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) [ 214.598542] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809) With kCFI, indirect calls are validated against their expected type versus actual type and failures occur when the two types do not match. Have you tried this tool with drm subsytem, IIRC there are also similar cases with callbacks expecting ptr to different struct than actually passed. The ultimate issue is that these sysfs functions are expecting to be called via dev_attr_show() but they may also be called via kobj_attr_show(), as certain files are created under two different kobjects that have two different sysfs_ops in intel_gt_sysfs_register(), hence the warnings above. When accessing the gt_ files under /sys/devices/pci:00/:00:02.0/drm/card0, which are using the same sysfs functions, there are no violations, meaning the functions are being called with the proper type. To make everything work properly, adjust certain functions to match the type of the ->show() and ->store() members in 'struct kobj_attribute'. Add a macro to generate functions for that can be called via both dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be called through both kobject locations without violating kCFI and adjust the attribute groups to account for this. [1]: https://lore.kernel.org/20220908215504.3686827-1-samitolva...@google.com/ Link: https://github.com/ClangBuiltLinux/lin
Re: [PATCH v2 2/2] drm/tests: Split drm_test_dp_mst_sideband_msg_req_decode into parameterized tests
On Tue, Sep 27, 2022 at 07:12:06PM -0300, Maíra Canal wrote: > The drm_test_dp_mst_sideband_msg_req_decode repeats the same test > structure with different parameters. This could be better represented > by parameterized tests, provided by KUnit. > > In order to convert the tests to parameterized tests, the test case for > the client ID was changed: instead of using get_random_bytes to generate > the client ID, the client ID is now hardcoded in the test case. Generally "random" usage is not incompatible with parameterized tests, we can create parameterized tests that use random data. The idea is to pass a function that generates the actual param (where we have a pointer to function as one of the members in "params" struct). For example, see "random_dp_query_enc_client_id" usage here: https://lore.kernel.org/dri-devel/20220117232259.180459-7-michal.winiar...@intel.com/ In this case, we just compare data going in with data going out (and the data itself is not transformed in any way), so it doesn't really matter for coverage and we can hardcode. -Michał > So, convert drm_test_dp_mst_sideband_msg_req_decode into parameterized > tests and make the tests' allocations and prints completely managed by KUnit. > > Signed-off-by: Maíra Canal > --- > v1 -> v2: > https://lore.kernel.org/dri-devel/20220925222719.345424-1-mca...@igalia.com/T/#m056610a23a63109484afeafefb5846178c4d36b2 > - Mention on the commit message the change on the test case for the client ID > (Michał Winiarski). > --- > .../gpu/drm/tests/drm_dp_mst_helper_test.c| 370 -- > 1 file changed, 243 insertions(+), 127 deletions(-)
Re: [PATCH] dt-bindings: display: st, stm32-dsi: Handle data-lanes in DSI port node
On Tue, 27 Sep 2022 01:45:01 +0200, Marek Vasut wrote: > Handle 'data-lanes' property of the DSI output endpoint, it is possible > to describe DSI link with 1 or 2 data lanes this way. > > Signed-off-by: Marek Vasut > --- > Cc: Alexandre Torgue > Cc: Krzysztof Kozlowski > Cc: Maxime Coquelin > Cc: Philippe Cornu > Cc: Rob Herring > Cc: Yannick Fertre > Cc: devicet...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: linux-st...@st-md-mailman.stormreply.com > To: linux-arm-ker...@lists.infradead.org > --- > .../bindings/display/st,stm32-dsi.yaml | 17 ++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > Applied, thanks!
Re: [PATCH V3 2/3] dt-bindings: display: panel: Add NewVision NV3051D bindings
On Mon, 26 Sep 2022 14:14:27 -0500, Chris Morgan wrote: > From: Chris Morgan > > Add documentation for the NewVision NV3051D panel bindings. > Note that for the two expected consumers of this panel binding > the underlying LCD model is unknown. Name "anbernic,rg353p-panel" > is used because the hardware itself is known as "anbernic,rg353p". > > Signed-off-by: Chris Morgan > --- > .../display/panel/newvision,nv3051d.yaml | 63 +++ > 1 file changed, 63 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > Reviewed-by: Rob Herring
Re: [PATCH V3 1/2] dt-bindings: display: panel: Add Samsung AMS495QA01 bindings
On Mon, 26 Sep 2022 11:43:32 -0500, Chris Morgan wrote: > From: Chris Morgan > > Add documentation for the Samsung AMS495QA01 panel. > > Signed-off-by: Chris Morgan > Signed-off-by: Maya Matuszczyk > --- > .../display/panel/samsung,ams495qa01.yaml | 56 +++ > 1 file changed, 56 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/samsung,ams495qa01.yaml > Reviewed-by: Rob Herring
Re: [PATCH v2 2/2] drm/panel: simple: Use dev_err_probe() to simplify code
Hi, On Wed, Sep 28, 2022 at 6:57 PM Yuan Can wrote: > > In the probe path, dev_err() can be replaced with dev_err_probe() > which will check if error code is -EPROBE_DEFER and prints the > error name. It also sets the defer probe reason which can be > checked later through debugfs. > > Signed-off-by: Yuan Can > Reviewed-by: Douglas Anderson > --- > drivers/gpu/drm/panel/panel-simple.c | 9 +++-- > 1 file changed, 3 insertions(+), 6 deletions(-) Pushed to drm-misc-next: c9b48b91e2fb drm/panel: simple: Use dev_err_probe() to simplify code For the other patches in your original series, I'll assume folks paying more attention to those panel drivers will commit them. If they totally stall for a while, though, then yell and I can try taking a look at them. :-) -Doug
Re: [PATCH v2 1/2] drm/panel: panel-edp: Use dev_err_probe() to simplify code
Hi, On Wed, Sep 28, 2022 at 6:56 PM Yuan Can wrote: > > In the probe path, dev_err() can be replaced with dev_err_probe() > which will check if error code is -EPROBE_DEFER and prints the > error name. It also sets the defer probe reason which can be > checked later through debugfs. > > Signed-off-by: Yuan Can > Reviewed-by: Douglas Anderson > --- > drivers/gpu/drm/panel/panel-edp.c | 22 ++ > 1 file changed, 6 insertions(+), 16 deletions(-) Pushed to drm-misc-next: b28d204a7c19 drm/panel: panel-edp: Use dev_err_probe() to simplify code
Re: [PATCH v5] drm/i915/mtl: enable local stolen memory
On Thu, Sep 29, 2022 at 05:16:58PM +0530, Aravind Iddamsetty wrote: > As an integrated GPU, MTL does not have local memory and HAS_LMEM() > returns false. However the platform's stolen memory is presented via > BAR2 (i.e., the BAR we traditionally consider to be the GMADR on IGFX) > and should be managed by the driver the same way that local memory is > on dgpu platforms (which includes setting the "lmem" bit on page table > entries). We use the term "local stolen memory" to refer to this > model. > > The major difference from the traditional BAR2 (GMADR) is that > the stolen area is mapped via the BAR2 while in the former BAR2 is an > aperture into the GTT VA through which access are made into stolen area. > > BSPEC: 53098, 63830 > > v2: > 1. dropped is_dsm_invalid, updated valid_stolen_size check from Lucas > (Jani, Lucas) > 2. drop lmembar_is_igpu_stolen > 3. revert to referring GFXMEM_BAR as GEN12_LMEM_BAR (Lucas) > > v3:(Jani) > 1. rename get_mtl_gms_size to mtl_get_gms_size > 2. define register for MMIO address > > v4:(Matt) > 1. Use REG_FIELD_GET to read GMS value > 2. replace the calculations with SZ_256M/SZ_8M > > v5: Include more details to commit message on how it is different from > earlier platforms (Anshuman) > > Cc: Matt Roper > Cc: Lucas De Marchi > Cc: Jani Nikula > > Signed-off-by: CQ Tang > Signed-off-by: Aravind Iddamsetty > Original-author: CQ Tang Reviewed-by: Matt Roper > --- > drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 83 ++ > drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h| 3 + > drivers/gpu/drm/i915/i915_reg.h| 4 ++ > 4 files changed, 76 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c > b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c > index c5a4035c99cd..910086974454 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c > @@ -77,9 +77,9 @@ void i915_gem_stolen_remove_node(struct drm_i915_private > *i915, > mutex_unlock(&i915->mm.stolen_lock); > } > > -static bool valid_stolen_size(struct resource *dsm) > +static bool valid_stolen_size(struct drm_i915_private *i915, struct resource > *dsm) > { > - return dsm->start != 0 && dsm->end > dsm->start; > + return (dsm->start != 0 || HAS_BAR2_SMEM_STOLEN(i915)) && dsm->end > > dsm->start; > } > > static int adjust_stolen(struct drm_i915_private *i915, > @@ -88,7 +88,7 @@ static int adjust_stolen(struct drm_i915_private *i915, > struct i915_ggtt *ggtt = to_gt(i915)->ggtt; > struct intel_uncore *uncore = ggtt->vm.gt->uncore; > > - if (!valid_stolen_size(dsm)) > + if (!valid_stolen_size(i915, dsm)) > return -EINVAL; > > /* > @@ -135,7 +135,7 @@ static int adjust_stolen(struct drm_i915_private *i915, > } > } > > - if (!valid_stolen_size(dsm)) > + if (!valid_stolen_size(i915, dsm)) > return -EINVAL; > > return 0; > @@ -149,8 +149,11 @@ static int request_smem_stolen(struct drm_i915_private > *i915, > /* >* With stolen lmem, we don't need to request system memory for the >* address range since it's local to the gpu. > + * > + * Starting MTL, in IGFX devices the stolen memory is exposed via > + * BAR2 and shall be considered similar to stolen lmem. >*/ > - if (HAS_LMEM(i915)) > + if (HAS_LMEM(i915) || HAS_BAR2_SMEM_STOLEN(i915)) > return 0; > > /* > @@ -385,8 +388,6 @@ static void icl_get_stolen_reserved(struct > drm_i915_private *i915, > > drm_dbg(&i915->drm, "GEN6_STOLEN_RESERVED = 0x%016llx\n", reg_val); > > - *base = reg_val & GEN11_STOLEN_RESERVED_ADDR_MASK; > - > switch (reg_val & GEN8_STOLEN_RESERVED_SIZE_MASK) { > case GEN8_STOLEN_RESERVED_1M: > *size = 1024 * 1024; > @@ -404,6 +405,12 @@ static void icl_get_stolen_reserved(struct > drm_i915_private *i915, > *size = 8 * 1024 * 1024; > MISSING_CASE(reg_val & GEN8_STOLEN_RESERVED_SIZE_MASK); > } > + > + if (HAS_BAR2_SMEM_STOLEN(i915)) > + /* the base is initialized to stolen top so subtract size to > get base */ > + *base -= *size; > + else > + *base = reg_val & GEN11_STOLEN_RESERVED_ADDR_MASK; > } > > /* > @@ -833,6 +840,29 @@ static const struct intel_memory_region_ops > i915_region_stolen_lmem_ops = { > .init_object = _i915_gem_object_stolen_init, > }; > > +static int mtl_get_gms_size(struct intel_uncore *uncore) > +{ > + u16 ggc, gms; > + > + ggc = intel_uncore_read16(uncore, GGC); > + > + /* check GGMS, should be fixed 0x3 (8MB) */ > + if ((ggc & GGMS_MASK) != GGMS_MASK) > + return -EIO; > + > + /* return valid GMS value, -EIO if invalid */ > + gms = REG_FIELD_GET(GMS_MASK, ggc); > + switch (gms) { > + case 0x0 ...
[PATCH v3 3/4] drm: lcdif: Switch to limited range for RGB to YUV conversion
Up to and including v1.3, HDMI supported limited quantization range only for YCbCr. HDMI v1.4 introduced selectable quantization ranges, but this feature isn't supported in the dw-hdmi driver that is used in conjunction with the LCDIF in the i.MX8MP. The HDMI YCbCr output is thus always advertised in the AVI infoframe as limited range. The LCDIF driver, on the other hand, configures the CSC to produce full range YCbCr. This mismatch results in loss of details and incorrect colours. Fix it by switching to limited range YCbCr. The coefficients are copied from drivers/media/platforms/nxp/imx-pxp.c for coherency, as the hardware is most likely identical. Fixes: 9db35bb349a0 ("drm: lcdif: Add support for i.MX8MP LCDIF variant") Signed-off-by: Laurent Pinchart Reviewed-by: Marek Vasut Reviewed-by: Kieran Bingham Reviewed-by: Liu Ying --- Changes since v2: - List floating point coefficient values in comment - Fix typo in the commit message Changes since v1: - Use coefficients from imx-pxp.c --- drivers/gpu/drm/mxsfb/lcdif_kms.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c index 1f22ea5896d5..08e4880ec6cf 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c @@ -53,16 +53,22 @@ static void lcdif_set_formats(struct lcdif_drm_private *lcdif, writel(DISP_PARA_LINE_PATTERN_UYVY_H, lcdif->base + LCDC_V8_DISP_PARA); - /* CSC: BT.601 Full Range RGB to YCbCr coefficients. */ - writel(CSC0_COEF0_A2(0x096) | CSC0_COEF0_A1(0x04c), + /* +* CSC: BT.601 Limited Range RGB to YCbCr coefficients. +* +* |Y | | 0.2568 0.5041 0.0979| |R| |16 | +* |Cb| = |-0.1482 -0.2910 0.4392| * |G| + |128| +* |Cr| | 0.4392 0.4392 -0.3678| |B| |128| +*/ + writel(CSC0_COEF0_A2(0x081) | CSC0_COEF0_A1(0x041), lcdif->base + LCDC_V8_CSC0_COEF0); - writel(CSC0_COEF1_B1(0x7d5) | CSC0_COEF1_A3(0x01d), + writel(CSC0_COEF1_B1(0x7db) | CSC0_COEF1_A3(0x019), lcdif->base + LCDC_V8_CSC0_COEF1); - writel(CSC0_COEF2_B3(0x080) | CSC0_COEF2_B2(0x7ac), + writel(CSC0_COEF2_B3(0x070) | CSC0_COEF2_B2(0x7b6), lcdif->base + LCDC_V8_CSC0_COEF2); - writel(CSC0_COEF3_C2(0x795) | CSC0_COEF3_C1(0x080), + writel(CSC0_COEF3_C2(0x7a2) | CSC0_COEF3_C1(0x070), lcdif->base + LCDC_V8_CSC0_COEF3); - writel(CSC0_COEF4_D1(0x000) | CSC0_COEF4_C3(0x7ec), + writel(CSC0_COEF4_D1(0x010) | CSC0_COEF4_C3(0x7ee), lcdif->base + LCDC_V8_CSC0_COEF4); writel(CSC0_COEF5_D3(0x080) | CSC0_COEF5_D2(0x080), lcdif->base + LCDC_V8_CSC0_COEF5); -- Regards, Laurent Pinchart
[PATCH v3 2/4] drm: lcdif: Don't use BIT() for multi-bit register fields
The BIT() macro is meant to represent a single bit. Don't use it for values of register fields that span multiple bits. Signed-off-by: Laurent Pinchart Reviewed-by: Marek Vasut Reviewed-by: Kieran Bingham Reviewed-by: Liu Ying --- Changes since v1: - Use hex for field values --- drivers/gpu/drm/mxsfb/lcdif_regs.h | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/lcdif_regs.h b/drivers/gpu/drm/mxsfb/lcdif_regs.h index 013f2cace2a0..0d5d9bedd94a 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_regs.h +++ b/drivers/gpu/drm/mxsfb/lcdif_regs.h @@ -138,9 +138,9 @@ #define DISP_PARA_DISP_ON BIT(31) #define DISP_PARA_SWAP_EN BIT(30) -#define DISP_PARA_LINE_PATTERN_UYVY_H (GENMASK(29, 28) | BIT(26)) -#define DISP_PARA_LINE_PATTERN_RGB565 GENMASK(28, 26) -#define DISP_PARA_LINE_PATTERN_RGB888 0 +#define DISP_PARA_LINE_PATTERN_UYVY_H (0xd << 26) +#define DISP_PARA_LINE_PATTERN_RGB565 (0x7 << 26) +#define DISP_PARA_LINE_PATTERN_RGB888 (0x0 << 26) #define DISP_PARA_LINE_PATTERN_MASKGENMASK(29, 26) #define DISP_PARA_DISP_MODE_MASK GENMASK(25, 24) #define DISP_PARA_BGND_R_MASK GENMASK(23, 16) @@ -202,18 +202,18 @@ #define CTRLDESCL0_5_ENBIT(31) #define CTRLDESCL0_5_SHADOW_LOAD_ENBIT(30) -#define CTRLDESCL0_5_BPP_16_RGB565 BIT(26) -#define CTRLDESCL0_5_BPP_16_ARGB1555 (BIT(26) | BIT(24)) -#define CTRLDESCL0_5_BPP_16_ARGB (BIT(26) | BIT(25)) -#define CTRLDESCL0_5_BPP_YCbCr422 (BIT(26) | BIT(25) | BIT(24)) -#define CTRLDESCL0_5_BPP_24_RGB888 BIT(27) -#define CTRLDESCL0_5_BPP_32_ARGB (BIT(27) | BIT(24)) -#define CTRLDESCL0_5_BPP_32_ABGR (BIT(27) | BIT(25)) +#define CTRLDESCL0_5_BPP_16_RGB565 (0x4 << 24) +#define CTRLDESCL0_5_BPP_16_ARGB1555 (0x5 << 24) +#define CTRLDESCL0_5_BPP_16_ARGB (0x6 << 24) +#define CTRLDESCL0_5_BPP_YCbCr422 (0x7 << 24) +#define CTRLDESCL0_5_BPP_24_RGB888 (0x8 << 24) +#define CTRLDESCL0_5_BPP_32_ARGB (0x9 << 24) +#define CTRLDESCL0_5_BPP_32_ABGR (0xa << 24) #define CTRLDESCL0_5_BPP_MASK GENMASK(27, 24) -#define CTRLDESCL0_5_YUV_FORMAT_Y2VY1U 0 -#define CTRLDESCL0_5_YUV_FORMAT_Y2UY1V BIT(14) -#define CTRLDESCL0_5_YUV_FORMAT_VY2UY1 BIT(15) -#define CTRLDESCL0_5_YUV_FORMAT_UY2VY1 (BIT(15) | BIT(14)) +#define CTRLDESCL0_5_YUV_FORMAT_Y2VY1U (0x0 << 14) +#define CTRLDESCL0_5_YUV_FORMAT_Y2UY1V (0x1 << 14) +#define CTRLDESCL0_5_YUV_FORMAT_VY2UY1 (0x2 << 14) +#define CTRLDESCL0_5_YUV_FORMAT_UY2VY1 (0x3 << 14) #define CTRLDESCL0_5_YUV_FORMAT_MASK GENMASK(15, 14) #define CSC0_CTRL_CSC_MODE_RGB2YCbCr GENMASK(2, 1) -- Regards, Laurent Pinchart
[PATCH v3 4/4] drm: lcdif: Add support for YUV planes
From: Kieran Bingham The LCDIF includes a color space converter that supports YUV input. Use it to support YUV planes, either through the converter if the output format is RGB, or in conversion bypass mode otherwise. Signed-off-by: Kieran Bingham Signed-off-by: Laurent Pinchart Reviewed-by: Marek Vasut Reviewed-by: Kieran Bingham --- Changes since v2: - Fix YUV to RGB coefficients - List floating point coefficient values in comment - Express CSC coefficients with three hex digits - Move | to the end of the line - Add comment header before RGB formats Changes since v1: - Support all YCbCr encodings and quantization ranges - Drop incorrect comment --- drivers/gpu/drm/mxsfb/lcdif_kms.c | 260 - drivers/gpu/drm/mxsfb/lcdif_regs.h | 5 +- 2 files changed, 224 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c index 08e4880ec6cf..81454b53936f 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -32,13 +33,119 @@ /* - * CRTC */ + +/* + * Despite the reference manual stating the opposite, the D1, D2 and D3 offset + * values are added to Y, U and V, not subtracted. They must thus be programmed + * with negative values. + */ +static const u32 lcdif_yuv2rgb_coeffs[3][2][6] = { + [DRM_COLOR_YCBCR_BT601] = { + [DRM_COLOR_YCBCR_LIMITED_RANGE] = { + /* +* BT.601 limited range: +* +* |R| |1.1644 0. 1.5960| |Y - 16 | +* |G| = |1.1644 -0.3917 -0.8129| * |Cb - 128| +* |B| |1.1644 2.0172 0.| |Cr - 128| +*/ + CSC0_COEF0_A1(0x12a) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x199) | CSC0_COEF1_B1(0x12a), + CSC0_COEF2_B2(0x79c) | CSC0_COEF2_B3(0x730), + CSC0_COEF3_C1(0x12a) | CSC0_COEF3_C2(0x204), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x1f0), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + [DRM_COLOR_YCBCR_FULL_RANGE] = { + /* +* BT.601 full range: +* +* |R| |1. 0. 1.4020| |Y - 0 | +* |G| = |1. -0.3441 -0.7141| * |Cb - 128| +* |B| |1. 1.7720 0.| |Cr - 128| +*/ + CSC0_COEF0_A1(0x100) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x167) | CSC0_COEF1_B1(0x100), + CSC0_COEF2_B2(0x7a8) | CSC0_COEF2_B3(0x749), + CSC0_COEF3_C1(0x100) | CSC0_COEF3_C2(0x1c6), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x000), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + }, + [DRM_COLOR_YCBCR_BT709] = { + [DRM_COLOR_YCBCR_LIMITED_RANGE] = { + /* +* Rec.709 limited range: +* +* |R| |1.1644 0. 1.7927| |Y - 16 | +* |G| = |1.1644 -0.2132 -0.5329| * |Cb - 128| +* |B| |1.1644 2.1124 0.| |Cr - 128| +*/ + CSC0_COEF0_A1(0x12a) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x1cb) | CSC0_COEF1_B1(0x12a), + CSC0_COEF2_B2(0x7c9) | CSC0_COEF2_B3(0x778), + CSC0_COEF3_C1(0x12a) | CSC0_COEF3_C2(0x21d), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x1f0), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + [DRM_COLOR_YCBCR_FULL_RANGE] = { + /* +* Rec.709 full range: +* +* |R| |1. 0. 1.5748| |Y - 0 | +* |G| = |1. -0.1873 -0.4681| * |Cb - 128| +* |B| |1. 1.8556 0.| |Cr - 128| +*/ + CSC0_COEF0_A1(0x100) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x193) | CSC0_COEF1_B1(0x100), + CSC0_COEF2_B2(0x7d0) | CSC0_COEF2_B3(0x788), + CSC0_COEF3_C1(0x100) | CSC0_COEF3_C2(0x1db), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x000), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + }, + [DRM_COLOR_YCBCR_BT2020] = { + [DRM_COLOR_YCBCR_LIMITED_RANGE] = { +
[PATCH v3 1/4] drm: lcdif: Fix indentation in lcdif_regs.h
A couple of the register macro values are incorrectly indented. Fix them. Signed-off-by: Laurent Pinchart Reviewed-by: Marek Vasut Reviewed-by: Kieran Bingham Reviewed-by: Liu Ying --- drivers/gpu/drm/mxsfb/lcdif_regs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/lcdif_regs.h b/drivers/gpu/drm/mxsfb/lcdif_regs.h index 8e8bef175bf2..013f2cace2a0 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_regs.h +++ b/drivers/gpu/drm/mxsfb/lcdif_regs.h @@ -130,7 +130,7 @@ #define CTRL_FETCH_START_OPTION_BPVBIT(9) #define CTRL_FETCH_START_OPTION_RESV GENMASK(9, 8) #define CTRL_FETCH_START_OPTION_MASK GENMASK(9, 8) -#define CTRL_NEG BIT(4) +#define CTRL_NEG BIT(4) #define CTRL_INV_PXCK BIT(3) #define CTRL_INV_DEBIT(2) #define CTRL_INV_VSBIT(1) @@ -186,7 +186,7 @@ #define INT_ENABLE_D1_PLANE_PANIC_EN BIT(0) #define CTRLDESCL0_1_HEIGHT(n) (((n) & 0x) << 16) -#define CTRLDESCL0_1_HEIGHT_MASK GENMASK(31, 16) +#define CTRLDESCL0_1_HEIGHT_MASK GENMASK(31, 16) #define CTRLDESCL0_1_WIDTH(n) ((n) & 0x) #define CTRLDESCL0_1_WIDTH_MASKGENMASK(15, 0) -- Regards, Laurent Pinchart
[PATCH v3 0/4] drm: lcdif: Improve YUV support
Hello, This small patch series improves YUV support in the i.MX8MP LCDIF driver. After patches 1/4 and 2/4 that fix tiny cosmetic issues, patch 3/4 fixes YUV quantization range for the RGB to YUV conversion. Patch 4/4 addresses the other direction and adds support for YUV planes. Please see individual patches for details. Compared to v2, review comments have been taken into account, and the YUV to RGB coefficients in patch 4/4 have been fixed (they were blatantly wrong due to a stupid mistake). The series has been tested on a Polyhex Debix board with the currently out-of-tree HDMI encoder support patches developed by Lucas Stach, with the kmstest and the libcamera 'cam' applications. There is a know issue with the way the driver programs the format and pitch, which produces incorrect output when testing YUV formats with the legacy (non-atomic) KMS API, in particular with the modetest application. The framebuffer is accessed from the plane state in function called from the .atomic_enable() handler, which in some circumstances results in the format and/or pitch of the old frame buffer being used. This issue preexists, and can be triggered by selecting a different RGB format with modetest (XR15 for instance). It should be fixed separately, and I wouldn't consider it as a blocker for this series as YUV formats can already be used correctly when using the KMS atomic API. Kieran Bingham (1): drm: lcdif: Add support for YUV planes Laurent Pinchart (3): drm: lcdif: Fix indentation in lcdif_regs.h drm: lcdif: Don't use BIT() for multi-bit register fields drm: lcdif: Switch to limited range for RGB to YUV conversion drivers/gpu/drm/mxsfb/lcdif_kms.c | 232 ++--- drivers/gpu/drm/mxsfb/lcdif_regs.h | 37 ++--- 2 files changed, 229 insertions(+), 40 deletions(-) -- Regards, Laurent Pinchart
Re: [Freedreno] [PATCH -next] drm/msm/msm_gem_shrinker: fix compile error in can_block()
On Thu, Sep 29, 2022 at 4:51 AM Akhil P Oommen wrote: > > On 9/29/2022 3:00 PM, Yang Yingliang wrote: > > I got the compile error: > > > >drivers/gpu/drm/msm/msm_gem_shrinker.c: In function ‘can_block’: > >drivers/gpu/drm/msm/msm_gem_shrinker.c:29:21: error: ‘__GFP_ATOMIC’ > > undeclared (first use in this function); did you mean ‘GFP_ATOMIC’? > > if (sc->gfp_mask & __GFP_ATOMIC) > > ^~~~ > > GFP_ATOMIC > >drivers/gpu/drm/msm/msm_gem_shrinker.c:29:21: note: each undeclared > > identifier is reported only once for each function it appears in > > > > __GFP_ATOMIC is dropped by commit 6708fe6bec50 ("mm: discard __GFP_ATOMIC"). > > Use __GFP_HIGH instead. > > > > Fixes: 025d27239a2f ("drm/msm/gem: Evict active GEM objects when necessary") > > Signed-off-by: Yang Yingliang > > --- > > drivers/gpu/drm/msm/msm_gem_shrinker.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c > > b/drivers/gpu/drm/msm/msm_gem_shrinker.c > > index 58e0513be5f4..6a0de6cdb82b 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c > > +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c > > @@ -26,7 +26,7 @@ static bool can_swap(void) > > > > static bool can_block(struct shrink_control *sc) > > { > > - if (sc->gfp_mask & __GFP_ATOMIC) > > + if (sc->gfp_mask & __GFP_HIGH) > > return false; > > return current_is_kswapd() || (sc->gfp_mask & __GFP_RECLAIM); > > } > > Reviewed-by: Akhil P Oommen > Somehow the original patch didn't show up in my inbox, but I've sent this: https://patchwork.freedesktop.org/series/109255/ I guess __GFP_HIGH could also be used to detect GFP_ATOMIC, but checking that direct reclaim is ok seems safer (ie. it should always be safe to sleep in that case) BR, -R > > -Akhil.
Re: [PATCH v2 4/4] drm: lcdif: Add support for YUV planes
Hi Liu, On Thu, Sep 29, 2022 at 05:53:37PM +0800, Liu Ying wrote: > On Wed, 2022-09-28 at 03:58 +0300, Laurent Pinchart wrote: > > From: Kieran Bingham > > > > The LCDIF includes a color space converter that supports YUV input. Use > > it to support YUV planes, either through the converter if the output > > format is RGB, or in conversion bypass mode otherwise. > > > > Signed-off-by: Kieran Bingham > > Signed-off-by: Laurent Pinchart > > --- > > Changes since v1: > > > > - Support all YCbCr encodings and quantization ranges > > - Drop incorrect comment > > --- > > drivers/gpu/drm/mxsfb/lcdif_kms.c | 183 + > > drivers/gpu/drm/mxsfb/lcdif_regs.h | 5 +- > > 2 files changed, 164 insertions(+), 24 deletions(-) > > I have a chance to test this series and find that my > 'koe,tx26d202vm0bwa' 1920x1200 LVDS panel connected with i.MX8mp EVK > can only show the test pattern at the top half of the display with YUV > fb. Looks like something with wrong stride. XR24 fb is ok, but RG16 fb > has similar issue. Anything I missed? > > The command I'm using to test YUV fb: > modetest -M imx-lcdif -P 31@35:1920x1200@YUYV Thanks for the bug report. The problem didn't occur with kmstest nor the libcamera 'cam' test application, but I have been able to reproduce it with modetest. The modetest application uses the legacy mode setting API by default, so it exercises code paths in the driver in different ways, uncovering a few preexisting issues. The problem is caused by the fact that the functions called from the .atomic_enable() handler access the frame buffer from the plane state and configure the hardware using that information. Depending on how the device is configured, the display can be enabled with one frame buffer, and then immediately switch to a different frame buffer that has a different format and/or pitch. Properties of the frame buffer should only be accessed from the plane .atomic_update() operation. Fixing this requires quite a bit of refactoring of the driver, which I won't have time to work on at the moment. Marek, would you have some time to look at this ? As the issue isn't introduced by this series but preexists (you should be able to reproduce it by selecting the XR15 format instead of XR24 for instance), can YUV support be merged already (after I send v3) ? -- Regards, Laurent Pinchart
Re: [PATCH 2/7] mm: Free device private pages have zero refcount
Alistair Popple wrote: > > Jason Gunthorpe writes: > > > On Mon, Sep 26, 2022 at 04:03:06PM +1000, Alistair Popple wrote: > >> Since 27674ef6c73f ("mm: remove the extra ZONE_DEVICE struct page > >> refcount") device private pages have no longer had an extra reference > >> count when the page is in use. However before handing them back to the > >> owning device driver we add an extra reference count such that free > >> pages have a reference count of one. > >> > >> This makes it difficult to tell if a page is free or not because both > >> free and in use pages will have a non-zero refcount. Instead we should > >> return pages to the drivers page allocator with a zero reference count. > >> Kernel code can then safely use kernel functions such as > >> get_page_unless_zero(). > >> > >> Signed-off-by: Alistair Popple > >> --- > >> arch/powerpc/kvm/book3s_hv_uvmem.c | 1 + > >> drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 1 + > >> drivers/gpu/drm/nouveau/nouveau_dmem.c | 1 + > >> lib/test_hmm.c | 1 + > >> mm/memremap.c| 5 - > >> mm/page_alloc.c | 6 ++ > >> 6 files changed, 10 insertions(+), 5 deletions(-) > > > > I think this is a great idea, but I'm surprised no dax stuff is > > touched here? > > free_zone_device_page() shouldn't be called for pgmap->type == > MEMORY_DEVICE_FS_DAX so I don't think we should have to worry about DAX > there. Except that the folio code looks like it might have introduced a > bug. AFAICT put_page() always calls > put_devmap_managed_page(&folio->page) but folio_put() does not (although > folios_put() does!). So it seems folio_put() won't end up calling > __put_devmap_managed_page_refs() as I think it should. > > I think you're right about the change to __init_zone_device_page() - I > should limit it to DEVICE_PRIVATE/COHERENT pages only. But I need to > look at Dan's patch series more closely as I suspect it might be better > to rebase this patch on top of that. Apologies for the delay I was travelling the past few days. Yes, I think this patch slots in nicely to avoid the introduction of an init_mode [1]: https://lore.kernel.org/nvdimm/166329940343.2786261.6047770378829215962.st...@dwillia2-xfh.jf.intel.com/ Mind if I steal it into my series?
Re: [PATCH v8 12/12] dt-bindings: display/msm: add support for the display on SM8250
On Sat, 24 Sep 2022 15:36:11 +0300, Dmitry Baryshkov wrote: > Add DPU and MDSS schemas to describe MDSS and DPU blocks on the Qualcomm > SM8250 platform. > > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/mdss-common.yaml | 4 +- > .../bindings/display/msm/qcom,sm8250-dpu.yaml | 92 + > .../display/msm/qcom,sm8250-mdss.yaml | 330 ++ > 3 files changed, 424 insertions(+), 2 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sm8250-mdss.yaml > Reviewed-by: Rob Herring
Re: [PATCH v8 11/12] dt-bindings: display/msm: add missing device nodes to mdss-* schemas
On Sat, 24 Sep 2022 15:36:10 +0300, Dmitry Baryshkov wrote: > Add missing device nodes (DSI, PHYs, DP/eDP) to the existing MDSS > schemas. > > Signed-off-by: Dmitry Baryshkov > --- > .../display/msm/qcom,msm8998-mdss.yaml| 153 + > .../display/msm/qcom,qcm2290-mdss.yaml| 81 + > .../display/msm/qcom,sc7180-mdss.yaml | 179 +++ > .../display/msm/qcom,sc7280-mdss.yaml | 292 ++ > .../display/msm/qcom,sdm845-mdss.yaml | 153 + > 5 files changed, 858 insertions(+) > Reviewed-by: Rob Herring
Re: [PATCH v8 10/12] dt-bindings: display/msm: split dpu-qcm2290 into DPU and MDSS parts
On Sat, 24 Sep 2022 15:36:09 +0300, Dmitry Baryshkov wrote: > In order to make the schema more readable, split dpu-qcm2290 into the DPU > and MDSS parts, each one describing just a single device binding. > > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/dpu-qcm2290.yaml | 148 -- > .../display/msm/qcom,qcm2290-dpu.yaml | 84 ++ > .../display/msm/qcom,qcm2290-mdss.yaml| 117 ++ > 3 files changed, 201 insertions(+), 148 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,qcm2290-dpu.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml > Reviewed-by: Rob Herring
Re: [Freedreno] [PATCH] drm/msm/gem: Unpin objects slightly later
On Fri, Sep 23, 2022 at 3:41 PM Rob Clark wrote: > > From: Rob Clark > > The introduction of 025d27239a2f exposes a problem with f371bcc0c2ac, in > that we need to keep the object pinned in the time the submit is queued > up in the gpu scheduler. Otherwise the shrinker will see it as a thing > that can be evicted if we wait for it to be signaled. But if the > shrinker path is waiting on it with the obj lock held, the job cannot be > scheduled, as that also requires briefly grabbing the obj lock, leading > to deadlock. (Not to mention, we don't want the shrinker to evict an > obj queued up in gpu scheduler.) > > Fixes: f371bcc0c2ac ("drm/msm/gem: Unpin buffers earlier") > Fixes: 025d27239a2f ("drm/msm/gem: Evict active GEM objects when necessary") > Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/19 > Signed-off-by: Rob Clark Tested-by: Chia-I Wu > --- > drivers/gpu/drm/msm/msm_gem_submit.c | 4 ++-- > drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > b/drivers/gpu/drm/msm/msm_gem_submit.c > index 5599d93ec0d2..c670591995e6 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -501,11 +501,11 @@ static int submit_reloc(struct msm_gem_submit *submit, > struct msm_gem_object *ob > */ > static void submit_cleanup(struct msm_gem_submit *submit, bool error) > { > - unsigned cleanup_flags = BO_LOCKED | BO_OBJ_PINNED; > + unsigned cleanup_flags = BO_LOCKED; > unsigned i; > > if (error) > - cleanup_flags |= BO_VMA_PINNED; > + cleanup_flags |= BO_VMA_PINNED | BO_OBJ_PINNED; > > for (i = 0; i < submit->nr_bos; i++) { > struct msm_gem_object *msm_obj = submit->bos[i].obj; > diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c > b/drivers/gpu/drm/msm/msm_ringbuffer.c > index cad4c3525f0b..57a8e9564540 100644 > --- a/drivers/gpu/drm/msm/msm_ringbuffer.c > +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c > @@ -25,7 +25,8 @@ static struct dma_fence *msm_job_run(struct drm_sched_job > *job) > > msm_gem_lock(obj); > msm_gem_unpin_vma_fenced(submit->bos[i].vma, fctx); > - submit->bos[i].flags &= ~BO_VMA_PINNED; > + msm_gem_unpin_locked(obj); > + submit->bos[i].flags &= ~(BO_VMA_PINNED | BO_OBJ_PINNED); > msm_gem_unlock(obj); > } > > -- > 2.37.2 >
Re: [PATCH v8 09/12] dt-bindings: display/msm: split dpu-msm8998 into DPU and MDSS parts
On Sat, 24 Sep 2022 15:36:08 +0300, Dmitry Baryshkov wrote: > In order to make the schema more readable, split dpu-msm8998 into the DPU > and MDSS parts, each one describing just a single device binding. > > Signed-off-by: Dmitry Baryshkov > --- > .../display/msm/qcom,msm8998-dpu.yaml | 95 +++ > ...pu-msm8998.yaml => qcom,msm8998-mdss.yaml} | 47 ++--- > 2 files changed, 101 insertions(+), 41 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,msm8998-dpu.yaml > rename Documentation/devicetree/bindings/display/msm/{dpu-msm8998.yaml => > qcom,msm8998-mdss.yaml} (69%) > Reviewed-by: Rob Herring
Re: [PATCH v8 08/12] dt-bindings: display/msm: split dpu-sdm845 into DPU and MDSS parts
On Sat, 24 Sep 2022 15:36:07 +0300, Dmitry Baryshkov wrote: > In order to make the schema more readable, split dpu-sdm845 into the DPU > and MDSS parts, each one describing just a single device binding. > > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/dpu-sdm845.yaml | 148 -- > .../bindings/display/msm/qcom,sdm845-dpu.yaml | 90 +++ > .../display/msm/qcom,sdm845-mdss.yaml | 117 ++ > 3 files changed, 207 insertions(+), 148 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sdm845-dpu.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml > Reviewed-by: Rob Herring
Re: [PATCH v8 07/12] dt-bindings: display/msm: split dpu-sc7280 into DPU and MDSS parts
On Sat, 24 Sep 2022 15:36:06 +0300, Dmitry Baryshkov wrote: > In order to make the schema more readable, split dpu-sc7280 into the DPU > and MDSS parts, each one describing just a single device binding. > > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/dpu-sc7280.yaml | 162 -- > .../bindings/display/msm/qcom,sc7280-dpu.yaml | 98 +++ > .../display/msm/qcom,sc7280-mdss.yaml | 130 ++ > 3 files changed, 228 insertions(+), 162 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml > Reviewed-by: Rob Herring
Re: [PATCH v8 06/12] dt-bindings: display/msm: split dpu-sc7180 into DPU and MDSS parts
On Sat, 24 Sep 2022 15:36:05 +0300, Dmitry Baryshkov wrote: > In order to make the schema more readable, split dpu-sc7180 into the DPU > and MDSS parts, each one describing just a single device binding. > > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/dpu-sc7180.yaml | 158 -- > .../bindings/display/msm/qcom,sc7180-dpu.yaml | 95 +++ > .../display/msm/qcom,sc7180-mdss.yaml | 125 ++ > 3 files changed, 220 insertions(+), 158 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml > Reviewed-by: Rob Herring
Re: [PATCH v8 05/12] dt-bindings: display/msm: move common MDSS properties to mdss-common.yaml
On Sat, 24 Sep 2022 15:36:04 +0300, Dmitry Baryshkov wrote: > Move properties common to all MDSS DT nodes to the mdss-common.yaml. > > This extends qcom,msm8998-mdss schema to allow interconnect nodes, which > will be added later, once msm8998 gains interconnect support. > > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/dpu-msm8998.yaml | 41 + > .../bindings/display/msm/dpu-qcm2290.yaml | 51 ++-- > .../bindings/display/msm/dpu-sc7180.yaml | 50 ++- > .../bindings/display/msm/dpu-sc7280.yaml | 50 ++- > .../bindings/display/msm/dpu-sdm845.yaml | 54 ++-- > .../bindings/display/msm/mdss-common.yaml | 83 +++ > 6 files changed, 111 insertions(+), 218 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/display/msm/mdss-common.yaml > Reviewed-by: Rob Herring
Re: [PATCH v8 04/12] dt-bindings: display/msm: move common DPU properties to dpu-common.yaml
On Sat, 24 Sep 2022 15:36:03 +0300, Dmitry Baryshkov wrote: > Move properties common to all DPU DT nodes to the dpu-common.yaml. > > Note, this removes description of individual DPU port@ nodes. However > such definitions add no additional value. The reg values do not > correspond to hardware INTF indices. The driver discovers and binds > these ports not paying any care for the order of these items. Thus just > leave the reference to graph.yaml#/properties/ports and the description. > > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/dpu-common.yaml | 52 +++ > .../bindings/display/msm/dpu-msm8998.yaml | 44 +--- > .../bindings/display/msm/dpu-qcm2290.yaml | 39 +- > .../bindings/display/msm/dpu-sc7180.yaml | 43 +-- > .../bindings/display/msm/dpu-sc7280.yaml | 43 +-- > .../bindings/display/msm/dpu-sdm845.yaml | 44 +--- > 6 files changed, 62 insertions(+), 203 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-common.yaml > Reviewed-by: Rob Herring
Re: [PATCH v2 2/8] mm: Free device private pages have zero refcount
On 2022-09-28 08:01, Alistair Popple wrote: Since 27674ef6c73f ("mm: remove the extra ZONE_DEVICE struct page refcount") device private pages have no longer had an extra reference count when the page is in use. However before handing them back to the owning device driver we add an extra reference count such that free pages have a reference count of one. This makes it difficult to tell if a page is free or not because both free and in use pages will have a non-zero refcount. Instead we should return pages to the drivers page allocator with a zero reference count. Kernel code can then safely use kernel functions such as get_page_unless_zero(). Signed-off-by: Alistair Popple Acked-by: Felix Kuehling Cc: Jason Gunthorpe Cc: Michael Ellerman Cc: Felix Kuehling Cc: Alex Deucher Cc: Christian König Cc: Ben Skeggs Cc: Lyude Paul Cc: Ralph Campbell Cc: Alex Sierra Cc: John Hubbard Cc: Dan Williams --- This will conflict with Dan's series to fix reference counts for DAX[1]. At the moment this only makes changes for device private and coherent pages, however if DAX is fixed to remove the extra refcount then we should just be able to drop the checks for private/coherent pages and treat them the same. [1] - https://lore.kernel.org/linux-mm/166329930818.2786261.6086109734008025807.st...@dwillia2-xfh.jf.intel.com/ --- arch/powerpc/kvm/book3s_hv_uvmem.c | 2 +- drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 2 +- drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +- include/linux/memremap.h | 1 + lib/test_hmm.c | 2 +- mm/memremap.c| 9 + mm/page_alloc.c | 8 7 files changed, 22 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c index d4eacf4..9d8de68 100644 --- a/arch/powerpc/kvm/book3s_hv_uvmem.c +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c @@ -718,7 +718,7 @@ static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm) dpage = pfn_to_page(uvmem_pfn); dpage->zone_device_data = pvt; - lock_page(dpage); + zone_device_page_init(dpage); return dpage; out_clear: spin_lock(&kvmppc_uvmem_bitmap_lock); diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c index 776448b..97a6845 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c @@ -223,7 +223,7 @@ svm_migrate_get_vram_page(struct svm_range *prange, unsigned long pfn) page = pfn_to_page(pfn); svm_range_bo_ref(prange->svm_bo); page->zone_device_data = prange->svm_bo; - lock_page(page); + zone_device_page_init(page); } static void diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c index 1635661..b092988 100644 --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c @@ -326,7 +326,7 @@ nouveau_dmem_page_alloc_locked(struct nouveau_drm *drm) return NULL; } - lock_page(page); + zone_device_page_init(page); return page; } diff --git a/include/linux/memremap.h b/include/linux/memremap.h index 1901049..f68bf6d 100644 --- a/include/linux/memremap.h +++ b/include/linux/memremap.h @@ -182,6 +182,7 @@ static inline bool folio_is_device_coherent(const struct folio *folio) } #ifdef CONFIG_ZONE_DEVICE +void zone_device_page_init(struct page *page); void *memremap_pages(struct dev_pagemap *pgmap, int nid); void memunmap_pages(struct dev_pagemap *pgmap); void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap); diff --git a/lib/test_hmm.c b/lib/test_hmm.c index 89463ff..688c15d 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -627,8 +627,8 @@ static struct page *dmirror_devmem_alloc_page(struct dmirror_device *mdevice) goto error; } + zone_device_page_init(dpage); dpage->zone_device_data = rpage; - lock_page(dpage); return dpage; error: diff --git a/mm/memremap.c b/mm/memremap.c index 25029a4..1c2c038 100644 --- a/mm/memremap.c +++ b/mm/memremap.c @@ -505,8 +505,17 @@ void free_zone_device_page(struct page *page) /* * Reset the page count to 1 to prepare for handing out the page again. */ + if (page->pgmap->type != MEMORY_DEVICE_PRIVATE && + page->pgmap->type != MEMORY_DEVICE_COHERENT) + set_page_count(page, 1); +} + +void zone_device_page_init(struct page *page) +{ set_page_count(page, 1); + lock_page(page); } +EXPORT_SYMBOL_GPL(zone_device_page_init); #ifdef CONFIG_FS_DAX bool __put_devmap_managed_page_refs(struct page *page, int refs) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 9d49803..4df1e43 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -6744,6 +6744,14 @@
Re: [PATCH 12/13] drm/scheduler: rework entity flush, kill and fini
On 2022-09-29 09:21, Christian König wrote: This was buggy because when we had to wait for entities which were killed as well we would just deadlock. Instead move all the dependency handling into the callbacks so that will all happen asynchronously. Signed-off-by: Christian König --- drivers/gpu/drm/scheduler/sched_entity.c | 197 +++ 1 file changed, 92 insertions(+), 105 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 1bb1437a8fed..1d448e376811 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -139,6 +139,74 @@ bool drm_sched_entity_is_ready(struct drm_sched_entity *entity) return true; } +static void drm_sched_entity_kill_jobs_irq_work(struct irq_work *wrk) +{ + struct drm_sched_job *job = container_of(wrk, typeof(*job), work); + + drm_sched_fence_scheduled(job->s_fence); + drm_sched_fence_finished(job->s_fence); + WARN_ON(job->s_fence->parent); + job->sched->ops->free_job(job); +} + +/* Signal the scheduler finished fence when the entity in question is killed. */ +static void drm_sched_entity_kill_jobs_cb(struct dma_fence *f, + struct dma_fence_cb *cb) +{ + struct drm_sched_job *job = container_of(cb, struct drm_sched_job, +finish_cb); + int r; + + dma_fence_put(f); + + /* Wait for all dependencies to avoid data corruptions */ + while (!xa_empty(&job->dependencies)) { + f = xa_erase(&job->dependencies, job->last_dependency++); + r = dma_fence_add_callback(f, &job->finish_cb, + drm_sched_entity_kill_jobs_cb); + if (!r) + return; + + dma_fence_put(f); + } + + init_irq_work(&job->work, drm_sched_entity_kill_jobs_irq_work); + irq_work_queue(&job->work); +} + +/* Remove the entity from the scheduler and kill all pending jobs */ +static void drm_sched_entity_kill(struct drm_sched_entity *entity) +{ + struct drm_sched_job *job; + struct dma_fence *prev; + + if (!entity->rq) + return; + + spin_lock(&entity->rq_lock); + entity->stopped = true; + drm_sched_rq_remove_entity(entity->rq, entity); + spin_unlock(&entity->rq_lock); + + /* Make sure this entity is not used by the scheduler at the moment */ + wait_for_completion(&entity->entity_idle); Does it really stop processing in case more jobs are pending in entity queue already ? It probably makes sense to integrate drm_sched_entity_is_idle into drm_sched_entity_is_ready to prevent rq selecting this entity in such case + + prev = dma_fence_get(entity->last_scheduled); + while ((job = to_drm_sched_job(spsc_queue_pop(&entity->job_queue { + struct drm_sched_fence *s_fence = job->s_fence; + + dma_fence_set_error(&s_fence->finished, -ESRCH); + + dma_fence_get(&s_fence->finished); + if (!prev || dma_fence_add_callback(prev, &job->finish_cb, + drm_sched_entity_kill_jobs_cb)) + drm_sched_entity_kill_jobs_cb(NULL, &job->finish_cb); + + prev = &s_fence->finished; + } + dma_fence_put(prev); +} + /** * drm_sched_entity_flush - Flush a context entity * @@ -179,91 +247,13 @@ long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout) /* For killed process disable any more IBs enqueue right now */ last_user = cmpxchg(&entity->last_user, current->group_leader, NULL); if ((!last_user || last_user == current->group_leader) && - (current->flags & PF_EXITING) && (current->exit_code == SIGKILL)) { - spin_lock(&entity->rq_lock); - entity->stopped = true; - drm_sched_rq_remove_entity(entity->rq, entity); - spin_unlock(&entity->rq_lock); - } + (current->flags & PF_EXITING) && (current->exit_code == SIGKILL)) + drm_sched_entity_kill(entity); This reverts 'drm/scheduler: only kill entity if last user is killed v2' and so might break this use case - when entity gets through FD into child process. Why this needs to be removed ? Andrey return ret; } EXPORT_SYMBOL(drm_sched_entity_flush); -static void drm_sched_entity_kill_jobs_irq_work(struct irq_work *wrk) -{ - struct drm_sched_job *job = container_of(wrk, typeof(*job), work); - - drm_sched_fence_finished(job->s_fence); - WARN_ON(job->s_fence->parent); - job->sched->ops->free_job(job); -} - - -/* Signal the scheduler finished fence when the entity in question is killed. */ -static void drm_sched_entity_kill_jobs_cb(struct dma_fence *f, -
Re: [PATCH] drm/sched: Add FIFO sched policy to run queue
Inlined: On 2022-09-29 14:46, Luben Tuikov wrote: > From: Andrey Grodzovsky > > When many entities are competing for the same run queue > on the same scheduler, we observe an unusually long wait > times and some jobs get starved. This has been observed on GPUVis. > > The issue is due to the Round Robin policy used by schedulers > to pick up the next entity's job queue for execution. Under stress > of many entities and long job queues within entity some > jobs could be stuck for very long time in it's entity's > queue before being popped from the queue and executed > while for other entities with smaller job queues a job > might execute earlier even though that job arrived later > then the job in the long queue. > > Fix: > Add FIFO selection policy to entities in run queue, chose next entity > on run queue in such order that if job on one entity arrived > earlier then job on another entity the first job will start > executing earlier regardless of the length of the entity's job > queue. > > v2: > Switch to rb tree structure for entities based on TS of > oldest job waiting in the job queue of an entity. Improves next > entity extraction to O(1). Entity TS update > O(log N) where N is the number of entities in the run-queue > > Drop default option in module control parameter. > > v3: > Various cosmetical fixes and minor refactoring of fifo update function. > (Luben) > > v4: > Switch drm_sched_rq_select_entity_fifo to in order search (Luben) > > v5: Fix up drm_sched_rq_select_entity_fifo loop (Luben) > > v6: Add missing drm_sched_rq_remove_fifo_locked > > Cc: Luben Tuikov > Cc: Christian König > Cc: Direct Rendering Infrastructure - Development > > Cc: AMD Graphics > Signed-off-by: Andrey Grodzovsky > Tested-by: Yunxiang Li (Teddy) > --- > drivers/gpu/drm/scheduler/sched_entity.c | 26 ++- > drivers/gpu/drm/scheduler/sched_main.c | 97 +++- > include/drm/gpu_scheduler.h | 32 > 3 files changed, 149 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/scheduler/sched_entity.c > b/drivers/gpu/drm/scheduler/sched_entity.c > index 6b25b2f4f5a308..f3ffce3c9304d5 100644 > --- a/drivers/gpu/drm/scheduler/sched_entity.c > +++ b/drivers/gpu/drm/scheduler/sched_entity.c > @@ -73,6 +73,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity, > entity->priority = priority; > entity->sched_list = num_sched_list > 1 ? sched_list : NULL; > entity->last_scheduled = NULL; > + RB_CLEAR_NODE(&entity->rb_tree_node); > > if(num_sched_list) > entity->rq = &sched_list[0]->sched_rq[entity->priority]; > @@ -417,14 +418,16 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct > drm_sched_entity *entity) > > sched_job = to_drm_sched_job(spsc_queue_peek(&entity->job_queue)); > if (!sched_job) > - return NULL; > + goto skip; > > while ((entity->dependency = > drm_sched_job_dependency(sched_job, entity))) { > trace_drm_sched_job_wait_dep(sched_job, entity->dependency); > > - if (drm_sched_entity_add_dependency_cb(entity)) > - return NULL; > + if (drm_sched_entity_add_dependency_cb(entity)) { > + sched_job = NULL; > + goto skip; > + } > } > > /* skip jobs from entity that marked guilty */ > @@ -443,6 +446,16 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct > drm_sched_entity *entity) > smp_wmb(); > > spsc_queue_pop(&entity->job_queue); > + > + /* > + * It's when head job is extracted we can access the next job (or empty) > + * queue and update the entity location in the min heap accordingly. > + */ > +skip: > + if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) > + drm_sched_rq_update_fifo(entity, > + (sched_job ? sched_job->submit_ts : > ktime_get())); > + > return sched_job; > } If there's a next job, you should update the entity's timestamp to that of the next job. Also you shouldn't have to update the timestamp when there is no next job. And to be true to the comment above, you should have this here: -skip: - if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) - drm_sched_rq_update_fifo(entity, -(sched_job ? sched_job->submit_ts : ktime_get())); + if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) { + struct drm_sched_job *next; + + next = to_drm_sched_job(spsc_queue_peek(&entity->job_queue)); + if (next) + drm_sched_rq_update_fifo(entity, next->submit_ts); + } +Out: return sched_job; And then change the "goto skip;" to "goto Out;" in the preceding code. This way you update the entity's timestamp to that of the next (oldest) job in the entity's job queue, if there is a next
Re: [PATCH] drm/atomic-helper: Don't allocated plane state in CRTC check
On Thu, Sep 29, 2022 at 04:07:14PM +0200, Thomas Zimmermann wrote: > In drm_atomic_helper_check_crtc_state(), do not add a new plane state > to the global state if it does not exist already. Adding a new plane > state will results in overhead for the plane during the atomic-commit > step. > > For the test in drm_atomic_helper_check_crtc_state() to succeed, it is > important that the CRTC has an enabled primary plane after the commit. > This can be a newly enabled plane or an already enabled plane. So if a > plane is not part of the commit, use the plane's existing state. The new > helper drm_atomic_get_next_plane_state() returns the correct instance. > > Signed-off-by: Thomas Zimmermann > Fixes: d6b9af1097fe ("drm/atomic-helper: Add helper > drm_atomic_helper_check_crtc_state()") > Cc: Thomas Zimmermann > Cc: Jocelyn Falempe > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/drm_atomic_helper.c | 4 +--- > include/drm/drm_atomic.h| 20 > 2 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 98cc3137c062..463d4f3fa443 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -960,9 +960,7 @@ int drm_atomic_helper_check_crtc_state(struct > drm_crtc_state *crtc_state, > > if (plane->type != DRM_PLANE_TYPE_PRIMARY) > continue; > - plane_state = drm_atomic_get_plane_state(state, plane); > - if (IS_ERR(plane_state)) > - return PTR_ERR(plane_state); > + plane_state = drm_atomic_get_next_plane_state(state, > plane); > if (plane_state->fb && plane_state->crtc) { Hmm. Why this is even looking at these. If the plane is in the crtc's plane_mask then surely plane_state->crtc must already point to this crtc? And I don't think ->fb and ->crtc are allowed to disagree, so if one is set then surely the other one must be as well or we'd return an error at some point somewhere? > has_primary_plane = true; > break; > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 10b1990bc1f6..4006f2d459e3 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -623,6 +623,26 @@ drm_atomic_get_new_plane_state(struct drm_atomic_state > *state, > return state->planes[drm_plane_index(plane)].new_state; > } > > +/** > + * drm_atomic_get_next_plane_state - get plane state after atomic commit > + * @state: global atomic state object > + * @plane: plane to grab > + * > + * This function returns the plane state that the given plane will have > + * after the atomic commit. This will be the new plane state if the plane > + * is part of the global atomic state, or the current state otherwise. > + */ > +static inline struct drm_plane_state * > +drm_atomic_get_next_plane_state(struct drm_atomic_state *state, > + struct drm_plane *plane) > +{ > + struct drm_plane_state *plane_state = > drm_atomic_get_new_plane_state(state, plane); > + > + if (plane_state) > + return plane_state; > + return plane->state; You're not holding the appropriate lock so that does not look safe. Even if you know somewhere, somehow this sort of thing to be safe I don't think it's a good idea to promote this with any kind of official function because someone will shoot themselves in the foot with it. > +} > + > /** > * drm_atomic_get_existing_connector_state - get connector state, if it > exists > * @state: global atomic state object > -- > 2.37.3 -- Ville Syrjälä Intel
[PATCH] drm/sched: Add FIFO sched policy to run queue
From: Andrey Grodzovsky When many entities are competing for the same run queue on the same scheduler, we observe an unusually long wait times and some jobs get starved. This has been observed on GPUVis. The issue is due to the Round Robin policy used by schedulers to pick up the next entity's job queue for execution. Under stress of many entities and long job queues within entity some jobs could be stuck for very long time in it's entity's queue before being popped from the queue and executed while for other entities with smaller job queues a job might execute earlier even though that job arrived later then the job in the long queue. Fix: Add FIFO selection policy to entities in run queue, chose next entity on run queue in such order that if job on one entity arrived earlier then job on another entity the first job will start executing earlier regardless of the length of the entity's job queue. v2: Switch to rb tree structure for entities based on TS of oldest job waiting in the job queue of an entity. Improves next entity extraction to O(1). Entity TS update O(log N) where N is the number of entities in the run-queue Drop default option in module control parameter. v3: Various cosmetical fixes and minor refactoring of fifo update function. (Luben) v4: Switch drm_sched_rq_select_entity_fifo to in order search (Luben) v5: Fix up drm_sched_rq_select_entity_fifo loop (Luben) v6: Add missing drm_sched_rq_remove_fifo_locked Cc: Luben Tuikov Cc: Christian König Cc: Direct Rendering Infrastructure - Development Cc: AMD Graphics Signed-off-by: Andrey Grodzovsky Tested-by: Yunxiang Li (Teddy) --- drivers/gpu/drm/scheduler/sched_entity.c | 26 ++- drivers/gpu/drm/scheduler/sched_main.c | 97 +++- include/drm/gpu_scheduler.h | 32 3 files changed, 149 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 6b25b2f4f5a308..f3ffce3c9304d5 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -73,6 +73,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity, entity->priority = priority; entity->sched_list = num_sched_list > 1 ? sched_list : NULL; entity->last_scheduled = NULL; + RB_CLEAR_NODE(&entity->rb_tree_node); if(num_sched_list) entity->rq = &sched_list[0]->sched_rq[entity->priority]; @@ -417,14 +418,16 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct drm_sched_entity *entity) sched_job = to_drm_sched_job(spsc_queue_peek(&entity->job_queue)); if (!sched_job) - return NULL; + goto skip; while ((entity->dependency = drm_sched_job_dependency(sched_job, entity))) { trace_drm_sched_job_wait_dep(sched_job, entity->dependency); - if (drm_sched_entity_add_dependency_cb(entity)) - return NULL; + if (drm_sched_entity_add_dependency_cb(entity)) { + sched_job = NULL; + goto skip; + } } /* skip jobs from entity that marked guilty */ @@ -443,6 +446,16 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct drm_sched_entity *entity) smp_wmb(); spsc_queue_pop(&entity->job_queue); + + /* +* It's when head job is extracted we can access the next job (or empty) +* queue and update the entity location in the min heap accordingly. +*/ +skip: + if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) + drm_sched_rq_update_fifo(entity, +(sched_job ? sched_job->submit_ts : ktime_get())); + return sched_job; } @@ -502,11 +515,13 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job) { struct drm_sched_entity *entity = sched_job->entity; bool first; + ktime_t ts = ktime_get(); trace_drm_sched_job(sched_job, entity); atomic_inc(entity->rq->sched->score); WRITE_ONCE(entity->last_user, current->group_leader); first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node); + sched_job->submit_ts = ts; /* first job wakes up scheduler */ if (first) { @@ -518,8 +533,13 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job) DRM_ERROR("Trying to push to a killed entity\n"); return; } + drm_sched_rq_add_entity(entity->rq, entity); spin_unlock(&entity->rq_lock); + + if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) + drm_sched_rq_update_fifo(entity, ts); + drm_sched_wakeup(entity->rq->sched); } } diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c ind
[PATCH v3 6/6] amd/display: indicate support for atomic async page-flips on DC
amdgpu_dm_commit_planes() already sets the flip_immediate flag for async page-flips. This flag is used to set the UNP_FLIP_CONTROL register. Thus, no additional change is required to handle async page-flips with the atomic uAPI. v2: make it clear this commit is about DC and not only DCN Signed-off-by: Simon Ser Reviewed-by: André Almeida Reviewed-by: Alex Deucher Cc: Joshua Ashton Cc: Melissa Wen Cc: Harry Wentland Cc: Nicholas Kazlauskas --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 7500e82cf06a..44235345fd57 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3808,7 +3808,6 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev) adev_to_drm(adev)->mode_config.prefer_shadow = 1; /* indicates support for immediate flip */ adev_to_drm(adev)->mode_config.async_page_flip = true; - adev_to_drm(adev)->mode_config.atomic_async_page_flip_not_supported = true; adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; -- 2.37.3
[PATCH v3 2/6] amd/display: only accept async flips for fast updates
Up until now, amdgpu was silently degrading to vsync when user-space requested an async flip but the hardware didn't support it. The hardware doesn't support immediate flips when the update changes the FB pitch, the DCC state, the rotation, enables or disables CRTCs or planes, etc. This is reflected in the dm_crtc_state.update_type field: UPDATE_TYPE_FAST means that immediate flip is supported. Silently degrading async flips to vsync is not the expected behavior from a uAPI point-of-view. Xorg expects async flips to fail if unsupported, to be able to fall back to a blit. i915 already behaves this way. This patch aligns amdgpu with uAPI expectations and returns a failure when an async flip is not possible. v2: new patch Signed-off-by: Simon Ser Reviewed-by: André Almeida Reviewed-by: Alex Deucher Cc: Joshua Ashton Cc: Melissa Wen Cc: Harry Wentland Cc: Nicholas Kazlauskas --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 10 ++ 2 files changed, 18 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 7b19f444624c..44235345fd57 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7629,7 +7629,15 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, /* * Only allow immediate flips for fast updates that don't * change FB pitch, DCC state, rotation or mirroing. +* +* dm_crtc_helper_atomic_check() only accepts async flips with +* fast updates. */ + if (crtc->state->async_flip && + acrtc_state->update_type != UPDATE_TYPE_FAST) + drm_warn_once(state->dev, + "[PLANE:%d:%s] async flip with non-fast update\n", + plane->base.id, plane->name); bundle->flip_addrs[planes_count].flip_immediate = crtc->state->async_flip && acrtc_state->update_type == UPDATE_TYPE_FAST; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c index 594fe8a4d02b..97ead857f507 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c @@ -388,6 +388,16 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, return -EINVAL; } + /* Only allow async flips for fast updates that don't change the FB +* pitch, the DCC state, rotation, etc. */ + if (crtc_state->async_flip && + dm_crtc_state->update_type != UPDATE_TYPE_FAST) { + drm_dbg_atomic(crtc->dev, + "[CRTC:%d:%s] async flips are only supported for fast updates\n", + crtc->base.id, crtc->name); + return -EINVAL; + } + /* In some use cases, like reset, no stream is attached */ if (!dm_crtc_state->stream) return 0; -- 2.37.3
[PATCH v3 5/6] drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
This new kernel capability indicates whether async page-flips are supported via the atomic uAPI. DRM clients can use it to check for support before feeding DRM_MODE_PAGE_FLIP_ASYNC to the kernel. Make it clear that DRM_CAP_ASYNC_PAGE_FLIP is for legacy uAPI only. Signed-off-by: Simon Ser Reviewed-by: André Almeida Reviewed-by: Alex Deucher Cc: Daniel Vetter Cc: Joshua Ashton Cc: Melissa Wen Cc: Harry Wentland Cc: Nicholas Kazlauskas Cc: Ville Syrjälä --- drivers/gpu/drm/drm_ioctl.c | 5 + include/uapi/drm/drm.h | 10 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index ca2a6e6101dc..5b1591e2b46c 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -302,6 +302,11 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ case DRM_CAP_CRTC_IN_VBLANK_EVENT: req->value = 1; break; + case DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP: + req->value = drm_core_check_feature(dev, DRIVER_ATOMIC) && +dev->mode_config.async_page_flip && + !dev->mode_config.atomic_async_page_flip_not_supported; + break; default: return -EINVAL; } diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 642808520d92..b1962628ecda 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -706,7 +706,8 @@ struct drm_gem_open { /** * DRM_CAP_ASYNC_PAGE_FLIP * - * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC. + * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for legacy + * page-flips. */ #define DRM_CAP_ASYNC_PAGE_FLIP0x7 /** @@ -767,6 +768,13 @@ struct drm_gem_open { * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects". */ #define DRM_CAP_SYNCOBJ_TIMELINE 0x14 +/** + * DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP + * + * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for atomic + * commits. + */ +#define DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP 0x15 /* DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap { -- 2.37.3
[PATCH v3 3/6] drm: introduce drm_mode_config.atomic_async_page_flip_not_supported
This new field indicates whether the driver has the necessary logic to support async page-flips via the atomic uAPI. This is leveraged by the next commit to allow user-space to use this functionality. All atomic drivers setting drm_mode_config.async_page_flip are updated to also set drm_mode_config.atomic_async_page_flip_not_supported. We will gradually check and update these drivers to properly handle drm_crtc_state.async_flip in their atomic logic. The goal of this negative flag is the same as fb_modifiers_not_supported: we want to eventually get rid of all drivers missing atomic support for async flips. New drivers should not set this flag, instead they should support atomic async flips (if they support async flips at all). IOW, we don't want more drivers with async flip support for legacy but not atomic. v2: only set the flag on atomic drivers (remove it on amdgpu DCE and on radeon) Signed-off-by: Simon Ser Reviewed-by: André Almeida Reviewed-by: Alex Deucher Cc: Daniel Vetter Cc: Joshua Ashton Cc: Melissa Wen Cc: Harry Wentland Cc: Nicholas Kazlauskas Cc: Ville Syrjälä --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 + drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 1 + drivers/gpu/drm/i915/display/intel_display.c | 1 + drivers/gpu/drm/nouveau/nouveau_display.c | 1 + drivers/gpu/drm/vc4/vc4_kms.c | 1 + include/drm/drm_mode_config.h | 11 +++ 6 files changed, 16 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 44235345fd57..7500e82cf06a 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3808,6 +3808,7 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev) adev_to_drm(adev)->mode_config.prefer_shadow = 1; /* indicates support for immediate flip */ adev_to_drm(adev)->mode_config.async_page_flip = true; + adev_to_drm(adev)->mode_config.atomic_async_page_flip_not_supported = true; adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index f7e7f4e919c7..ffb3a2fa797f 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -639,6 +639,7 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev) dev->mode_config.max_height = dc->desc->max_height; dev->mode_config.funcs = &mode_config_funcs; dev->mode_config.async_page_flip = true; + dev->mode_config.atomic_async_page_flip_not_supported = true; return 0; } diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 40fbf8a296e2..e025b3499c9d 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8621,6 +8621,7 @@ static void intel_mode_config_init(struct drm_i915_private *i915) mode_config->helper_private = &intel_mode_config_funcs; mode_config->async_page_flip = HAS_ASYNC_FLIPS(i915); + mode_config->atomic_async_page_flip_not_supported = true; /* * Maximum framebuffer dimensions, chosen to match diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index a2f5df568ca5..2b5c4f24aedd 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -699,6 +699,7 @@ nouveau_display_create(struct drm_device *dev) dev->mode_config.async_page_flip = false; else dev->mode_config.async_page_flip = true; + dev->mode_config.atomic_async_page_flip_not_supported = true; drm_kms_helper_poll_init(dev); drm_kms_helper_poll_disable(dev); diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 4419e810103d..3fe59c6b2cf0 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -1047,6 +1047,7 @@ int vc4_kms_load(struct drm_device *dev) dev->mode_config.helper_private = &vc4_mode_config_helpers; dev->mode_config.preferred_depth = 24; dev->mode_config.async_page_flip = true; + dev->mode_config.atomic_async_page_flip_not_supported = true; ret = vc4_ctm_obj_init(vc4); if (ret) diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 6b5e01295348..1b535d94f2f4 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -917,6 +917,17 @@ struct drm_mode_config { */ bool async_page_flip; + /** +* @atomic_async_page_flip_not_supported: +* +* If true, the driver does not support async page-flips with the +* atomic uAPI. This is only used by old drivers which
[PATCH v3 4/6] drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits
If the driver supports it, allow user-space to supply the DRM_MODE_PAGE_FLIP_ASYNC flag to request an async page-flip. Set drm_crtc_state.async_flip accordingly. Document that drivers will reject atomic commits if an async flip isn't possible. This allows user-space to fall back to something else. For instance, Xorg falls back to a blit. Another option is to wait as close to the next vblank as possible before performing the page-flip to reduce latency. v2: document new uAPI v3: add comment about Intel hardware which needs one last sync page-flip before being able to switch to async (Ville, Pekka) Signed-off-by: Simon Ser Co-developed-by: André Almeida Signed-off-by: André Almeida Reviewed-by: André Almeida Reviewed-by: Alex Deucher Cc: Daniel Vetter Cc: Joshua Ashton Cc: Melissa Wen Cc: Harry Wentland Cc: Nicholas Kazlauskas Cc: Ville Syrjälä --- drivers/gpu/drm/drm_atomic_uapi.c | 28 +--- include/uapi/drm/drm_mode.h | 9 + 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 79730fa1dd8e..ee24ed7e2edb 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1278,6 +1278,18 @@ static void complete_signaling(struct drm_device *dev, kfree(fence_state); } +static void +set_async_flip(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + crtc_state->async_flip = true; + } +} + int drm_mode_atomic_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { @@ -1318,9 +1330,16 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, } if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) { - drm_dbg_atomic(dev, - "commit failed: invalid flag DRM_MODE_PAGE_FLIP_ASYNC\n"); - return -EINVAL; + if (!dev->mode_config.async_page_flip) { + drm_dbg_atomic(dev, + "commit failed: DRM_MODE_PAGE_FLIP_ASYNC not supported\n"); + return -EINVAL; + } + if (dev->mode_config.atomic_async_page_flip_not_supported) { + drm_dbg_atomic(dev, + "commit failed: DRM_MODE_PAGE_FLIP_ASYNC not supported with atomic\n"); + return -EINVAL; + } } /* can't test and expect an event at the same time. */ @@ -1418,6 +1437,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, if (ret) goto out; + if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) + set_async_flip(state); + if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) { ret = drm_atomic_check_only(state); } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 86a292c3185a..b39e78117b18 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -942,6 +942,15 @@ struct hdr_output_metadata { * Request that the page-flip is performed as soon as possible, ie. with no * delay due to waiting for vblank. This may cause tearing to be visible on * the screen. + * + * When used with atomic uAPI, the driver will return an error if the hardware + * doesn't support performing an asynchronous page-flip for this update. + * User-space should handle this, e.g. by falling back to a regular page-flip. + * + * Note, some hardware might need to perform one last synchronous page-flip + * before being able to switch to asynchronous page-flips. As an exception, + * the driver will return success even though that first page-flip is not + * asynchronous. */ #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4 -- 2.37.3
[PATCH v3 1/6] drm: document DRM_MODE_PAGE_FLIP_ASYNC
This is a subset of [1], included here because a subsequent patch needs to document the behavior of this flag under the atomic uAPI. v2: new patch [1]: https://patchwork.freedesktop.org/patch/500177/ Signed-off-by: Simon Ser Reviewed-by: André Almeida Reviewed-by: Alex Deucher --- include/uapi/drm/drm_mode.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index fa953309d9ce..86a292c3185a 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -936,6 +936,13 @@ struct hdr_output_metadata { }; #define DRM_MODE_PAGE_FLIP_EVENT 0x01 +/** + * DRM_MODE_PAGE_FLIP_ASYNC + * + * Request that the page-flip is performed as soon as possible, ie. with no + * delay due to waiting for vblank. This may cause tearing to be visible on + * the screen. + */ #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4 #define DRM_MODE_PAGE_FLIP_TARGET_RELATIVE 0x8 -- 2.37.3
[PATCH v3 0/6] Add support for atomic async page-flips
This series adds support for DRM_MODE_PAGE_FLIP_ASYNC for atomic commits, aka. "immediate flip" (which might result in tearing). The feature was only available via the legacy uAPI, however for gaming use-cases it may be desirable to enable it via the atomic uAPI too. - Patchwork: https://patchwork.freedesktop.org/series/107683/ - User-space patch: https://github.com/Plagman/gamescope/pull/595 - IGT patch: https://patchwork.freedesktop.org/series/107681/ Main changes in v2: add docs, fail atomic commit if async flip isn't possible. Changes in v3: add a note in the documentation about Intel hardware, add R-b tags. Tested on an AMD Picasso iGPU (Simon) and an AMD Vangogh GPU (André). Simon Ser (6): drm: document DRM_MODE_PAGE_FLIP_ASYNC amd/display: only accept async flips for fast updates drm: introduce drm_mode_config.atomic_async_page_flip_not_supported drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP amd/display: indicate support for atomic async page-flips on DC .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++ .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 10 +++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 1 + drivers/gpu/drm/drm_atomic_uapi.c | 28 +-- drivers/gpu/drm/drm_ioctl.c | 5 drivers/gpu/drm/i915/display/intel_display.c | 1 + drivers/gpu/drm/nouveau/nouveau_display.c | 1 + drivers/gpu/drm/vc4/vc4_kms.c | 1 + include/drm/drm_mode_config.h | 11 include/uapi/drm/drm.h| 10 ++- include/uapi/drm/drm_mode.h | 16 +++ 11 files changed, 88 insertions(+), 4 deletions(-) -- 2.37.3
Re: [PATCH 1/2] drm/scheduler: fix fence ref counting
Series is Reviewed-by: Andrey Grodzovsky Andrey On 2022-09-29 14:01, Christian König wrote: We leaked dependency fences when processes were beeing killed. Additional to that grab a reference to the last scheduled fence. Signed-off-by: Christian König --- drivers/gpu/drm/scheduler/sched_entity.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 6b25b2f4f5a3..7ef1a086a6fb 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -207,6 +207,7 @@ static void drm_sched_entity_kill_jobs_cb(struct dma_fence *f, struct drm_sched_job *job = container_of(cb, struct drm_sched_job, finish_cb); + dma_fence_put(f); INIT_WORK(&job->work, drm_sched_entity_kill_jobs_work); schedule_work(&job->work); } @@ -234,8 +235,10 @@ static void drm_sched_entity_kill_jobs(struct drm_sched_entity *entity) struct drm_sched_fence *s_fence = job->s_fence; /* Wait for all dependencies to avoid data corruptions */ - while ((f = drm_sched_job_dependency(job, entity))) + while ((f = drm_sched_job_dependency(job, entity))) { dma_fence_wait(f, false); + dma_fence_put(f); + } drm_sched_fence_scheduled(s_fence); dma_fence_set_error(&s_fence->finished, -ESRCH); @@ -250,6 +253,7 @@ static void drm_sched_entity_kill_jobs(struct drm_sched_entity *entity) continue; } + dma_fence_get(entity->last_scheduled); r = dma_fence_add_callback(entity->last_scheduled, &job->finish_cb, drm_sched_entity_kill_jobs_cb);
Re: [PATCH 3/3] dma-buf: Check status of enable-signaling bit on debug
Am 29.09.22 um 20:30 schrieb Yadav, Arvind: On 9/29/2022 11:48 PM, Christian König wrote: Am 27.09.22 um 19:24 schrieb Arvind Yadav: Fence signaling must be enabled to make sure that the dma_fence_is_signaled_locked() function ever returns true. Since drivers and implementations sometimes mess this up, this ensures correct behaviour when DMABUF_DEBUG_ENABLE_SIGNALING is used during debugging. This should make any implementation bugs resulting in not signaled fences much more obvious. Are all IGT tests now passing with this? That would be a bit unfortunate because it means we still have missed the bug in drm_syncobj. IGT has these test cases related to syncobj (syncobj_basic, syncobj_timeline and syncobj_wait)and all are passing. I will check syncobj and let you know. Maybe CC the Intel list and let their CI systems take a look. That's usually rather valuable. Thanks, Christian. ~Arvind Christian. Signed-off-by: Arvind Yadav --- include/linux/dma-fence.h | 5 + 1 file changed, 5 insertions(+) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 775cdc0b4f24..5156dc6be0a6 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -398,6 +398,11 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence); static inline bool dma_fence_is_signaled_locked(struct dma_fence *fence) { +#ifdef CONFIG_DMABUF_DEBUG_ENABLE_SIGNALING + if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) + return false; +#endif + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return true;
Re: [PATCH v2 1/8] mm/memory.c: Fix race when faulting a device private page
On 2022-09-28 08:01, Alistair Popple wrote: When the CPU tries to access a device private page the migrate_to_ram() callback associated with the pgmap for the page is called. However no reference is taken on the faulting page. Therefore a concurrent migration of the device private page can free the page and possibly the underlying pgmap. This results in a race which can crash the kernel due to the migrate_to_ram() function pointer becoming invalid. It also means drivers can't reliably read the zone_device_data field because the page may have been freed with memunmap_pages(). Close the race by getting a reference on the page while holding the ptl to ensure it has not been freed. Unfortunately the elevated reference count will cause the migration required to handle the fault to fail. To avoid this failure pass the faulting page into the migrate_vma functions so that if an elevated reference count is found it can be checked to see if it's expected or not. Do we really have to drag the fault_page all the way into the migrate structure? Is the elevated refcount still needed at that time? Maybe it would be easier to drop the refcount early in the ops->migrage_to_ram callbacks, so we won't have to deal with it in all the migration code. Regards, Felix Signed-off-by: Alistair Popple Cc: Jason Gunthorpe Cc: John Hubbard Cc: Ralph Campbell Cc: Michael Ellerman Cc: Felix Kuehling Cc: Lyude Paul --- arch/powerpc/kvm/book3s_hv_uvmem.c | 15 ++- drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 17 +++-- drivers/gpu/drm/amd/amdkfd/kfd_migrate.h | 2 +- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 11 +--- include/linux/migrate.h | 8 ++- lib/test_hmm.c | 7 ++--- mm/memory.c | 16 +++- mm/migrate.c | 34 ++--- mm/migrate_device.c | 18 + 9 files changed, 87 insertions(+), 41 deletions(-) diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c index 5980063..d4eacf4 100644 --- a/arch/powerpc/kvm/book3s_hv_uvmem.c +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c @@ -508,10 +508,10 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm) static int __kvmppc_svm_page_out(struct vm_area_struct *vma, unsigned long start, unsigned long end, unsigned long page_shift, - struct kvm *kvm, unsigned long gpa) + struct kvm *kvm, unsigned long gpa, struct page *fault_page) { unsigned long src_pfn, dst_pfn = 0; - struct migrate_vma mig; + struct migrate_vma mig = { 0 }; struct page *dpage, *spage; struct kvmppc_uvmem_page_pvt *pvt; unsigned long pfn; @@ -525,6 +525,7 @@ static int __kvmppc_svm_page_out(struct vm_area_struct *vma, mig.dst = &dst_pfn; mig.pgmap_owner = &kvmppc_uvmem_pgmap; mig.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE; + mig.fault_page = fault_page; /* The requested page is already paged-out, nothing to do */ if (!kvmppc_gfn_is_uvmem_pfn(gpa >> page_shift, kvm, NULL)) @@ -580,12 +581,14 @@ static int __kvmppc_svm_page_out(struct vm_area_struct *vma, static inline int kvmppc_svm_page_out(struct vm_area_struct *vma, unsigned long start, unsigned long end, unsigned long page_shift, - struct kvm *kvm, unsigned long gpa) + struct kvm *kvm, unsigned long gpa, + struct page *fault_page) { int ret; mutex_lock(&kvm->arch.uvmem_lock); - ret = __kvmppc_svm_page_out(vma, start, end, page_shift, kvm, gpa); + ret = __kvmppc_svm_page_out(vma, start, end, page_shift, kvm, gpa, + fault_page); mutex_unlock(&kvm->arch.uvmem_lock); return ret; @@ -736,7 +739,7 @@ static int kvmppc_svm_page_in(struct vm_area_struct *vma, bool pagein) { unsigned long src_pfn, dst_pfn = 0; - struct migrate_vma mig; + struct migrate_vma mig = { 0 }; struct page *spage; unsigned long pfn; struct page *dpage; @@ -994,7 +997,7 @@ static vm_fault_t kvmppc_uvmem_migrate_to_ram(struct vm_fault *vmf) if (kvmppc_svm_page_out(vmf->vma, vmf->address, vmf->address + PAGE_SIZE, PAGE_SHIFT, - pvt->kvm, pvt->gpa)) + pvt->kvm, pvt->gpa, vmf->page)) return VM_FAULT_SIGBUS; else return 0; diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c index b059a77..776448b 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c @@ -409,7 +409,7 @@ svm_migra
Re: [PATCH 3/3] dma-buf: Check status of enable-signaling bit on debug
On 9/29/2022 11:48 PM, Christian König wrote: Am 27.09.22 um 19:24 schrieb Arvind Yadav: Fence signaling must be enabled to make sure that the dma_fence_is_signaled_locked() function ever returns true. Since drivers and implementations sometimes mess this up, this ensures correct behaviour when DMABUF_DEBUG_ENABLE_SIGNALING is used during debugging. This should make any implementation bugs resulting in not signaled fences much more obvious. Are all IGT tests now passing with this? That would be a bit unfortunate because it means we still have missed the bug in drm_syncobj. IGT has these test cases related to syncobj (syncobj_basic, syncobj_timeline and syncobj_wait)and all are passing. I will check syncobj and let you know. ~Arvind Christian. Signed-off-by: Arvind Yadav --- include/linux/dma-fence.h | 5 + 1 file changed, 5 insertions(+) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 775cdc0b4f24..5156dc6be0a6 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -398,6 +398,11 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence); static inline bool dma_fence_is_signaled_locked(struct dma_fence *fence) { +#ifdef CONFIG_DMABUF_DEBUG_ENABLE_SIGNALING + if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) + return false; +#endif + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return true;
Re: [PATCH v4 15/15] vfio: Add struct device to vfio_device
On Thu, 29 Sep 2022 14:49:56 -0300 Jason Gunthorpe wrote: > On Thu, Sep 29, 2022 at 10:55:19AM -0600, Alex Williamson wrote: > > Hi Kevin, > > > > This introduced the regression discovered here: > > > > https://lore.kernel.org/all/20220928125650.0a2ea297.alex.william...@redhat.com/ > > > > Seems we're not releasing the resources when removing an mdev. This is > > a regression, so it needs to be fixed or reverted before the merge > > window. Thanks, > > My guess at the fix for this: > > https://lore.kernel.org/r/0-v1-013609965fe8+9d-vfio_gvt_unregister_...@nvidia.com Indeed this seems to work I'll look for acks and further reviews from Intel folks. Thanks! Alex
Re: [PATCH v3] drm: document uAPI page-flip flags
On Thu, Sep 29, 2022 at 8:11 PM Simon Ser wrote: > > On Wednesday, September 28th, 2022 at 12:06, Pekka Paalanen > wrote: > > > > +/** > > > + * DRM_MODE_PAGE_FLIP_FLAGS > > > + * > > > + * Bitmask of flags suitable for &drm_mode_crtc_page_flip_target.flags. > > > > Should this mention also drm_mode_crtc_page_flip.flags? > > > > UAPI header defines both structs. > > drm_mode_crtc_page_flip is "v1", drm_mode_crtc_page_flip_target is "v2". The > latter just replaces a reserved field with a new one. So I figured "v1" is > mostly kept around for backwards compat and everybody should use "v2" for > simplicity's sake. > > > > +/** > > > + * DRM_MODE_ATOMIC_ALLOW_MODESET > > > + * > > > + * Allow the update to result in temporary or transient visible > > > artifacts while > > > + * the update is being applied. Applying the update may also take > > > significantly > > > + * more time than a page flip. All visual artifacts will disappear by > > > the time > > > + * the update is completed, as signalled throught the vblank event's > > > timestamp > > > > typo: throught > > Indeed! > > > > + * (see struct drm_event_vblank). > > > + * > > > + * This flag must be set when the KMS update might cause visible > > > artifacts. > > > + * Without this flag such KMS update will return a EINVAL error. What > > > kind of > > > + * update may cause visible artifacts depends on the driver and the > > > hardware. > > > + * User-space that needs to know beforehand if an update might cause > > > visible > > > + * artifacts can use &DRM_MODE_ATOMIC_TEST_ONLY without > > > + * &DRM_MODE_ATOMIC_ALLOW_MODESET to see if it fails. > > > + * > > > + * To the best of the driver's knowledge, visual artifacts are > > > guaranteed to > > > + * not appear when this flag is not set. Some sinks might display visual > > > + * artifacts outside of the driver's control. > > > > Ok, so we kept the "visual artifacts" semantics and allow monitors to > > do otherwise. > > > > I'm still not sure what this means for things like infoframe data where > > changing a certain field (e.g. HDR_OUTPUT_METADATA structure's EOTF > > field) has a high risk of causing a visual glitch. I cannot imagine why > > a monitor manufacturer would not be able to avoid the glitch if they > > wanted to. The glitch might or might not happen, and we cannot know in > > advance or afterwards whether it did happen, but it is probable that > > many monitors will glitch. > > > > I think "To the best of driver's knowledge" means that if someone > > reports a monitor to glitch, the driver/kernel would need to add that > > field to the "needs modeset" set. But doing so can regress other > > monitors that didn't glitch, so it needs to be a monitor quirk. > > > > This is not something for this patch, but would it be possible to agree > > on the kernel development policy here? Should glitches be reported and > > added to monitor quirks list? Or should drivers be defensive from the > > start and require modeset if the field is known to cause glitches on > > some monitors? > > Good question. I am not sure there is even a desire to have a quirks table for > this among driver developers. > > This reminds me of some VRR displays showing visual artifacts when user-space > changes its page-flip rate. If we took this definition by the letter, the > kernel should require ALLOW_MODESET at each page-flip... (The better solution > would be to just to have a slew rate implemented in the kernel. I think it's > implemented to some extent in amdgpu/i915 but still some monitors are having > issues.) > Honestly the right thing to do here is give up on the "no visual artifacts" phrashing. This is really about not having to interrupt the stream which definitely results in visual artifacts but avoiding that doesn't guarantee no artifacts.
Re: [PATCH 3/3] dma-buf: Check status of enable-signaling bit on debug
Am 27.09.22 um 19:24 schrieb Arvind Yadav: Fence signaling must be enabled to make sure that the dma_fence_is_signaled_locked() function ever returns true. Since drivers and implementations sometimes mess this up, this ensures correct behaviour when DMABUF_DEBUG_ENABLE_SIGNALING is used during debugging. This should make any implementation bugs resulting in not signaled fences much more obvious. Are all IGT tests now passing with this? That would be a bit unfortunate because it means we still have missed the bug in drm_syncobj. Christian. Signed-off-by: Arvind Yadav --- include/linux/dma-fence.h | 5 + 1 file changed, 5 insertions(+) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 775cdc0b4f24..5156dc6be0a6 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -398,6 +398,11 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence); static inline bool dma_fence_is_signaled_locked(struct dma_fence *fence) { +#ifdef CONFIG_DMABUF_DEBUG_ENABLE_SIGNALING + if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) + return false; +#endif + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return true;
Re: [PATCH 2/3] dma-buf: Enable signaling on fence for sw_sync
Am 27.09.22 um 19:24 schrieb Arvind Yadav: Here's enabling software signaling on fence for sw_sync. Signed-off-by: Arvind Yadav Reviewed-by: Christian König --- drivers/dma-buf/sw_sync.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 348b3a9170fa..d2a52ceac14e 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -244,6 +244,8 @@ static struct sync_pt *sync_pt_create(struct sync_timeline *obj, obj->context, value); INIT_LIST_HEAD(&pt->link); + dma_fence_enable_sw_signaling(&pt->base); + spin_lock_irq(&obj->lock); if (!dma_fence_is_signaled_locked(&pt->base)) { struct rb_node **p = &obj->pt_tree.rb_node;
Re: [PATCH 1/3] dma-buf: Remove the signaled bit status check
Am 27.09.22 um 19:24 schrieb Arvind Yadav: Remove the extra signaled bit status check because it is returning early when the fence is already signaled and __dma_fence_enable_signaling is checking the status of signaled bit again. Signed-off-by: Arvind Yadav Reviewed-by: Christian König --- drivers/dma-buf/dma-fence.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 406b4e26f538..11ef20f70ee0 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -648,11 +648,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb, if (WARN_ON(!fence || !func)) return -EINVAL; - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) { - INIT_LIST_HEAD(&cb->node); - return -ENOENT; - } - spin_lock_irqsave(fence->lock, flags); if (__dma_fence_enable_signaling(fence)) {
Re: [PATCH v3] drm: document uAPI page-flip flags
On Wednesday, September 28th, 2022 at 12:06, Pekka Paalanen wrote: > > +/** > > + * DRM_MODE_PAGE_FLIP_FLAGS > > + * > > + * Bitmask of flags suitable for &drm_mode_crtc_page_flip_target.flags. > > Should this mention also drm_mode_crtc_page_flip.flags? > > UAPI header defines both structs. drm_mode_crtc_page_flip is "v1", drm_mode_crtc_page_flip_target is "v2". The latter just replaces a reserved field with a new one. So I figured "v1" is mostly kept around for backwards compat and everybody should use "v2" for simplicity's sake. > > +/** > > + * DRM_MODE_ATOMIC_ALLOW_MODESET > > + * > > + * Allow the update to result in temporary or transient visible artifacts > > while > > + * the update is being applied. Applying the update may also take > > significantly > > + * more time than a page flip. All visual artifacts will disappear by the > > time > > + * the update is completed, as signalled throught the vblank event's > > timestamp > > typo: throught Indeed! > > + * (see struct drm_event_vblank). > > + * > > + * This flag must be set when the KMS update might cause visible artifacts. > > + * Without this flag such KMS update will return a EINVAL error. What kind > > of > > + * update may cause visible artifacts depends on the driver and the > > hardware. > > + * User-space that needs to know beforehand if an update might cause > > visible > > + * artifacts can use &DRM_MODE_ATOMIC_TEST_ONLY without > > + * &DRM_MODE_ATOMIC_ALLOW_MODESET to see if it fails. > > + * > > + * To the best of the driver's knowledge, visual artifacts are guaranteed > > to > > + * not appear when this flag is not set. Some sinks might display visual > > + * artifacts outside of the driver's control. > > Ok, so we kept the "visual artifacts" semantics and allow monitors to > do otherwise. > > I'm still not sure what this means for things like infoframe data where > changing a certain field (e.g. HDR_OUTPUT_METADATA structure's EOTF > field) has a high risk of causing a visual glitch. I cannot imagine why > a monitor manufacturer would not be able to avoid the glitch if they > wanted to. The glitch might or might not happen, and we cannot know in > advance or afterwards whether it did happen, but it is probable that > many monitors will glitch. > > I think "To the best of driver's knowledge" means that if someone > reports a monitor to glitch, the driver/kernel would need to add that > field to the "needs modeset" set. But doing so can regress other > monitors that didn't glitch, so it needs to be a monitor quirk. > > This is not something for this patch, but would it be possible to agree > on the kernel development policy here? Should glitches be reported and > added to monitor quirks list? Or should drivers be defensive from the > start and require modeset if the field is known to cause glitches on > some monitors? Good question. I am not sure there is even a desire to have a quirks table for this among driver developers. This reminds me of some VRR displays showing visual artifacts when user-space changes its page-flip rate. If we took this definition by the letter, the kernel should require ALLOW_MODESET at each page-flip... (The better solution would be to just to have a slew rate implemented in the kernel. I think it's implemented to some extent in amdgpu/i915 but still some monitors are having issues.)
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
If it is supposed to be a non-linear luminance curve, which one is it? It would be much clearer if user space can control linear luminance and use whatever definition of perceived brightness it wants. The obvious downside of it is that it requires bits to encode changes that users can't perceive. What about backlights which only have a few predefined luminance levels? How would user space differentiate between the continuous and discrete backlight? What about self-emitting displays? They usually increase the dynamic range when they increase in brightness because the black level doesn't rise. They also probably employ some tonemapping to adjust for that. What about the range of the backlight? What about the absolute luminance of the backlight? I want to know about that all in user space. I understand that most of the time the kernel doesn't have answers to those questions right now but the API should account for all of that. On Wed, Sep 28, 2022 at 1:14 PM Ville Syrjälä wrote: > > On Wed, Sep 28, 2022 at 01:57:18PM +0300, Ville Syrjälä wrote: > > On Wed, Sep 28, 2022 at 01:04:01PM +0300, Jani Nikula wrote: > > > On Fri, 09 Sep 2022, Hans de Goede wrote: > > > > Hi all, > > > > > > > > Here is v2 of my "drm/kms: control display brightness through > > > > drm_connector properties" RFC: > > > > > > > > Changes from version 1: > > > > - Drop bl_brightness_0_is_min_brightness from list of new connector > > > > properties. > > > > - Clearly define that 0 is always min-brightness when setting the > > > > brightness > > > > through the connector properties. > > > > - Drop bl_brightness_control_method from list of new connector > > > > properties. > > > > - Phase 1 of the plan has been completed > > > > > > > > As discussed already several times in the past: > > > > https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/ > > > > > > > > https://lore.kernel.org/all/4b17ba08-39f3-57dd-5aad-d37d844b0...@linux.intel.com/ > > > > > > > > The current userspace API for brightness control offered by > > > > /sys/class/backlight devices has various issues: > > > > > > > > 1. There is no way to map the backlight device to a specific > > > >display-output / panel (1) > > > > 2. Controlling the brightness requires root-rights requiring > > > >desktop-environments to use suid-root helpers for this. > > > > 3. The meaning of 0 is not clearly defined, it can be either off, > > > >or minimum brightness at which the display is still readable > > > >(in a low light environment) > > > > 4. It's not possible to change both the gamma and the brightness in the > > > >same KMS atomic commit. You'd want to be able to reduce brightness to > > > >conserve power, and counter the effects of that by changing gamma to > > > >reach a visually similar image. And you'd want to have the changes > > > > take > > > >effect at the same time instead of reducing brightness at some frame > > > > and > > > >change gamma at some other frame. This is pretty much impossible to > > > > do > > > >via the sysfs interface. > > > > > > > > As already discussed on various conference's hallway tracks > > > > and as has been proposed on the dri-devel list once before (2), > > > > it seems that there is consensus that the best way to to solve these > > > > 2 issues is to add support for controlling a video-output's brightness > > > > through properties on the drm_connector. > > > > > > > > This RFC outlines my plan to try and actually implement this, > > > > which has 3 phases: > > > > > > > > > > > > Phase 1: Stop registering multiple /sys/class/backlight devs for a > > > > single display > > > > = > > > > > > > > On x86 there can be multiple firmware + direct-hw-access methods > > > > for controlling the backlight and in some cases the kernel registers > > > > multiple backlight-devices for a single internal laptop LCD panel. > > > > > > > > A plan to fix this was posted here: > > > > https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ > > > > And a pull-req actually implementing this plan has been send out this > > > > week: > > > > https://lore.kernel.org/dri-devel/261afe3d-7790-e945-adf6-a2c96c9b1...@redhat.com/ > > > > > > > > > > > > Phase 2: Add drm_connector properties mirroring the matching backlight > > > > device > > > > = > > > > > > > > The plan is to add a drm_connector helper function, which optionally > > > > takes > > > > a pointer to the backlight device for the GPU's native backlight device, > > > > which will then mirror the backlight settings from the backlight device > > > > in a set of read/write brightness* properties on the connector. > > > > > > > > This function can then be called by GPU drivers for the drm_connector > > > > for > > > > the internal panel and it will then take care of every
[PATCH 2/2] drm/sched: add missing NULL check in drm_sched_get_cleanup_job
Otherwise we would crash if the job is not resubmitted. Signed-off-by: Christian König --- drivers/gpu/drm/scheduler/sched_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 4f2395d1a791..23e5e8275dc7 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -829,7 +829,8 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched) job = list_first_entry_or_null(&sched->pending_list, struct drm_sched_job, list); - if (job && dma_fence_is_signaled(job->s_fence->parent)) { + if (job && (!job->s_fence->parent || + dma_fence_is_signaled(job->s_fence->parent))) { /* remove job from pending_list */ list_del_init(&job->list); -- 2.25.1
[PATCH 1/2] drm/scheduler: fix fence ref counting
We leaked dependency fences when processes were beeing killed. Additional to that grab a reference to the last scheduled fence. Signed-off-by: Christian König --- drivers/gpu/drm/scheduler/sched_entity.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 6b25b2f4f5a3..7ef1a086a6fb 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -207,6 +207,7 @@ static void drm_sched_entity_kill_jobs_cb(struct dma_fence *f, struct drm_sched_job *job = container_of(cb, struct drm_sched_job, finish_cb); + dma_fence_put(f); INIT_WORK(&job->work, drm_sched_entity_kill_jobs_work); schedule_work(&job->work); } @@ -234,8 +235,10 @@ static void drm_sched_entity_kill_jobs(struct drm_sched_entity *entity) struct drm_sched_fence *s_fence = job->s_fence; /* Wait for all dependencies to avoid data corruptions */ - while ((f = drm_sched_job_dependency(job, entity))) + while ((f = drm_sched_job_dependency(job, entity))) { dma_fence_wait(f, false); + dma_fence_put(f); + } drm_sched_fence_scheduled(s_fence); dma_fence_set_error(&s_fence->finished, -ESRCH); @@ -250,6 +253,7 @@ static void drm_sched_entity_kill_jobs(struct drm_sched_entity *entity) continue; } + dma_fence_get(entity->last_scheduled); r = dma_fence_add_callback(entity->last_scheduled, &job->finish_cb, drm_sched_entity_kill_jobs_cb); -- 2.25.1
[PATCH 3/4] drm/panel: Add driver for JDI LPM102A188A
The JDI LPM102A188A is a 2560x1800 IPS panel found in the Google Pixel C. This driver is based on the downstream GPLv2 driver released by Google written by Sean Paul [1], which was then adapted to the newer kernel APIs. [1]: https://android.googlesource.com/kernel/tegra/+/refs/heads/android-tegra-dragon-3.18-oreo/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c Signed-off-by: Diogo Ivo --- drivers/gpu/drm/panel/Kconfig | 11 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c | 513 ++ 3 files changed, 525 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 38799effd00a..532a15a38b60 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -203,6 +203,17 @@ config DRM_PANEL_JDI_LT070ME05000 The panel has a 1200(RGB)×1920 (WUXGA) resolution and uses 24 bit per pixel. +config DRM_PANEL_JDI_LPM102A188A + tristate "JDI LPM102A188A DSI panel" + depends on OF && GPIOLIB + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for JDI LPM102A188A DSI + control mode panel as found in Google Pixel C devices. + The panel has a 2560×1800 resolution. It provides a MIPI DSI interface + to the host. + config DRM_PANEL_JDI_R63452 tristate "JDI R63452 Full HD DSI panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 42a7ab54234b..774a5ce0ebb8 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o obj-$(CONFIG_DRM_PANEL_INNOLUX_EJ030NA) += panel-innolux-ej030na.o obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o +obj-$(CONFIG_DRM_PANEL_JDI_LPM102A188A) += panel-jdi-lpm102a188a.o obj-$(CONFIG_DRM_PANEL_JDI_R63452) += panel-jdi-fhd-r63452.o obj-$(CONFIG_DRM_PANEL_KHADAS_TS050) += panel-khadas-ts050.o obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o diff --git a/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c b/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c new file mode 100644 index ..c7f13719d4ff --- /dev/null +++ b/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c @@ -0,0 +1,513 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2014 Google, Inc. + * + * Copyright (C) 2022 Diogo Ivo + * + * Adapted from the downstream Pixel C driver written by Sean Paul + */ + +#include +#include +#include +#include +#include +#include + +#include + +#include +#include +#include + +struct jdi_panel { + struct drm_panel base; + struct mipi_dsi_device *link1; + struct mipi_dsi_device *link2; + + struct regulator *supply; + struct regulator *ddi_supply; + struct backlight_device *backlight; + + struct gpio_desc *enable_gpio; + struct gpio_desc *reset_gpio; + + const struct drm_display_mode *mode; + + bool prepared; + bool enabled; +}; + +static inline struct jdi_panel *to_panel_jdi(struct drm_panel *panel) +{ + return container_of(panel, struct jdi_panel, base); +} + +static void jdi_wait_frames(struct jdi_panel *jdi, unsigned int frames) +{ + unsigned int refresh = drm_mode_vrefresh(jdi->mode); + + if (WARN_ON(frames > refresh)) + return; + + msleep(1000 / (refresh / frames)); +} + +static int jdi_panel_disable(struct drm_panel *panel) +{ + struct jdi_panel *jdi = to_panel_jdi(panel); + + if (!jdi->enabled) + return 0; + + backlight_disable(jdi->backlight); + + jdi_wait_frames(jdi, 2); + + jdi->enabled = false; + + return 0; +} + +static int jdi_panel_unprepare(struct drm_panel *panel) +{ + struct jdi_panel *jdi = to_panel_jdi(panel); + int ret; + + if (!jdi->prepared) + return 0; + + ret = mipi_dsi_dcs_set_display_off(jdi->link1); + if (ret < 0) + dev_err(panel->dev, "failed to set display off: %d\n", ret); + ret = mipi_dsi_dcs_set_display_off(jdi->link2); + if (ret < 0) + dev_err(panel->dev, "failed to set display off: %d\n", ret); + + /* Specified by JDI @ 50ms, subject to change */ + msleep(50); + + ret = mipi_dsi_dcs_enter_sleep_mode(jdi->link1); + if (ret < 0) + dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret); + ret = mipi_dsi_dcs_enter_sleep_mode(jdi->link2); + if (ret < 0) + dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret); + + /* Specified by JDI @ 150ms, subject to change */ + msleep(150); + + gpiod_set_value(jdi->
[PATCH 4/4] arm64: dts: smaug: Add display panel node
The Google Pixel C has a JDI LPM102A188A display panel. Add a DT node for it. Tested on Pixel C. Signed-off-by: Diogo Ivo --- arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 72 +++ 1 file changed, 72 insertions(+) diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts index 20d092812984..271ef70747f1 100644 --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts @@ -31,6 +31,39 @@ memory { }; host1x@5000 { + dc@5420 { + status = "okay"; + }; + + dsia: dsi@5430 { + avdd-dsi-csi-supply = <&vdd_dsi_csi>; + nvidia,boot-on; + status = "okay"; + + link2: panel@0 { + compatible = "jdi,lpm102a188a"; + reg = <0>; + }; + }; + + dsib: dsi@5440 { + avdd-dsi-csi-supply = <&vdd_dsi_csi>; + nvidia,ganged-mode = <&dsia>; + nvidia,boot-on; + status = "okay"; + + link1: panel@0 { + compatible = "jdi,lpm102a188a"; + reg = <0>; + power-supply = <&pplcd_vdd>; + ddi-supply = <&pp1800_lcdio>; + enable-gpios = <&gpio TEGRA_GPIO(V, 1) GPIO_ACTIVE_HIGH>; + reset-gpios = <&gpio TEGRA_GPIO(V, 2) GPIO_ACTIVE_LOW>; + link2 = <&link2>; + backlight = <&backlight>; + }; + }; + dpaux: dpaux@545c { status = "okay"; }; @@ -1627,6 +1660,37 @@ nau8825@1a { status = "okay"; }; + backlight: lp8557-backlight@2c { + compatible = "ti,lp8557"; + reg = <0x2c>; + power-supply = <&pplcd_vdd>; + enable-supply = <&pp1800_lcdio>; + bl-name = "lp8557-backlight"; + dev-ctrl = /bits/ 8 <0x01>; + init-brt = /bits/ 8 <0x80>; + + /* Full scale current, 20mA */ + rom_11h { + rom-addr = /bits/ 8 <0x11>; + rom-val = /bits/ 8 <0x05>; + }; + /* Frequency = 4.9kHz, magic undocumented val */ + rom_12h { + rom-addr = /bits/ 8 <0x12>; + rom-val = /bits/ 8 <0x29>; + }; + /* Boost freq = 1MHz, BComp option = 1 */ + rom_13h { + rom-addr = /bits/ 8 <0x13>; + rom-val = /bits/ 8 <0x03>; + }; + /* 4V OV, 6 output LED string enabled */ + rom_14h { + rom-addr = /bits/ 8 <0x14>; + rom-val = /bits/ 8 <0xbf>; + }; + }; + audio-codec@2d { compatible = "realtek,rt5677"; reg = <0x2d>; @@ -1908,4 +1972,12 @@ usbc_vbus: regulator-usbc-vbus { regulator-min-microvolt = <500>; regulator-max-microvolt = <500>; }; + + vdd_dsi_csi: regulator-vdd-dsi-csi { + compatible = "regulator-fixed"; + regulator-name = "AVDD_DSI_CSI_1V2"; + regulator-min-microvolt = <120>; + regulator-max-microvolt = <120>; + vin-supply = <&pp1200_avdd>; + }; }; -- 2.37.3
[PATCH 1/4] dt-bindings: display: Add bindings for JDI LPM102A188A
The LPM102A188A is a 10.2" 2560x1800 IPS panel found in the Google Pixel C. Signed-off-by: Diogo Ivo --- .../display/panel/jdi,lpm102a188a.yaml| 100 ++ 1 file changed, 100 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/jdi,lpm102a188a.yaml diff --git a/Documentation/devicetree/bindings/display/panel/jdi,lpm102a188a.yaml b/Documentation/devicetree/bindings/display/panel/jdi,lpm102a188a.yaml new file mode 100644 index ..97f9db7152da --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/jdi,lpm102a188a.yaml @@ -0,0 +1,100 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/jdi,lpm102a188a.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: JDI LPM102A188A 2560x1800 10.2" DSI Panel + +maintainers: + - Diogo Ivo + +description: | + This panel requires a dual-channel DSI host to operate. It supports two modes: + - left-right: each channel drives the left or right half of the screen + - even-odd: each channel drives the even or odd lines of the screen + + Each of the DSI channels controls a separate DSI peripheral. The peripheral + driven by the first link (DSI-LINK1) is considered the primary peripheral + and controls the device. The 'link2' property contains a phandle to the + peripheral driven by the second link (DSI-LINK2). + +allOf: + - $ref: panel-common.yaml# + +properties: + compatible: +const: jdi,lpm102a188a + + reg: true + enable-gpios: true + reset-gpios: true + power-supply: true + backlight: true + + ts-reset-gpios: +maxItems: 1 +description: | + Specifier for a GPIO connected to the touchscreen reset control signal. + The reset signal is active low. + + ddi-supply: +description: The regulator that provides IOVCC (1.8V). + + link2: +$ref: /schemas/types.yaml#/definitions/phandle +description: | + phandle to the DSI peripheral on the secondary link. Note that the + presence of this property marks the containing node as DSI-LINK1. + +required: + - compatible + - reg + +if: + required: +- link2 +then: + required: +- power-supply +- ddi-supply +- enable-gpios +- reset-gpios + +additionalProperties: false + +examples: + - | +#include +#include + +dsia: dsi@5430 { +#address-cells = <1>; +#size-cells = <0>; +reg = <0x0 0x5430 0x0 0x0004>; + +link2: panel@0 { +compatible = "jdi,lpm102a188a"; +reg = <0>; +}; +}; + +dsib: dsi@5440{ +#address-cells = <1>; +#size-cells = <0>; +reg = <0x0 0x5440 0x0 0x0004>; +nvidia,ganged-mode = <&dsia>; + +link1: panel@0 { +compatible = "jdi,lpm102a188a"; +reg = <0>; +power-supply = <&pplcd_vdd>; +ddi-supply = <&pp1800_lcdio>; +enable-gpios = <&gpio TEGRA_GPIO(V, 1) GPIO_ACTIVE_HIGH>; +reset-gpios = <&gpio TEGRA_GPIO(V, 2) GPIO_ACTIVE_LOW>; +link2 = <&link2>; +backlight = <&backlight>; +}; +}; + +... -- 2.37.3
[PATCH 0/4] Add JDI LPM102A188A display panel support
Hello! These patches add support for the JDI LPM102A188A display panel, found in the Google Pixel C. Patch 1 adds the DT bindings for the panel. Patch 2 adds an optional register clear to the Tegra DSI driver. Patch 3 adds the panel driver, which is based on the downstream kernel driver published by Google and developed by Sean Paul. Patch 4 adds the DT node for the Google Pixel C. There is one point in this series on which I would like to ask for some advice: Since the device's bootloader leaves the display on and in patch 3 I have assumed that the panel must be reset when probing, I was forced to add patch 2, discovered by poking at the DSI module's registers until the panel initialization sequence succeeded. However, if it is okay to keep the panel on from the bootloader then it would be possible to forego this second patch. Any comments on this would be highly appreciated. Thank you! Diogo Ivo (4): dt-bindings: display: Add bindings for JDI LPM102A188A drm/tegra: dsi: Clear enable register if powered by bootloader drm/panel: Add driver for JDI LPM102A188A arm64: dts: smaug: Add display panel node .../display/panel/jdi,lpm102a188a.yaml| 100 arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 72 +++ drivers/gpu/drm/panel/Kconfig | 11 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c | 511 ++ drivers/gpu/drm/tegra/dsi.c | 29 + 6 files changed, 724 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/jdi,lpm102a188a.yaml create mode 100644 drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c -- 2.37.3
[PATCH 2/4] drm/tegra: dsi: Clear enable register if powered by bootloader
In cases where the DSI module is left on by the bootloader some panels may fail to initialize if the enable register is not cleared before the panel's initialization sequence. Clear it and add an optional device tree property to inform the driver if this is the case. Signed-off-by: Diogo Ivo --- drivers/gpu/drm/tegra/dsi.c | 29 + 1 file changed, 29 insertions(+) diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c index de1333dc0d86..f66514379913 100644 --- a/drivers/gpu/drm/tegra/dsi.c +++ b/drivers/gpu/drm/tegra/dsi.c @@ -78,6 +78,8 @@ struct tegra_dsi { unsigned int video_fifo_depth; unsigned int host_fifo_depth; + bool enabled; + /* for ganged-mode support */ struct tegra_dsi *master; struct tegra_dsi *slave; @@ -460,6 +462,8 @@ static void tegra_dsi_enable(struct tegra_dsi *dsi) value |= DSI_POWER_CONTROL_ENABLE; tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL); + dsi->enabled = true; + if (dsi->slave) tegra_dsi_enable(dsi->slave); } @@ -737,6 +741,8 @@ static void tegra_dsi_disable(struct tegra_dsi *dsi) value &= ~DSI_POWER_CONTROL_ENABLE; tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL); + dsi->enabled = false; + if (dsi->slave) tegra_dsi_disable(dsi->slave); @@ -912,6 +918,27 @@ static void tegra_dsi_encoder_enable(struct drm_encoder *encoder) u32 value; int err; + /* If the bootloader enabled DSI it needs to be disabled +* in order for the panel initialization commands to be +* properly sent. +*/ + if (dsi->enabled) { + if (dc) { + value = tegra_dc_readl(dc, DC_DISP_DISP_WIN_OPTIONS); + value &= ~DSI_ENABLE; + tegra_dc_writel(dc, value, DC_DISP_DISP_WIN_OPTIONS); + + tegra_dc_commit(dc); + } + + err = tegra_dsi_wait_idle(dsi, 100); + if (err < 0) + dev_dbg(dsi->dev, "failed to idle DSI: %d\n", err); + + /* disable DSI controller */ + tegra_dsi_disable(dsi); + } + err = tegra_dsi_prepare(dsi); if (err < 0) { dev_err(dsi->dev, "failed to prepare: %d\n", err); @@ -1573,6 +1600,8 @@ static int tegra_dsi_probe(struct platform_device *pdev) dsi->output.connector.polled = DRM_CONNECTOR_POLL_HPD; + /* Check if the DSI module was left on by bootloader. */ + dsi->enabled = of_property_read_bool(pdev->dev.of_node, "nvidia,boot-on"); /* * Assume these values by default. When a DSI peripheral driver * attaches to the DSI host, the parameters will be taken from -- 2.37.3
Re: [PATCH v4 15/15] vfio: Add struct device to vfio_device
On Thu, Sep 29, 2022 at 10:55:19AM -0600, Alex Williamson wrote: > Hi Kevin, > > This introduced the regression discovered here: > > https://lore.kernel.org/all/20220928125650.0a2ea297.alex.william...@redhat.com/ > > Seems we're not releasing the resources when removing an mdev. This is > a regression, so it needs to be fixed or reverted before the merge > window. Thanks, My guess at the fix for this: https://lore.kernel.org/r/0-v1-013609965fe8+9d-vfio_gvt_unregister_...@nvidia.com Jason
Re: [PATCH 05/16] drm/i915/vm_bind: Implement bind and unbind of object
On Thu, Sep 29, 2022 at 06:28:39PM +0100, Matthew Auld wrote: On 29/09/2022 17:38, Niranjana Vishwanathapura wrote: On Thu, Sep 29, 2022 at 11:49:30AM +0100, Matthew Auld wrote: On 28/09/2022 07:19, Niranjana Vishwanathapura wrote: Add uapi and implement support for bind and unbind of an object at the specified GPU virtual addresses. The vm_bind mode is not supported in legacy execbuf2 ioctl. It will be supported only in the newer execbuf3 ioctl. Signed-off-by: Niranjana Vishwanathapura Signed-off-by: Prathap Kumar Valsan Signed-off-by: Andi Shyti --- drivers/gpu/drm/i915/Makefile | 1 + .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 5 + drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h | 26 ++ .../drm/i915/gem/i915_gem_vm_bind_object.c | 306 ++ drivers/gpu/drm/i915/gt/intel_gtt.c | 10 + drivers/gpu/drm/i915/gt/intel_gtt.h | 17 + drivers/gpu/drm/i915/i915_driver.c | 3 + drivers/gpu/drm/i915/i915_vma.c | 1 + drivers/gpu/drm/i915/i915_vma_types.h | 14 + include/uapi/drm/i915_drm.h | 112 +++ 10 files changed, 495 insertions(+) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index a26edcdadc21..9bf939ef18ea 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -166,6 +166,7 @@ gem-y += \ gem/i915_gem_ttm_move.o \ gem/i915_gem_ttm_pm.o \ gem/i915_gem_userptr.o \ + gem/i915_gem_vm_bind_object.o \ gem/i915_gem_wait.o \ gem/i915_gemfs.o i915-y += \ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index cd75b0ca2555..f85f10cf9c34 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -781,6 +781,11 @@ static int eb_select_context(struct i915_execbuffer *eb) if (unlikely(IS_ERR(ctx))) return PTR_ERR(ctx); + if (ctx->vm->vm_bind_mode) { + i915_gem_context_put(ctx); + return -EOPNOTSUPP; + } + eb->gem_context = ctx; if (i915_gem_context_has_full_ppgtt(ctx)) eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h new file mode 100644 index ..36262a6357b5 --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2022 Intel Corporation + */ + +#ifndef __I915_GEM_VM_BIND_H +#define __I915_GEM_VM_BIND_H + +#include + +struct drm_device; +struct drm_file; +struct i915_address_space; +struct i915_vma; + +struct i915_vma * +i915_gem_vm_bind_lookup_vma(struct i915_address_space *vm, u64 va); + +int i915_gem_vm_bind_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); +int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); + +void i915_gem_vm_unbind_all(struct i915_address_space *vm); + +#endif /* __I915_GEM_VM_BIND_H */ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c new file mode 100644 index ..e529162abd2c --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c @@ -0,0 +1,306 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2022 Intel Corporation + */ + +#include + +#include + +#include "gem/i915_gem_context.h" +#include "gem/i915_gem_vm_bind.h" + +#include "gt/intel_gpu_commands.h" + +#define START(node) ((node)->start) +#define LAST(node) ((node)->last) + +INTERVAL_TREE_DEFINE(struct i915_vma, rb, u64, __subtree_last, + START, LAST, static inline, i915_vm_bind_it) + +#undef START +#undef LAST + +/** + * DOC: VM_BIND/UNBIND ioctls + * + * DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer + * objects (BOs) or sections of a BOs at specified GPU virtual addresses on a + * specified address space (VM). Multiple mappings can map to the same physical + * pages of an object (aliasing). These mappings (also referred to as persistent + * mappings) will be persistent across multiple GPU submissions (execbuf calls) + * issued by the UMD, without user having to provide a list of all required + * mappings during each submission (as required by older execbuf mode). + * + * The VM_BIND/UNBIND calls allow UMDs to request a timeline out fence for + * signaling the completion of bind/unbind operation. + * + * VM_BIND feature is advertised to user via I915_PARAM_VM_BIND_VERSION. + * User has to opt-in for VM_BIND mode of binding for an address space (VM) + * during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension. + * + * VM_BIND/UNBIND ioctl calls executed on different CPU threads co
[PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call
When converting to directly create the vfio_device the mdev driver has to put a vfio_register_emulated_iommu_dev() in the probe() and a pairing vfio_unregister_group_dev() in the remove. This was missed for gvt, add it. Cc: sta...@vger.kernel.org Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use vfio_register_emulated_iommu_dev") Reported-by: Alex Williamson Signed-off-by: Jason Gunthorpe --- drivers/gpu/drm/i915/gvt/kvmgt.c | 1 + 1 file changed, 1 insertion(+) Should go through Alex's tree. diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 41bba40feef8f4..9003145adb5a93 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev) if (WARN_ON_ONCE(vgpu->attached)) return; + vfio_unregister_group_dev(&vgpu->vfio_device); vfio_put_device(&vgpu->vfio_device); } base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c -- 2.37.3
Re: [PATCH 05/16] drm/i915/vm_bind: Implement bind and unbind of object
On 29/09/2022 17:38, Niranjana Vishwanathapura wrote: On Thu, Sep 29, 2022 at 11:49:30AM +0100, Matthew Auld wrote: On 28/09/2022 07:19, Niranjana Vishwanathapura wrote: Add uapi and implement support for bind and unbind of an object at the specified GPU virtual addresses. The vm_bind mode is not supported in legacy execbuf2 ioctl. It will be supported only in the newer execbuf3 ioctl. Signed-off-by: Niranjana Vishwanathapura Signed-off-by: Prathap Kumar Valsan Signed-off-by: Andi Shyti --- drivers/gpu/drm/i915/Makefile | 1 + .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 5 + drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h | 26 ++ .../drm/i915/gem/i915_gem_vm_bind_object.c | 306 ++ drivers/gpu/drm/i915/gt/intel_gtt.c | 10 + drivers/gpu/drm/i915/gt/intel_gtt.h | 17 + drivers/gpu/drm/i915/i915_driver.c | 3 + drivers/gpu/drm/i915/i915_vma.c | 1 + drivers/gpu/drm/i915/i915_vma_types.h | 14 + include/uapi/drm/i915_drm.h | 112 +++ 10 files changed, 495 insertions(+) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index a26edcdadc21..9bf939ef18ea 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -166,6 +166,7 @@ gem-y += \ gem/i915_gem_ttm_move.o \ gem/i915_gem_ttm_pm.o \ gem/i915_gem_userptr.o \ + gem/i915_gem_vm_bind_object.o \ gem/i915_gem_wait.o \ gem/i915_gemfs.o i915-y += \ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index cd75b0ca2555..f85f10cf9c34 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -781,6 +781,11 @@ static int eb_select_context(struct i915_execbuffer *eb) if (unlikely(IS_ERR(ctx))) return PTR_ERR(ctx); + if (ctx->vm->vm_bind_mode) { + i915_gem_context_put(ctx); + return -EOPNOTSUPP; + } + eb->gem_context = ctx; if (i915_gem_context_has_full_ppgtt(ctx)) eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h new file mode 100644 index ..36262a6357b5 --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2022 Intel Corporation + */ + +#ifndef __I915_GEM_VM_BIND_H +#define __I915_GEM_VM_BIND_H + +#include + +struct drm_device; +struct drm_file; +struct i915_address_space; +struct i915_vma; + +struct i915_vma * +i915_gem_vm_bind_lookup_vma(struct i915_address_space *vm, u64 va); + +int i915_gem_vm_bind_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); +int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); + +void i915_gem_vm_unbind_all(struct i915_address_space *vm); + +#endif /* __I915_GEM_VM_BIND_H */ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c new file mode 100644 index ..e529162abd2c --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c @@ -0,0 +1,306 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2022 Intel Corporation + */ + +#include + +#include + +#include "gem/i915_gem_context.h" +#include "gem/i915_gem_vm_bind.h" + +#include "gt/intel_gpu_commands.h" + +#define START(node) ((node)->start) +#define LAST(node) ((node)->last) + +INTERVAL_TREE_DEFINE(struct i915_vma, rb, u64, __subtree_last, + START, LAST, static inline, i915_vm_bind_it) + +#undef START +#undef LAST + +/** + * DOC: VM_BIND/UNBIND ioctls + * + * DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer + * objects (BOs) or sections of a BOs at specified GPU virtual addresses on a + * specified address space (VM). Multiple mappings can map to the same physical + * pages of an object (aliasing). These mappings (also referred to as persistent + * mappings) will be persistent across multiple GPU submissions (execbuf calls) + * issued by the UMD, without user having to provide a list of all required + * mappings during each submission (as required by older execbuf mode). + * + * The VM_BIND/UNBIND calls allow UMDs to request a timeline out fence for + * signaling the completion of bind/unbind operation. + * + * VM_BIND feature is advertised to user via I915_PARAM_VM_BIND_VERSION. + * User has to opt-in for VM_BIND mode of binding for an address space (VM) + * during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension. + * + * VM_BIND/UNBIND ioctl calls executed on different CPU threads concurrently + * are not ordered. Furthermore, parts of the VM_BI
Re: [PATCH 07/16] drm/i915/vm_bind: Add support to handle object evictions
On 28/09/2022 07:19, Niranjana Vishwanathapura wrote: Support eviction by maintaining a list of evicted persistent vmas for rebinding during next submission. Ensure the list do not include persistent vmas that are being purged. Signed-off-by: Niranjana Vishwanathapura Signed-off-by: Andi Shyti Acked-by: Matthew Auld --- .../drm/i915/gem/i915_gem_vm_bind_object.c| 6 ++ drivers/gpu/drm/i915/gt/intel_gtt.c | 2 ++ drivers/gpu/drm/i915/gt/intel_gtt.h | 4 drivers/gpu/drm/i915/i915_vma.c | 19 +++ drivers/gpu/drm/i915/i915_vma.h | 10 ++ drivers/gpu/drm/i915/i915_vma_types.h | 10 ++ 6 files changed, 51 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c index 809c78455d2e..958139ed6da3 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c @@ -85,6 +85,12 @@ static void i915_gem_vm_bind_remove(struct i915_vma *vma, bool release_obj) { lockdep_assert_held(&vma->vm->vm_bind_lock); + spin_lock(&vma->vm->vm_rebind_lock); + if (!list_empty(&vma->vm_rebind_link)) + list_del_init(&vma->vm_rebind_link); + i915_vma_set_purged(vma); + spin_unlock(&vma->vm->vm_rebind_lock); + list_del_init(&vma->vm_bind_link); list_del_init(&vma->non_priv_vm_bind_link); i915_vm_bind_it_remove(vma, &vma->vm->va); diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index da4f9dee0397..6db31197fa87 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -296,6 +296,8 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass) INIT_LIST_HEAD(&vm->non_priv_vm_bind_list); vm->root_obj = i915_gem_object_create_internal(vm->i915, PAGE_SIZE); GEM_BUG_ON(IS_ERR(vm->root_obj)); + INIT_LIST_HEAD(&vm->vm_rebind_list); + spin_lock_init(&vm->vm_rebind_lock); } void *__px_vaddr(struct drm_i915_gem_object *p) diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h index 3f2e87d3bf34..b73d35b4e05d 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.h +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h @@ -273,6 +273,10 @@ struct i915_address_space { struct list_head vm_bind_list; /** @vm_bound_list: List of vm_binding completed */ struct list_head vm_bound_list; + /* @vm_rebind_list: list of vmas to be rebinded */ + struct list_head vm_rebind_list; + /* @vm_rebind_lock: protects vm_rebound_list */ + spinlock_t vm_rebind_lock; /* @va: tree of persistent vmas */ struct rb_root_cached va; struct list_head non_priv_vm_bind_list; diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 89c276163916..84ed3d1a17a6 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -241,6 +241,7 @@ vma_create(struct drm_i915_gem_object *obj, INIT_LIST_HEAD(&vma->vm_bind_link); INIT_LIST_HEAD(&vma->non_priv_vm_bind_link); + INIT_LIST_HEAD(&vma->vm_rebind_link); return vma; err_unlock: @@ -1686,6 +1687,14 @@ static void force_unbind(struct i915_vma *vma) if (!drm_mm_node_allocated(&vma->node)) return; + /* +* Persistent vma should have been purged by now. +* If not, issue a warning and purge it. +*/ + if (GEM_WARN_ON(i915_vma_is_persistent(vma) && + !i915_vma_is_purged(vma))) + i915_vma_set_purged(vma); + atomic_and(~I915_VMA_PIN_MASK, &vma->flags); WARN_ON(__i915_vma_unbind(vma)); GEM_BUG_ON(drm_mm_node_allocated(&vma->node)); @@ -2048,6 +2057,16 @@ int __i915_vma_unbind(struct i915_vma *vma) __i915_vma_evict(vma, false); drm_mm_remove_node(&vma->node); /* pairs with i915_vma_release() */ + + if (i915_vma_is_persistent(vma)) { + spin_lock(&vma->vm->vm_rebind_lock); + if (list_empty(&vma->vm_rebind_link) && + !i915_vma_is_purged(vma)) + list_add_tail(&vma->vm_rebind_link, + &vma->vm->vm_rebind_list); + spin_unlock(&vma->vm->vm_rebind_lock); + } + return 0; } diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h index 51e712de380a..48731855b5b0 100644 --- a/drivers/gpu/drm/i915/i915_vma.h +++ b/drivers/gpu/drm/i915/i915_vma.h @@ -152,6 +152,16 @@ static inline void i915_vma_set_persistent(struct i915_vma *vma) set_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); } +static inline bool i915_vma_is_purged(const struct i915_vma *vma) +{ + return test_bit(I915_VMA_PURGED_BIT, __i915_vma_flags(vma)); +} + +static inl
Re: [Linaro-mm-sig] Re: [PATCH v4 5/6] drm/sched: Use parent fence instead of finished
Am 29.09.22 um 17:31 schrieb Steven Price: On 29/09/2022 15:57, Christian König wrote: Am 29.09.22 um 16:53 schrieb Steven Price: On 14/09/2022 17:43, Arvind Yadav wrote: Using the parent fence instead of the finished fence to get the job status. This change is to avoid GPU scheduler timeout error which can cause GPU reset. I'm able to reproduce crashes on Panfrost and I believe this commit is the cause. Specifically it's possible for job->s_fence->parent to be NULL. The underlying issue seems to involve drm_sched_resubmit_jobs_ext() - if the run_jobs() callback returns an error it will set s_fence->parent to NULL after signalling s_fence->finished: fence = sched->ops->run_job(s_job); i++; if (IS_ERR_OR_NULL(fence)) { if (IS_ERR(fence)) dma_fence_set_error(&s_fence->finished, PTR_ERR(fence)); s_job->s_fence->parent = NULL; I don't understand the reasoning behind this change, but it doesn't seem right to be using the parent fence when we have code which can be setting that pointer to NULL. Since I don't understand the reasoning my only suggestion is to revert this patch (and potentially the dependent patch "dma-buf: Check status of enable-signaling bit on debug"?). Can anyone suggest a better fix? Well, first of all please absolutely don't use drm_sched_resubmit_jobs_ext()! Panfrost isn't using drm_sched_resubmit_jobs_ext() directly but via drm_sched_resubmit_jobs(). Yeah, but it's the same problem that this isn't designed very well. It was an extremely bad idea in amdgpu to approach GPU by re-submitting jobs and it was an even worse idea to push this into the scheduler. The design of dma_fence is that you submit that once and *only* once and then get a result for this submission. If re-submission is desirable it should be done in userspace or at least higher levels. Panfrost has an interesting feature where it's possible to rescue a job during a GPU reset. Because jobs are queued on the GPU if the job hasn't actually started executing then it's quite possible to safely resubmit it from the kernel driver and user space doesn't need to be involved. That's actually fine. E.g. when you can save the hardware state and restart it there is nothing as far as I can see which speaks against that. The problem is rather pushing this into the scheduler because and trying to fit the square pig through a round hole. You either end up allocating memory while inside a GPU reset (which is illegal because allocating memory could need to wait for the reset to finish). Or you end up re-using the same dma_fence object twice (which in turn is illegal from the dma_fence design). The benefit of this is if another process has hung the GPU that processes jobs can be killed off without affecting any other innocent processes. One option would be to hide all this from the scheduler, but I can't see how to do that without also hiding the actual reset from the scheduler. Admittedly at the moment Panfrost is far too aggressive at resetting and will perform a GPU reset in conditions where it's completely unnecessary. There's work to do there but I haven't had the time to look at it yet. Apart from that, yes a NULL check is missing here but that should be trivial to fix. What I'm struggling to get my head round is whether it's correct to always treat the job as signalled just because s_fence->parent is NULL? Well s_fence parent will never be set to something else than NULL in this situation, isn't it? The problem with using the finished fence is that this is actually the public interface of the scheduler instead of the internal state. In other words s_fence->parent is what the scheduler deals with to produce the s_fence->finished state. Christian. Thanks, Steve Thanks, Christian. Thanks, Steve Signed-off-by: Arvind Yadav Reviewed-by: Andrey Grodzovsky --- changes in v1,v2 - Enable signaling for finished fence in sche_main() is removed --- drivers/gpu/drm/scheduler/sched_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index e0ab14e0fb6b..2ac28ad11432 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -829,7 +829,7 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched) job = list_first_entry_or_null(&sched->pending_list, struct drm_sched_job, list); - if (job && dma_fence_is_signaled(&job->s_fence->finished)) { + if (job && dma_fence_is_signaled(job->s_fence->parent)) { /* remove job from pending_list */ list_del_init(&job->list); @@ -841,7 +841,7 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched) if (next) { next->s_fence->scheduled.timestamp = - job->s_fence->finished.timestamp; + job->s_fence->parent->ti
Re: [PATCH 04/16] drm/i915/vm_bind: Add support to create persistent vma
On 28/09/2022 07:19, Niranjana Vishwanathapura wrote: Add i915_vma_instance_persistent() to create persistent vmas. Persistent vmas will use i915_gtt_view to support partial binding. vma_lookup is tied to segment of the object instead of section of VA space. Hence, it do not support aliasing. ie., multiple mappings (at different VA) point to the same gtt_view of object. Skip vma_lookup for persistent vmas to support aliasing. Signed-off-by: Niranjana Vishwanathapura Signed-off-by: Andi Shyti Acked-by: Matthew Auld
Re: [PATCH v4 15/15] vfio: Add struct device to vfio_device
Hi Kevin, This introduced the regression discovered here: https://lore.kernel.org/all/20220928125650.0a2ea297.alex.william...@redhat.com/ Seems we're not releasing the resources when removing an mdev. This is a regression, so it needs to be fixed or reverted before the merge window. Thanks, Alex On Wed, 21 Sep 2022 18:44:01 +0800 Kevin Tian wrote: > From: Yi Liu > > and replace kref. With it a 'vfio-dev/vfioX' node is created under the > sysfs path of the parent, indicating the device is bound to a vfio > driver, e.g.: > > /sys/devices/pci\:6f/\:6f\:01.0/vfio-dev/vfio0 > > It is also a preparatory step toward adding cdev for supporting future > device-oriented uAPI. > > Add Documentation/ABI/testing/sysfs-devices-vfio-dev. > > Suggested-by: Jason Gunthorpe > Signed-off-by: Yi Liu > Signed-off-by: Kevin Tian > Reviewed-by: Jason Gunthorpe > --- > .../ABI/testing/sysfs-devices-vfio-dev| 8 +++ > MAINTAINERS | 1 + > drivers/vfio/vfio_main.c | 64 +++ > include/linux/vfio.h | 6 +- > 4 files changed, 65 insertions(+), 14 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-devices-vfio-dev > > diff --git a/Documentation/ABI/testing/sysfs-devices-vfio-dev > b/Documentation/ABI/testing/sysfs-devices-vfio-dev > new file mode 100644 > index ..e21424fd9666 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-devices-vfio-dev > @@ -0,0 +1,8 @@ > +What: /sys/...//vfio-dev/vfioX/ > +Date: September 2022 > +Contact: Yi Liu > +Description: > + This directory is created when the device is bound to a > + vfio driver. The layout under this directory matches what > + exists for a standard 'struct device'. 'X' is a unique > + index marking this device in vfio. > diff --git a/MAINTAINERS b/MAINTAINERS > index d30f26e07cd3..02c8f11b1c17 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -21312,6 +21312,7 @@ R:Cornelia Huck > L: k...@vger.kernel.org > S: Maintained > T: git git://github.com/awilliam/linux-vfio.git > +F: Documentation/ABI/testing/sysfs-devices-vfio-dev > F: Documentation/driver-api/vfio.rst > F: drivers/vfio/ > F: include/linux/vfio.h > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > index c27449613a1d..f9d10dbcf3e6 100644 > --- a/drivers/vfio/vfio_main.c > +++ b/drivers/vfio/vfio_main.c > @@ -49,6 +49,8 @@ static struct vfio { > struct mutexgroup_lock; /* locks group_list */ > struct ida group_ida; > dev_t group_devt; > + struct class*device_class; > + struct ida device_ida; > } vfio; > > struct vfio_iommu_driver { > @@ -485,12 +487,13 @@ static struct vfio_device *vfio_group_get_device(struct > vfio_group *group, > * VFIO driver API > */ > /* Release helper called by vfio_put_device() */ > -void vfio_device_release(struct kref *kref) > +static void vfio_device_release(struct device *dev) > { > struct vfio_device *device = > - container_of(kref, struct vfio_device, kref); > + container_of(dev, struct vfio_device, device); > > vfio_release_device_set(device); > + ida_free(&vfio.device_ida, device->index); > > /* >* kvfree() cannot be done here due to a life cycle mess in > @@ -500,7 +503,6 @@ void vfio_device_release(struct kref *kref) >*/ > device->ops->release(device); > } > -EXPORT_SYMBOL_GPL(vfio_device_release); > > /* > * Allocate and initialize vfio_device so it can be registered to vfio > @@ -548,6 +550,13 @@ int vfio_init_device(struct vfio_device *device, struct > device *dev, > { > int ret; > > + ret = ida_alloc_max(&vfio.device_ida, MINORMASK, GFP_KERNEL); > + if (ret < 0) { > + dev_dbg(dev, "Error to alloc index\n"); > + return ret; > + } > + > + device->index = ret; > init_completion(&device->comp); > device->dev = dev; > device->ops = ops; > @@ -558,11 +567,15 @@ int vfio_init_device(struct vfio_device *device, struct > device *dev, > goto out_uninit; > } > > - kref_init(&device->kref); > + device_initialize(&device->device); > + device->device.release = vfio_device_release; > + device->device.class = vfio.device_class; > + device->device.parent = device->dev; > return 0; > > out_uninit: > vfio_release_device_set(device); > + ida_free(&vfio.device_ida, device->index); > return ret; > } > EXPORT_SYMBOL_GPL(vfio_init_device); > @@ -659,6 +672,7 @@ static int __vfio_register_dev(struct vfio_device *device, > struct vfio_group *group) > { > struct vfio_device *existing_device; > + int ret; > > i
Re: [Intel-gfx] [PATCH] drm/i915: Fix CFI violations in gt_sysfs
On Thu, Sep 29, 2022 at 06:46:34PM +0200, Andi Shyti wrote: > Hi Nathan, > > thanks for this refactoring... looks good even though i would > have split it in more patches as this is doing quite many things. Right, sorry about that :/ I initially thought the problem was much simpler and the diff was more reasonable before I truly saw what was happening and by that point, trying to break things apart felt like navigating a mine field. I will definitely keep that in mind for the future though. > But I will not be stubborn, I understand that it's not trivial to > have things split. I will give my r-b for now but I will check it > again before applying it as it's very easy to get tangled-up in > between all those defines: > > Reviewed-by: Andi Shyti Appreciate it! I don't have access to some of the hardware that is special cased in this code so I definitely would not mind some additional eyes and testing for this change. > For now I'm quite surprised to see how easily our CI gives green > lights :-P > > On Sat, Sep 24, 2022 at 09:39:30PM -0700, Nathan Chancellor wrote: > > On Fri, Sep 23, 2022 at 09:57:47PM -0700, Kees Cook wrote: > > > On Thu, Sep 22, 2022 at 12:51:27PM -0700, Nathan Chancellor wrote: > > > > [...] > > > > To make everything work properly, adjust certain functions to match the > > > > type of the ->show() and ->store() members in 'struct kobj_attribute'. > > > > Add a macro to generate functions for that can be called via both > > > > dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be > > > > called through both kobject locations without violating kCFI and adjust > > > > the attribute groups to account for this. > > > > > > This was quite a roller coaster! I think the solution looks good, even > > > if I'm suspicious of the original design that has the same stuff > > > available twice in different places. (I have a dim memory of rdma > > > needing a refactoring like this too?) > > > > Right, I noticed this comment in intel_gt_sysfs_register() once I fully > > saw what was going on: > > > > /* > > * We need to make things right with the > > * ABI compatibility. The files were originally > > * generated under the parent directory. > > * > > * We generate the files only for gt 0 > > * to avoid duplicates. > > */ > > > > Makes it seem like there will be userspace breakage if these files do > > not exist? I figured this was the cleanest solution within those > > parameters. > > i915 went multi-gt (multitile) so that some interfaces, like the > power management files, have effect only on one of the tiles. > This means that we needed to move some of the files inside the > newly created gt0/gt1 directory. > > But, because some of those files are part of an ABI > specification, we can't simply transfer them from the original > position so that we needed to make a "hard" copy (actually the > original files now take the role of affecting all the GTs instead > of only one). > > The complexity of this file comes from the necessity of > minimizing code duplication, otherwise we could have had two > perfectly identical files (which looking at the final result it > wouldn't have been a completely bad idea after all). > > Thanks again, will let you know when I will get this into our > branch. Ah, that all makes sense, good to know that this solution will allow all of that to continue to work. If there are any issues or further comments, I am happy to follow up in whatever way I need to. Thanks again for the review and tentative acceptance! Cheers, Nathan
Re: [Intel-gfx] [PATCH] drm/i915: Fix CFI violations in gt_sysfs
Hi Nathan, thanks for this refactoring... looks good even though i would have split it in more patches as this is doing quite many things. But I will not be stubborn, I understand that it's not trivial to have things split. I will give my r-b for now but I will check it again before applying it as it's very easy to get tangled-up in between all those defines: Reviewed-by: Andi Shyti For now I'm quite surprised to see how easily our CI gives green lights :-P On Sat, Sep 24, 2022 at 09:39:30PM -0700, Nathan Chancellor wrote: > On Fri, Sep 23, 2022 at 09:57:47PM -0700, Kees Cook wrote: > > On Thu, Sep 22, 2022 at 12:51:27PM -0700, Nathan Chancellor wrote: > > > [...] > > > To make everything work properly, adjust certain functions to match the > > > type of the ->show() and ->store() members in 'struct kobj_attribute'. > > > Add a macro to generate functions for that can be called via both > > > dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be > > > called through both kobject locations without violating kCFI and adjust > > > the attribute groups to account for this. > > > > This was quite a roller coaster! I think the solution looks good, even > > if I'm suspicious of the original design that has the same stuff > > available twice in different places. (I have a dim memory of rdma > > needing a refactoring like this too?) > > Right, I noticed this comment in intel_gt_sysfs_register() once I fully > saw what was going on: > > /* >* We need to make things right with the >* ABI compatibility. The files were originally >* generated under the parent directory. >* >* We generate the files only for gt 0 >* to avoid duplicates. >*/ > > Makes it seem like there will be userspace breakage if these files do > not exist? I figured this was the cleanest solution within those > parameters. i915 went multi-gt (multitile) so that some interfaces, like the power management files, have effect only on one of the tiles. This means that we needed to move some of the files inside the newly created gt0/gt1 directory. But, because some of those files are part of an ABI specification, we can't simply transfer them from the original position so that we needed to make a "hard" copy (actually the original files now take the role of affecting all the GTs instead of only one). The complexity of this file comes from the necessity of minimizing code duplication, otherwise we could have had two perfectly identical files (which looking at the final result it wouldn't have been a completely bad idea after all). Thanks again, will let you know when I will get this into our branch. Andi > > Reviewed-by: Kees Cook > > Thanks for looking it over! > > Cheers, > Nathan
Re: [PATCH 05/16] drm/i915/vm_bind: Implement bind and unbind of object
On Thu, Sep 29, 2022 at 11:49:30AM +0100, Matthew Auld wrote: On 28/09/2022 07:19, Niranjana Vishwanathapura wrote: Add uapi and implement support for bind and unbind of an object at the specified GPU virtual addresses. The vm_bind mode is not supported in legacy execbuf2 ioctl. It will be supported only in the newer execbuf3 ioctl. Signed-off-by: Niranjana Vishwanathapura Signed-off-by: Prathap Kumar Valsan Signed-off-by: Andi Shyti --- drivers/gpu/drm/i915/Makefile | 1 + .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 5 + drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h | 26 ++ .../drm/i915/gem/i915_gem_vm_bind_object.c| 306 ++ drivers/gpu/drm/i915/gt/intel_gtt.c | 10 + drivers/gpu/drm/i915/gt/intel_gtt.h | 17 + drivers/gpu/drm/i915/i915_driver.c| 3 + drivers/gpu/drm/i915/i915_vma.c | 1 + drivers/gpu/drm/i915/i915_vma_types.h | 14 + include/uapi/drm/i915_drm.h | 112 +++ 10 files changed, 495 insertions(+) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index a26edcdadc21..9bf939ef18ea 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -166,6 +166,7 @@ gem-y += \ gem/i915_gem_ttm_move.o \ gem/i915_gem_ttm_pm.o \ gem/i915_gem_userptr.o \ + gem/i915_gem_vm_bind_object.o \ gem/i915_gem_wait.o \ gem/i915_gemfs.o i915-y += \ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index cd75b0ca2555..f85f10cf9c34 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -781,6 +781,11 @@ static int eb_select_context(struct i915_execbuffer *eb) if (unlikely(IS_ERR(ctx))) return PTR_ERR(ctx); + if (ctx->vm->vm_bind_mode) { + i915_gem_context_put(ctx); + return -EOPNOTSUPP; + } + eb->gem_context = ctx; if (i915_gem_context_has_full_ppgtt(ctx)) eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h new file mode 100644 index ..36262a6357b5 --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2022 Intel Corporation + */ + +#ifndef __I915_GEM_VM_BIND_H +#define __I915_GEM_VM_BIND_H + +#include + +struct drm_device; +struct drm_file; +struct i915_address_space; +struct i915_vma; + +struct i915_vma * +i915_gem_vm_bind_lookup_vma(struct i915_address_space *vm, u64 va); + +int i915_gem_vm_bind_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); +int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void *data, +struct drm_file *file); + +void i915_gem_vm_unbind_all(struct i915_address_space *vm); + +#endif /* __I915_GEM_VM_BIND_H */ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c new file mode 100644 index ..e529162abd2c --- /dev/null +++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c @@ -0,0 +1,306 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2022 Intel Corporation + */ + +#include + +#include + +#include "gem/i915_gem_context.h" +#include "gem/i915_gem_vm_bind.h" + +#include "gt/intel_gpu_commands.h" + +#define START(node) ((node)->start) +#define LAST(node) ((node)->last) + +INTERVAL_TREE_DEFINE(struct i915_vma, rb, u64, __subtree_last, +START, LAST, static inline, i915_vm_bind_it) + +#undef START +#undef LAST + +/** + * DOC: VM_BIND/UNBIND ioctls + * + * DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer + * objects (BOs) or sections of a BOs at specified GPU virtual addresses on a + * specified address space (VM). Multiple mappings can map to the same physical + * pages of an object (aliasing). These mappings (also referred to as persistent + * mappings) will be persistent across multiple GPU submissions (execbuf calls) + * issued by the UMD, without user having to provide a list of all required + * mappings during each submission (as required by older execbuf mode). + * + * The VM_BIND/UNBIND calls allow UMDs to request a timeline out fence for + * signaling the completion of bind/unbind operation. + * + * VM_BIND feature is advertised to user via I915_PARAM_VM_BIND_VERSION. + * User has to opt-in for VM_BIND mode of binding for an address space (VM) + * during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension. + * + * VM_BIND/UNBIND ioctl calls executed on different CPU threads concurrently + * are not ordered. Furthermore, parts
Re: [PATCH] drm/msm: Fix build break with recent mm tree
On 9/29/22 09:14, Rob Clark wrote: > From: Rob Clark > > 9178e3dcb121 ("mm: discard __GFP_ATOMIC") removed __GFP_ATOMIC, > replacing it with a check for not __GFP_DIRECT_RECLAIM. > > Reported-by: Randy Dunlap > Reported-by: Stephen Rothwell > Signed-off-by: Rob Clark Acked-by: Randy Dunlap # build-tested Thanks. > --- > Sorry, this was reported by Stephen earlier in the month, while > I was on the other side of the globe and jetlagged. Unfortunately > I forgot about it by the time I got back home. Applying this patch > after 025d27239a2f ("drm/msm/gem: Evict active GEM objects when necessary") > but before or after 9178e3dcb121 ("mm: discard __GFP_ATOMIC") should > resolve the build breakage. > > drivers/gpu/drm/msm/msm_gem_shrinker.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c > b/drivers/gpu/drm/msm/msm_gem_shrinker.c > index 473ced14e520..8f83454ceedf 100644 > --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c > +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c > @@ -27,7 +27,7 @@ static bool can_swap(void) > > static bool can_block(struct shrink_control *sc) > { > - if (sc->gfp_mask & __GFP_ATOMIC) > + if (!(sc->gfp_mask & __GFP_DIRECT_RECLAIM)) > return false; > return current_is_kswapd() || (sc->gfp_mask & __GFP_RECLAIM); > } -- ~Randy
[PATCH v4 19/30] drm/connector: Add a function to lookup a TV mode by its name
As part of the command line parsing rework coming in the next patches, we'll need to lookup drm_connector_tv_mode values by their name, already defined in drm_tv_mode_enum_list. In order to avoid any code duplication, let's do a function that will perform a lookup of a TV mode name and return its value. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_connector.c | 24 include/drm/drm_connector.h | 2 ++ 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 8edc347ef664..ead5b30c193c 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -991,6 +991,30 @@ static const struct drm_prop_enum_list drm_tv_mode_enum_list[] = { }; DRM_ENUM_NAME_FN(drm_get_tv_mode_name, drm_tv_mode_enum_list) +/** + * drm_get_tv_mode_from_name - Translates a TV mode name into its enum value + * @name: TV Mode name we want to convert + * @len: Length of @name + * + * Translates @name into an enum drm_connector_tv_mode. + * + * Returns: the enum value on success, a negative errno otherwise. + */ +int drm_get_tv_mode_from_name(const char *name, size_t len) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(drm_tv_mode_enum_list); i++) { + const struct drm_prop_enum_list *item = &drm_tv_mode_enum_list[i]; + + if (strlen(item->name) == len && !strncmp(item->name, name, len)) + return item->type; + } + + return -EINVAL; +} +EXPORT_SYMBOL(drm_get_tv_mode_from_name); + static const struct drm_prop_enum_list drm_tv_select_enum_list[] = { { DRM_MODE_SUBCONNECTOR_Automatic, "Automatic" }, /* DVI-I and TV-out */ { DRM_MODE_SUBCONNECTOR_Composite, "Composite" }, /* TV-out */ diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index a501db7d..a33f24a76738 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1864,6 +1864,8 @@ const char *drm_get_dp_subconnector_name(int val); const char *drm_get_content_protection_name(int val); const char *drm_get_hdcp_content_type_name(int val); +int drm_get_tv_mode_from_name(const char *name, size_t len); + int drm_mode_create_dvi_i_properties(struct drm_device *dev); void drm_connector_attach_dp_subconnector_property(struct drm_connector *connector); -- b4 0.11.0-dev-7da52
[PATCH v4 29/30] drm/vc4: vec: Add support for more analog TV standards
From: Mateusz Kwiatkowski Add support for the following composite output modes (all of them are somewhat more obscure than the previously defined ones): - NTSC_443 - NTSC-style signal with the chroma subcarrier shifted to 4.43361875 MHz (the PAL subcarrier frequency). Never used for broadcasting, but sometimes used as a hack to play NTSC content in PAL regions (e.g. on VCRs). - PAL_N - PAL with alternative chroma subcarrier frequency, 3.58205625 MHz. Used as a broadcast standard in Argentina, Paraguay and Uruguay to fit 576i50 with colour in 6 MHz channel raster. - PAL60 - 480i60 signal with PAL-style color at normal European PAL frequency. Another non-standard, non-broadcast mode, used in similar contexts as NTSC_443. Some displays support one but not the other. - SECAM - French frequency-modulated analog color standard; also have been broadcast in Eastern Europe and various parts of Africa and Asia. Uses the same 576i50 timings as PAL. Also added some comments explaining color subcarrier frequency registers. Acked-by: Noralf Trønnes Signed-off-by: Mateusz Kwiatkowski Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_vec.c | 84 ++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index b342dc9cf69c..8d37d7ba9b2a 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -46,6 +46,7 @@ #define VEC_CONFIG0_YDEL(x)((x) << 26) #define VEC_CONFIG0_CDEL_MASK GENMASK(25, 24) #define VEC_CONFIG0_CDEL(x)((x) << 24) +#define VEC_CONFIG0_SECAM_STD BIT(21) #define VEC_CONFIG0_PBPR_FIL BIT(18) #define VEC_CONFIG0_CHROMA_GAIN_MASK GENMASK(17, 16) #define VEC_CONFIG0_CHROMA_GAIN_UNITY (0 << 16) @@ -76,6 +77,27 @@ #define VEC_SOFT_RESET 0x10c #define VEC_CLMP0_START0x144 #define VEC_CLMP0_END 0x148 + +/* + * These set the color subcarrier frequency + * if VEC_CONFIG1_CUSTOM_FREQ is enabled. + * + * VEC_FREQ1_0 contains the most significant 16-bit half-word, + * VEC_FREQ3_2 contains the least significant 16-bit half-word. + * 0x8000 seems to be equivalent to the pixel clock + * (which itself is the VEC clock divided by 8). + * + * Reference values (with the default pixel clock of 13.5 MHz): + * + * NTSC (3579545.[45] Hz) - 0x21F07C1F + * PAL (4433618.75 Hz) - 0x2A098ACB + * PAL-M (3575611.[888111] Hz) - 0x21E6EFE3 + * PAL-N (3582056.25 Hz) - 0x21F69446 + * + * NOTE: For SECAM, it is used as the Dr center frequency, + * regardless of whether VEC_CONFIG1_CUSTOM_FREQ is enabled or not; + * that is specified as 4406250 Hz, which corresponds to 0x29C71C72. + */ #define VEC_FREQ3_20x180 #define VEC_FREQ1_00x184 @@ -118,6 +140,14 @@ #define VEC_INTERRUPT_CONTROL 0x190 #define VEC_INTERRUPT_STATUS 0x194 + +/* + * Db center frequency for SECAM; the clock for this is the same as for + * VEC_FREQ3_2/VEC_FREQ1_0, which is used for Dr center frequency. + * + * This is specified as 425 Hz, which corresponds to 0x284BDA13. + * That is also the default value, so no need to set it explicitly. + */ #define VEC_FCW_SECAM_B0x198 #define VEC_SECAM_GAIN_VAL 0x19c @@ -197,6 +227,10 @@ enum vc4_vec_tv_mode_id { VC4_VEC_TV_MODE_NTSC_J, VC4_VEC_TV_MODE_PAL, VC4_VEC_TV_MODE_PAL_M, + VC4_VEC_TV_MODE_NTSC_443, + VC4_VEC_TV_MODE_PAL_60, + VC4_VEC_TV_MODE_PAL_N, + VC4_VEC_TV_MODE_SECAM, }; struct vc4_vec_tv_mode { @@ -239,6 +273,12 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = { .config0 = VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, + { + .mode = DRM_MODE_TV_MODE_NTSC_443, + .config0 = VEC_CONFIG0_NTSC_STD, + .config1 = VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, + .custom_freq = 0x2a098acb, + }, { .mode = DRM_MODE_TV_MODE_NTSC_J, .config0 = VEC_CONFIG0_NTSC_STD, @@ -254,6 +294,17 @@ static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = { .config0 = VEC_CONFIG0_PAL_M_STD, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, + { + .mode = DRM_MODE_TV_MODE_PAL_N, + .config0 = VEC_CONFIG0_PAL_N_STD, + .config1 = VEC_CONFIG1_C_CVBS_CVBS, + }, + { + .mode = DRM_MODE_TV_MODE_SECAM, + .config0 = VEC_CONFIG0_SECAM_STD, + .config1 = VEC_CONFIG1_C_CVBS_CVBS, + .custom_freq = 0x29c71c72, + }, }; static inline const struct vc4_vec_tv_mode * @@ -273,9 +324,13 @@ vc4_vec_tv_mode_lookup(unsigned int mode) static const struct drm_prop_enum_l
[PATCH v4 27/30] drm/vc4: vec: Check for VEC output constraints
From: Mateusz Kwiatkowski The VEC can accept pretty much any relatively reasonable mode, but still has a bunch of constraints to meet. Let's create an atomic_check() implementation that will make sure we don't end up accepting a non-functional mode. Acked-by: Noralf Trønnes Signed-off-by: Mateusz Kwiatkowski Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_vec.c | 48 +++ 1 file changed, 48 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index 90e375a8a8f9..1fcb7baf874e 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -453,6 +453,7 @@ static int vc4_vec_encoder_atomic_check(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { + const struct drm_display_mode *mode = &crtc_state->adjusted_mode; const struct vc4_vec_tv_mode *vec_mode; vec_mode = &vc4_vec_tv_modes[conn_state->tv.legacy_mode]; @@ -461,6 +462,53 @@ static int vc4_vec_encoder_atomic_check(struct drm_encoder *encoder, !drm_mode_equal(vec_mode->mode, &crtc_state->adjusted_mode)) return -EINVAL; + if (mode->crtc_hdisplay % 4) + return -EINVAL; + + if (!(mode->crtc_hsync_end - mode->crtc_hsync_start)) + return -EINVAL; + + switch (mode->vtotal) { + case 525: + if (mode->crtc_vtotal > 262) + return -EINVAL; + + if (mode->crtc_vdisplay < 1 || mode->crtc_vdisplay > 253) + return -EINVAL; + + if (!(mode->crtc_vsync_start - mode->crtc_vdisplay)) + return -EINVAL; + + if ((mode->crtc_vsync_end - mode->crtc_vsync_start) != 3) + return -EINVAL; + + if ((mode->crtc_vtotal - mode->crtc_vsync_end) < 4) + return -EINVAL; + + break; + + case 625: + if (mode->crtc_vtotal > 312) + return -EINVAL; + + if (mode->crtc_vdisplay < 1 || mode->crtc_vdisplay > 305) + return -EINVAL; + + if (!(mode->crtc_vsync_start - mode->crtc_vdisplay)) + return -EINVAL; + + if ((mode->crtc_vsync_end - mode->crtc_vsync_start) != 3) + return -EINVAL; + + if ((mode->crtc_vtotal - mode->crtc_vsync_end) < 2) + return -EINVAL; + + break; + + default: + return -EINVAL; + } + return 0; } -- b4 0.11.0-dev-7da52