Re: [Intel-gfx] [PATCH 0/6] Introduce struct cdclk_step
> -Original Message- > From: Ville Syrjälä > Sent: Monday, September 26, 2022 10:30 AM > To: Srivatsa, Anusha > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > ; Vivi, Rodrigo ; Navare, > Manasi D ; Roper, Matthew D > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > On Mon, Sep 26, 2022 at 05:21:40PM +, Srivatsa, Anusha wrote: > > > > > > > -Original Message- > > > From: Ville Syrjälä > > > Sent: Friday, September 23, 2022 12:04 PM > > > To: Srivatsa, Anusha > > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > > ; Vivi, Rodrigo ; > > > Navare, Manasi D ; Roper, Matthew D > > > > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > > > On Fri, Sep 23, 2022 at 04:56:53PM +, Srivatsa, Anusha wrote: > > > > > > > > > > > > > -Original Message- > > > > > From: Ville Syrjälä > > > > > Sent: Tuesday, September 20, 2022 2:59 PM > > > > > To: Srivatsa, Anusha > > > > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > > > > ; Vivi, Rodrigo > > > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > > > > > > > On Tue, Sep 20, 2022 at 06:48:46PM +, Srivatsa, Anusha wrote: > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > From: Ville Syrjälä > > > > > > > Sent: Tuesday, September 20, 2022 1:20 AM > > > > > > > To: Srivatsa, Anusha > > > > > > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > > > > > > ; Vivi, Rodrigo > > > > > > > > > > > > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > > > > > > > > > > > On Fri, Sep 16, 2022 at 05:43:58PM -0700, Anusha Srivatsa wrote: > > > > > > > > This is a prep series for the actual cdclk refactoring > > > > > > > > that will be sent following this. Idea is to have a struct > > > > > > > > - cdclk_step that holds the following: > > > > > > > > - cdclk action (squash, crawl or modeset) > > > > > > > > - cdclk frequency > > > > > > > > which gets populated in atomic check. Driver uses the > > > > > > > > populated values during atomic commit to do the suitable > > > > > > > > sequence of actions like programming squash ctl registers > > > > > > > > in case of squashing or PLL sequence incase of modeset and so > on. > > > > > > > > > > > > > > > > This series just addresses the initial idea. The actual > > > > > > > > plumming in the atomic commit phase will be sent shortly. > > > > > > > > > > > > > > OK, people keep ignoring what I say so I just typed up the > > > > > > > code quickly. This to me seems like the most straightforward > > > > > > > way to do what > > > > > we need: > > > > > > > https://github.com/vsyrjala/linux.git cdclk_crawl_and_squash > > > > > > > > > > > > > > Totally untested ofc, apart from me doing a quick scan > > > > > > > through our cdclk tables for the crawl+squahs platforms to > > > > > > > make sure that this approach should produce sane values. > > > > > > Ville, > > > > > > Why have a mid cdclk_config? Cant we use the new-cdclk-config > > > > > > for this > > > > > purpose? > > > > > > > > > > You either > > > > > - start at old, crawl to mid, then squash to new > > > > > - start at old, squash to mid, then crawl to new > > > > > > > > Tested the changes on TGL(legacy) and DG2(squash). Took some time > > > > to > > > understand the code but the mid cdclk config logic works. @Ville > > > Syrjälä does replacing the intel_cdclk_can_squash() and > > > intel_cdclk_can_crawl() with your new cdclk_crawl_and_squash in atomic > check make sense? > > > > > > I don't think we should need any real logic at that point. > > > If we can squash and crawl then I think we can always do the cdclk > > > change w/o a modeset, at least with what we currently have defined > > > in the cdclk tables. > > > > @Ville Syrjälä is this patch in your radar to be sending out to the ML? Or > should I send it on your behalf? > > You can take over again if you want. Will do. Thanks Ville! Anusha > -- > Ville Syrjälä > Intel
Re: [Intel-gfx] [PATCH 0/6] Introduce struct cdclk_step
On Mon, Sep 26, 2022 at 05:21:40PM +, Srivatsa, Anusha wrote: > > > > -Original Message- > > From: Ville Syrjälä > > Sent: Friday, September 23, 2022 12:04 PM > > To: Srivatsa, Anusha > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > ; Vivi, Rodrigo ; Navare, > > Manasi D ; Roper, Matthew D > > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > On Fri, Sep 23, 2022 at 04:56:53PM +, Srivatsa, Anusha wrote: > > > > > > > > > > -Original Message- > > > > From: Ville Syrjälä > > > > Sent: Tuesday, September 20, 2022 2:59 PM > > > > To: Srivatsa, Anusha > > > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > > > ; Vivi, Rodrigo > > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > > > > > On Tue, Sep 20, 2022 at 06:48:46PM +, Srivatsa, Anusha wrote: > > > > > > > > > > > > > > > > -Original Message- > > > > > > From: Ville Syrjälä > > > > > > Sent: Tuesday, September 20, 2022 1:20 AM > > > > > > To: Srivatsa, Anusha > > > > > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > > > > > ; Vivi, Rodrigo > > > > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > > > > > > > > > On Fri, Sep 16, 2022 at 05:43:58PM -0700, Anusha Srivatsa wrote: > > > > > > > This is a prep series for the actual cdclk refactoring that > > > > > > > will be sent following this. Idea is to have a struct - > > > > > > > cdclk_step that holds the following: > > > > > > > - cdclk action (squash, crawl or modeset) > > > > > > > - cdclk frequency > > > > > > > which gets populated in atomic check. Driver uses the > > > > > > > populated values during atomic commit to do the suitable > > > > > > > sequence of actions like programming squash ctl registers in > > > > > > > case of squashing or PLL sequence incase of modeset and so on. > > > > > > > > > > > > > > This series just addresses the initial idea. The actual > > > > > > > plumming in the atomic commit phase will be sent shortly. > > > > > > > > > > > > OK, people keep ignoring what I say so I just typed up the code > > > > > > quickly. This to me seems like the most straightforward way to > > > > > > do what > > > > we need: > > > > > > https://github.com/vsyrjala/linux.git cdclk_crawl_and_squash > > > > > > > > > > > > Totally untested ofc, apart from me doing a quick scan through > > > > > > our cdclk tables for the crawl+squahs platforms to make sure > > > > > > that this approach should produce sane values. > > > > > Ville, > > > > > Why have a mid cdclk_config? Cant we use the new-cdclk-config for > > > > > this > > > > purpose? > > > > > > > > You either > > > > - start at old, crawl to mid, then squash to new > > > > - start at old, squash to mid, then crawl to new > > > > > > Tested the changes on TGL(legacy) and DG2(squash). Took some time to > > understand the code but the mid cdclk config logic works. @Ville Syrjälä > > does > > replacing the intel_cdclk_can_squash() and intel_cdclk_can_crawl() with your > > new cdclk_crawl_and_squash in atomic check make sense? > > > > I don't think we should need any real logic at that point. > > If we can squash and crawl then I think we can always do the cdclk change > > w/o a modeset, at least with what we currently have defined in the cdclk > > tables. > > @Ville Syrjälä is this patch in your radar to be sending out to the ML? Or > should I send it on your behalf? You can take over again if you want. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 0/6] Introduce struct cdclk_step
> -Original Message- > From: Ville Syrjälä > Sent: Friday, September 23, 2022 12:04 PM > To: Srivatsa, Anusha > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > ; Vivi, Rodrigo ; Navare, > Manasi D ; Roper, Matthew D > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > On Fri, Sep 23, 2022 at 04:56:53PM +, Srivatsa, Anusha wrote: > > > > > > > -Original Message- > > > From: Ville Syrjälä > > > Sent: Tuesday, September 20, 2022 2:59 PM > > > To: Srivatsa, Anusha > > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > > ; Vivi, Rodrigo > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > > > On Tue, Sep 20, 2022 at 06:48:46PM +, Srivatsa, Anusha wrote: > > > > > > > > > > > > > -Original Message- > > > > > From: Ville Syrjälä > > > > > Sent: Tuesday, September 20, 2022 1:20 AM > > > > > To: Srivatsa, Anusha > > > > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > > > > ; Vivi, Rodrigo > > > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > > > > > > > On Fri, Sep 16, 2022 at 05:43:58PM -0700, Anusha Srivatsa wrote: > > > > > > This is a prep series for the actual cdclk refactoring that > > > > > > will be sent following this. Idea is to have a struct - > > > > > > cdclk_step that holds the following: > > > > > > - cdclk action (squash, crawl or modeset) > > > > > > - cdclk frequency > > > > > > which gets populated in atomic check. Driver uses the > > > > > > populated values during atomic commit to do the suitable > > > > > > sequence of actions like programming squash ctl registers in > > > > > > case of squashing or PLL sequence incase of modeset and so on. > > > > > > > > > > > > This series just addresses the initial idea. The actual > > > > > > plumming in the atomic commit phase will be sent shortly. > > > > > > > > > > OK, people keep ignoring what I say so I just typed up the code > > > > > quickly. This to me seems like the most straightforward way to > > > > > do what > > > we need: > > > > > https://github.com/vsyrjala/linux.git cdclk_crawl_and_squash > > > > > > > > > > Totally untested ofc, apart from me doing a quick scan through > > > > > our cdclk tables for the crawl+squahs platforms to make sure > > > > > that this approach should produce sane values. > > > > Ville, > > > > Why have a mid cdclk_config? Cant we use the new-cdclk-config for > > > > this > > > purpose? > > > > > > You either > > > - start at old, crawl to mid, then squash to new > > > - start at old, squash to mid, then crawl to new > > > > Tested the changes on TGL(legacy) and DG2(squash). Took some time to > understand the code but the mid cdclk config logic works. @Ville Syrjälä does > replacing the intel_cdclk_can_squash() and intel_cdclk_can_crawl() with your > new cdclk_crawl_and_squash in atomic check make sense? > > I don't think we should need any real logic at that point. > If we can squash and crawl then I think we can always do the cdclk change > w/o a modeset, at least with what we currently have defined in the cdclk > tables. @Ville Syrjälä is this patch in your radar to be sending out to the ML? Or should I send it on your behalf? Anusha > -- > Ville Syrjälä > Intel
Re: [Intel-gfx] [PATCH 0/6] Introduce struct cdclk_step
On Fri, Sep 23, 2022 at 04:56:53PM +, Srivatsa, Anusha wrote: > > > > -Original Message- > > From: Ville Syrjälä > > Sent: Tuesday, September 20, 2022 2:59 PM > > To: Srivatsa, Anusha > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > ; Vivi, Rodrigo > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > On Tue, Sep 20, 2022 at 06:48:46PM +, Srivatsa, Anusha wrote: > > > > > > > > > > -Original Message- > > > > From: Ville Syrjälä > > > > Sent: Tuesday, September 20, 2022 1:20 AM > > > > To: Srivatsa, Anusha > > > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > > > ; Vivi, Rodrigo > > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > > > > > On Fri, Sep 16, 2022 at 05:43:58PM -0700, Anusha Srivatsa wrote: > > > > > This is a prep series for the actual cdclk refactoring that will > > > > > be sent following this. Idea is to have a struct - cdclk_step that > > > > > holds the following: > > > > > - cdclk action (squash, crawl or modeset) > > > > > - cdclk frequency > > > > > which gets populated in atomic check. Driver uses the populated > > > > > values during atomic commit to do the suitable sequence of actions > > > > > like programming squash ctl registers in case of squashing or PLL > > > > > sequence incase of modeset and so on. > > > > > > > > > > This series just addresses the initial idea. The actual plumming > > > > > in the atomic commit phase will be sent shortly. > > > > > > > > OK, people keep ignoring what I say so I just typed up the code > > > > quickly. This to me seems like the most straightforward way to do what > > we need: > > > > https://github.com/vsyrjala/linux.git cdclk_crawl_and_squash > > > > > > > > Totally untested ofc, apart from me doing a quick scan through our > > > > cdclk tables for the crawl+squahs platforms to make sure that this > > > > approach should produce sane values. > > > Ville, > > > Why have a mid cdclk_config? Cant we use the new-cdclk-config for this > > purpose? > > > > You either > > - start at old, crawl to mid, then squash to new > > - start at old, squash to mid, then crawl to new > > Tested the changes on TGL(legacy) and DG2(squash). Took some time to > understand the code but the mid cdclk config logic works. @Ville Syrjälä does > replacing the intel_cdclk_can_squash() and intel_cdclk_can_crawl() with your > new cdclk_crawl_and_squash in atomic check make sense? I don't think we should need any real logic at that point. If we can squash and crawl then I think we can always do the cdclk change w/o a modeset, at least with what we currently have defined in the cdclk tables. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 0/6] Introduce struct cdclk_step
> -Original Message- > From: Ville Syrjälä > Sent: Tuesday, September 20, 2022 2:59 PM > To: Srivatsa, Anusha > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > ; Vivi, Rodrigo > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > On Tue, Sep 20, 2022 at 06:48:46PM +, Srivatsa, Anusha wrote: > > > > > > > -Original Message- > > > From: Ville Syrjälä > > > Sent: Tuesday, September 20, 2022 1:20 AM > > > To: Srivatsa, Anusha > > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > > ; Vivi, Rodrigo > > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > > > On Fri, Sep 16, 2022 at 05:43:58PM -0700, Anusha Srivatsa wrote: > > > > This is a prep series for the actual cdclk refactoring that will > > > > be sent following this. Idea is to have a struct - cdclk_step that > > > > holds the following: > > > > - cdclk action (squash, crawl or modeset) > > > > - cdclk frequency > > > > which gets populated in atomic check. Driver uses the populated > > > > values during atomic commit to do the suitable sequence of actions > > > > like programming squash ctl registers in case of squashing or PLL > > > > sequence incase of modeset and so on. > > > > > > > > This series just addresses the initial idea. The actual plumming > > > > in the atomic commit phase will be sent shortly. > > > > > > OK, people keep ignoring what I say so I just typed up the code > > > quickly. This to me seems like the most straightforward way to do what > we need: > > > https://github.com/vsyrjala/linux.git cdclk_crawl_and_squash > > > > > > Totally untested ofc, apart from me doing a quick scan through our > > > cdclk tables for the crawl+squahs platforms to make sure that this > > > approach should produce sane values. > > Ville, > > Why have a mid cdclk_config? Cant we use the new-cdclk-config for this > purpose? > > You either > - start at old, crawl to mid, then squash to new > - start at old, squash to mid, then crawl to new Tested the changes on TGL(legacy) and DG2(squash). Took some time to understand the code but the mid cdclk config logic works. @Ville Syrjälä does replacing the intel_cdclk_can_squash() and intel_cdclk_can_crawl() with your new cdclk_crawl_and_squash in atomic check make sense? Anusha > -- > Ville Syrjälä > Intel
Re: [Intel-gfx] [PATCH 0/6] Introduce struct cdclk_step
On Tue, Sep 20, 2022 at 06:48:46PM +, Srivatsa, Anusha wrote: > > > > -Original Message- > > From: Ville Syrjälä > > Sent: Tuesday, September 20, 2022 1:20 AM > > To: Srivatsa, Anusha > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > > ; Vivi, Rodrigo > > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > > > On Fri, Sep 16, 2022 at 05:43:58PM -0700, Anusha Srivatsa wrote: > > > This is a prep series for the actual cdclk refactoring that will be > > > sent following this. Idea is to have a struct - cdclk_step that holds > > > the following: > > > - cdclk action (squash, crawl or modeset) > > > - cdclk frequency > > > which gets populated in atomic check. Driver uses the populated values > > > during atomic commit to do the suitable sequence of actions like > > > programming squash ctl registers in case of squashing or PLL sequence > > > incase of modeset and so on. > > > > > > This series just addresses the initial idea. The actual plumming in > > > the atomic commit phase will be sent shortly. > > > > OK, people keep ignoring what I say so I just typed up the code quickly. > > This > > to me seems like the most straightforward way to do what we need: > > https://github.com/vsyrjala/linux.git cdclk_crawl_and_squash > > > > Totally untested ofc, apart from me doing a quick scan through our cdclk > > tables for the crawl+squahs platforms to make sure that this approach > > should produce sane values. > Ville, > Why have a mid cdclk_config? Cant we use the new-cdclk-config for this > purpose? You either - start at old, crawl to mid, then squash to new - start at old, squash to mid, then crawl to new -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 0/6] Introduce struct cdclk_step
> -Original Message- > From: Ville Syrjälä > Sent: Tuesday, September 20, 2022 1:20 AM > To: Srivatsa, Anusha > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > ; Vivi, Rodrigo > Subject: Re: [PATCH 0/6] Introduce struct cdclk_step > > On Fri, Sep 16, 2022 at 05:43:58PM -0700, Anusha Srivatsa wrote: > > This is a prep series for the actual cdclk refactoring that will be > > sent following this. Idea is to have a struct - cdclk_step that holds > > the following: > > - cdclk action (squash, crawl or modeset) > > - cdclk frequency > > which gets populated in atomic check. Driver uses the populated values > > during atomic commit to do the suitable sequence of actions like > > programming squash ctl registers in case of squashing or PLL sequence > > incase of modeset and so on. > > > > This series just addresses the initial idea. The actual plumming in > > the atomic commit phase will be sent shortly. > > OK, people keep ignoring what I say so I just typed up the code quickly. This > to me seems like the most straightforward way to do what we need: > https://github.com/vsyrjala/linux.git cdclk_crawl_and_squash > > Totally untested ofc, apart from me doing a quick scan through our cdclk > tables for the crawl+squahs platforms to make sure that this approach > should produce sane values. Ville, Why have a mid cdclk_config? Cant we use the new-cdclk-config for this purpose? Anusha > > > > Cc: Ville Syrjälä > > Cc: Uma Shankar > > Cc: Rodrigo Vivi > > > > Anusha Srivatsa (6): > > drm/i915/display Add dg2_prog_squash_ctl() helper > > drm/i915/display: add cdclk action struct to cdclk_config > > drm/i915/display: Embed the new struct steps for squashing > > drm/i915/display: Embed the new struct steps for crawling > > drm/i915/display: Embed the new struct steps for modeset > > drm/i915/display: Dump the new cdclk config values > > > > drivers/gpu/drm/i915/display/intel_cdclk.c | 77 > > +- drivers/gpu/drm/i915/display/intel_cdclk.h | > > 16 - > > 2 files changed, 74 insertions(+), 19 deletions(-) > > > > -- > > 2.25.1 > > -- > Ville Syrjälä > Intel
Re: [Intel-gfx] [PATCH 0/6] Introduce struct cdclk_step
On Fri, Sep 16, 2022 at 05:43:58PM -0700, Anusha Srivatsa wrote: > This is a prep series for the actual cdclk refactoring > that will be sent following this. Idea is to have a > struct - cdclk_step that holds the following: > - cdclk action (squash, crawl or modeset) > - cdclk frequency > which gets populated in atomic check. Driver > uses the populated values during atomic commit > to do the suitable sequence of actions like > programming squash ctl registers in case of squashing > or PLL sequence incase of modeset and so on. > > This series just addresses the initial idea. The actual plumming > in the atomic commit phase will be sent shortly. OK, people keep ignoring what I say so I just typed up the code quickly. This to me seems like the most straightforward way to do what we need: https://github.com/vsyrjala/linux.git cdclk_crawl_and_squash Totally untested ofc, apart from me doing a quick scan through our cdclk tables for the crawl+squahs platforms to make sure that this approach should produce sane values. > > Cc: Ville Syrjälä > Cc: Uma Shankar > Cc: Rodrigo Vivi > > Anusha Srivatsa (6): > drm/i915/display Add dg2_prog_squash_ctl() helper > drm/i915/display: add cdclk action struct to cdclk_config > drm/i915/display: Embed the new struct steps for squashing > drm/i915/display: Embed the new struct steps for crawling > drm/i915/display: Embed the new struct steps for modeset > drm/i915/display: Dump the new cdclk config values > > drivers/gpu/drm/i915/display/intel_cdclk.c | 77 +- > drivers/gpu/drm/i915/display/intel_cdclk.h | 16 - > 2 files changed, 74 insertions(+), 19 deletions(-) > > -- > 2.25.1 -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 0/6] Introduce struct cdclk_step
Please find the review commenst for the respective patches. Also as a general rule, please add/ copy all folks nvolved in offline discussions/ triage help in order to accelerate reviews and get feedback from all. Manasi On Fri, Sep 16, 2022 at 05:43:58PM -0700, Anusha Srivatsa wrote: > This is a prep series for the actual cdclk refactoring > that will be sent following this. Idea is to have a > struct - cdclk_step that holds the following: > - cdclk action (squash, crawl or modeset) > - cdclk frequency > which gets populated in atomic check. Driver > uses the populated values during atomic commit > to do the suitable sequence of actions like > programming squash ctl registers in case of squashing > or PLL sequence incase of modeset and so on. > > This series just addresses the initial idea. The actual plumming > in the atomic commit phase will be sent shortly. > > Cc: Ville Syrjälä > Cc: Uma Shankar > Cc: Rodrigo Vivi > > Anusha Srivatsa (6): > drm/i915/display Add dg2_prog_squash_ctl() helper > drm/i915/display: add cdclk action struct to cdclk_config > drm/i915/display: Embed the new struct steps for squashing > drm/i915/display: Embed the new struct steps for crawling > drm/i915/display: Embed the new struct steps for modeset > drm/i915/display: Dump the new cdclk config values > > drivers/gpu/drm/i915/display/intel_cdclk.c | 77 +- > drivers/gpu/drm/i915/display/intel_cdclk.h | 16 - > 2 files changed, 74 insertions(+), 19 deletions(-) > > -- > 2.25.1 >
