| Message ID | 20210721063530.16678-1-chris.chiu@canonical.com |
|---|---|
| Headers | show |
| Series | Fix garbage display when scrolling on | expand |
On 21.07.21 08:35, chris.chiu@canonical.com wrote: > From: Chris Chiu <chris.chiu@canonical.com> > > BugLink: https://bugs.launchpad.net/bugs/1926579 > > [Impact] > On Dell TGL platforms screen shows garbage when browsing website by scrolling mouse > > [Fix] > This patch fixes the issue > https://patchwork.freedesktop.org/patch/430153/?series=89348&rev=1 > It needs dependant commits on top of Hirsute base to access the stepping info to specifically disable PSR2 when stepping is B1 from A0. > > [Test] > Verified on Dell TGL-H platforms. > > [Where problems could occur] > It disable PSR2 on A0 and B0 TGL-H platforms, should introduce no regression. > > V2: Add more dependant patches to avoid as many conflicts as possible and add explanations for all backported section. > > Aditya Swarup (3): > drm/i915/tgl: Add bound checks and simplify TGL REVID macros > drm/i915/tgl: Use TGL stepping info for applying WAs > drm/i915/adl_s: Add display WAs for ADL-S > > Caz Yokoyama (1): > drm/i915/adl_s: Add ADL-S platform info and PCI ids > > Daniele Ceraolo Spurio (1): > drm/i915: split gen8+ flush and bb_start emission functions > > Gwan-gyeong Mun (1): > drm/i915/display: Disable PSR2 if TGL Display stepping is B1 from A0 > > Jani Nikula (7): > drm/i915/pm: replace I915_READ()/WRITE() with > intel_uncore_read()/write() > drm/i915: remove unused ADLS_REVID_* macros > drm/i915: split out stepping info to a new file > drm/i915: add new helpers for accessing stepping info > drm/i915: switch KBL to the new stepping scheme > drm/i915: switch TGL and ADL to the new stepping scheme > drm/i915: rename DISP_STEPPING->DISPLAY_STEP and GT_STEPPING->GT_STEP > > John Harrison (1): > drm/i915: Correct location of Wa_1408615072 > > Lucas De Marchi (3): > drm/i915: remove WA_SET_BIT_MASKED() > drm/i915: remove WA_CLR_BIT_MASKED() > drm/i915: remove WA_SET_FIELD_MASKED() > > Swathi Dhanavanthri (1): > drm/i915/dg1: Implement WA_16011163337 > > drivers/gpu/drm/i915/Makefile | 2 + > .../drm/i915/display/intel_display_power.c | 9 +- > drivers/gpu/drm/i915/display/intel_psr.c | 11 +- > drivers/gpu/drm/i915/display/intel_sprite.c | 6 +- > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 618 +++++++++++++++++ > drivers/gpu/drm/i915/gt/gen8_engine_cs.h | 36 + > drivers/gpu/drm/i915/gt/intel_lrc.c | 629 +----------------- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 260 ++++---- > drivers/gpu/drm/i915/i915_drv.c | 3 +- > drivers/gpu/drm/i915/i915_drv.h | 97 +-- > drivers/gpu/drm/i915/i915_pci.c | 13 + > drivers/gpu/drm/i915/intel_device_info.c | 7 +- > drivers/gpu/drm/i915/intel_device_info.h | 5 + > drivers/gpu/drm/i915/intel_pm.c | 558 ++++++++-------- > drivers/gpu/drm/i915/intel_step.c | 106 +++ > drivers/gpu/drm/i915/intel_step.h | 40 ++ > include/drm/i915_pciids.h | 11 + > 17 files changed, 1297 insertions(+), 1114 deletions(-) > create mode 100644 drivers/gpu/drm/i915/gt/gen8_engine_cs.c > create mode 100644 drivers/gpu/drm/i915/gt/gen8_engine_cs.h > create mode 100644 drivers/gpu/drm/i915/intel_step.c > create mode 100644 drivers/gpu/drm/i915/intel_step.h > Thanks for adding explanations. Though with a size like this, you should at least provide pull request style information in the cover email. This does not only simplify getting the set but also allows to look at things from various angles (as in show overall diff or individual patches). In my opinion SRU set should not strife for the least conflicts but for the least change. Only for LTS kernels there is the possible advantage to make future changes simpler. The kernels in between do not live that long. So there it is "the more one changes, the more one breaks". And as mentioned yesterday, the i915 driver breaks a lot. And it cannot be tested well. We don't have all the existing revisions of GPUs and even if we had, we could not have someone actually look at the screen. -Stefan
On Wed, Jul 21, 2021 at 3:21 PM Stefan Bader <stefan.bader@canonical.com> wrote: > On 21.07.21 08:35, chris.chiu@canonical.com wrote: > > From: Chris Chiu <chris.chiu@canonical.com> > > > > BugLink: https://bugs.launchpad.net/bugs/1926579 > > > > [Impact] > > On Dell TGL platforms screen shows garbage when browsing website by > scrolling mouse > > > > [Fix] > > This patch fixes the issue > > https://patchwork.freedesktop.org/patch/430153/?series=89348&rev=1 > > It needs dependant commits on top of Hirsute base to access the stepping > info to specifically disable PSR2 when stepping is B1 from A0. > > > > [Test] > > Verified on Dell TGL-H platforms. > > > > [Where problems could occur] > > It disable PSR2 on A0 and B0 TGL-H platforms, should introduce no > regression. > > > > V2: Add more dependant patches to avoid as many conflicts as possible > and add explanations for all backported section. > > > > Aditya Swarup (3): > > drm/i915/tgl: Add bound checks and simplify TGL REVID macros > > drm/i915/tgl: Use TGL stepping info for applying WAs > > drm/i915/adl_s: Add display WAs for ADL-S > > > > Caz Yokoyama (1): > > drm/i915/adl_s: Add ADL-S platform info and PCI ids > > > > Daniele Ceraolo Spurio (1): > > drm/i915: split gen8+ flush and bb_start emission functions > > > > Gwan-gyeong Mun (1): > > drm/i915/display: Disable PSR2 if TGL Display stepping is B1 from A0 > > > > Jani Nikula (7): > > drm/i915/pm: replace I915_READ()/WRITE() with > > intel_uncore_read()/write() > > drm/i915: remove unused ADLS_REVID_* macros > > drm/i915: split out stepping info to a new file > > drm/i915: add new helpers for accessing stepping info > > drm/i915: switch KBL to the new stepping scheme > > drm/i915: switch TGL and ADL to the new stepping scheme > > drm/i915: rename DISP_STEPPING->DISPLAY_STEP and GT_STEPPING->GT_STEP > > > > John Harrison (1): > > drm/i915: Correct location of Wa_1408615072 > > > > Lucas De Marchi (3): > > drm/i915: remove WA_SET_BIT_MASKED() > > drm/i915: remove WA_CLR_BIT_MASKED() > > drm/i915: remove WA_SET_FIELD_MASKED() > > > > Swathi Dhanavanthri (1): > > drm/i915/dg1: Implement WA_16011163337 > > > > drivers/gpu/drm/i915/Makefile | 2 + > > .../drm/i915/display/intel_display_power.c | 9 +- > > drivers/gpu/drm/i915/display/intel_psr.c | 11 +- > > drivers/gpu/drm/i915/display/intel_sprite.c | 6 +- > > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 618 +++++++++++++++++ > > drivers/gpu/drm/i915/gt/gen8_engine_cs.h | 36 + > > drivers/gpu/drm/i915/gt/intel_lrc.c | 629 +----------------- > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 260 ++++---- > > drivers/gpu/drm/i915/i915_drv.c | 3 +- > > drivers/gpu/drm/i915/i915_drv.h | 97 +-- > > drivers/gpu/drm/i915/i915_pci.c | 13 + > > drivers/gpu/drm/i915/intel_device_info.c | 7 +- > > drivers/gpu/drm/i915/intel_device_info.h | 5 + > > drivers/gpu/drm/i915/intel_pm.c | 558 ++++++++-------- > > drivers/gpu/drm/i915/intel_step.c | 106 +++ > > drivers/gpu/drm/i915/intel_step.h | 40 ++ > > include/drm/i915_pciids.h | 11 + > > 17 files changed, 1297 insertions(+), 1114 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/gt/gen8_engine_cs.c > > create mode 100644 drivers/gpu/drm/i915/gt/gen8_engine_cs.h > > create mode 100644 drivers/gpu/drm/i915/intel_step.c > > create mode 100644 drivers/gpu/drm/i915/intel_step.h > > > Thanks for adding explanations. Though with a size like this, you should > at > least provide pull request style information in the cover email. This does > not > only simplify getting the set but also allows to look at things from > various > angles (as in show overall diff or individual patches). > In my opinion SRU set should not strife for the least conflicts but for > the > least change. Only for LTS kernels there is the possible advantage to make > future changes simpler. The kernels in between do not live that long. So > there > it is "the more one changes, the more one breaks". And as mentioned > yesterday, > the i915 driver breaks a lot. And it cannot be tested well. We don't have > all > the existing revisions of GPUs and even if we had, we could not have > someone > actually look at the screen. > > -Stefan > > > Thanks for the clear explanations. I don't have to waste so much time and effort just for fixing conflicts and apply the same functions for old platforms. I'll propose a much simpler v3.
On 21.07.21 12:53, Chris Chiu wrote: > > > On Wed, Jul 21, 2021 at 3:21 PM Stefan Bader <stefan.bader@canonical.com > <mailto:stefan.bader@canonical.com>> wrote: > > On 21.07.21 08:35, chris.chiu@canonical.com > <mailto:chris.chiu@canonical.com> wrote: > > From: Chris Chiu <chris.chiu@canonical.com <mailto:chris.chiu@canonical.com>> > > > > BugLink: https://bugs.launchpad.net/bugs/1926579 > <https://bugs.launchpad.net/bugs/1926579> > > > > [Impact] > > On Dell TGL platforms screen shows garbage when browsing website by > scrolling mouse > > > > [Fix] > > This patch fixes the issue > > https://patchwork.freedesktop.org/patch/430153/?series=89348&rev=1 > <https://patchwork.freedesktop.org/patch/430153/?series=89348&rev=1> > > It needs dependant commits on top of Hirsute base to access the stepping > info to specifically disable PSR2 when stepping is B1 from A0. > > > > [Test] > > Verified on Dell TGL-H platforms. > > > > [Where problems could occur] > > It disable PSR2 on A0 and B0 TGL-H platforms, should introduce no regression. > > > > V2: Add more dependant patches to avoid as many conflicts as possible and > add explanations for all backported section. > > > > Aditya Swarup (3): > > drm/i915/tgl: Add bound checks and simplify TGL REVID macros > > drm/i915/tgl: Use TGL stepping info for applying WAs > > drm/i915/adl_s: Add display WAs for ADL-S > > > > Caz Yokoyama (1): > > drm/i915/adl_s: Add ADL-S platform info and PCI ids > > > > Daniele Ceraolo Spurio (1): > > drm/i915: split gen8+ flush and bb_start emission functions > > > > Gwan-gyeong Mun (1): > > drm/i915/display: Disable PSR2 if TGL Display stepping is B1 from A0 > > > > Jani Nikula (7): > > drm/i915/pm: replace I915_READ()/WRITE() with > > intel_uncore_read()/write() > > drm/i915: remove unused ADLS_REVID_* macros > > drm/i915: split out stepping info to a new file > > drm/i915: add new helpers for accessing stepping info > > drm/i915: switch KBL to the new stepping scheme > > drm/i915: switch TGL and ADL to the new stepping scheme > > drm/i915: rename DISP_STEPPING->DISPLAY_STEP and GT_STEPPING->GT_STEP > > > > John Harrison (1): > > drm/i915: Correct location of Wa_1408615072 > > > > Lucas De Marchi (3): > > drm/i915: remove WA_SET_BIT_MASKED() > > drm/i915: remove WA_CLR_BIT_MASKED() > > drm/i915: remove WA_SET_FIELD_MASKED() > > > > Swathi Dhanavanthri (1): > > drm/i915/dg1: Implement WA_16011163337 > > > > drivers/gpu/drm/i915/Makefile | 2 + > > .../drm/i915/display/intel_display_power.c | 9 +- > > drivers/gpu/drm/i915/display/intel_psr.c | 11 +- > > drivers/gpu/drm/i915/display/intel_sprite.c | 6 +- > > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 618 +++++++++++++++++ > > drivers/gpu/drm/i915/gt/gen8_engine_cs.h | 36 + > > drivers/gpu/drm/i915/gt/intel_lrc.c | 629 +----------------- > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 260 ++++---- > > drivers/gpu/drm/i915/i915_drv.c | 3 +- > > drivers/gpu/drm/i915/i915_drv.h | 97 +-- > > drivers/gpu/drm/i915/i915_pci.c | 13 + > > drivers/gpu/drm/i915/intel_device_info.c | 7 +- > > drivers/gpu/drm/i915/intel_device_info.h | 5 + > > drivers/gpu/drm/i915/intel_pm.c | 558 ++++++++-------- > > drivers/gpu/drm/i915/intel_step.c | 106 +++ > > drivers/gpu/drm/i915/intel_step.h | 40 ++ > > include/drm/i915_pciids.h | 11 + > > 17 files changed, 1297 insertions(+), 1114 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/gt/gen8_engine_cs.c > > create mode 100644 drivers/gpu/drm/i915/gt/gen8_engine_cs.h > > create mode 100644 drivers/gpu/drm/i915/intel_step.c > > create mode 100644 drivers/gpu/drm/i915/intel_step.h > > > Thanks for adding explanations. Though with a size like this, you should at > least provide pull request style information in the cover email. This does not > only simplify getting the set but also allows to look at things from various > angles (as in show overall diff or individual patches). > In my opinion SRU set should not strife for the least conflicts but for the > least change. Only for LTS kernels there is the possible advantage to make > future changes simpler. The kernels in between do not live that long. So there > it is "the more one changes, the more one breaks". And as mentioned yesterday, > the i915 driver breaks a lot. And it cannot be tested well. We don't have all > the existing revisions of GPUs and even if we had, we could not have someone > actually look at the screen. > > -Stefan > > > > Thanks for the clear explanations. I don't have to waste so much time and effort > just for fixing conflicts and apply the same functions for old platforms. I'll > propose a much simpler v3. Thanks, I see the v3 on the list and this is much more something that I would consider as SRU. Nice and and small and one even can understand things. ;) NACKing this thread for clarity. -Stefan
From: Chris Chiu <chris.chiu@canonical.com> BugLink: https://bugs.launchpad.net/bugs/1926579 [Impact] On Dell TGL platforms screen shows garbage when browsing website by scrolling mouse [Fix] This patch fixes the issue https://patchwork.freedesktop.org/patch/430153/?series=89348&rev=1 It needs dependant commits on top of Hirsute base to access the stepping info to specifically disable PSR2 when stepping is B1 from A0. [Test] Verified on Dell TGL-H platforms. [Where problems could occur] It disable PSR2 on A0 and B0 TGL-H platforms, should introduce no regression. V2: Add more dependant patches to avoid as many conflicts as possible and add explanations for all backported section. Aditya Swarup (3): drm/i915/tgl: Add bound checks and simplify TGL REVID macros drm/i915/tgl: Use TGL stepping info for applying WAs drm/i915/adl_s: Add display WAs for ADL-S Caz Yokoyama (1): drm/i915/adl_s: Add ADL-S platform info and PCI ids Daniele Ceraolo Spurio (1): drm/i915: split gen8+ flush and bb_start emission functions Gwan-gyeong Mun (1): drm/i915/display: Disable PSR2 if TGL Display stepping is B1 from A0 Jani Nikula (7): drm/i915/pm: replace I915_READ()/WRITE() with intel_uncore_read()/write() drm/i915: remove unused ADLS_REVID_* macros drm/i915: split out stepping info to a new file drm/i915: add new helpers for accessing stepping info drm/i915: switch KBL to the new stepping scheme drm/i915: switch TGL and ADL to the new stepping scheme drm/i915: rename DISP_STEPPING->DISPLAY_STEP and GT_STEPPING->GT_STEP John Harrison (1): drm/i915: Correct location of Wa_1408615072 Lucas De Marchi (3): drm/i915: remove WA_SET_BIT_MASKED() drm/i915: remove WA_CLR_BIT_MASKED() drm/i915: remove WA_SET_FIELD_MASKED() Swathi Dhanavanthri (1): drm/i915/dg1: Implement WA_16011163337 drivers/gpu/drm/i915/Makefile | 2 + .../drm/i915/display/intel_display_power.c | 9 +- drivers/gpu/drm/i915/display/intel_psr.c | 11 +- drivers/gpu/drm/i915/display/intel_sprite.c | 6 +- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 618 +++++++++++++++++ drivers/gpu/drm/i915/gt/gen8_engine_cs.h | 36 + drivers/gpu/drm/i915/gt/intel_lrc.c | 629 +----------------- drivers/gpu/drm/i915/gt/intel_workarounds.c | 260 ++++---- drivers/gpu/drm/i915/i915_drv.c | 3 +- drivers/gpu/drm/i915/i915_drv.h | 97 +-- drivers/gpu/drm/i915/i915_pci.c | 13 + drivers/gpu/drm/i915/intel_device_info.c | 7 +- drivers/gpu/drm/i915/intel_device_info.h | 5 + drivers/gpu/drm/i915/intel_pm.c | 558 ++++++++-------- drivers/gpu/drm/i915/intel_step.c | 106 +++ drivers/gpu/drm/i915/intel_step.h | 40 ++ include/drm/i915_pciids.h | 11 + 17 files changed, 1297 insertions(+), 1114 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/gen8_engine_cs.c create mode 100644 drivers/gpu/drm/i915/gt/gen8_engine_cs.h create mode 100644 drivers/gpu/drm/i915/intel_step.c create mode 100644 drivers/gpu/drm/i915/intel_step.h