On Thu, Dec 03, 2020 at 06:35:29PM +0200, Jani Nikula wrote:
> On Wed, 02 Dec 2020, "Navare, Manasi" <manasi.d.navare@xxxxxxxxx> wrote:
> > On Tue, Dec 01, 2020 at 02:21:56PM -0800, Navare, Manasi wrote:
> >> On Tue, Nov 10, 2020 at 12:39:08PM +0200, Jani Nikula wrote:
> >> > > +{
> >> > > + struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
> >> >
> >> > I kind of feel like either the function should a) ensure it's okay to do
> >> > intel_attached_dp() and return false if not, or b) just use struct
> >> > intel_dp as the parameter.
> >> >
> >> > As it is, passing a non-dp connector to this function will fail either
> >> > subtly or spectacularly, but not graciously.
> >
> > Actually after doing some code rewriting, I think it is best to still pass
> > the drm connector but I am adding the check now for connector type
> > Only if its eDP or DP now I get the intel_dp.
> > Future we can add HDMI there as well.
>
> It's fine, as long as we don't cast to intel_dp unless we know it's
> intel_dp.
>
> >> > > +
> >> > > + /*
> >> > > + * DP Sink is capable of Variable refresh video timings if
> >> > > + * Ignore MSA bit is set in DPCD.
> >> > > + * EDID monitor range also should be atleast 10 for reasonable
> >> > > + * Adaptive sync/ VRR end user experience.
> >> > > + */
> >> >
> >> > Please fix typos etc.
> >>
> >> Did I miss some typos, I dont see any in the above comment?
>
> Odd capitalization, "atleast", "sync/ VRR", maybe also reflow the
> paragraph.
>
> I know it's nitpicking, but other people will read the comment many,
> many more times than you write it. ;)
Okay yes will remove the odd capitalizations.
Manasi
>
>
> BR,
> Jani.
>
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
