| Message ID | 20220919130505.1984383-1-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Enable Pipewriteback | expand |
Hi Jani, All patches have been reviewed by Arun it would be great if you could have a Look and give an ACK. Regards, Suraj Kandpal > Subject: [PATCH v6 0/3] Enable Pipewriteback > > A patch series was floated in the drm mailing list which aimed to change the > drm_connector and drm_encoder fields to pointer in the > drm_connector_writeback structure, this received a huge pushback from the > community but since i915 expects each connector present in the drm_device > list to be a intel_connector but drm_writeback framework makes us have a > connector which cannot be embedded in an intel_connector structure. > [1] > https://patchwork.kernel.org/project/dri- > devel/patch/20220202081702.22119-1-suraj.kandpal@intel.com/ > [2] > https://patchwork.kernel.org/project/dri- > devel/patch/20220202085429.22261-6-suraj.kandpal@intel.com/ > Since no one had an issue with encoder field being changed into a pointer it > was decided to break the connector and encoder pointer changes into two > different series.The encoder field changes is currently being worked upon by > Abhinav Kumar and the changes have been merged. > [3]https://patchwork.kernel.org/project/dri-devel/list/?series=633565 > Going forward we use a drm_connector which is not embedded in > intel_connector. > We also create a intel_encoder to avoid changes to many iterators but no > intel_connector. We also changed all iterators that > > ---v2 > solving BAT issues > > ---v3 > -removing unecessary comments from i915_reg.h [Arun] -moving wd_init into > its own if condition [Arun] -fixing comment styling and alignment in intel_wd.c > [Arun] -removing continue from loop and calling function if condition is met > [Arun] -removing useless arguments from intel_queue_writeback_job and > intel_enabling_capture [Arun] > > --v4 > Adding Reviewed-by to patches which were previously reviewd > > --v5 > Adding Reviewed-by for patch 3 > > --v6 > Solving BAT issue > changes for checkpatch to pass > > Suraj Kandpal (3): > drm/i915: Define WD trancoder for i915 > drm/i915 : Changing intel_connector iterators > drm/i915: Enabling WD Transcoder > > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/display/intel_acpi.c | 8 +- > drivers/gpu/drm/i915/display/intel_crtc.c | 6 + > .../drm/i915/display/intel_crtc_state_dump.c | 1 + > drivers/gpu/drm/i915/display/intel_ddi.c | 6 + > drivers/gpu/drm/i915/display/intel_display.c | 68 +- > drivers/gpu/drm/i915/display/intel_display.h | 18 +- > .../drm/i915/display/intel_display_debugfs.c | 13 +- > .../drm/i915/display/intel_display_types.h | 32 +- > drivers/gpu/drm/i915/display/intel_dpll.c | 6 + > .../drm/i915/display/intel_modeset_setup.c | 119 ++- > .../drm/i915/display/intel_modeset_verify.c | 17 +- > drivers/gpu/drm/i915/display/intel_opregion.c | 3 + > drivers/gpu/drm/i915/display/intel_wd.c | 694 ++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_wd.h | 48 ++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_irq.c | 8 +- > drivers/gpu/drm/i915/i915_pci.c | 7 +- > drivers/gpu/drm/i915/i915_reg.h | 137 ++++ > 19 files changed, 1137 insertions(+), 56 deletions(-) create mode 100644 > drivers/gpu/drm/i915/display/intel_wd.c > create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h > > -- > 2.25.1
Hi Suraj, CI is not green https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107440v9/shards-all.html?testfilter=kms_writeback 2/4 subtests are passing writeback-fb-id subtest can be fixed by asserting EINVAL for i915 we need to fix writeback-check-output first On local validation, we were getting CRC mismatch however CI results show different issue https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107440v9/shard-tglu-4/igt@kms_writeback@writeback-check-output.html#dmesg-warnings575 Please check the same if its relevant. On 19-Sep-22 6:35 PM, Kandpal, Suraj wrote: > A patch series was floated in the drm mailing list which aimed to change > the drm_connector and drm_encoder fields to pointer in the > drm_connector_writeback structure, this received a huge pushback from > the community but since i915 expects each connector present in the > drm_device list to be a intel_connector but drm_writeback framework > makes us have a connector which cannot be embedded in an intel_connector > structure. > [1] > https://patchwork.kernel.org/project/dri-devel/patch/20220202081702.22119-1-suraj.kandpal@intel.com/ > [2] > https://patchwork.kernel.org/project/dri-devel/patch/20220202085429.22261-6-suraj.kandpal@intel.com/ > Since no one had an issue with encoder field being changed into a > pointer it was decided to break the connector and encoder pointer > changes into two different series.The encoder field changes is > currently being worked upon by Abhinav Kumar and the changes have been > merged. > [3]https://patchwork.kernel.org/project/dri-devel/list/?series=633565 > Going forward we use a drm_connector which is not embedded in > intel_connector. > We also create a intel_encoder to avoid changes to many > iterators but no intel_connector. We also changed all iterators that > > ---v2 > solving BAT issues > > ---v3 > -removing unecessary comments from i915_reg.h [Arun] > -moving wd_init into its own if condition [Arun] > -fixing comment styling and alignment in intel_wd.c [Arun] > -removing continue from loop and calling function if condition is met > [Arun] > -removing useless arguments from intel_queue_writeback_job and > intel_enabling_capture [Arun] > > --v4 > Adding Reviewed-by to patches which were previously reviewd > > --v5 > Adding Reviewed-by for patch 3 > > --v6 > Solving BAT issue > changes for checkpatch to pass > > Suraj Kandpal (3): > drm/i915: Define WD trancoder for i915 > drm/i915 : Changing intel_connector iterators > drm/i915: Enabling WD Transcoder > > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/display/intel_acpi.c | 8 +- > drivers/gpu/drm/i915/display/intel_crtc.c | 6 + > .../drm/i915/display/intel_crtc_state_dump.c | 1 + > drivers/gpu/drm/i915/display/intel_ddi.c | 6 + > drivers/gpu/drm/i915/display/intel_display.c | 68 +- > drivers/gpu/drm/i915/display/intel_display.h | 18 +- > .../drm/i915/display/intel_display_debugfs.c | 13 +- > .../drm/i915/display/intel_display_types.h | 32 +- > drivers/gpu/drm/i915/display/intel_dpll.c | 6 + > .../drm/i915/display/intel_modeset_setup.c | 119 ++- > .../drm/i915/display/intel_modeset_verify.c | 17 +- > drivers/gpu/drm/i915/display/intel_opregion.c | 3 + > drivers/gpu/drm/i915/display/intel_wd.c | 694 ++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_wd.h | 48 ++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_irq.c | 8 +- > drivers/gpu/drm/i915/i915_pci.c | 7 +- > drivers/gpu/drm/i915/i915_reg.h | 137 ++++ > 19 files changed, 1137 insertions(+), 56 deletions(-) > create mode 100644 drivers/gpu/drm/i915/display/intel_wd.c > create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h >
Hi Swati, > Hi Suraj, > > CI is not green > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107440v9/shards- > all.html?testfilter=kms_writeback > > 2/4 subtests are passing > writeback-fb-id subtest can be fixed by asserting EINVAL for i915 we need to Also we will be sending new patches following this series that sends a fake vblank when a writeback job on the pipe with wb connector attached instead of failing it atomic check. So no change in this test will be required > fix writeback-check-output first On local validation, we were getting CRC > mismatch however CI results show different issue > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_107440v9/shard-tglu- > 4/igt@kms_writeback@writeback-check-output.html#dmesg-warnings575 > Please check the same if its relevant. This is still a CRC mismatch which is happening kms_writeback:1432) igt_kms-DEBUG: display: } (kms_writeback:1432) igt_debugfs-DEBUG: Opening debugfs directory '/sys/kernel/debug/dri/0' (kms_writeback:1432) igt_debugfs-DEBUG: CRC mismatch at index 0: 0xcccb1dc5 != 0x34ab9dc5 (kms_writeback:1432) igt_debugfs-CRITICAL: Test assertion failure function igt_assert_crc_equal, file ../lib/igt_debugfs.c:491: (kms_writeback:1432) igt_debugfs-CRITICAL: Failed assertion: !mismatch || igt_skip_crc_compare (kms_writeback:1432) igt_core-INFO: Stack trace: (kms_writeback:1432) igt_core-INFO: #0 ../lib/igt_core.c:1819 __igt_fail_assert() (kms_writeback:1432) igt_core-INFO: #1 ../lib/igt_debugfs.c:492 igt_assert_crc_equal() (kms_writeback:1432) igt_core-INFO: #2 ../tests/kms_writeback.c:343 writeback_sequence() (kms_writeback:1432) igt_core-INFO: #3 ../tests/kms_writeback.c:360 __igt_unique____real_main480() (kms_writeback:1432) igt_core-INFO: #4 ../tests/kms_writeback.c:480 main() (kms_writeback:1432) igt_core-INFO: #5 ../csu/libc-start.c:344 __libc_start_main() (kms_writeback:1432) igt_core-INFO: #6 [_start+0x2a] **** END **** Subtest writeback-check-output: FAIL (0.137s) which is due to color formats for which the changes are going to be sent in subsequent patch The dmesg failure that is seen here is because of no vblank event being present when pipe is getting detached and should Be fixed with the next patch series that also solves the writeback_fb_id issue Regards, Suraj Kandpal > > On 19-Sep-22 6:35 PM, Kandpal, Suraj wrote: > > A patch series was floated in the drm mailing list which aimed to > > change the drm_connector and drm_encoder fields to pointer in the > > drm_connector_writeback structure, this received a huge pushback from > > the community but since i915 expects each connector present in the > > drm_device list to be a intel_connector but drm_writeback framework > > makes us have a connector which cannot be embedded in an > > intel_connector structure. > > [1] > > https://patchwork.kernel.org/project/dri-devel/patch/20220202081702.22 > > 119-1-suraj.kandpal@intel.com/ > > [2] > > https://patchwork.kernel.org/project/dri-devel/patch/20220202085429.22 > > 261-6-suraj.kandpal@intel.com/ Since no one had an issue with encoder > > field being changed into a pointer it was decided to break the > > connector and encoder pointer changes into two different series.The > > encoder field changes is currently being worked upon by Abhinav Kumar > > and the changes have been merged. > > [3]https://patchwork.kernel.org/project/dri-devel/list/?series=633565 > > Going forward we use a drm_connector which is not embedded in > > intel_connector. > > We also create a intel_encoder to avoid changes to many iterators but > > no intel_connector. We also changed all iterators that > > > > ---v2 > > solving BAT issues > > > > ---v3 > > -removing unecessary comments from i915_reg.h [Arun] -moving wd_init > > into its own if condition [Arun] -fixing comment styling and alignment > > in intel_wd.c [Arun] -removing continue from loop and calling function > > if condition is met [Arun] -removing useless arguments from > > intel_queue_writeback_job and intel_enabling_capture [Arun] > > > > --v4 > > Adding Reviewed-by to patches which were previously reviewd > > > > --v5 > > Adding Reviewed-by for patch 3 > > > > --v6 > > Solving BAT issue > > changes for checkpatch to pass > > > > Suraj Kandpal (3): > > drm/i915: Define WD trancoder for i915 > > drm/i915 : Changing intel_connector iterators > > drm/i915: Enabling WD Transcoder > > > > drivers/gpu/drm/i915/Makefile | 1 + > > drivers/gpu/drm/i915/display/intel_acpi.c | 8 +- > > drivers/gpu/drm/i915/display/intel_crtc.c | 6 + > > .../drm/i915/display/intel_crtc_state_dump.c | 1 + > > drivers/gpu/drm/i915/display/intel_ddi.c | 6 + > > drivers/gpu/drm/i915/display/intel_display.c | 68 +- > > drivers/gpu/drm/i915/display/intel_display.h | 18 +- > > .../drm/i915/display/intel_display_debugfs.c | 13 +- > > .../drm/i915/display/intel_display_types.h | 32 +- > > drivers/gpu/drm/i915/display/intel_dpll.c | 6 + > > .../drm/i915/display/intel_modeset_setup.c | 119 ++- > > .../drm/i915/display/intel_modeset_verify.c | 17 +- > > drivers/gpu/drm/i915/display/intel_opregion.c | 3 + > > drivers/gpu/drm/i915/display/intel_wd.c | 694 ++++++++++++++++++ > > drivers/gpu/drm/i915/display/intel_wd.h | 48 ++ > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/i915_irq.c | 8 +- > > drivers/gpu/drm/i915/i915_pci.c | 7 +- > > drivers/gpu/drm/i915/i915_reg.h | 137 ++++ > > 19 files changed, 1137 insertions(+), 56 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/display/intel_wd.c > > create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h > > > > -- > ~Swati Sharma
Also, please specify in cover-letter, current series WB support is for TGL+ since WB is supported from SKL+ as per bspec https://gfxspecs.intel.com/Predator/Home/Index/4290 On 19-Sep-22 6:35 PM, Kandpal, Suraj wrote: > A patch series was floated in the drm mailing list which aimed to change > the drm_connector and drm_encoder fields to pointer in the > drm_connector_writeback structure, this received a huge pushback from > the community but since i915 expects each connector present in the > drm_device list to be a intel_connector but drm_writeback framework > makes us have a connector which cannot be embedded in an intel_connector > structure. > [1] > https://patchwork.kernel.org/project/dri-devel/patch/20220202081702.22119-1-suraj.kandpal@intel.com/ > [2] > https://patchwork.kernel.org/project/dri-devel/patch/20220202085429.22261-6-suraj.kandpal@intel.com/ > Since no one had an issue with encoder field being changed into a > pointer it was decided to break the connector and encoder pointer > changes into two different series.The encoder field changes is > currently being worked upon by Abhinav Kumar and the changes have been > merged. > [3]https://patchwork.kernel.org/project/dri-devel/list/?series=633565 > Going forward we use a drm_connector which is not embedded in > intel_connector. > We also create a intel_encoder to avoid changes to many > iterators but no intel_connector. We also changed all iterators that > > ---v2 > solving BAT issues > > ---v3 > -removing unecessary comments from i915_reg.h [Arun] > -moving wd_init into its own if condition [Arun] > -fixing comment styling and alignment in intel_wd.c [Arun] > -removing continue from loop and calling function if condition is met > [Arun] > -removing useless arguments from intel_queue_writeback_job and > intel_enabling_capture [Arun] > > --v4 > Adding Reviewed-by to patches which were previously reviewd > > --v5 > Adding Reviewed-by for patch 3 > > --v6 > Solving BAT issue > changes for checkpatch to pass > > Suraj Kandpal (3): > drm/i915: Define WD trancoder for i915 > drm/i915 : Changing intel_connector iterators > drm/i915: Enabling WD Transcoder > > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/display/intel_acpi.c | 8 +- > drivers/gpu/drm/i915/display/intel_crtc.c | 6 + > .../drm/i915/display/intel_crtc_state_dump.c | 1 + > drivers/gpu/drm/i915/display/intel_ddi.c | 6 + > drivers/gpu/drm/i915/display/intel_display.c | 68 +- > drivers/gpu/drm/i915/display/intel_display.h | 18 +- > .../drm/i915/display/intel_display_debugfs.c | 13 +- > .../drm/i915/display/intel_display_types.h | 32 +- > drivers/gpu/drm/i915/display/intel_dpll.c | 6 + > .../drm/i915/display/intel_modeset_setup.c | 119 ++- > .../drm/i915/display/intel_modeset_verify.c | 17 +- > drivers/gpu/drm/i915/display/intel_opregion.c | 3 + > drivers/gpu/drm/i915/display/intel_wd.c | 694 ++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_wd.h | 48 ++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_irq.c | 8 +- > drivers/gpu/drm/i915/i915_pci.c | 7 +- > drivers/gpu/drm/i915/i915_reg.h | 137 ++++ > 19 files changed, 1137 insertions(+), 56 deletions(-) > create mode 100644 drivers/gpu/drm/i915/display/intel_wd.c > create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h >