On Fri, 14 Oct 2016, "Sharma, Shashank" <shashank.sharma@xxxxxxxxx> wrote: > Regards > > Shashank > > > On 10/14/2016 8:02 PM, Jani Nikula wrote: >> On Fri, 14 Oct 2016, Shashank Sharma <shashank.sharma@xxxxxxxxx> wrote: >>> Many GEN9 boards come with on-board lspcon cards. >>> Fot these boards, VBT configuration should properly point out >>> if a particular port contains lspcon device, so that driver can >>> initialize it properly. >>> >>> This patch adds a utility function, which checks the VBT flag >>> for lspcon bit, and tells us if a port is configured to have a >>> lspcon device or not. >>> >>> V2: Fixed review comments from Ville >>> - Do not forget PORT_D while checking lspcon for GEN9 >>> >>> V3: Addressed review comments from Rodrigo >>> - Create a HAS_LSPCON() macro for better use case handling. >>> - Do not dump warnings for non-gen-9 platforms, it will be noise. >>> >>> V4: Rebase >>> V5: Rebase >>> V6: Pass dev_priv to HAS_LSPCON() macro >>> >>> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx> >>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >> I was hoping you'd use the version I rebased and sent, put it first in >> the series, and rebase the rest on that. The point is, this series has >> taken so long that lspcon devices have proliferated all over the place, >> and we'll be getting more and more bugs about them. If this patch was >> first, with the debug logging, we could at least get that to 4.9, maybe >> stable kernels, and we'd immediately know the reason. I think it'll be a >> hard sell to get the whole series to 4.9 kernel. >> >> BR, >> Jani. > Jani, > The patch got its first r-b since a long time. > After that, it was waiting to be merged, for long time. > > Recently, when Imre was asked to test the patches, and he found one > issue specific to APL. > We were trying to fix a suspend-resume issue, which was fixed with the > last patch. > Now this patch is ready to be merged, just waiting for Imre's r-b. > > Third patch just gives information about if LSPCON is available or not, > which is not a big help for anything as such. > So instead of changing the sequence, and confusing the reviewers, I > thought it would be better to send the whole series and > get this merged as-it-is. Fine, let's merge this as-is... after patch 1/5 has been posted to dri-devel and/or has received an ack from Dave Airlie that it's fine to merge through our tree. In the bigger scheme of things, if this patch 3/5 had been first in the series all along, we could have merged this *months* ago. This is how series should be organized. Simple things first. Having the debug information *is* valuable. You'd see that if you had to go through our incoming bugs. We've had plenty of LSPCON bugs since the day Skylake was launched. Yes, quite a long time now. If 3/5 was first in the series, we could backport that to v4.9 and older and reduce our debugging time. BR, Jani. > > Regards > Shashank >> >>> --- >>> drivers/gpu/drm/i915/i915_drv.h | 5 ++++ >>> drivers/gpu/drm/i915/intel_bios.c | 49 +++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 54 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >>> index fe875b2..7bab2f1 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.h >>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>> @@ -2864,6 +2864,8 @@ struct drm_i915_cmd_table { >>> >>> #define HAS_GMCH_DISPLAY(dev_priv) ((dev_priv)->info.has_gmch_display) >>> >>> +#define HAS_LSPCON(dev_priv) (IS_GEN9(dev_priv)) >>> + >>> /* DPF == dynamic parity feature */ >>> #define HAS_L3_DPF(dev_priv) ((dev_priv)->info.has_l3_dpf) >>> #define NUM_L3_SLICES(dev_priv) (IS_HSW_GT3(dev_priv) ? \ >>> @@ -3631,6 +3633,9 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv, enum por >>> bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port); >>> bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv, >>> enum port port); >>> +bool intel_bios_is_lspcon_present(struct drm_i915_private *dev_priv, >>> + enum port port); >>> + >>> >>> /* intel_opregion.c */ >>> #ifdef CONFIG_ACPI >>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c >>> index 83667e8..32e1def 100644 >>> --- a/drivers/gpu/drm/i915/intel_bios.c >>> +++ b/drivers/gpu/drm/i915/intel_bios.c >>> @@ -1763,3 +1763,52 @@ intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv, >>> >>> return false; >>> } >>> + >>> +/** >>> + * intel_bios_is_lspcon_present - if LSPCON is attached on %port >>> + * @dev_priv: i915 device instance >>> + * @port: port to check >>> + * >>> + * Return true if LSPCON is present on this port >>> + */ >>> +bool >>> +intel_bios_is_lspcon_present(struct drm_i915_private *dev_priv, >>> + enum port port) >>> +{ >>> + int i; >>> + >>> + if (!HAS_LSPCON(dev_priv)) >>> + return false; >>> + >>> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) { >>> + if (!dev_priv->vbt.child_dev[i].common.lspcon) >>> + continue; >>> + >>> + switch (dev_priv->vbt.child_dev[i].common.dvo_port) { >>> + case DVO_PORT_DPA: >>> + case DVO_PORT_HDMIA: >>> + if (port == PORT_A) >>> + return true; >>> + break; >>> + case DVO_PORT_DPB: >>> + case DVO_PORT_HDMIB: >>> + if (port == PORT_B) >>> + return true; >>> + break; >>> + case DVO_PORT_DPC: >>> + case DVO_PORT_HDMIC: >>> + if (port == PORT_C) >>> + return true; >>> + break; >>> + case DVO_PORT_DPD: >>> + case DVO_PORT_HDMID: >>> + if (port == PORT_D) >>> + return true; >>> + break; >>> + default: >>> + break; >>> + } >>> + } >>> + >>> + return false; >>> +} > -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx