On Tue, 14 Mar 2023 at 18:48, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > > > On 3/14/2023 9:43 AM, Dmitry Baryshkov wrote: > > On 14/03/2023 18:35, Abhinav Kumar wrote: > >> > >> > >> On 3/14/2023 3:58 AM, Dmitry Baryshkov wrote: > >>> On 14/03/2023 07:08, Abhinav Kumar wrote: > >>>> > >>>> > >>>> On 3/9/2023 4:57 PM, Dmitry Baryshkov wrote: > >>>>> Enable SmartDMA features for the rest of the platforms where it is > >>>>> supposed to work. > >>>>> > >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > >>>> > >>>> I am so glad we split this. Without visual validation check we > >>>> wouldnt have caught the issues and would have ended up with a blank > >>>> half screen on our sc7280 CBs on DP. > >>> > >>> yes, the r_pipe was indeed mea culpa, when I didn't fully validate > >>> the results of a rebase. However this doesn't seem to be an > >>> sc7280-specific question. Are there any platform-specific issues with > >>> SmartDMA/multirect/planes revealed during testing? In other words, > >>> were there any issues which warrant this split? > >>> > > Missed this question. > > Well the sc7280 issue was itself coming from a platform specific > maxlinewidth. So like I wrote there, my monitor supported 2560x1600 and > the maxlinewidth for sc7280 is 2400. Thats why I could catch this. > > With RB5 or other platforms in the previous patch, this would not have > been caught without forcing some conditions. > > >>>> > >>>> For sc7280, I validated the foll cases: > >>> > >>> Thanks a lot! > >>> > >>>> > >>>> 1) Basic Chrome UI comes up > >>>> 2) Validated some browser use-cases and played some youtube videos > >>>> 3) Validated multiple plug-in/plug-out cases with DP connected > >>>> 4) IGT test cases with DP connected: > >>>> - kms_atomic > >>>> - kms_atomic_transition > >>>> - kms_pipe_basic_crc > >>>> > >>>> Some notes: > >>>> > >>>> 1) I wanted to test 4k with this too but my monitor only supports > >>>> 4k@60 which is not possible with 24bpp with 2 lanes and due to the > >>>> HBR3 black screen issue I could not test that so far. But since I > >>>> have that issue even with 1080P and without these changes, it should > >>>> have no effect due to this series. > >>>> > >>>> 2) I wanted to test some YUV overlay cases but I still cannot find > >>>> one on chrome. Its always using RGB. Will check with others. > >>> > >>> Testing YUV would definitely be a must, especially for the SSPP > >>> allocation. Can you possibly check whether contemporaty Xv > >>> implementation employs YUV planes? You can try that using `mpv -xo > >>> xv` or `mplayer -vo xv` > >>> > >> > >> Let me get some feedback from CrOS folks here instead of just trial > >> and error to find out. > >> > >>>> > >>>> With these two noted, this change and this series has my > >>>> > >>>> Tested-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> > >>>> > >> > >> There is no guarantee we will end up finding the YUV case, you could > >> have retained the Tested-by for the efforts which were already put in > >> instead of just blatantly removing it. > > > > I can add it back for v6 (or move sc7280 to the previous patch if you'd > > prefer that). Also I think the Tested-by should have included the > > #sc7280 comment. If you don't mind I'll add it. > > > > My tag had all the comments of what I tested and what I didnt. Not inline. Thus once patchwork picked your tag, all context (testing on sc7280) was lost. > >>>> Only for sc7280 device. > >>>> > >>>> I still cannot give my R-b on this change till others validate > >>>> visually and ensure things arent broken for them. > >>>> > >>>> If we don't get enough validation and if only sc7280 remains in this > >>>> change, please re-post it with only that and I will give my R-b too. > >>> > > -- With best wishes Dmitry