<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <blockquote type="cite" style="color: #000000;"> <blockquote type="cite" style="color: #000000;"> <pre wrap="">Think about two situations where:- - Monitor supports scrambling and scdc, but we will not enable it, as the current mode is 1080P@148 MHz - Monitor supports scrambling and scdc, and we will enable it, as the current mode is 4k@596 Mhz To differentiate between these two, we have: config->hdmi_scrambling which shows scrambling enabled on monitor by source display_info->hdmi.scdc->scrambling which indicates monitor supports scrambling Does it make sense ? Or you prefer some changes here ? </pre> </blockquote> <pre wrap="">Is there any harm in disabling scrambling twice? If not I'd say just disable it on every modeset if it is not needed. Then there's no need to know the previous state at the moment we actually enable/disable it. </pre> </blockquote> <pre wrap="">> Agreed. Just explicitly set the state to what we want every time. - Cool, so I would load the crtc_state only once (in compute_config), and then disable it every time during ddi_disable. Thanks for this clarification guys, I will send a follow up patch series addressing all the comments soon. Regards Shashank </pre> <div class="moz-cite-prefix">On 2/23/2017 6:51 PM, Ville Syrjälä wrote: </div> <blockquote cite="mid:20170223132135.GV31595@intel.com" type="cite"> <blockquote type="cite" style="color: #000000;"> <blockquote type="cite" style="color: #000000;"> <pre wrap="">> > Actually design is slightly different. The state's hdmi_scrambling/clock > > bool's indicate that the sink's > > scrambling/high_clock_ratio is enabled/set by source (and needs to be > > disabled), whereas connecotr's->display_info->scdc.scrambling > > is to indicate monitor's capability to support scrambling and scdc </pre> </blockquote> <pre wrap="">> > The crtc_state shouldn't be changed to represent changes in the hardware state. > It is computed during the atomic check phase, and after that it should represent > the state that needs to be commited. Perhaps the code in hdmi_compute_config() > just needs to take the previous state into account? </pre> </blockquote> <pre wrap="">The previous state is irrelevant. We just need to compute these things based on what we're going to do next. And this stuff doesn't really track the state of the sink, rather it tracks the state of the source. The sink state just follows along to match. So in the readout path we just want to read out the state of the source. </pre> <blockquote type="cite" style="color: #000000;"> <pre wrap="">> </pre> <blockquote type="cite" style="color: #000000;"> <pre wrap="">> > > > Think about two situations where:- > > - Monitor supports scrambling and scdc, but we will not enable it, as > > the current mode is 1080P@148 MHz > > - Monitor supports scrambling and scdc, and we will enable it, as the > > current mode is 4k@596 Mhz > > > > To differentiate between these two, we have: > > config->hdmi_scrambling which shows scrambling enabled on monitor by source > > display_info->hdmi.scdc->scrambling which indicates monitor supports > > scrambling > > > > Does it make sense ? Or you prefer some changes here ? </pre> </blockquote> <pre wrap="">> > Is there any harm in disabling scrambling twice? If not I'd say just disable it > on every modeset if it is not needed. Then there's no need to know the previous > state at the moment we actually enable/disable it. </pre> </blockquote> <pre wrap="">Agreed. Just explicitly set the state to what we want every time. </pre> </blockquote> </body> </html>