[Intel-gfx] [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0

[He, Shuang]

> -----Original Message-----
> From: Lespiau, Damien
> Sent: Saturday, November 15, 2014 6:55 PM
> To: He, Shuang
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 4/4] drm/i915/skl: Read out crtl1 for eDP/DPLL0
> 
> Hi Shuang,
> 
> You wanted suggestions, so how about:
> 
> For both examples, to determine the size of the column, I'd take the
> length of the longest value of that column (including the title) and add
> 4 to account for spacing. Left alignment for text, right alignement for
> numbers (of course, all of that is debatable).
[He, Shuang] Hello, Damien, Thanks for your great input, we'll check how those suggestion could be integrated, I've created one jira task to track it: https://jira01.devtools.intel.com/browse/VIZ-4672

> 
> On Fri, Nov 14, 2014 at 09:28:03PM -0800, shuang.he at intel.com wrote:
> > Tested-By: PRC QA PRTS (Patch Regression Test System Contact:
> shuang.he at intel.com)
> > -------------------------------------Summary-------------------------------------
> > Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
> > BYT: pass/total=290/291->290/291
> > PNV: pass/total=352/356->356/356
> > ILK: pass/total=371/372->364/372
> > IVB: pass/total=545/546->545/546
> > SNB: pass/total=424/425->424/425
> > HSW: pass/total=579/579->579/579
> > BDW: pass/total=434/435->434/435
> 
> For this one:
> 
>   - a bit more spacing
>   - some table-like alignment. That's assuming the mail client will use
>     monospaced fonts for text emails, it really should.
>   - add the delta so it's easier to parse the interesting information
>   - sort by gens
[He, Shuang] yes, this makes sense

>   - baseline_drm_intel_nightly_pass_rate can really just be
>     drm-intel-nightly
[He, Shuang] yes, this makes sense

>   - it's not just a single patch applied, so series applied?
[He, Shuang] yes, it's series applied

> 
> Platform    Delta    drm-intel-nightly    Series Applied
> --------------------------------------------------------
> PNV            +4              352/356           356/356
> ILK            -7              371/372           364/372
> SNB             0              424/425           424/425
> IVB             0              545/546           545/546
> BYT             0              290/291           290/291
> HSW             0              579/579           579/579
> BDW             0              434/435           434/435
> 
> 
> > -------------------------------------Detailed-------------------------------------
> > test_platform: test_suite, test_case, result_with_drm_intel_nightly(count,
> machine_id...)...->result_with_patch_applied(count, machine_id)...
> > PNV: Intel_gpu_tools, igt_gen3_mixed_blits, DMESG_WARN(1, M23) ->
> PASS(4, M7)
> > PNV: Intel_gpu_tools, igt_gen3_render_mixed_blits, CRASH(1, M23) ->
> PASS(1, M7)
> > PNV: Intel_gpu_tools, igt_gen3_render_tiledx_blits, CRASH(1, M23) -> PASS(1,
> M7)
> > PNV: Intel_gpu_tools, igt_gen3_render_tiledy_blits, CRASH(1, M23) -> PASS(1,
> M7)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank,
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(2, M26)PASS(2,
> M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible,
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3,
> M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-modeset-vs-hang-interruptible,
> PASS(4, M37M26) -> NSPT(1, M26)PASS(3, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-wf_vblank-interruptible,
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3,
> M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_plain-flip, PASS(4, M37M26) ->
> DMESG_WARN(2, M26)PASS(2, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_plain-flip-interruptible, PASS(4, M37M26) ->
> DMESG_WARN(2, M26)PASS(2, M26)
> > ILK: Intel_gpu_tools, igt_kms_flip_rcs-flip-vs-modeset-interruptible, PASS(4,
> M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
> 
> And for this one:
> 
>   - it's not nessary to repeat the test suite name for every single row,
>     if needed at all (I don't think it is when replying to kernel
>     patches), it can be put beforehand.
[He, Shuang] this makes sense

>   - sorted by gen
>   - The machine id isn't useful in this report
[He, Shuang] Some time, test result diffs because of Machine have changed, machine id is put there to help understand the issue

>   - It'd be nice to add a some explanations on how to decipher the
>     result column (what is the count, what the various states mean).
>     Maybe after the table as it only needs to be read a few times to get
>     it.
[He, Shuang] yes, it may help. I'm always be hesitating on how to put these things in the right position. I'll see if your way could help

> 
> Platform    Test
> drm-intel-nightly        Series Applied
> ----------------------------------------------------------------------------------------------------------------
> ---
> ILK         igt_kms_flip_flip-vs-absolute-wf_vblank
> DMESG_WARN(1) PASS(3)    DMESG_WARN(2) PASS(2)
> ILK         igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible
> DMESG_WARN(1) PASS(3)    DMESG_WARN(1) PASS(3)
> ..
> 
> HTH,
[He, Shuang] By the way, What is HTH? "Hope to help"? :)

> 
> --
> Damien