On 27/01/2018 09:05, Jani Nikula wrote:
On Fri, 26 Jan 2018, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
On Fri, Jan 26, 2018 at 10:12:00AM +0000, Jani Nikula wrote:
On Thu, 25 Jan 2018, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
The only difference is that this SKUs has the full
Port A/E split named as Port F.
But since SKUs differences don't matter on the platform
definition group and ids, let's merge all off them together.
v2: Really include the PCI IDs to the picidlist[];
v3: Add the PCI Id for another SKU (Anusha).
v4: Update IDs, really include to pciidlists again.
v5: Unify all GT2 IDs.
v6: Unify in a way that we don't break early-quirks.c
v7: Remove GT reference since it doesn't matter here (Paulo)
Also move IS_CNL_WITH_PORT_F macro to this patch to
make it easier for review this part and also to get
used sooner.
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_pci.c | 5 ++---
include/drm/i915_pciids.h | 18 +++++++-----------
3 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 454d8f937fae..5702ebf17974 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2648,6 +2648,8 @@ intel_info(const struct drm_i915_private *dev_priv)
(dev_priv)->info.gt == 2)
#define IS_CFL_GT3(dev_priv) (IS_COFFEELAKE(dev_priv) && \
(dev_priv)->info.gt == 3)
+#define IS_CNL_WITH_PORT_F(dev_priv) (IS_CANNONLAKE(dev_priv) && \
+ (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
I'd be happy if bit 2 in device id actually meant "port F", but I'm not
so happy with coming up with rules like this for coincidental things.
the port F thing is just that we have no name for that SKU :(
but from the spec organization this bit seems to represent that
group. Although I fully agree with you that this is horrible.
More generally, I'm not all that happy about *any* of the INTEL_DEVID
uses in i915_drv.h because it spreads out the device id information, so
I'd rather not add more. I'd rather move towards single point of truth
for device ids.
Yeah. I guess I agree with you.
There should be something inside the device info right?
even if we have to separate that in two groups.
That's the thing. It also worries me to add everything in device info,
because it grows the device info structs for all devices, and you also
need to add more device infos to cover all combinations. Unless you
initialize them runtime, which is also a bit meh.
If you agree with this approach I can follow up with a patch/series
that kill INTEL_DEVID in favor of device info structs.
Or do you have any other idea for solving this?
Not really. Cc: Chris and Tvrtko who seem to come up with ideas for
things like this on a regular basis. ;)
If I understood the gist, if single point of truth for device ids is the
desire, that seems to imply there has to some place where flags are
toggled at runtime.
Getting rid of INTEL_DEVID checks sprinkled around randomly, as courtesy
of i915_drv.h "is-platform-variant" macros would be nice, I agree.
So perhaps a runtime pass which would set a new field in device_info,
like INTEL_INFO()->platform_variant wouldn't be that bad. Device ID
checks would then be moved from i915_drv.h into intel_device_info.c, and
the i915_drv.h macros would become simple checks.
Wrt size consideration, I am not sure if it would be feasible to split
struct intel_device_info in const friendly part and the rest - so that
the instances in i915_pci.c could be smaller, and copied over to
larger/complete version in struct drm_i915_private. On a quick glance it
doesn't seem that there are many, if any, fields which are set at
runtime only.
So no magical ideas from me I'm afraid.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
