dri-devel June 2022

dri-devel@lists.freedesktop.org

332 participants
697 discussions

[PATCH AUTOSEL 5.17 03/43] drm/amd/display: Read Golden Settings Table from VBIOS
by Sasha Levin 14 Jun '22

14 Jun '22

From: Sherry Wang <YAO.WANG1(a)amd.com> [ Upstream commit 4b81dd2cc6f4f4e8cea0ed6ee8d5193a8ae14a72 ] [Why] Dmub read AUX_DPHY_RX_CONTROL0 from Golden Setting Table, but driver will set it to default value 0x103d1110, which causes issue in some case [How] Remove the driver code, use the value set by dmub in dp_aux_init Reviewed-by: Nicholas Kazlauskas <Nicholas.Kazlauskas(a)amd.com> Acked-by: Jasdeep Dhillon <jdhillon(a)amd.com> Tested-by: Daniel Wheeler <daniel.wheeler(a)amd.com> Signed-off-by: Sherry Wang <YAO.WANG1(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c index 8b9b1a5309ba..0190a90260e9 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c @@ -230,9 +230,7 @@ static void enc31_hw_init(struct link_encoder *enc) AUX_RX_PHASE_DETECT_LEN, [21,20] = 0x3 default is 3 AUX_RX_DETECTION_THRESHOLD [30:28] = 1 */ - AUX_REG_WRITE(AUX_DPHY_RX_CONTROL0, 0x103d1110); - - AUX_REG_WRITE(AUX_DPHY_TX_CONTROL, 0x21c7a); + // dmub will read AUX_DPHY_RX_CONTROL0/AUX_DPHY_TX_CONTROL from vbios table in dp_aux_init //AUX_DPHY_TX_REF_CONTROL'AUX_TX_REF_DIV HW default is 0x32; // Set AUX_TX_REF_DIV Divider to generate 2 MHz reference from refclk -- 2.35.1

1 0

[PATCH AUTOSEL 5.18 15/47] drm/amdkfd: add pinned BOs to kfd_bo_list
by Sasha Levin 14 Jun '22

14 Jun '22

From: Lang Yu <Lang.Yu(a)amd.com> [ Upstream commit 4fac4fcf4500bce515b0f32195e7bb86aa0246c6 ] The kfd_bo_list is used to restore process BOs after evictions. As page tables could be destroyed during evictions, we should also update pinned BOs' page tables during restoring to make sure they are valid. So for pinned BOs, 1, Validate them and update their page tables. 2, Don't add eviction fence for them. v2: - Don't handle pinned ones specially in BO validation.(Felix) Signed-off-by: Lang Yu <Lang.Yu(a)amd.com> Acked-by: Christian König <christian.koenig(a)amd.com> Reviewed-by: Felix Kuehling <Felix.Kuehling(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index cd89d2e46852..f4509656ea8c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -1955,9 +1955,6 @@ int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev, return -EINVAL; } - /* delete kgd_mem from kfd_bo_list to avoid re-validating - * this BO in BO's restoring after eviction. - */ mutex_lock(&mem->process_info->lock); ret = amdgpu_bo_reserve(bo, true); @@ -1980,7 +1977,6 @@ int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev, amdgpu_amdkfd_remove_eviction_fence( bo, mem->process_info->eviction_fence); - list_del_init(&mem->validate_list.head); if (size) *size = amdgpu_bo_size(bo); @@ -2544,12 +2540,15 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef) process_info->eviction_fence = new_fence; *ef = dma_fence_get(&new_fence->base); - /* Attach new eviction fence to all BOs */ + /* Attach new eviction fence to all BOs except pinned ones */ list_for_each_entry(mem, &process_info->kfd_bo_list, - validate_list.head) + validate_list.head) { + if (mem->bo->tbo.pin_count) + continue; + amdgpu_bo_fence(mem->bo, &process_info->eviction_fence->base, true); - + } /* Attach eviction fence to PD / PT BOs */ list_for_each_entry(peer_vm, &process_info->vm_list_head, vm_list_node) { -- 2.35.1

1 0

[PATCH AUTOSEL 5.18 06/47] drm/amdkfd: Use mmget_not_zero in MMU notifier
by Sasha Levin 14 Jun '22

14 Jun '22

From: Philip Yang <Philip.Yang(a)amd.com> [ Upstream commit fa582c6f3684ac0098a9d02ddf0ed52a02b37127 ] MMU notifier callback may pass in mm with mm->mm_users==0 when process is exiting, use mmget_no_zero to avoid accessing invalid mm in deferred list work after mm is gone. Signed-off-by: Philip Yang <Philip.Yang(a)amd.com> Reviewed-by: Felix Kuehling <Felix.Kuehling(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c index 3b8856b4cece..5979335d7afd 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c @@ -2286,6 +2286,8 @@ svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni, if (range->event == MMU_NOTIFY_RELEASE) return true; + if (!mmget_not_zero(mni->mm)) + return true; start = mni->interval_tree.start; last = mni->interval_tree.last; @@ -2312,6 +2314,7 @@ svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni, } svm_range_unlock(prange); + mmput(mni->mm); return true; } -- 2.35.1

1 0

[PATCH AUTOSEL 5.18 05/47] drm/amdgpu: Resolve RAS GFX error count issue after cold boot on Arcturus
by Sasha Levin 14 Jun '22

14 Jun '22

From: Candice Li <candice.li(a)amd.com> [ Upstream commit 2a460963350ec6b1534d28d7f943b5f84815aff2 ] Adjust the sequence for ras late init and separate ras reset error status from query status. v2: squash in fix from Candice Signed-off-by: Candice Li <candice.li(a)amd.com> Reviewed-by: Hawking Zhang <Hawking.Zhang(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 9 ++++++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 27 ++++++++++++++++++++----- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c index 28a736c507bb..bd3b32e5ba9e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c @@ -625,17 +625,20 @@ int amdgpu_get_gfx_off_status(struct amdgpu_device *adev, uint32_t *value) int amdgpu_gfx_ras_late_init(struct amdgpu_device *adev, struct ras_common_if *ras_block) { int r; - r = amdgpu_ras_block_late_init(adev, ras_block); - if (r) - return r; if (amdgpu_ras_is_supported(adev, ras_block->block)) { if (!amdgpu_persistent_edc_harvesting_supported(adev)) amdgpu_ras_reset_error_status(adev, AMDGPU_RAS_BLOCK__GFX); + r = amdgpu_ras_block_late_init(adev, ras_block); + if (r) + return r; + r = amdgpu_irq_get(adev, &adev->gfx.cp_ecc_error_irq, 0); if (r) goto late_fini; + } else { + amdgpu_ras_feature_enable_on_boot(adev, ras_block, 0); } return 0; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 424c22a841f4..3f96dadf2698 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -195,6 +195,13 @@ static ssize_t amdgpu_ras_debugfs_read(struct file *f, char __user *buf, if (amdgpu_ras_query_error_status(obj->adev, &info)) return -EINVAL; + /* Hardware counter will be reset automatically after the query on Vega20 and Arcturus */ + if (obj->adev->ip_versions[MP0_HWIP][0] != IP_VERSION(11, 0, 2) && + obj->adev->ip_versions[MP0_HWIP][0] != IP_VERSION(11, 0, 4)) { + if (amdgpu_ras_reset_error_status(obj->adev, info.head.block)) + dev_warn(obj->adev->dev, "Failed to reset error counter and error status"); + } + s = snprintf(val, sizeof(val), "%s: %lu\n%s: %lu\n", "ue", info.ue_count, "ce", info.ce_count); @@ -548,9 +555,10 @@ static ssize_t amdgpu_ras_sysfs_read(struct device *dev, if (amdgpu_ras_query_error_status(obj->adev, &info)) return -EINVAL; - if (obj->adev->asic_type == CHIP_ALDEBARAN) { + if (obj->adev->ip_versions[MP0_HWIP][0] != IP_VERSION(11, 0, 2) && + obj->adev->ip_versions[MP0_HWIP][0] != IP_VERSION(11, 0, 4)) { if (amdgpu_ras_reset_error_status(obj->adev, info.head.block)) - DRM_WARN("Failed to reset error counter and error status"); + dev_warn(obj->adev->dev, "Failed to reset error counter and error status"); } return sysfs_emit(buf, "%s: %lu\n%s: %lu\n", "ue", info.ue_count, @@ -1023,9 +1031,6 @@ int amdgpu_ras_query_error_status(struct amdgpu_device *adev, } } - if (!amdgpu_persistent_edc_harvesting_supported(adev)) - amdgpu_ras_reset_error_status(adev, info->head.block); - return 0; } @@ -1145,6 +1150,12 @@ int amdgpu_ras_query_error_count(struct amdgpu_device *adev, if (res) return res; + if (adev->ip_versions[MP0_HWIP][0] != IP_VERSION(11, 0, 2) && + adev->ip_versions[MP0_HWIP][0] != IP_VERSION(11, 0, 4)) { + if (amdgpu_ras_reset_error_status(adev, info.head.block)) + dev_warn(adev->dev, "Failed to reset error counter and error status"); + } + ce += info.ce_count; ue += info.ue_count; } @@ -1705,6 +1716,12 @@ static void amdgpu_ras_log_on_err_counter(struct amdgpu_device *adev) continue; amdgpu_ras_query_error_status(adev, &info); + + if (adev->ip_versions[MP0_HWIP][0] != IP_VERSION(11, 0, 2) && + adev->ip_versions[MP0_HWIP][0] != IP_VERSION(11, 0, 4)) { + if (amdgpu_ras_reset_error_status(adev, info.head.block)) + dev_warn(adev->dev, "Failed to reset error counter and error status"); + } } } -- 2.35.1

1 0

[PATCH AUTOSEL 5.18 04/47] drm/amd/display: Read Golden Settings Table from VBIOS
by Sasha Levin 14 Jun '22

14 Jun '22

From: Sherry Wang <YAO.WANG1(a)amd.com> [ Upstream commit 4b81dd2cc6f4f4e8cea0ed6ee8d5193a8ae14a72 ] [Why] Dmub read AUX_DPHY_RX_CONTROL0 from Golden Setting Table, but driver will set it to default value 0x103d1110, which causes issue in some case [How] Remove the driver code, use the value set by dmub in dp_aux_init Reviewed-by: Nicholas Kazlauskas <Nicholas.Kazlauskas(a)amd.com> Acked-by: Jasdeep Dhillon <jdhillon(a)amd.com> Tested-by: Daniel Wheeler <daniel.wheeler(a)amd.com> Signed-off-by: Sherry Wang <YAO.WANG1(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c index d94fd1010deb..8b12b4111c88 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_dio_link_encoder.c @@ -230,9 +230,7 @@ static void enc31_hw_init(struct link_encoder *enc) AUX_RX_PHASE_DETECT_LEN, [21,20] = 0x3 default is 3 AUX_RX_DETECTION_THRESHOLD [30:28] = 1 */ - AUX_REG_WRITE(AUX_DPHY_RX_CONTROL0, 0x103d1110); - - AUX_REG_WRITE(AUX_DPHY_TX_CONTROL, 0x21c7a); + // dmub will read AUX_DPHY_RX_CONTROL0/AUX_DPHY_TX_CONTROL from vbios table in dp_aux_init //AUX_DPHY_TX_REF_CONTROL'AUX_TX_REF_DIV HW default is 0x32; // Set AUX_TX_REF_DIV Divider to generate 2 MHz reference from refclk -- 2.35.1

1 0

[PATCH v6 0/9] Add new formats support to vkms
by Igor Torrente 14 Jun '22

14 Jun '22

Summary ======= This series of patches refactor some vkms components in order to introduce new formats to the planes and writeback connector. Now in the blend function, the plane's pixels are converted to ARGB16161616 and then blended together. The CRC is calculated based on the ARGB1616161616 buffer. And if required, this buffer is copied/converted to the writeback buffer format. And to handle the pixel conversion, new functions were added to convert from a specific format to ARGB16161616 (the reciprocal is also true). Tests ===== This patch series was tested using the following igt tests: -t ".*kms_plane.*" -t ".*kms_writeback.*" -t ".*kms_cursor_crc*" -t ".*kms_flip.*" New tests passing ------------------- - pipe-A-cursor-size-change - pipe-A-cursor-alpha-transparent Performance ----------- It's running slightly faster than the current implementation. Results running the IGT[1] test `igt@kms_cursor_crc@pipe-a-cursor-512x512-onscreen` ten times: | Frametime | |:--------------------------------------------:| | Implementation | Current | This commit | |:---------------:|:---------:|:--------------:| | frametime range | 9~22 ms | 10~22 ms | | Average | 11.4 ms | 12.32 ms | Memory consumption ================== It consumes less memory than the current implementation in the common case (more detail in the commit message). | Memory consumption (output dimensions) | |:--------------------------------------:| | Current | This patch | |:------------------:|:-----------------:| | Width * Heigth | 2 * Width | [1] IGT commit id: bc3f6833a12221a46659535dac06ebb312490eb4 XRGB to ARGB behavior ===================== During the development, I decided to always fill the alpha channel of the output pixel whenever the conversion from a format without an alpha channel to ARGB16161616 is necessary. Therefore, I ignore the value received from the XRGB and overwrite the value with 0xFFFF. Primary plane and CRTC size =========================== This patch series reworks the blend function to accept a primary plane with a different size and position from CRTC. Because now we need to fill the background, we had a loss in performance with this change Alpha channel output for XRGB formats ===================================== There's still an open question about which value the writeback alpha channel should be for XRGB formats. The current igt test implementation is expecting the channel to not be change. But it's not entirely clear if this should be the behavior followed by vkms (or any other driver). Open issue: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/118 --- Igor Torrente (9): drm: vkms: Replace hardcoded value of `vkms_composer.map` to DRM_FORMAT_MAX_PLANES drm: vkms: Rename `vkms_composer` to `vkms_frame_info` drm: drm_atomic_helper: Add a new helper to deal with the writeback connector validation drm: vkms: get the reference to `drm_framebuffer` instead if coping it drm: vkms: Add fb information to `vkms_writeback_job` drm: vkms: Refactor the plane composer to accept new formats drm: vkms: Supports to the case where primary plane doesn't match the CRTC drm: vkms: Adds XRGB_16161616 and ARGB_1616161616 formats drm: vkms: Add support to the RGB565 format Documentation/gpu/vkms.rst | 7 +- drivers/gpu/drm/drm_atomic_helper.c | 39 ++++ drivers/gpu/drm/vkms/Makefile | 1 + drivers/gpu/drm/vkms/vkms_composer.c | 314 ++++++++++++-------------- drivers/gpu/drm/vkms/vkms_drv.h | 39 +++- drivers/gpu/drm/vkms/vkms_formats.c | 300 ++++++++++++++++++++++++ drivers/gpu/drm/vkms/vkms_formats.h | 12 + drivers/gpu/drm/vkms/vkms_plane.c | 50 ++-- drivers/gpu/drm/vkms/vkms_writeback.c | 39 +++- include/drm/drm_atomic_helper.h | 3 + 10 files changed, 584 insertions(+), 220 deletions(-) create mode 100644 drivers/gpu/drm/vkms/vkms_formats.c create mode 100644 drivers/gpu/drm/vkms/vkms_formats.h -- 2.30.2

1 9

[PATCH v2] drm/msm: use for_each_sgtable_sg to iterate over scatterlist
by Jonathan Marek 13 Jun '22

13 Jun '22

The dma_map_sgtable() call (used to invalidate cache) overwrites sgt->nents with 1, so msm_iommu_pagetable_map maps only the first physical segment. To fix this problem use for_each_sgtable_sg(), which uses orig_nents. Fixes: b145c6e65eb0 ("drm/msm: Add support to create a local pagetable") Signed-off-by: Jonathan Marek <jonathan(a)marek.ca> --- v2: use for_each_sgtable_sg and update commit message drivers/gpu/drm/msm/msm_iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index bcaddbba564df..a54ed354578b5 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -58,7 +58,7 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova, u64 addr = iova; unsigned int i; - for_each_sg(sgt->sgl, sg, sgt->nents, i) { + for_each_sgtable_sg(sgt, sg, i) { size_t size = sg->length; phys_addr_t phys = sg_phys(sg); -- 2.26.1

1 0

[PATCH v4 1/2] dt-bindings: lcdif: Add compatible for i.MX8MP
by Marek Vasut 13 Jun '22

13 Jun '22

Add compatible string for i.MX8MP LCDIF variant. This is called LCDIFv3 and is completely different from the LCDIFv3 found in i.MX23 in that it has a completely scrambled register layout compared to all previous LCDIF variants. The new LCDIFv3 also supports 36bit address space. However, except for the complete bit reshuffling, this is still LCDIF and it still works like one. Signed-off-by: Marek Vasut <marex(a)denx.de> Cc: Alexander Stein <alexander.stein(a)ew.tq-group.com> Cc: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com> Cc: Lucas Stach <l.stach(a)pengutronix.de> Cc: Peng Fan <peng.fan(a)nxp.com> Cc: Rob Herring <robh+dt(a)kernel.org> Cc: Robby Cai <robby.cai(a)nxp.com> Cc: Sam Ravnborg <sam(a)ravnborg.org> Cc: Stefan Agner <stefan(a)agner.ch> Cc: devicetree(a)vger.kernel.org --- V2: No change V3: No change V4: No change --- Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml index 900a56cae80e6..876015a44a1e6 100644 --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -20,6 +20,7 @@ properties: - fsl,imx23-lcdif - fsl,imx28-lcdif - fsl,imx6sx-lcdif + - fsl,imx8mp-lcdif - items: - enum: - fsl,imx6sl-lcdif -- 2.35.1

5 9

DRM/KMS PWL API Thoughts and Questions
by Harry Wentland 13 Jun '22

13 Jun '22

(I'm sending this as an email as lowest common denominator but feel an issue on the color-and-hdr repo would be a better interface for productive discussion. Please pop over to https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/10 if you agree. Hopefully we can drive the discussion there but if there is a strong preference for email that works as well. :) ) I've wanted to start a thread to discuss the use of PWL APIs that were introduced by Uma a year ago and for which Bhanuprakash provided IGT tests. I have come to like the API but as we're getting closer to a real-world use of it I have a few questions and comments. As with a lot of complex APIs the devil is in the details. Some of those details are currently underspecified, or underdocumented and it's important that we all interpret the API the same way. **The API** The original patches posted by Uma: https://patchwork.freedesktop.org/series/90822/ https://patchwork.freedesktop.org/series/90826/ The IGT tests for PWL API: https://patchwork.freedesktop.org/series/96895/ I've rebased the kernel patches on a slightly more recent kernel, along with an AMD implementation: https://gitlab.freedesktop.org/hwentland/linux/-/tree/color-and-hdr I've also rebased them on an IGT tree, but that's not too up-to-date: https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/tree/color-and-hdr **Why do I like the API?** In order to allow HW composition of HDR planes in linear space we need the ability to program at least a per-CRTC regamma (inv_EOTF) to go from linear to wire format post-blending. Since userspace might want to apply corrections on top of a simple transfer function (such as PQ, BT.709, etc.) it would like a way to set a custom LUT. The existing CRTC gamma LUT defines equally spaced entries. As Pekka shows in [1] equally-spaced LUTs have unacceptable error for regamma/inv_EOTF. Hence we need finer granularity of our entries near zero while coarse granularity works fine toward the brighter values. [1] https://gitlab.freedesktop.org/pq/color-and-hdr/-/merge_requests/9 HW (at least AMD and Intel HW) implements this ability as segmented piece-wise linear LUTs. These define segments of equally spaced entries. These segments are constrained by the HW implementation. I like how the PWL API allows different drivers to specify the constraints imposed by different HW while allowing userspace a generic way of parsing the PWL. This also avoids complex calculations in the kernel driver, which might be required for other APIs one could envision. If anyone likes I can elaborate on some ideas for an alternate API, though all of them will require non-trivial transformations by the kernel driver in order to program them to HW. **Nitpicks** The API defines everything inside the segments, including flags and values that apply to the entire PWL, such as DRM_MODE_LUT_GAMMA, DRM_MODE_LUT_REFLECT_NEGATIVE, input_bpc, and output_bpc. If these don't stay constant for segments it might complicate the interpretation of segments. I suggest we consider these as effectively applying to the entire PWL. We could encode them in an overall drm_color_lut struct that includes an array of drm_color_lut_range but that's probably not necessary, hence why I called this out as a nitpick. I would just like us to be aware of this ambiguity and document that these values applies to the entire PWL. **How to read the PWL** Let me first give a summary for how this LUT is used in userspace. If you're familiar with this please review and comment if I got things wrong. As I mentioned, a lot of this is underspecified at the moment so you're reading my interpretation. You can see this behavior in plane_degamma_test [2] in the kms_color.c IGT test suite. I suggest the plane_degamma_test here here instead of the test_pipe_gamma test as the latter still has Intelisms (assumptions around Intel driver/HW behavior) and will not work for other drivers. Iterate over all enums in PLANE_DEGAMMA_MODE and find a suitable one. How do we find the suitable one? More on that below. Once we have the right PLANE_DEGAMMA_MODE we read the blob for the blob ID associated with the PLANE_DEGAMMA_MODE enum. We interpret the blob as an array of drm_color_lut_range. See get_segment_data [3]. We can think of our LUT/PWL as f(x) = y. For a traditional equally spaced LUT with 1024 entries x would be 0, 1, 2, ..., 1023. For a PWL LUT we need to parse the segment data provided in drm_color_lut_range. Let's look at the 2nd-last entry of the nonlinear_pwl definition for the AMD driver [4] (I've correct it here and dropped the DRM_MODE_LUT_REUSE_LAST but it's still incorrect in the link) and simplify it to 4 entries for sake of readability: { .flags = (DRM_MODE_LUT_GAMMA | DRM_MODE_LUT_REFLECT_NEGATIVE | DRM_MODE_LUT_INTERPOLATE | DRM_MODE_LUT_NON_DECREASING), .count = 4, .input_bpc = 13, .output_bpc = 18, .start = 1 << 12, .end = 1 << 13, .min = 0, .max = 1UL << 35 }, We see we have 16 entries in the region from (1 << 12) to (1 << 13). We see input_bpc is 13, so our 1.0 float value is 1 << 13. (Is it sensible to use input_bpc as our 1.0 floating point reference? Why?) Since this segment is not reusing the last entry (doesn't have DRM_MODE_LUT_REUSE_LAST) we divide the region between 1 << 12 and 1 << 13 into 4 equally spaced sections. step_size = (segment->end - segment->start) / count In our case our segment spans from 4096 to 8192 and yields a step_size of 1024. Note that we need to calculate this in floating point, otherwise we're not guaranteed equal spacing. This gives us these X entries for this particular segment: 4096, 5120, 6144, 7168 And normalized to 1 << 13 (input_bpc) for our 1.0 float value we get 0.5, 0.625, 0.75, 0.875 If the segment had the REUSE_LAST flag the values would look like 4096, 5461, 6826, 8192 and normalized to 0.5, 0.666626, 0.833252, 1 Though in the case of REUSE_LAST a more sensible definition might be with a count of 5 entries in this segment, instead of 16. But ultimatly that's up to the driver. I attached a simple C program that parses a PWL and helps illustrate my interpretation. You'll just need to copy-paste or include your PWL definition (instead of color_gamma.h), and point the PWL define to your PWL variable. To build it you'll need to copy the drm-uapi folder from my IGT repo into the same directory as pwl-parser.c and then just build it with "gcc -Idrm-uapi pwl-parser.c". The above entries come from running pwl-parser with the nonlinear_pwl from [5]. To illustrate these I added versions of your lut1d_error scripts that run on the SDR and HDR PWLs for AMD HW [6]. We should probably document the above in detail in the DRM/KMS API docs. [2] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/blob/color-and-hdr… [3] https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/blob/color-and-hdr… [4] https://gitlab.freedesktop.org/hwentland/linux/-/blob/color-and-hdr/drivers… [5] https://gitlab.freedesktop.org/hwentland/linux/-/blob/color-and-hdr/drivers… [6] https://gitlab.freedesktop.org/hwentland/color-and-hdr/-/tree/precision/oct… **How to provide PWL entries?** To provide the PWL values for each entry we'll have to sample our (custom) curve at the respective points specified by our PWL entries and providing those samples in a blob that is passed to PLANE_DEGAMMA_LUT. It's not much different from how we provide values for an equally spaced (legacy) LUT. As for sampling our curve, I remember seeing that Weston uses an LCMS function to sample the curve at required points. Last I checked it samples the curve at evenly spaced intervals but the LCMS function seemed to provide arbitrary sampling. **How to pick a suitable PWL definition?** Picking the right PWL definition out of the respective \_MODE enum isn't trivial. Without further information a userspace implementer has to understand the distribution of entries in all segments and perform a bunch of math to estimate the error for given curves. A simpler approach might be if we defined common naming for our PWL enums. We can define the commonly expected cases. The two important parameters are within-range vs overrange entries, and linear vs non-linear outputs. Within-range PWLs would cover [0.0, 1.0] entries and overrange [0.0, 128.0] to cover PQ and probably anything else. One could think of the within-range PWL as intended for SDR content and the over-range PWL for HDR content. Linear PWLs would be intended for linear luminance outputs (or near-linear), and can be represented by equally spaced LUTs, such as a single-segment definitions. Non-linear PWLs would be intended for linear to non-linear transforms; Linear PWLs for non-linear to linear transforms or OOTFs. This gives us four enums, plus one for bypass: DRM_MODE_LUT_BYPASS DRM_MODE_LUT_LINEAR_SDR DRM_MODE_LUT_NONLINEAR_SDR DRM_MODE_LUT_LINEAR_HDR DRM_MODE_LUT_NONLINEAR_HDR Drivers can provide appropriate PWLs based on the HW caps. Userspace can pick an appropriate one if it's available or fall back to either a sub-optimal PWL or to using a GPU transform instead. Thoughts? Thanks, Harry

2 1

[PATCH] drm/bridge: tc358767: don't fixup mode polarity
by Lucas Stach 13 Jun '22

13 Jun '22

It doesn't always work, as there is just a single adjusted mode, so if parts of the display pipeline disagree on the required sync polarity it's possible to end up with a invalid config. However it also isn't necessary for this bridge chip, as the DPI interface has a configurable sync polarity, so we can just set it up to match the mode. Signed-off-by: Lucas Stach <l.stach(a)pengutronix.de> --- drivers/gpu/drm/bridge/tc358767.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 485717c8f0b4..bd2cae711ca0 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -889,6 +889,7 @@ static int tc_set_edp_video_mode(struct tc_data *tc, u32 dp0_syncval; u32 bits_per_pixel = 24; u32 in_bw, out_bw; + u32 dpipxlfmt; /* * Recommended maximum number of symbols transferred in a transfer unit: @@ -938,10 +939,15 @@ static int tc_set_edp_video_mode(struct tc_data *tc, if (ret) return ret; - ret = regmap_write(tc->regmap, DPIPXLFMT, - VS_POL_ACTIVE_LOW | HS_POL_ACTIVE_LOW | - DE_POL_ACTIVE_HIGH | SUB_CFG_TYPE_CONFIG1 | - DPI_BPP_RGB888); + dpipxlfmt = DE_POL_ACTIVE_HIGH | SUB_CFG_TYPE_CONFIG1 | DPI_BPP_RGB888; + + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + dpipxlfmt |= VS_POL_ACTIVE_LOW; + + if (mode->flags & DRM_MODE_FLAG_NHSYNC) + dpipxlfmt |= HS_POL_ACTIVE_LOW; + + ret = regmap_write(tc->regmap, DPIPXLFMT, dpipxlfmt); if (ret) return ret; @@ -1471,18 +1477,6 @@ tc_edp_bridge_atomic_disable(struct drm_bridge *bridge, dev_err(tc->dev, "main link disable error: %d\n", ret); } -static bool tc_bridge_mode_fixup(struct drm_bridge *bridge, - const struct drm_display_mode *mode, - struct drm_display_mode *adj) -{ - /* Fixup sync polarities, both hsync and vsync are active low */ - adj->flags = mode->flags; - adj->flags |= (DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC); - adj->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); - - return true; -} - static int tc_common_atomic_check(struct drm_bridge *bridge, struct drm_bridge_state *bridge_state, struct drm_crtc_state *crtc_state, @@ -1758,7 +1752,6 @@ static const struct drm_bridge_funcs tc_edp_bridge_funcs = { .atomic_check = tc_edp_atomic_check, .atomic_enable = tc_edp_bridge_atomic_enable, .atomic_disable = tc_edp_bridge_atomic_disable, - .mode_fixup = tc_bridge_mode_fixup, .detect = tc_bridge_detect, .get_edid = tc_get_edid, .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, -- 2.30.2

2 1

← Newer
1
...
37
38
39
40
41
42
43
...
70
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel June 2022