[igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Force a full modeset when we force dsc enable

[Manasi Navare]

On Thu, Apr 04, 2019 at 02:02:42AM +0300, Imre Deak wrote:
> On Wed, Apr 03, 2019 at 03:48:24PM -0700, Manasi Navare wrote:
> > On Thu, Apr 04, 2019 at 12:42:33AM +0300, Imre Deak wrote:
> > > On Wed, Apr 03, 2019 at 01:49:44PM -0700, Manasi Navare wrote:
> > > > [...]
> > > > Currently we have inh i915_dsc_fec_support_show:
> > > > DSC_enabled:
> > > > DSC_sink_support:
> > > > FEC_sink_support:
> > > 
> > > Yes, and after the kernel change you'll have an additional
> > >   
> > >   Force DSC enable: yes/no
> > > 
> > > line here.
> > > 
> > > > And this node we open as a read only first and then as write node and
> > > > force_dsc_enable() from IGT just writes a 1 to this, so you are saying
> > > > that in the kernel _write() node, I accept this and write to
> > > > force_dsc_enable entry in this file instead of rewriting the whole
> > > > file?
> > > 
> > > I'm not sure what you mean. Currently "1" or "0" is written to the
> > > debugfs file which will set intel_dp->force_dsc_enable. This doesn't
> > > need to be changed at all, no need to rewrite the whole file.
> > > 
> > > > So in IGT also I will have to parse the file and write 1 for
> > > > force_dsc_enable entry?
> > > 
> > > You can get the original value of it similarly to how it's done already
> > > for 'DSC_Sink_Support', by reading the file and doing
> > > 
> > > 	strstr(buf, "Force DSC enable: yes");
> > > 
> > > on the content.
> > > 
> > > > I still cant figure out how the restore() function work, we write the
> > > > stored_values and write for each of the entries in the file? And then
> > > > in the kernel debugfs_write, get these and write back to each entry?
> > > 
> > > Only force_dsc_enable need to be restored, the only value that you
> > > change by writing to the debugfs file. All the rest are calculated
> > > values that you don't need to care about.
> > 
> > But now that I rething then, force_dsc_enable is always 0, it s a variable
> > that we use to force DSC only if IGT asks to, so for test we force it to 1
> > and then after the test always clear it since the original value is always 0.
> > So why do we need a separate restore() for just force_dsc_enable if the value
> > is always 0.
> 
> It's not necessarily 0. Before the test starts someone could've written
> 1 to that file.

Hmm, so now the use of this force_dsc_enable in kernel is only for this specific IGT
and the kernel doesnt touch this. But because in the future the kernel might set it to true,
we want to make sure that the IGT doesnt change the kernel behaviour after the test is run,
is my understanding correct?
So basically adding that now for future proofing it?

Also on the igt_install_exit_handler(), should the test_cleanup be now part of this handler?


Manasi

> 
> > 
> > Manasi
> > 
> > > 
> > > --Imre