| Message ID | 20190826180507.17802-6-David.Francis@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DSC MST support in DRM | expand |
On 2019-08-26 2:05 p.m., David Francis wrote: > Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not > support virtual DPCD registers, but do support DSC. > The DSC caps can be read from the physical aux, > like in SST DSC. These hubs have many different > DEVICE_IDs. Add a new quirk to detect this case. > > Cc: Lyude Paul <lyude@redhat.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: David Francis <David.Francis@amd.com> > --- > drivers/gpu/drm/drm_dp_helper.c | 2 ++ > include/drm/drm_dp_helper.h | 7 +++++++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 2cc21eff4cf3..fc39323e7d52 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { > { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) }, > /* CH7511 seems to leave SINK_COUNT zeroed */ > { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) }, > + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */ > + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) }, This seems to be generic OUI for Synaptics [1]. Could this cause us to cast the net too wide? Even if we check that it's DP_DPCD_REV >= 1.4 there's a good chance Synaptics is fixing this in the future and won't require the quirk. [1] https://aruljohn.com/mac/vendor/Synaptics Harry > }; > > #undef OUI > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 8364502f92cf..a1331b08705f 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -1434,6 +1434,13 @@ enum drm_dp_quirk { > * The driver should ignore SINK_COUNT during detection. > */ > DP_DPCD_QUIRK_NO_SINK_COUNT, > + /** > + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD: > + * > + * The device supports MST DSC despite not supporting Virtual DPCD. > + * The DSC caps can be read from the physical aux instead. > + */ > + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD, > }; > > /** >
On 2019-08-26 2:05 p.m., David Francis wrote: >> Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not >> support virtual DPCD registers, but do support DSC. >> The DSC caps can be read from the physical aux, >> like in SST DSC. These hubs have many different >> DEVICE_IDs. Add a new quirk to detect this case. >> >> Cc: Lyude Paul <lyude@redhat.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Signed-off-by: David Francis <David.Francis@amd.com> >> --- >> drivers/gpu/drm/drm_dp_helper.c | 2 ++ >> include/drm/drm_dp_helper.h | 7 +++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c >> index 2cc21eff4cf3..fc39323e7d52 100644 >> --- a/drivers/gpu/drm/drm_dp_helper.c >> +++ b/drivers/gpu/drm/drm_dp_helper.c >> @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { >> { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) }, >> /* CH7511 seems to leave SINK_COUNT zeroed */ >> { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) }, >> + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */ >> + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) }, > >This seems to be generic OUI for Synaptics [1]. Could this cause us to >cast the net too wide? > >Even if we check that it's DP_DPCD_REV >= 1.4 there's a good chance >Synaptics is fixing this in the future and won't require the quirk. > >[1] https://aruljohn.com/mac/vendor/Synaptics > > >Harry It's not pretty, but - Currently the net of "all Synaptics devices with rev>DP1.4" catches every device we care about and none we don't - If a future Synaptics device supports virtual DPCD properly, we will check for that first and never resort to the workaround (see patch 6/6) - We don't know any of the properties of any hypothetical future Synaptics hardware, so we can't create cases for them now Also, received offline review from AMD MST DSC (Non-Linux) expert Wenjing Liu, giving me permission o mark this patch Reviewed-by: Wenjing Liu <Wenjing.Liu@amd.com> > >> }; >> >> #undef OUI >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> index 8364502f92cf..a1331b08705f 100644 >> --- a/include/drm/drm_dp_helper.h >> +++ b/include/drm/drm_dp_helper.h >> @@ -1434,6 +1434,13 @@ enum drm_dp_quirk { >> * The driver should ignore SINK_COUNT during detection. >> */ >> DP_DPCD_QUIRK_NO_SINK_COUNT, >> + /** >> + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD: >> + * >> + * The device supports MST DSC despite not supporting Virtual DPCD. >> + * The DSC caps can be read from the physical aux instead. >> + */ >> + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD, >> }; >> >> /**
On 2019-08-26 3:14 p.m., Francis, David wrote: > On 2019-08-26 2:05 p.m., David Francis wrote: >>> Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not >>> support virtual DPCD registers, but do support DSC. >>> The DSC caps can be read from the physical aux, >>> like in SST DSC. These hubs have many different >>> DEVICE_IDs. Add a new quirk to detect this case. >>> >>> Cc: Lyude Paul <lyude@redhat.com> >>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>> Signed-off-by: David Francis <David.Francis@amd.com> >>> --- >>> drivers/gpu/drm/drm_dp_helper.c | 2 ++ >>> include/drm/drm_dp_helper.h | 7 +++++++ >>> 2 files changed, 9 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c >>> index 2cc21eff4cf3..fc39323e7d52 100644 >>> --- a/drivers/gpu/drm/drm_dp_helper.c >>> +++ b/drivers/gpu/drm/drm_dp_helper.c >>> @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { >>> { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) }, >>> /* CH7511 seems to leave SINK_COUNT zeroed */ >>> { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) }, >>> + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */ >>> + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) }, >> >> This seems to be generic OUI for Synaptics [1]. Could this cause us to >> cast the net too wide? >> >> Even if we check that it's DP_DPCD_REV >= 1.4 there's a good chance >> Synaptics is fixing this in the future and won't require the quirk. >> >> [1] https://aruljohn.com/mac/vendor/Synaptics >> >> >> Harry > > It's not pretty, but > - Currently the net of "all Synaptics devices with rev>DP1.4" catches every device we care about and none we don't > - If a future Synaptics device supports virtual DPCD properly, we will check for that first and never resort to the workaround (see patch 6/6) > - We don't know any of the properties of any hypothetical future Synaptics hardware, so we can't create cases for them now > That's fair enough. Thanks for the explanation. Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry > Also, received offline review from AMD MST DSC (Non-Linux) expert Wenjing Liu, giving me permission o mark this patch > Reviewed-by: Wenjing Liu <Wenjing.Liu@amd.com> > >> >>> }; >>> >>> #undef OUI >>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >>> index 8364502f92cf..a1331b08705f 100644 >>> --- a/include/drm/drm_dp_helper.h >>> +++ b/include/drm/drm_dp_helper.h >>> @@ -1434,6 +1434,13 @@ enum drm_dp_quirk { >>> * The driver should ignore SINK_COUNT during detection. >>> */ >>> DP_DPCD_QUIRK_NO_SINK_COUNT, >>> + /** >>> + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD: >>> + * >>> + * The device supports MST DSC despite not supporting Virtual DPCD. >>> + * The DSC caps can be read from the physical aux instead. >>> + */ >>> + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD, >>> }; >>> >>> /** > > ________________________________________ > From: Wentland, Harry <Harry.Wentland@amd.com> > Sent: August 26, 2019 3:05 PM > To: Francis, David; dri-devel@lists.freedesktop.org > Subject: Re: [PATCH v8 5/6] drm/dp_mst: Add new quirk for Synaptics MST hubs > > On 2019-08-26 2:05 p.m., David Francis wrote: >> Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not >> support virtual DPCD registers, but do support DSC. >> The DSC caps can be read from the physical aux, >> like in SST DSC. These hubs have many different >> DEVICE_IDs. Add a new quirk to detect this case. >> >> Cc: Lyude Paul <lyude@redhat.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Signed-off-by: David Francis <David.Francis@amd.com> >> --- >> drivers/gpu/drm/drm_dp_helper.c | 2 ++ >> include/drm/drm_dp_helper.h | 7 +++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c >> index 2cc21eff4cf3..fc39323e7d52 100644 >> --- a/drivers/gpu/drm/drm_dp_helper.c >> +++ b/drivers/gpu/drm/drm_dp_helper.c >> @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { >> { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) }, >> /* CH7511 seems to leave SINK_COUNT zeroed */ >> { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) }, >> + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */ >> + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) }, > > This seems to be generic OUI for Synaptics [1]. Could this cause us to > cast the net too wide? > > Even if we check that it's DP_DPCD_REV >= 1.4 there's a good chance > Synaptics is fixing this in the future and won't require the quirk. > > [1] https://aruljohn.com/mac/vendor/Synaptics > > Harry > >> }; >> >> #undef OUI >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> index 8364502f92cf..a1331b08705f 100644 >> --- a/include/drm/drm_dp_helper.h >> +++ b/include/drm/drm_dp_helper.h >> @@ -1434,6 +1434,13 @@ enum drm_dp_quirk { >> * The driver should ignore SINK_COUNT during detection. >> */ >> DP_DPCD_QUIRK_NO_SINK_COUNT, >> + /** >> + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD: >> + * >> + * The device supports MST DSC despite not supporting Virtual DPCD. >> + * The DSC caps can be read from the physical aux instead. >> + */ >> + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD, >> }; >> >> /** >>
On Mon, 26 Aug 2019, "Francis, David" <David.Francis@amd.com> wrote: > On 2019-08-26 2:05 p.m., David Francis wrote: >>> Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not >>> support virtual DPCD registers, but do support DSC. >>> The DSC caps can be read from the physical aux, >>> like in SST DSC. These hubs have many different >>> DEVICE_IDs. Add a new quirk to detect this case. >>> >>> Cc: Lyude Paul <lyude@redhat.com> >>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>> Signed-off-by: David Francis <David.Francis@amd.com> >>> --- >>> drivers/gpu/drm/drm_dp_helper.c | 2 ++ >>> include/drm/drm_dp_helper.h | 7 +++++++ >>> 2 files changed, 9 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c >>> index 2cc21eff4cf3..fc39323e7d52 100644 >>> --- a/drivers/gpu/drm/drm_dp_helper.c >>> +++ b/drivers/gpu/drm/drm_dp_helper.c >>> @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { >>> { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) }, >>> /* CH7511 seems to leave SINK_COUNT zeroed */ >>> { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) }, >>> + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */ >>> + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) }, >> >>This seems to be generic OUI for Synaptics [1]. Could this cause us to >>cast the net too wide? >> >>Even if we check that it's DP_DPCD_REV >= 1.4 there's a good chance >>Synaptics is fixing this in the future and won't require the quirk. >> >>[1] https://aruljohn.com/mac/vendor/Synaptics >> >> >>Harry > > It's not pretty, but > - Currently the net of "all Synaptics devices with rev>DP1.4" catches every device we care about and none we don't > - If a future Synaptics device supports virtual DPCD properly, we will check for that first and never resort to the workaround (see patch 6/6) > - We don't know any of the properties of any hypothetical future Synaptics hardware, so we can't create cases for them now One consideration is that if this causes certain behaviour in future hardware, and, by coincidence, that happens to be desired behaviour, and objectively fixing it (removing the quirk) causes a regression in that desired behaviour, it's a regression. I'm not sure what the right approach here is, but you really do need to be aware of the kind of corners you might paint yourself into. BR, Jani. > > Also, received offline review from AMD MST DSC (Non-Linux) expert Wenjing Liu, giving me permission o mark this patch > Reviewed-by: Wenjing Liu <Wenjing.Liu@amd.com> > >> >>> }; >>> >>> #undef OUI >>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >>> index 8364502f92cf..a1331b08705f 100644 >>> --- a/include/drm/drm_dp_helper.h >>> +++ b/include/drm/drm_dp_helper.h >>> @@ -1434,6 +1434,13 @@ enum drm_dp_quirk { >>> * The driver should ignore SINK_COUNT during detection. >>> */ >>> DP_DPCD_QUIRK_NO_SINK_COUNT, >>> + /** >>> + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD: >>> + * >>> + * The device supports MST DSC despite not supporting Virtual DPCD. >>> + * The DSC caps can be read from the physical aux instead. >>> + */ >>> + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD, >>> }; >>> >>> /** > > ________________________________________ > From: Wentland, Harry <Harry.Wentland@amd.com> > Sent: August 26, 2019 3:05 PM > To: Francis, David; dri-devel@lists.freedesktop.org > Subject: Re: [PATCH v8 5/6] drm/dp_mst: Add new quirk for Synaptics MST hubs > > On 2019-08-26 2:05 p.m., David Francis wrote: >> Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not >> support virtual DPCD registers, but do support DSC. >> The DSC caps can be read from the physical aux, >> like in SST DSC. These hubs have many different >> DEVICE_IDs. Add a new quirk to detect this case. >> >> Cc: Lyude Paul <lyude@redhat.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Signed-off-by: David Francis <David.Francis@amd.com> >> --- >> drivers/gpu/drm/drm_dp_helper.c | 2 ++ >> include/drm/drm_dp_helper.h | 7 +++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c >> index 2cc21eff4cf3..fc39323e7d52 100644 >> --- a/drivers/gpu/drm/drm_dp_helper.c >> +++ b/drivers/gpu/drm/drm_dp_helper.c >> @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { >> { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) }, >> /* CH7511 seems to leave SINK_COUNT zeroed */ >> { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) }, >> + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */ >> + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) }, > > This seems to be generic OUI for Synaptics [1]. Could this cause us to > cast the net too wide? > > Even if we check that it's DP_DPCD_REV >= 1.4 there's a good chance > Synaptics is fixing this in the future and won't require the quirk. > > [1] https://aruljohn.com/mac/vendor/Synaptics > > Harry > >> }; >> >> #undef OUI >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> index 8364502f92cf..a1331b08705f 100644 >> --- a/include/drm/drm_dp_helper.h >> +++ b/include/drm/drm_dp_helper.h >> @@ -1434,6 +1434,13 @@ enum drm_dp_quirk { >> * The driver should ignore SINK_COUNT during detection. >> */ >> DP_DPCD_QUIRK_NO_SINK_COUNT, >> + /** >> + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD: >> + * >> + * The device supports MST DSC despite not supporting Virtual DPCD. >> + * The DSC caps can be read from the physical aux instead. >> + */ >> + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD, >> }; >> >> /** >> > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 2cc21eff4cf3..fc39323e7d52 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) }, /* CH7511 seems to leave SINK_COUNT zeroed */ { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) }, + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */ + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) }, }; #undef OUI diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 8364502f92cf..a1331b08705f 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1434,6 +1434,13 @@ enum drm_dp_quirk { * The driver should ignore SINK_COUNT during detection. */ DP_DPCD_QUIRK_NO_SINK_COUNT, + /** + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD: + * + * The device supports MST DSC despite not supporting Virtual DPCD. + * The DSC caps can be read from the physical aux instead. + */ + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD, }; /**
Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not support virtual DPCD registers, but do support DSC. The DSC caps can be read from the physical aux, like in SST DSC. These hubs have many different DEVICE_IDs. Add a new quirk to detect this case. Cc: Lyude Paul <lyude@redhat.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: David Francis <David.Francis@amd.com> --- drivers/gpu/drm/drm_dp_helper.c | 2 ++ include/drm/drm_dp_helper.h | 7 +++++++ 2 files changed, 9 insertions(+)